Skip to content

[codex] add fold09 agent evaluation bundle#30

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

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

Conversation

@jmchandonia

Copy link
Copy Markdown
Collaborator

Summary

  • add tracked fold-09 holdout-17 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 225,524 rows vs 226,080 gold rows
  • key precision 1.000 and gold recall 0.998
  • shared-row VWC matched 225,556/225,564; interval matched 225,502/225,564
  • generated JSON matched all 7/7 top-level fields and covered all 7/7 gold mapping categories

Validation

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

Tasks:

  • Gather context on PR
  • Read evaluation files
  • Review Python harmonization script
  • Review JSON artifacts
  • Review documentation files
  • Provide comprehensive feedback

Summary

This PR adds a well-structured agent evaluation bundle for fold-09-holdout-17 with strong overall metrics (precision 1.000, recall 0.998). The artifacts are organized clearly and the evaluation methodology is sound. Below are specific observations by file.


harmonize_heldout.py — Code Quality

Hardcoded absolute paths (significant portability issue)

Lines 8–13 contain machine-specific absolute paths:

FOLD_DIR = Path('/scratch/jmc/data-harmonization-eval/.runs/fold-09-holdout-17')
BASE_DIR = Path('/h/jmc/ess-dive_wfsfa_soil_datasets')

These paths are non-portable and will fail for anyone trying to re-run the script outside the original machine. If reproducibility of the harmonization script is a goal for this benchmark, consider parameterizing these via CLI args or environment variables. This is especially notable because the evaluation framework appears to be designed for sandboxed execution, yet the script encodes the sandbox sandbox path directly.

Mutation of DataFrame slice (x)

Line 59–60:

x = df[['DateTime'] + vwc_cols].copy()
x['datetime_UTC'] = parse_local_to_utc(...)

The .copy() is correct here — no issue. This pattern is properly followed throughout.

interval_min computation via .diff()

Line 81:

long['interval_min'] = long.groupby(['site_id', 'depth_m'])['datetime_UTC'].diff().dt.total_seconds() / 60.0

The first row of each group will be NaN since .diff() produces no value for it. This is the expected behavior for a difference-based interval computation, and it explains the interval_min mismatch: 225502/225564 (62 mismatches ≈ 5 sites × 4 depths × ~3 groups, consistent with leading NaN values). This is a known limitation worth noting in the methodology — the gold standard appears to backfill or otherwise handle the first-row NaN.

skiprows=[1] skips the units header row

Line 56:

df = pd.read_csv(path, skiprows=[1])

This correctly skips the units row (second row, index 1). This is a subtle but important detail that the agent got right.

No explicit encoding specified for CSV reads

pd.read_csv uses the system default encoding; adding encoding='utf-8' would make the behavior explicit and portable.

trace() opens the log file on every call

Lines 34–37: Each trace() call opens and closes the file. For the small number of trace calls in this script this is fine, but worth noting as a design pattern.


mapping.json — Content

transformation field: 0/7 exact matches (noted in EVALUATION.md)

The agent's transformation descriptions are verbose and semantically accurate, but don't exactly match the gold standard strings. This is expected for free-text fields and the 0/7 exact match metric is somewhat misleading — the substance is correct as evidenced by the output equivalence. Consider whether fuzzy/semantic matching would give a more meaningful metric here.

Extra soil_water_potential category

The agent included a soil_water_potential mapping category that isn't in the gold standard. This is a valid addition (the category IS handled in the data, just populated with NA), but it inflates the category count. The extra_agent_categories field in evaluation_metrics.json correctly captures this.

source_pattern for depth: "MC_jm" vs regex

The gold standard uses "MC_jm" as the source pattern description (where j is a numeric placeholder). The agent script uses the regex r'MC_\d+(?:\.\d+)?m' which is more precise. This is a semantic difference in how the pattern is expressed, not a correctness issue.


evaluation_metrics.json — Structure

Well-structured and machine-readable. A few observations:

  • key_includes_dataset_index: false is important context for interpreting key precision/recall — this policy should be documented or linked from EVALUATION.md (it's mentioned but could be more prominent)
  • common_rows: 225564 > key_intersection: 225504 — this difference (60 rows) indicates duplicate keys in either agent or gold output that are being counted separately in the shared-row analysis. This is worth investigating to understand if there are legitimate duplicate timestamps within a site/depth group.

curator_bundle.json — Content

  • Well-formed and complete
  • The open_questions field (lines 294–297) is a useful transparency mechanism — the UTM coordinate issue and missing README are noted
  • similar_dataset_reference pointing to index 1 is correctly reasoned ("structurally similar wide-format")
  • readme_content: null is an honest representation of the missing README

Documentation

  • AGENT_ACTION_LOG.md: Clean and detailed trace. The DNS failure for the ESS-DIVE API (lines 34–35) is faithfully recorded, showing the agent correctly fell back to local data
  • EVALUATION.md: Clear and concise. The note about key_includes_dataset_index=False policy could be expanded slightly
  • mapping_notes.md: Accurate; correctly identifies the hourly averaging from 15-minute measurements
  • MANIFEST.json: Well-structured; note that n_exemplars: 27 but exemplar_indices has 18 entries — this appears to be a discrepancy worth investigating

Potential Bugs / Issues

  1. n_exemplars mismatch (MANIFEST.json): n_exemplars: 27 but only 18 indices listed. This is likely an error in the manifest generation.

  2. 556 missing gold rows: The recall gap is well within acceptable range but the missing rows are not explained. Given the interval_min NaN pattern, some may be due to dropped first-interval rows, but 556 >> (5 sites × 4 depths = 20 expected NaN intervals). This gap likely reflects actual missing time periods in the source data that the gold standard handles differently (e.g., gap-filling).

  3. 62 interval_min mismatches: The 62 mismatches in interval_min (225502/225564) likely correspond to the leading NaN values for each site/depth group (20 groups possible across 5 sites × 4 depths), but 62 suggests some groups start from a non-NaN position in the gold. This would be worth a quick diagnostic.


Security

No security concerns. The script reads local CSVs and writes a local output — no external data sources, credentials, or network calls at runtime. The DOI fetch in the action log was attempted at curation time (not run time) and failed gracefully.


Performance

The script processes ~225K rows across 5 files using pandas — appropriate and efficient for this data size. The melt() and groupby() operations are standard and performant.


Minor Suggestions

  • Add encoding='utf-8' to pd.read_csv() calls for portability
  • Consider documenting the hardcoded path issue in a comment within the script so future evaluators know the script is environment-specific
  • Clarify the n_exemplars field in MANIFEST.json
  • Add a brief note in EVALUATION.md explaining why common_rows > key_intersection

@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