DEV-1778: stamp consumed edited-models store provenance onto runs - #94
Conversation
Record which saved edited-models store STATE a run consumed via --apply-edited-models, so a graded result stays attributable after the per-task store is overwritten (latest-wins). - ConsumedEditedModels type + additive-optional consumed_edited_models field on SubmissionAnnotation (no schema_version bump; legacy annotations still validate). - store_content_fingerprint (content-manifest digest) captured race-free, pre-re-anchor, inside materialize_from_saved_store; apply_or_none now returns AppliedStore(scratch, store_fp). Best-effort with telemetry. - The single finalize hook stamps the record onto the row; threaded through every grade producer and the local + cloud grade call-sites (success and fail-everything paths). - driver.fetch aggregates the per-(db, instance_id) list into the run manifest before writing it; regrade re-stamps from the manifest so a regrade never erases the provenance.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR fingerprints applied edited-model stores, records provenance in task results and submission annotations, aggregates records into run manifests, and restores them during regrading. ChangesConsumed edited-model provenance
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EditedModelStore
participant OTFResolver
participant TaskData
participant Grader
participant SubmissionAnnotation
participant RunManifest
EditedModelStore->>OTFResolver: apply saved edited-model store
OTFResolver->>OTFResolver: compute store fingerprint
OTFResolver->>TaskData: record scratch path and fingerprint
TaskData->>Grader: pass consumed_edited_models
Grader->>SubmissionAnnotation: persist provenance
SubmissionAnnotation->>RunManifest: aggregate consumed records
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/cloud/test_consumed_edited_models_cloud_call_site.py`:
- Around line 10-51: Update test_cloud_call_site_stamps_consumed_on_annotation
to stub ray_app._grade_one_submission with a successful implementation that
receives and validates consumed_edited_models, then writes the annotation
deterministically. Ensure the test asserts that value is passed to the grading
call rather than relying on the fallback writer, and keep error-path coverage
for write_failed_submission_annotation in a separate forced-error test.
In `@tests/test_edited_models_apply.py`:
- Around line 296-297: Update the tar extraction block in the affected test to
gate filter="data" with hasattr(tarfile, "data_filter"), matching
materialize_from_saved_store. Use em._safe_extractall as the fallback on Python
3.11.0–3.11.3, while preserving filtered extraction on versions that support
data_filter.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ba68da39-ec4a-4d78-b8a9-ea4ab9dc0ec2
📒 Files selected for processing (22)
src/bird_interact_agents/agents/_edited_models_hook.pysrc/bird_interact_agents/agents/pydantic_ai_recursive/agent.pysrc/bird_interact_agents/cloud/driver.pysrc/bird_interact_agents/cloud/ray_app.pysrc/bird_interact_agents/eval/annotation_schema.pysrc/bird_interact_agents/eval/grade_in_place.pysrc/bird_interact_agents/eval/regrade.pysrc/bird_interact_agents/run.pysrc/bird_interact_agents/slayer_otf/edited_models.pysrc/bird_interact_agents/slayer_otf/runtime.pytests/cloud/test_consumed_edited_models_cloud_call_site.pytests/cloud/test_fetch_consumed_edited_models.pytests/slayer_otf/test_dev1672_hide_jsonb_stores.pytests/test_consumed_edited_models_call_sites.pytests/test_consumed_edited_models_fingerprint.pytests/test_consumed_edited_models_grade_threading.pytests/test_consumed_edited_models_hook.pytests/test_consumed_edited_models_manifest.pytests/test_consumed_edited_models_regrade.pytests/test_consumed_edited_models_schema.pytests/test_edited_models.pytests/test_edited_models_apply.py
- regrade: fall back to attempt-N.json's own consumed_edited_models when the manifest aggregate is absent (local runs), so a regrade never erases the provenance the annotation already had (Codex). - cloud call-site test: split into deterministic success/failure tests that assert _grade_one_submission / write_failed_submission_annotation each receive the consumed record, instead of relying on the fallback writer (CodeRabbit). - apply pre-re-anchor test: gate tarfile filter="data" on data_filter with the _safe_extractall fallback for Python 3.11.0-3.11.3 (CodeRabbit).
Only accept attempt-N.json's consumed_edited_models when its db/instance_id match the task being regraded (mirrors the manifest path), so a stale or mismatched attempt record is never stamped onto the wrong annotation (Codex).
Closes DEV-1778.
Why
With
--apply-edited-models, a run's model definitions come from a saved per-taskedited_models.tar.gzstore that changes between runs (latest-wins overwrite). Nothing on the result recorded which state of that store produced it, so a pass/fail couldn't be reproduced or attributed once the store moved on. This is the one transferable idea from PR #61 / DEV-1605 (consumed_reference), adapted to the edited-models store. Everything else in DEV-1605 (versioned OTF layout, flat-fallback abolition) is out of scope.What
ConsumedEditedModels{db, instance_id, store_fp}+ additive-optionalconsumed_edited_modelsfield onSubmissionAnnotation. Noschema_versionbump; legacy annotations still validate.store_content_fingerprint()= sha256 over the sorted-JSON ofcontent_manifest(root)(reuses the change-detector's exclusion set). Content-deterministic, gzip-mtime/order independent. Captured race-free insidematerialize_from_saved_storefrom the freshly-extracted scratch, before re-anchor/hide (so it identifies the consumed archive content, machine-portable). Best-effort — a failure degrades tostore_fp=Noneand logs.apply_or_none/materialize_from_saved_storenow returnAppliedStore(scratch, store_fp).row["consumed_edited_models"](only when identity + fp present); threaded through all 5 grade producers and both the local (run._grade_local_row) and cloud (ray_app._run_one_in_actor) grade call-sites, on the success and fail-everything paths (a task that applies then fails still records what it consumed).driver.fetchaggregates the per-(db, instance_id)list into the run manifest before writing it; local runs rely on the per-task annotation (no local manifest exists), and cloud derives its aggregate from the same annotations, so the two agree.consumed_edited_modelsfrom the run manifest (same sourceversion/agent_modelalready use), so a regrade never silently erases the provenance.Scope / non-goals
Stamp only on successful apply (fresh-cache / save-only / raw runs →
None). No store snapshotting for replay; no new cloud submit flag; noresults_dbchange; no DEV-1605 layout work.Tests
9 new test files (49 tests): fingerprint determinism + pre-re-anchor proof + best-effort failure; schema round-trip + legacy-validates; hook stamping + identity guards; grade-producer threading across all three annotation branches; collect/dedupe (sorted first-seen); cloud fetch aggregation (proven to precede the manifest write) + merge round-trip; regrade re-stamp; and end-to-end local + cloud grade call-site integration (success + grader-failure). Plan and tests were each Codex-reviewed; all findings folded.
Full non-integration suite green (4610 passed). The only failures on this machine are the pre-existing environmental
test_slayer_models_loadablecases (missing local mini-interact data), which fail identically onmain.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests