Repository navigation
fix(skill): persist Aider/Devin update manifests (#4812) - #4817
legendteddy wants to merge 1 commit into
Conversation
Save the incremental manifest after successful Aider and Devin monolith updates so later --update runs use the current baseline. Build the stamp set from this run's fresh extraction and clear semantic hashes for dispatched files that produced no output, matching the split-host update invariant. Add an executable rendered-skill regression for both monoliths and regenerate their committed artifacts. Co-Authored-By: ChatGPT <noreply@openai.com>
|
Thanks for the pull request, @legendteddy. A maintainer will review it soon. Want to talk it through while it is in review? Come join us on our Discord server. For longer-form discussion there is also GitHub Discussions. A couple of things that speed up review: make sure the test suite passes on Python 3.10 and 3.13, and that the change keeps extraction deterministic. |
![graphify-labs[bot]](https://avatars.githubusercontent.com/in/4356270?s=60&v=4){kind=small}
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Not checked: tests were not run; no formal proof of the changed code.
Graphify review — findings
Fixes the Aider and Devin monolith --update runbooks so a merge advances the manifest baseline: the merge snippet now calls save_manifest with the stamped files and clears semantic entries for dispatched documents, papers and images that earned no stamp. Previously --update never saved a manifest, so edited files were re-queued on every run, and because the save runs inside the merge snippet the baseline moves before the merged graph is written out. Adds a test that executes the rendered merge block against a one-document corpus and asserts the changed file is no longer queued, and relaxes the step-body parser to accept a closing quote with trailing whitespace.
Worth a look
- Manifest baseline saved before the merged graph is persisted —
tools/skillgen/fragments/core/aider.md:901· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
- Update-manifest fix applied to Devin fragment only, not Aider —
tools/skillgen/fragments/core/devin.md:1012· Escalate · medium- NOT verified (no proof, no reproducing execution) — needs human review
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 493 functions depend on the 475 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
render()— 15 callers, 5 callees - new:
test_monolith_update_saves_manifest_for_changed_file()— 0 callers, 9 callees
Verification — 493 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 493 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
465 of 465 test file(s) selected (100%) via static blast radius.
Escalated to a full run for safety — the selection is not trustworthy on its own (see below). CI should run the whole suite.
tests/pi.test.ts— full-run-safetytests/test_affected_cli.py— full-run-safetytests/test_affected_defines.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_affected_overrides.py— full-run-safetytests/test_affected_path_file_node.py— full-run-safetytests/test_agents_platform.py— full-run-safetytests/test_alias_folding.py— full-run-safetytests/test_analyze.py— full-run-safetytests/test_anthropic_custom_endpoint.py— full-run-safetytests/test_antigravity_install.py— full-run-safetytests/test_apm_fallback_version.py— full-run-safetytests/test_architecture_doc.py— full-run-safetytests/test_astro_extraction.py— full-run-safetytests/test_astro_import_ids.py— full-run-safetytests/test_atomic_canvas_export.py— full-run-safetytests/test_atomic_version_stamp.py— full-run-safetytests/test_atomic_writes.py— full-run-safetytests/test_backend_env_isolation.py— full-run-safetytests/test_backend_extras.py— full-run-safetytests/test_backend_opt_in.py— full-run-safetytests/test_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_benchmark_real_query.py— full-run-safetytests/test_binary_media_words.py— full-run-safetytests/test_blade_extractor.py— full-run-safetytests/test_build.py— full-run-safetytests/test_build_located_semantic_identity.py— full-run-safetytests/test_build_merge_dedup_scope.py— full-run-safetytests/test_build_merge_hyperedges_and_prune.py— full-run-safetytests/test_build_merge_shrink_guard.py— full-run-safetytests/test_builtin_global_type_refs.py— full-run-safetytests/test_c_include_resolution.py— full-run-safetytests/test_cache.py— full-run-safetytests/test_cache_long_paths.py— full-run-safetytests/test_cache_stale_import_target.py— full-run-safetytests/test_callflow_html.py— full-run-safetytests/test_cargo_introspect.py— full-run-safetytests/test_cargo_missing_manifest.py— full-run-safetytests/test_carried_hyperedge_remap.py— full-run-safetytests/test_case_sensitive_resolution.py— full-run-safetytests/test_charmap_encoding.py— full-run-safetytests/test_chunking.py— full-run-safetytests/test_cjs_module_extension.py— full-run-safetytests/test_claude_cli_backend.py— full-run-safetytests/test_claude_md.py— full-run-safetytests/test_cli_broken_pipe.py— full-run-safetytests/test_cli_export.py— full-run-safetytests/test_cli_help.py— full-run-safetytests/test_cloud_cta.py— full-run-safety- … and 415 more
non-code file(s) changed (
graphify/skill-aider.md,graphify/skill-devin.md,tools/skillgen/expected/graphify__skill-aider.md,tools/skillgen/expected/graphify__skill-devin.md,tools/skillgen/fragments/core/aider.md…) → running the full suite for safety (a code graph can't see config/fixture/data deps)
changed code file(s) with no mapped test (
graphify/skill-aider.md,graphify/skill-devin.md,tools/skillgen/expected/graphify__skill-aider.md,tools/skillgen/expected/graphify__skill-devin.md,tools/skillgen/fragments/core/aider.md…) — a coverage gap or a missing link — running the full suite rather than only the selected tests
Selection is safe under the controlled-regression assumption; always-run tests + a periodic full run are the backstops. Advisory — it never changes the check verdict.
· 1 grounded finding(s) anchored inline below; 1 more finding(s) on lines outside this diff (see the check run).
|
|
||
|
|
||
| @pytest.mark.parametrize("key", ["aider", "devin"]) | ||
| def test_monolith_update_saves_manifest_for_changed_file(key, tmp_path, monkeypatch): |
There was a problem hiding this comment.
test_monolith_update_saves_manifest_for_changed_file()
fans out to 9 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
What does this PR do?
Fixes #4812.
Aider and Devin's monolith
--updatepaths detected and merged incremental changes but never saved the manifest. The next update therefore diffed against the previous full-build baseline, re-queueing files that had already been handled and re-reporting deletions.This adds the same manifest-persistence invariant used by the split-host update runbook:
The save uses
new_extractionbefore any later step can reinterpret a merged graph as fresh output.Type of change
Verification & Invariants
Invariant: after a successful Aider/Devin
--update, a changed file that produced fresh output is no longer queued on the next unchanged run. A semantic file that produced no current-run output remains unstamped/retryable.CONTRIBUTING.mdandARCHITECTURE.md.How was this tested?
The 15 full-suite failures are environment/baseline failures on this Windows checkout: 7 require the optional
tree_sitter_sql, 4 require a nativesh, 2 Aider/Devin code-only tests cannot execute the Windows Python path through bash, and 2 memoization tests show the same Windows/getcwd behavior on cleanv8. The failure set is identical before and after this change.Limitations
This PR fixes only Aider/Devin manifest persistence. It does not change the split-host update path or the separate
.graphify_extract.json/Step 9 reuse issue tracked in #4811 / #2844.Graphify-specific checklist