[codex] add fold04 agent evaluation bundle

[jmchandonia]

Summary

• add tracked fold-04 holdout-5 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 515,825 rows vs 1,089,621 gold rows
• row precision was 1.000 on NaN-safe unique keys, but gold recall was 0.528 because the agent filtered to control-treatment rows only
• generated JSON covered all 8/8 gold mapping categories, with one extra gravimetric_water_content category

Validation

• python -m py_compile evaluations/fold-04-holdout-5/agent_outputs/harmonize_heldout.py
• targeted rg forbidden-path scan over evaluations/fold-04-holdout-5/AGENT_ACTION_LOG.md
• UV_CACHE_DIR=/scratch/jmc/data-harmonization-eval/.uv-cache uv run pytest

[claude]

Claude finished @jmchandonia's task —— View job

---

PR Review: fold04 agent evaluation bundle

Tasks:

• Gather context and understand the PR
• Read CLAUDE.md for repo guidelines (no CLAUDE.md found)
• Review Python harmonization script
• Review JSON mapping and curator bundle
• Review evaluation metrics and documentation
• Post comprehensive review feedback

---

Overall Assessment

This PR tracks a well-structured agent evaluation artifact. The files are clean, the evaluation pipeline is sound, and the key discrepancy (control-only vs. full dataset recall) is accurately diagnosed. Below are specific observations.

---

harmonize_heldout.py — Code Quality

Positive:

• Clean separation of parse_local_to_utc, ensure_harmonized_cols, harmonize, and main functions.
• Proper use of from __future__ import annotations for forward-reference compatibility.
• pd.to_numeric(..., errors="coerce") used defensively throughout.
• Timezone handling with ambiguous="NaT" and nonexistent="shift_forward" is robust.
• validate="many_to_one" on both merges is an excellent defensive check that will surface bad data early.

Issues / Suggestions:

1. interval_min clamp logic is incomplete (harmonize_heldout.py:68):

   x.loc[x["interval_min"] < 0, "interval_min"] = np.nan

   Negative intervals can't occur after a sort_values on the timestamp column, so this guard is vacuously true. However, diff() produces NaT for the first row of each group, which becomes NaN — so the first-row interval is already NaN correctly. The check is harmless but misleading.

2. merge on smeta may silently drop rows (harmonize_heldout.py:48):

   x = x.merge(smeta, on=["Sensor.SN", "SFA_Location"], how="left", validate="many_to_one")

   With how="left", rows in swc with no match in smeta will have NaN for Treatment and therefore be silently excluded by the subsequent .eq("ctl") filter. This is likely correct behavior but is not explicitly documented — a comment would add clarity, especially since mapping_notes.md mentions 1,089,621 raw rows vs. 515,825 control rows.

3. depth_m is always NaN (harmonize_heldout.py:55):

   x["depth_m"] = np.nan

   This is correct per the mapping notes (no depth column in source files), but it causes the NaN-safe key comparison in the evaluation to need a sentinel workaround (noted in evaluation_metrics.json:60). If the evaluation harness ever changes its keying strategy, this could become a silent issue. Worth documenting.

4. OUT_DIR is the script's own directory (harmonize_heldout.py:10):

   OUT_DIR = Path(__file__).resolve().parent

   This is a common pattern but makes the script write output next to itself, which can be surprising if the script is moved. Consider using a --output-dir CLI argument or a separate OUTPUT_DIR env variable for portability, similar to how HELDOUT_RAW_DIR is handled.

5. No logging or progress output during the merge — for a 1M-row dataset, a progress indicator or at least a print() after the merge step would help users verify the script is making progress.

---

mapping.json — JSON Mapping

Positive:

• All 8/8 gold mapping categories are present.
• destination_variable matches exactly for all 8 entries.
• unit_conversion is correctly populated (8/8 exact matches).

Issues:

1. Extra gravimetric_water_content category — The agent included this category even though gravimetric water content is not in the source data (it's always NaN). The gold standard apparently omits this category entirely. This is a meaningful divergence: the extra category signals that the agent includes it for completeness (as a "not available" placeholder), while gold omits absent variables. This suggests a documentation/convention question for the evaluation framework about whether to include or omit all-NA mappings.

2. transformation matches: 0/8 — None of the transformation descriptions match gold exactly. This is expected given natural language variation, but it indicates that the evaluation metric for transformation may need fuzzy/semantic matching rather than exact string comparison to be meaningful.

3. location_metadata_files and sensor_metadata_files do not match gold — The top-level field mismatch (2/7 exact) is driven by these two fields. The agent's values are likely structurally correct but differ in detail from gold. The evaluation correctly notes this as a top-level mismatch.

---

evaluation_metrics.json — Metrics Quality

Positive:

• Machine-readable metrics are well-structured.
• Notes field (evaluation_metrics.json:58-61) clearly explains the NaN sentinel and control-only filter decisions.
• row_precision: 1.0 with agent_extra_keys: 0 confirms no spurious rows.

Issues:

1. interval_min match rate is very low: 59007/516005 (~11.4%). This is a significant flag. The evaluation report (EVALUATION.md:35) records it but does not explain the cause. Possible reasons:

   • The gold uses a different grouping key for interval computation (e.g., by Sensor.SN directly rather than the derived replicate integer).
   • The gold computes intervals differently at group boundaries.
   • Floating-point rounding differences.
     This low match rate warrants investigation and explanation in the evaluation notes, even if the root cause is a known difference in methodology.

2. common_rows: 516005 vs. rows: 515825 — There are 180 more rows in common_rows than in the agent's output. This is likely due to the NaN-safe key expansion creating duplicate key matches, but it is not explained in the metrics and could be confusing.

---

EVALUATION.md — Documentation

Positive:

• Tables are clear and the recall/precision distinction is well-articulated.
• The anti-leakage audit result (0 matches) is prominently reported.
• The interpretation section (EVALUATION.md:39) correctly explains the control-only filter as the source of the recall gap.

Issue:

1. interval_min mismatch is not explained — As noted above, the 88.6% mismatch on interval_min is reported but unexplained. Given that this is a derived column (computed from the data), a divergence this large suggests a methodological difference that should be documented.

---

curator_bundle.json — Curator Decisions

Positive:

• File classifications are accurate and well-reasoned.
• Location resolution QC flags (g1/g2) are appropriate and transparent.
• Unresolved site IDs (WG-LSB1, WG-USA2R, etc.) are explicitly documented.

Minor:

• readme_content: null is fine, but worth noting in mapping_notes.md that no README was present.

---

Security Considerations

No security concerns. The script reads from env-configurable paths, does not execute arbitrary user input, and writes only to a local output directory. No secrets or credentials are present in any artifact.

---

Summary

The PR is well-executed. The key substantive finding — that the agent filtered to control-only rows while gold includes all treatment rows — is correctly identified and reported. The main items to consider before merge:

• Medium priority: Explain the interval_min mismatch (11.4% match rate) in either EVALUATION.md or evaluation_metrics.json notes.
• Minor: Document the common_rows > agent rows discrepancy in metrics.
• Minor: Add a comment in harmonize_heldout.py near the how="left" merge explaining that unmatched sensor rows are intentionally dropped via the subsequent Treatment == "ctl" filter.