Skip to content

[codex] add fold10 agent evaluation bundle#32

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

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

Conversation

@jmchandonia

Copy link
Copy Markdown
Collaborator

Summary

  • add tracked fold-10 holdout-18 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 416 rows vs 420 gold rows
  • no keyed rows matched gold under evaluator key policy
  • generated JSON covered all 9/9 gold mapping categories, with 34/45 field-level exact matches

Validation

  • python -m py_compile evaluations/fold-10-holdout-18/agent_outputs/harmonize_heldout.py
  • targeted rg forbidden-path scan over evaluations/fold-10-holdout-18/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-10-holdout-18 Agent Evaluation Bundle

Tasks:

  • Gather context and understand request
  • Read changed files
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Review evaluation metrics
  • Post final review

Code Quality & Best Practices

harmonize_heldout.py

Overall the script is clean and follows a consistent pattern. A few observations:

  1. Hardcoded absolute paths (harmonize_heldout.py:6-8): Both RAW_DIR and OUT_DIR use absolute paths specific to the author's local machine (/h/jmc/..., /scratch/jmc/...). This makes the script non-reproducible on any other system. Since this is an evaluation artifact it may be intentional, but it's worth noting.

  2. Depth mapping bug — '15-May' entry (harmonize_heldout.py:42): The mapping correctly identifies that '15-May' is an Excel date-mangling artifact of '5-15 cm' and maps it to 0.10. However, the lookup dictionary already has '5-15 cm' -> 0.10 on line 41, so the two entries are identical in value. This is correct behavior but the comments in mapping_notes.md and curator_bundle.json both explain the reasoning well.

  3. site_id_from_row uses row.get() on a pd.Series (harmonize_heldout.py:23-27): pd.Series.get() works, but the standard idiom for pandas is row['col'] with explicit error handling, or using .apply() with loc. The current approach silently returns None for missing columns (treated as 'None' in the string), which could produce unexpected site_id values if source columns change. The str(part).strip() at line 34 handles the None'None' conversion, but a missing column would produce a key component of 'None' rather than raising an error.

  4. Filter-then-copy pattern (harmonize_heldout.py:62): out[out['gravimetric_water_content_gH2O_gs'].notna()].copy() is correct and avoids SettingWithCopyWarning. Good practice.

  5. import numpy as np but only used for np.nan. This is fine since pandas is already importing numpy transitively, but it could also use float('nan') or pd.NA consistently.

  6. No logging or error handling: The script runs silently. If the raw CSV is missing or malformed, it will raise an unhandled exception with no context. For a benchmark harness this is acceptable but worth noting.


Potential Bugs / Issues

  1. Key mismatch causing 0% row match (evaluation_metrics.json:19-33): The evaluator reports key_intersection: 0 — zero rows matched between agent and gold outputs. The unique_agent_keys: 398 vs unique_gold_keys: 393 and zero intersection strongly suggests the site_id construction differs from the gold standard. This is the most significant quality gap. The EVALUATION.md notes key_includes_dataset_index=False, suggesting the key policy may be a contributing factor, but the complete mismatch warrants investigation.

  2. Row count discrepancy (416 vs 420 gold): The curator_bundle.json explains 4 rows are dropped for missing gravimetric_water_content_gH2O_gs. If the gold standard retains those rows (with NaN values) rather than dropping them, that would explain the 4-row difference. This is a behavioral difference worth flagging.

  3. '15-May' depth handling (harmonize_heldout.py:42, depth_to_m): If the source CSV has already been modified to '5-15 cm' (the un-mangled form), then the '15-May' key would never match and those rows would map to np.nan and potentially be filtered out in downstream steps (though currently only NaN gravimetric values are filtered). This is a fragile dependency on a specific Excel corruption artifact.

  4. mapping.json DOI field (mapping.json:4): "doi": "UNKNOWN_FROM_ALLOWED_INPUTS" — this is a placeholder string, not null/None. If downstream consumers check for falsy values or compare type strictly, this could cause issues. The gold mapping presumably has a real DOI or null.


Performance Considerations

  • The script reads the full CSV once and processes it with standard pandas operations. For a 420-row dataset, performance is not a concern.
  • df.apply(site_id_from_row, axis=1) (harmonize_heldout.py:52) applies a Python function row-by-row, which is slower than vectorized operations. For this dataset size it's negligible, but for scale a vectorized string concatenation would be preferred.

Security Concerns

  • No security concerns found. The script reads a local CSV and writes output to a local directory. No network calls, no credential handling, no user input.
  • The forbidden-path scan result of 0 matches is consistent with the action log, which only accesses permitted fold-local paths and the explicitly allowed raw data directory.

Evaluation Metrics Review

Aspect Finding
Schema compliance ✅ All 9 target columns present, schema exactly matches
Anti-leakage ✅ Zero forbidden-path matches
Mapping categories ✅ 9/9 gold categories covered
Field-level exact matches ✅ 34/45 (75.6%)
Row precision / recall ⚠️ 0.0 / 0.0 due to zero key intersection
Row count ⚠️ 416 vs 420 gold (4 rows dropped for missing GWC)

The transformation field has the lowest match rate (2/9), which is expected given the ambiguity in describing transformation logic in free text. The complete key mismatch (0/393 gold keys recovered) is the main area to investigate.


Summary

This is a well-structured evaluation bundle. The harmonization script is readable and correct for the described task. The main concerns are:

  1. Zero row-level key intersection — the site_id construction or timestamp format likely differs from gold in a way that prevents any row-level matching, even though the data values appear correct.
  2. Hardcoded local paths make the script non-portable.
  3. Excel-mangled depth label '15-May' is a fragile edge case that deserves a comment in the code.

The evaluation infrastructure itself (metrics JSON, EVALUATION.md, MANIFEST.json, AGENT_ACTION_LOG.md) is thorough and consistent.

@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