Skip to content

[codex] add fold02 agent evaluation bundle#27

Draft
jmchandonia wants to merge 1 commit into
mainfrom
codex/fold02-agent-evaluation
Draft

[codex] add fold02 agent evaluation bundle#27
jmchandonia wants to merge 1 commit into
mainfrom
codex/fold02-agent-evaluation

Conversation

@jmchandonia

Copy link
Copy Markdown
Collaborator

Summary

  • add tracked fold-02 holdout-15-26 sub-agent artifacts, including action trace and generated outputs
  • add an evaluation report comparing output against held-out gold for the two-dataset cluster
  • add machine-readable evaluation metrics

Evaluation Highlights

  • targeted forbidden-path scan found zero matches
  • agent produced 711 rows vs 586 gold rows
  • no keyed rows matched gold under dataset-index-aware keys
  • cluster mapping literal comparison was not computed by the helper

Validation

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

@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Claude finished @jmchandonia's task —— View job


PR Review: fold02 agent evaluation bundle

Tasks

  • Gather context and read key files
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Review security concerns
  • Evaluate test coverage and evaluation metrics

Overall Summary

This PR adds a complete evaluation bundle for fold-02 (holdout datasets 15 and 26). The harmonization script is well-structured and the anti-leakage audit passed. However, the evaluation reveals zero row-level matches against gold, which warrants attention. Several specific issues are noted below.


harmonize_heldout.py — Code Quality

Hardcoded absolute paths (critical for reproducibility)

harmonize_heldout.py:11-14 uses absolute machine-specific paths:

FOLD_DIR = Path('/scratch/jmc/data-harmonization-eval/.runs/fold-02-holdout-15-26')
RAW_BASE = Path('/h/jmc/ess-dive_wfsfa_soil_datasets')

These paths are specific to the author's machine and will fail for any other reviewer or CI system trying to reproduce the run. The PR notes py_compile passes (which only checks syntax), but the script cannot be re-executed without these paths existing. Consider parameterizing via environment variables or CLI arguments.

Floating-point precision in output

heldout_harmonized.csv:3 contains:

0.10099999999999999

This is a floating-point rounding artifact from / 100.0. Consider using round() or df.round(6) before writing CSVs to avoid noisy output that would fail exact string comparison during evaluation.

main() calls raw_path(15).stat().st_size in build_curator_bundle()

harmonize_heldout.py:241 — the file size is computed at bundle-build time from the raw path. This tightly couples the curator bundle to the execution environment; the stat call will fail outside the author's system and also means the curator_bundle.json was generated from a specific file on the local machine, making it non-reproducible from the repo alone.

UTM conversion is hand-rolled (harmonize_heldout.py:80-117)

The implementation substitutes for unavailable pyproj. This is reasonable given the sandbox constraint, but the accuracy of the hand-rolled inverse Transverse Mercator is not validated anywhere. The action log shows pyproj was tried first and replaced (log lines 33-35) — it would be useful to document this tradeoff in mapping_notes.md.


Evaluation Concerns

Zero matched rows (key mismatch)

evaluation_metrics.json shows key_intersection: 0 with key_includes_dataset_index: true. This strongly suggests the key schema used during evaluation differs from the one used to generate the gold. Possible causes:

  • Dataset 15: Agent assigns datetime_UTC = '2019-07-02T06:00:00Z' (Mountain midnight → UTC offset), but gold may use a different date string or no datetime at all (e.g., NaT or a different offset).
  • The gold for dataset 26 has 584 unique keys but the agent produces 425 rows — the 125-row discrepancy may reflect different row-inclusion logic.

This is flagged in EVALUATION.md with a note to "interpret with the key policy recorded in the metrics" but the root cause of the key mismatch is not analyzed. It would strengthen the evaluation to include a brief diagnostic (e.g., sample gold keys vs. sample agent keys) rather than just noting 0/0.

Row count divergence

Agent: 711 rows vs. Gold: 586 rows. The agent generates 286 rows for dataset 15 (101 sites × up to 3 replicates, minus NaN) and 425 rows for dataset 26. However, mapping_notes.md:7 says "286 long-format VWC readings" for dataset 15, while EVALUATION.md says the agent produced 711 total. The discrepancy (711 vs 286+425=711) is internally consistent, but the gold has only 586. The extra 125 rows likely reflect different row-filtering logic in the gold — this should be investigated.

JSON mapping comparison not computed

EVALUATION.md and evaluation_metrics.json both note: "cluster mapping literal comparison not computed". This means one of the three evaluation dimensions is entirely missing. If mapping quality is part of the benchmark, this gap should be addressed or explicitly flagged as a known limitation.


Potential Bugs

is_timeseries serialized as Python False, not JSON false

heldout_harmonized.csv:2:

False,,0.0,,

The boolean False is written as the Python string "False" in the CSV, not false. If downstream parsing expects a JSON-style boolean or lowercase string, this could cause comparison failures. Check whether the gold uses False, false, 0, or another representation.

gravimetric_water_content_gH2O_gs division guard is incomplete

harmonize_heldout.py:176-177:

x['gravimetric_water_content_gH2O_gs'] = (wet - dry) / dry
x.loc[(dry <= 0) | wet.isna() | dry.isna(), 'gravimetric_water_content_gH2O_gs'] = np.nan

The division by zero guard is applied after the division. On some pandas/numpy versions, 0 / 0 or negative dry weights could silently produce inf or -inf that then gets replaced with NaN, but on others a warning is raised. The guard should be applied as a mask before the division or using where.

replicate column is float (1.0, 2.0, 3.0) not int

heldout_harmonized.csv:2 shows replicate=1.0. If the gold encodes replicate as integer, string comparison will fail. This is consistent with the zero-match result.


Security / Data Integrity

No security concerns identified. The anti-leakage audit reports 0 matches for forbidden paths, which is the key security requirement for this benchmark. The agent correctly restricted itself to fold-local sandbox files and raw data only.


Test Coverage

  • py_compile is verified (syntax only).
  • No unit tests for the UTM conversion function — given it's hand-rolled from a mathematical formula, even a single spot-check comparison against a known lat/lon would add confidence.
  • The evaluation harness itself has a gap (json_mapping_equivalence not computed), which should be addressed to make the benchmark results complete.

Minor Issues

  • harmonize_heldout.py:301 — the notes string is built with an f-string containing \n but is assigned in a single long line, making it hard to read/maintain.
  • AGENT_ACTION_LOG.md contains two log formats: manually-written markdown bullets (lines 1-27) and script-generated log lines (lines 38-64). The mixed format is inconsistent and could confuse automated log parsers.
  • mapping_notes.md references qc d1 and qc_flag but then notes "qc_flag is not part of the target CSV schema" — this notation could be clarified for readers unfamiliar with the QC flag system.

@claude claude Bot mentioned this pull request Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant