Skip to content

[codex] add fold08 agent evaluation bundle#29

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

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

Conversation

@jmchandonia

Copy link
Copy Markdown
Collaborator

Summary

  • add tracked fold-08 holdout-10 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 and gold both had 309 rows, but no keyed rows matched under evaluator key policy
  • generated JSON matched all 7/7 top-level fields and covered all 8/8 gold mapping categories

Validation

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

Tasks:

  • Gather context and understand request
  • Read CLAUDE.md for repo guidelines (no CLAUDE.md found; used README.md)
  • Review all changed files
  • Post comprehensive review feedback

Overall Assessment

This PR adds a complete evaluation artifact bundle for fold-08-holdout-10. The artifacts are well-structured and consistent with the repository's evaluation framework. A few issues and observations are noted below.


harmonize_heldout.py — Code Quality

Hardcoded absolute paths (significant portability issue)

FOLD_DIR = Path('/scratch/jmc/data-harmonization-eval/.runs/fold-08-holdout-10')
RAW_PATH = Path('/h/jmc/ess-dive_wfsfa_soil_datasets/ess-dive-01092fc392bc46d-20240819T143818677/Soil_water_content_Fig4e.csv')
OUT_PATH = FOLD_DIR / 'agent_outputs' / 'heldout_harmonized.csv'

These paths (/scratch/jmc/..., /h/jmc/...) are machine-specific and will fail on any other system. The evaluations/fold-08-holdout-10/agent_outputs/heldout_harmonized.csv was generated from this script but the script itself cannot be re-run from the repository root. This is a reproducibility concern — the script is an artifact of the agent run, but if anyone tries to validate it independently, they'll need to update all paths manually.

Recommendation: Either document these as sandbox-local paths explicitly (e.g., a comment), or parameterize via CLI args or environment variables.

Timezone assumption

long['datetime_UTC'] = parse_local_to_utc(long['Date'], '%Y-%m-%d', 'America/Denver')

Date-only strings (no time component) are localized to America/Denver and converted to UTC, producing timestamps like 2017-05-06T06:00:00Z (UTC-6 offset). However, the source data (mapping_notes.md) doesn't explicitly state the timezone for this dataset — it's inferred from the PLM site location. This is reasonable domain knowledge, but should be documented more explicitly. Also, note that America/Denver observes daylight saving time (UTC-6 in summer, UTC-7 in winter), which the code handles implicitly via pytz/dateutil. The use of nonexistent='shift_forward' is a sensible choice, but ambiguous='NaT' would drop fall DST-overlap rows silently. For daily data this is practically a non-issue, but worth noting.

raw.iloc[1:] skips row without validation

data = raw.iloc[1:].copy()

The code assumes row 0 is always a units row, based on the note in mapping_notes.md. If the raw file ever changes format, this will silently corrupt data. A defensive assertion would improve robustness:

assert raw.iloc[0]['Date'] == 'YYYY-MM-DD', f"Unexpected units row: {raw.iloc[0]}"

interval_min for first observation per site is NaN

This is expected behavior and is correctly documented in mapping_notes.md. The CSV output leaves the first row per site with no interval, which is consistent. No issue here.

Regex for column selection

value_cols = [c for c in data.columns if re.search(r'vol_water_content', c, flags=re.IGNORECASE)]

This is functional and appropriately flexible. Note the source columns have a space before the underscore (PLM1 _vol_water_content) — the regex would match these correctly.

site_id extraction

long['site_id'] = 'ER-' + long['source_variable'].str.extract(r'(PLM\d+)')[0]

This correctly strips the space and trailing column suffix. If a column name didn't match PLM\d+, the extracted group would be NaN, silently producing ER-nan rows. These would then be filtered out by the out['site_id'].notna() check at line 72, so data loss would occur quietly. A warning log would be helpful.


evaluation_metrics.json — Metrics Interpretation

Key intersection = 0 despite matching row counts

The evaluation reports key_intersection: 0 with unique_agent_keys: 309 and unique_gold_keys: 103. The PR description notes: "no keyed rows matched under evaluator key policy" and references key_includes_dataset_index=False.

