Skip to content

[codex] add fold12 agent evaluation bundle#35

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

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

Conversation

@jmchandonia

Copy link
Copy Markdown
Collaborator

Summary

  • add tracked fold-12 holdout-24 sub-agent artifacts, including action trace and generated outputs
  • add an evaluation report and machine-readable metrics against held-out gold

Evaluation Highlights

  • targeted forbidden-path scan found zero matches
  • agent produced 5,736 rows vs 3,204 gold rows
  • recovered all gold keys with 1.000 recall but had 2,532 extra keys, precision 0.559
  • all shared-row target values matched 3,204/3,204
  • generated JSON matched all 7/7 top-level fields and covered all 8/8 gold mapping categories

Validation

  • python -m py_compile evaluations/fold-12-holdout-24/agent_outputs/harmonize_heldout.py
  • targeted rg forbidden-path scan over evaluations/fold-12-holdout-24/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: fold-12 agent evaluation bundle

Tasks:

  • Gather context and analyze PR
  • Review evaluation files (MANIFEST, EVALUATION, AGENT_ACTION_LOG)
  • Review agent outputs (harmonize_heldout.py, mapping.json, curator_bundle.json)
  • Review metrics and evaluation data
  • Post comprehensive feedback

Overall Assessment

This PR correctly bundles a sub-agent evaluation artifact for held-out dataset 24. The evaluation infrastructure is sound, the action trace is complete, and the performance metrics are clearly reported. A few observations worth noting:


harmonize_heldout.py

Hardcoded absolute paths (harmonize_heldout.py:8-15):

SANDBOX = Path('/scratch/jmc/data-harmonization-eval/.runs/fold-12-holdout-24')
BASE_DIR = Path('/h/jmc/ess-dive_wfsfa_soil_datasets')

The script is non-portable as-is — it will fail on any machine other than the original author's. This is acceptable if the script is purely a tracked artifact (not expected to be re-run), but worth noting. Consider documenting this expectation in the README or MANIFEST if re-execution is not intended.

Row count discrepancy (harmonize_heldout.py:59-88):
The agent produces 5,736 rows vs 3,204 gold rows — a difference of 2,532. Inspection of the CSV shows water_potential_kPa values are empty for rows in P1,0.15 in early 2022. The agent includes rows even where water potential has NaN values (no filtering of null measurement rows). The gold standard may filter out null-measurement rows. This accounts for the lower precision (0.559) and explains the agent_extra_keys: 2532 in evaluation_metrics.json. The agent could consider dropping rows where all measurement columns are null.

Interval computation produces NaN for first rows (harmonize_heldout.py:83-88):
groupby(...).diff() produces NaT for the first row of each group, leaving interval_min empty for the first observation of each site-depth pair. The CSV confirms this (line 2: 2022-01-01 07:00:00+00:00,P1,0.15,1,True,,,,). This is a known limitation noted in mapping_notes.md, so it's properly documented.

Location validation logic (harmonize_heldout.py:90-93):

valid_sites = set(loc['ID'].astype(str))
missing_sites = sorted(set(long['site_id'].astype(str)) - valid_sites)
if missing_sites:
    raise ValueError(...)

Good defensive check. However, loc['ID'] column name is assumed — if the Locations.csv header differs, this silently reads an incorrect column. The curator_bundle.json confirms the column is indeed ID, so this is fine for this dataset, but brittle in general.

No filtering of non-MP columns (harmonize_heldout.py:63-66):
x = df[['Timestamp'] + mp_cols].copy() correctly drops non-relevant columns before melting. Good.


mapping.json

Extra gravimetric_water_content category (mapping.json:98-107):
The agent includes a gravimetric_water_content mapping entry that is flagged as extra_agent_categories in evaluation_metrics.json. This doesn't harm correctness (the destination column is still populated with NA), but indicates the agent included an explicit NA mapping that the gold standard omits. This is a minor schema divergence worth tracking in future folds.

transformation field: 0/8 exact matches (evaluation_metrics.json:55):
All 8 transformation descriptions differ from gold. These are natural-language fields so fuzzy mismatch is expected, but a 0/8 exact match rate suggests the agent's phrasing is systematically different from the gold standard's canonical wording. This may be worth monitoring across folds.

source_pattern field: 5/8 matches (evaluation_metrics.json:54):
Three source patterns don't match gold exactly. Given the regex-like P[12]_MP_j format used, the mismatches are likely minor formatting variations (e.g., j vs {depth} or similar). The functional meaning appears correct.


AGENT_ACTION_LOG.md

DNS failure noted but gracefully handled (lines 38-40):
The agent attempted to fetch ESS-DIVE API metadata but encountered a sandbox DNS failure. It correctly continued using local data. The anti-leakage scan (line 64) was still run and passed with 0 matches. This is good resilience.

Two API search attempts (lines 38-40):
The agent tried the ESS-DIVE API twice (lines 38 and 40) despite the first returning a DNS failure — minimal concern, but the log clearly shows the agent understood the failure reason before retrying.


EVALUATION.md

key_includes_dataset_index=False policy note (EVALUATION.md:52):
The last line points readers to evaluation_metrics.json for the key policy. The key design choice to exclude dataset_index from join keys is documented only in the JSON and not in EVALUATION.md itself. Adding a brief note explaining this directly in EVALUATION.md would improve clarity.


evaluation_metrics.json

Well-structured and machine-readable. All fields are present and self-consistent with EVALUATION.md. No issues.


curator_bundle.json

row_count_estimate: 956 while the harmonized CSV has 5,736 rows:
The curator bundle records row_count_estimate: 956 for the source file, which is the number of raw rows (one row per timestamp). After melting 6 MP columns × ~956 rows, you get ~5,736 long-format rows. This is correct behavior, but calling it an "estimate" when it's an exact count from a profiling read is slightly misleading.

readme_content: null (curator_bundle.json:91):
The open question in mapping_notes.md mentions that no data dictionary was present locally. The kPa unit assignment is inferred — this is appropriately flagged as an open question.


Security / Safety

No security concerns. The action log confirms a targeted forbidden-path scan returned 0 matches. The harmonization script does not access network resources, write to unexpected paths, or execute dynamic code. No credentials or secrets are embedded.


Summary

Area Status
Anti-leakage compliance ✅ Pass (0 forbidden-path matches)
Schema correctness ✅ Pass (9/9 target columns)
Gold recall ✅ 1.000 (all gold keys recovered)
Row precision ⚠️ 0.559 (2,532 extra rows — likely null-measurement rows not filtered)
JSON mapping top-level ✅ 7/7 fields match
JSON mapping categories ✅ 8/8 gold categories present (+1 extra agent category)
transformation descriptions ⚠️ 0/8 exact matches (natural-language phrasing divergence)
Script portability ⚠️ Hardcoded absolute paths (acceptable if artifact-only)

The main actionable finding is the 2,532 extra rows from non-null NaN-measurement rows not being filtered, which drives the lower precision. All other issues are minor documentation or style concerns.

@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