Repository navigation
Conversation
Validate every current-run chunk before cache writes and reconstruct incremental merge payloads from current dispatch, walked owned cache entries, and structural inputs. Foreign stubs and stale aggregates must not replace undispatched files. Fixes Graphify-Labs#3947 AI-Assisted-By: OpenAI Codex
|
Thanks for the pull request, @xiehuanyi. 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.
Graphify review — findings
Replaces the hand-rolled chunk numbering and glob merge in every generated skill with controller helpers. prepare_semantic_dispatch now records each chunk's exact FILE_LIST and absolute CHUNK_PATH under a run-scoped manifest, and collect_semantic_dispatch merges only that run's chunks, so stale chunk files from earlier runs never fill a gap. All current-run chunks are validated before any cache or merge write: a foreign source_file, missing provenance, or malformed JSON stops the run for re-extraction, while missing outputs only warn unless more than half are absent; fully cached runs still go through B3 with an empty dispatch.
Worth a look
- Chunk paths may resolve under the scan root instead of cwd (#1392 regression) —
graphify/skill-amp.md:282· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
- B1 chunking never merges small directories, so one subagent is dispatched per directory —
graphify/skill-agents.md:280· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Review partial — this diff was larger than one review pass covers, so later files were not reviewed; some findings may be missing.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 948 functions depend on the 948 functions this change touches.
Health — this change adds coupling hotspots:
- new:
load_skill_update_extraction()— 4 callers, 5 callees - new:
test_generated_update_ignores_stale_payload_and_loads_only_owned_cache()— 0 callers, 7 callees - new:
test_generated_cached_assembly_reads_owned_chunks_not_old_aggregate()— 0 callers, 6 callees - new:
test_generated_update_reports_prefix_but_keeps_known_external_reference()— 0 callers, 6 callees
Verification — 948 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: 948 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
330 of 330 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/test_affected_cli.py— full-run-safetytests/test_affected_member_seed.py— full-run-safetytests/test_agents_platform.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_benchmark.py— full-run-safetytests/test_benchmark_raw_graph.py— full-run-safetytests/test_blade_extractor.py— full-run-safetytests/test_build.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_cache.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_cluster.py— full-run-safetytests/test_cluster_exclude_hubs.py— full-run-safetytests/test_cobol_extractor.py— full-run-safetytests/test_codebuddy.py— full-run-safetytests/test_community_hub_labels.py— full-run-safetytests/test_community_labels_skill.py— full-run-safetytests/test_confidence.py— full-run-safetytests/test_corrupt_graph_json.py— full-run-safetytests/test_cpp_method_declarations.py— full-run-safetytests/test_cpp_nested_and_cli.py— full-run-safetytests/test_cpp_objc_cross_file_calls.py— full-run-safetytests/test_cpp_preprocess.py— full-run-safetytests/test_cross_extension_reexport_self_cycle.py— full-run-safety- … and 280 more
non-code file(s) changed (
graphify/skill-agents.md,graphify/skill-amp.md,graphify/skill-claw.md,graphify/skill-codex.md,graphify/skill-copilot.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-agents.md,graphify/skill-amp.md,graphify/skill-claw.md,graphify/skill-codex.md,graphify/skill-copilot.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.
Docs that may be stale (advisory)
CHANGELOG.md§ 0.9.75 (2026-10-04) (lines 24-44): references changed symbolsnodeCHANGELOG.md§ 0.9.73 (2026-09-30) (lines 61-76): references changed symbolsnodeCHANGELOG.md§ 0.9.65 (2026-09-20) (lines 162-174): references changed symbolsnodeCHANGELOG.md§ 0.9.64 (2026-09-18) (lines 175-186): references changed symbolsnodeCHANGELOG.md§ 0.9.62 (2026-09-15) (lines 195-213): references changed symbolsnodeCHANGELOG.md§ 0.9.39 (2026-08-10) (lines 506-513): references changed symbolsnodeCHANGELOG.md§ 0.9.37 (2026-08-08) (lines 523-530): references changed symbolsnodeCHANGELOG.md§ 0.9.35 (2026-08-06) (lines 538-546): references changed symbolsnodeCHANGELOG.md§ 0.9.22 (2026-07-20) (lines 679-688): references changed symbolsnodeCHANGELOG.md§ 0.9.19 (2026-07-18) (lines 709-719): references changed symbolsnode
…and 10 more.
· 4 grounded finding(s) anchored inline below.
| return merged | ||
|
|
||
|
|
||
| def load_skill_update_extraction( |
There was a problem hiding this comment.
load_skill_update_extraction()
high coupling complexity (Ca·Ce = 20).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| collect_semantic_dispatch(tmp_path) | ||
|
|
||
|
|
||
| def test_generated_cached_assembly_reads_owned_chunks_not_old_aggregate(tmp_path, monkeypatch): |
There was a problem hiding this comment.
test_generated_cached_assembly_reads_owned_chunks_not_old_aggregate()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| assert (output / ".graphify_semantic.json").read_bytes() == before | ||
|
|
||
|
|
||
| def test_generated_update_reports_prefix_but_keeps_known_external_reference(tmp_path, monkeypatch, capsys): |
There was a problem hiding this comment.
test_generated_update_reports_prefix_but_keeps_known_external_reference()
fans out to 6 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
| assert (output / "graph.json").read_bytes() == graph_before | ||
|
|
||
|
|
||
| def test_generated_update_ignores_stale_payload_and_loads_only_owned_cache(tmp_path, monkeypatch): |
There was a problem hiding this comment.
test_generated_update_ignores_stale_payload_and_loads_only_owned_cache()
fans out to 7 callees (efferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Thanks @xiehuanyi — the diagnosis of #3947 is correct: the skill runbook builds The blocker is scope. At +2479/-927 with a brand-new packaged module ( Could you split this into a focused change that extends the existing |
Keep fresh source ownership checks before caching and update merges, on current v8. AI assistance: Codex drafted the patch and regression tests. Co-Authored-By: Codex <noreply@openai.com>
Reconcile the earlier PR head by keeping the tested focused tree on v8, following maintainer scope feedback. The earlier packaged helper and dispatch changes are superseded. Co-Authored-By: Codex <noreply@openai.com>
xiehuanyi
changed the title
When a semantic subagent emits a stub attributed to an undispatched file,
build_mergecan replace that file’s rich existing nodes even while the total graph grows. The generated Step B3 now applies the existingscope_semantic_resultguard to each fresh result, using.graphify_uncached.txt, before cache writes and assembly with cached or AST data.Refs #3947.
The shared extraction prompts also require edge-only external references with complete known IDs, and the update runbook documents the destructive
source_filekey. The implementation is seven lines in the shared collection fragment, plus prompt/runbook guidance and generated copies. It uses currentv8(5c7b847).Validation:
v8before the guard and pass afterward for relative and absolute source paths. They run the actual B3, Part C and update merge blocks, preserving 23 rich foreign nodes despite a growing incoming graph, a valid external edge and cached nodes.v8; the normalized logs are identical, with no new diagnostics.Boundary: this follows the existing library guard’s dispatched-set policy. It does not introduce a per-chunk ownership protocol or a central target-ID resolver. The existing library policy for records without
source_fileremains applicable.AI assistance: Codex drafted the implementation and regression tests; disclosed in commit metadata.