This is a significant finding worth clarifying. The gold data has 103 unique keys but the agent produced 309 rows (3 sites × 103 rows each). The key policy not including dataset_index means the key is likely (site_id, datetime_UTC) or similar. With 309 agent rows and 103 gold keys, a complete overlap would be expected if site_id/datetime matched — the zero intersection suggests either:

  1. The gold uses different site_id values (e.g., PLM1 vs ER-PLM1), or
  2. The gold timestamps differ (e.g., no UTC conversion, or different date format)

The EVALUATION.md acknowledges this with "Output equivalence should be interpreted with the key policy recorded in the metrics", but the root cause of the mismatch isn't explained. This makes the row_precision: 0.0 and gold_recall: 0.0 metrics uninterpretable without understanding whether it's a real content failure or a key normalization artifact.

Recommendation: Add a brief note to EVALUATION.md explaining the suspected cause of zero key intersection (e.g., site_id prefix difference, UTC conversion difference from gold).


mapping.json — Mapping Accuracy

transformation: 0/8 exact matches

The evaluation reports zero exact transformation string matches against gold. This is the weakest area of the mapping output. The transformations in mapping.json are written as natural language descriptions (e.g., "Skip the first units row, parse date strings with format '%Y-%m-%d' in America/Denver local time, and convert to UTC."), which are likely written in a different style than the gold standard. This is expected in generative evaluation but worth flagging as a systematic scoring issue — transformation descriptions may have high semantic overlap with gold but zero lexical overlap.

Extra agent category: gravimetric_water_content

The agent included a mapping entry for gravimetric_water_content (with null source pattern, populated as NA) which is not in the gold mapping. This is a minor over-specification — the agent correctly documents that the field is absent, even if gold omits this explicit no-op mapping.


curator_bundle.json — Curator Output

interval_min in time_series_inference

"interval_min": 20160.0,
"reasoning": "Each PLM site has repeated observations from 2017-05-06 to 2021-05-02; sampling is irregular but the median interval is 14 days."

The stated interval_min of 20160.0 matches 14 days (14 × 1440 = 20160), which is consistent with the reasoning. However, the harmonized CSV shows intervals as short as 2880 minutes (2 days) and as large as 24480 minutes (17 days), so 20160 is the median but not a fixed interval. The field name interval_min suggests a minimum interval, not the median — this semantic ambiguity in the schema could cause confusion.

readme_content: null

Acceptable given the package didn't have a parseable README, but worth noting that this could affect downstream curator quality scores.


AGENT_ACTION_LOG.md — Process Audit

The trace is well-structured and timestamps are consistent. Two external metadata lookups (entries 10–11) show a DNS failure on first attempt with a retry — this is documented appropriately. The trace ends at entry 25 (truncated with just 25→), which may be an artifact of how the log was written. If entry 25 represents the final validation step, consider ensuring it has a complete record.


EVALUATION.md — Documentation

The evaluation report is clear and the table format is consistent. The anti-leakage audit result (0 forbidden path matches) is appropriately prominent.

Minor: The PR body claims "agent and gold both had 309 rows, but no keyed rows matched" — but evaluation_metrics.json shows gold has 103 unique keys (not 309 rows via the key lens). The 309 rows figure for gold refers to the raw row count, while 103 is the unique-key count. This distinction should be made clearer, as row count equality (309/309) alongside zero key intersection can be misleading.


Security

No security concerns. No credentials, tokens, or sensitive paths beyond sandbox-local filesystem paths that are already machine-specific.


Summary

Area Status
Schema conformance ✅ All 9 target columns present
Code executability ✅ Verified with py_compile
Forbidden path audit ✅ 0 matches
JSON mapping coverage ✅ 8/8 gold categories
Row count match ✅ 309/309
Row-level equivalence ⚠️ 0/0 shared rows (key policy issue)
Transformation matches ⚠️ 0/8 (likely lexical vs semantic scoring gap)
Path portability ⚠️ Hardcoded sandbox paths in .py

The most actionable item is adding clarification to EVALUATION.md about the zero key-intersection root cause, to help readers distinguish between a genuine content failure and a key normalization artifact.

@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