[codex] add fold06 agent evaluation bundle

[jmchandonia]

Summary

• add tracked fold-06 holdout-8 sub-agent artifacts, including action trace and generated outputs
• add an evaluation report comparing output, JSON mapping, and Python behavior against held-out gold
• add machine-readable evaluation metrics

Evaluation Highlights

• targeted forbidden-path scan found zero matches
• agent produced 195,524 rows vs 195,446 gold rows
• key precision 0.994 and gold recall 0.994 on unique keys
• shared-row matches: interval 194,247/194,263; VWC 191,225/194,263; water potential 178,229/194,263
• generated JSON covered all 8/8 gold mapping categories

Validation

• python -m py_compile evaluations/fold-06-holdout-8/agent_outputs/harmonize_heldout.py
• targeted rg forbidden-path scan over evaluations/fold-06-holdout-8/AGENT_ACTION_LOG.md

[claude]

Claude finished @jmchandonia's task —— View job

---

PR Review: fold-06-holdout-8 Agent Evaluation Bundle

Tasks

• Gather context and understand request
• Read repository CLAUDE.md (not present)
• Review key files
• Analyze code quality, security, and correctness
• Post review feedback

---

Overall Assessment

This PR looks well-structured for an agent evaluation bundle. The Python harmonization script is clean and idiomatic, metrics are comprehensive, and the anti-leakage audit was conducted. Here is detailed feedback on each area:

---

Code Quality — harmonize_heldout.py

Strengths:

• Clean, idiomatic pandas use with proper melt/pivot_table pattern for wide-to-long transformation
• Correct use of from __future__ import annotations, pathlib.Path, and type hints
• Proper sentinel value handling (-9999 → NaN) before numeric conversion
• Row filtering logic is correct: drops rows only where both VWC and water potential are missing

Issues / Suggestions:

1. Hardcoded absolute paths (harmonize_heldout.py:9-12): RUN_DIR and RAW_DIR are hardcoded to machine-specific paths (/scratch/jmc/..., /h/jmc/...). This makes the script non-reproducible by anyone else. Consider making these configurable via CLI args or environment variables for a production harness.

2. interval_min computed on per-file basis but not reset at boundaries (harmonize_heldout.py:66): The interval is computed as x['datetime'].diff(), which means the first row of each file will have NaN. When files from different years are concatenated, the first row of each year will lose its interval. This is correct behavior for within-file intervals but may explain the 16 unmatched interval_min rows (194,247/194,263 matched). This is worth documenting explicitly.

3. aggfunc='first' in pivot_table (harmonize_heldout.py:95): Using 'first' silently drops duplicates at the same (datetime, depth) key without warning. If duplicates exist in the source data, this could silently discard measurements. Consider adding a pre-pivot duplicate check or at minimum a logged warning.

4. log_action appends to an absolute path (harmonize_heldout.py:35): The LOG_PATH will fail if run in a different environment (same hardcoded path issue). The logging itself is a nice design pattern for the benchmark.

5. Minor — ensure_harmonized_cols called twice (harmonize_heldout.py:105,115): Once in harmonize_file and once in main. The second call in main is redundant after pd.concat since columns are already guaranteed present from each frame. Not harmful but slightly noisy.

---

JSON Mapping Quality — mapping.json

Strengths:

• All 8 gold mapping categories present (8/8)
• Correct destination variables, file lists, and unit conversions
• Clear, human-readable transformation descriptions

Issues:

1. sensor_metadata_files mismatch (evaluation_metrics.json:46): top_level_matches.sensor_metadata_files = false. The agent assigned the same InstallationMethods files to both location_metadata_files and sensor_metadata_files, but the gold apparently differs here. This is the only top-level field miss (6/7). It would be helpful to document what the gold expects here in the evaluation notes.

2. transformation field: 0/8 exact matches (evaluation_metrics.json:51): All transformation strings differ from gold. The transformations are semantically correct but verbosity/phrasing differs. This is an expected weakness of free-text fields and should probably be noted as "semantically equivalent but phrasing differs" in the evaluation report.

3. latitude/longitude mapping categories in mapping.json but not in OUTPUT_COLUMNS in the Python script: The mapping correctly documents lat/lon sourcing, but the Python script doesn't emit them in the output. This is consistent with the gold schema (OUTPUT_COLUMNS has 9 columns without lat/lon), but the discrepancy between the mapping doc and the code is slightly confusing. A comment in mapping_notes.md explaining this would help.

---

Evaluation Report — EVALUATION.md and evaluation_metrics.json

Strengths:

• Machine-readable metrics are well-structured
• Anti-leakage audit is clearly documented
• Shared-row value match table is informative

Issues:

1. water_potential_kPa match rate is notably lower (178,229/194,263 = 91.7%): This is ~16K rows with non-matching water potential values. The EVALUATION.md doesn't explain this discrepancy. Given the VWC match is 98.4%, the water potential miss rate stands out and warrants investigation/documentation (e.g., different depth sensor sets between agent and gold, or different NaN handling).

2. key_includes_dataset_index=False policy: The evaluation note mentions this but it's not obvious why the key omits dataset index. If multiple datasets are evaluated and merged, row-key collisions across datasets could produce misleading metrics. This is a design choice worth explicitly justifying in EVALUATION.md.

3. 1,269 extra agent rows / 1,183 missing gold rows: The agent has slightly more rows than gold (~78 net extra). This likely relates to how boundary timestamps or NaN rows are handled differently. Not a major issue, but worth noting in EVALUATION.md alongside the interval_min observation.

---

Security Concerns

• No security concerns identified. The script reads from a fixed local path and writes output locally. No network requests, credential handling, or shell injection vectors.
• The failed DNS lookup attempt (curl https://api.ess-dive.lbl.gov/... in AGENT_ACTION_LOG.md lines 47-48) confirms the sandbox correctly blocked external network access during execution — this is a positive signal.

---

Performance

• The wide-to-long melt + pivot_table pattern is appropriate for this data volume (~195K rows). No performance concerns for the expected scale.
• Reading 4 CSVs sequentially is fine. Parallel reads would not be necessary at this scale.

---

Test Coverage

• py_compile check is minimal — it only verifies syntax, not runtime behavior.
• Consider adding: a row-count assertion in the script itself, or a post-run validation that checks the output CSV schema matches OUTPUT_COLUMNS.
• No unit tests exist for the harmonization logic (e.g., parse_fixed_mst_to_utc, extract_depth_m). Given this is an evaluation artifact rather than production code, this is acceptable, but worth noting if scripts are reused across folds.

---

Summary

The PR is well-executed. Key items to consider before merging:

• Document the water_potential_kPa 91.7% match rate — explain if it's expected
• Hardcoded paths are acceptable for a benchmark artifact but should be noted in the evaluation docs
• The sensor_metadata_files top-level miss and transformation 0/8 exact matches are known weaknesses worth documenting more explicitly in EVALUATION.md