Skip to content

fix(extract): preserve existing semantic layer on --code-only --force - #2925

Closed
santhiprakash wants to merge 2 commits into
Graphify-Labs:v8from
santhiprakash:fix/code-only-force-merge
Closed

fix(extract): preserve existing semantic layer on --code-only --force#2925
santhiprakash wants to merge 2 commits into
Graphify-Labs:v8from
santhiprakash:fix/code-only-force-merge

Conversation

@santhiprakash

Copy link
Copy Markdown
Contributor

What

graphify extract --code-only --force previously rewrote graph.json with only the AST tier, silently dropping every doc/paper/image node plus its connected hyperedges. --force disables incremental mode so the merge path that would otherwise carry the surviving semantic tier forward never ran.

A code-only run cannot touch the semantic tier at all (no LLM dispatch), so discarding the existing semantic layer is a destructive side effect with no correctness justification.

Why

Reproduces per issue #2923:

graphify extract . --backend claude-cli     # 6 nodes (2 code + 4 doc/concept)
graphify extract . --code-only --force     # 2 nodes — 4 doc/concept lost

The second command's user-facing output (skipping 1 non-code file(s) — no LLM extraction) reads as a cost-saving notice, not a data-loss warning. The semantic layer is only the only record of those nodes for any subsequent query, and the only recovery is the timestamped backup folder the tool itself dropped to disk — exactly the path graphify update and the git-hook _rebuild_code correctly avoid.

Fix

Re-enable the incremental merge when --force and --code-only are combined and an existing graph.json is present. The AST tier is still fully replaced (full re-scan, semantic cache reads skipped); doc/paper/image nodes are carried forward via build_merge / merge_raw_extraction. graph_stale_sources still prunes semantic nodes for files deleted from disk between the prior extract and this one, so the merge cannot resurrect nodes for sources that no longer exist.

A new [graphify extract] --force --code-only: full AST re-scan, existing semantic layer preserved (no semantic pass this run) console line announces the merge path so the user can see the combination did the right thing.

Verification

  • New: test_code_only_force_preserves_existing_semantic_layer — seeded graph with AST + SEMANTIC nodes; --code-only --force preserves the SEMANTIC tier. Without the fix the assertion fires (semantic nodes remaining: set()).
  • New: test_code_only_force_prunes_removed_semantic_filesNOTES.txt deleted between seed and re-run; its semantic nodes are pruned (graph_stale_sources catches the dead file).
  • Existing tests/test_extract_code_only_cli.py (12 tests) all green.
  • uv run pytest tests/test_extract.py tests/test_build.py tests/test_build_merge_hyperedges_and_prune.py tests/test_build_merge_shrink_guard.py tests/test_extract_cli.py tests/test_extraction_spec_ids.py → 393 passed, 4 skipped.
  • uv run ruff check graphify/cli.py tests/test_extract_code_only_cli.py → clean.
  • uv run ruff format --check → only pre-existing reformat churn (none of the lines this PR touches).

Closes

#2923

…Graphify-Labs#2923)

graphify extract --code-only --force previously rewrote graph.json with only
the AST tier, silently dropping every doc/paper/image node plus its connected
hyperedges. --force disables incremental mode so the merge path that would
otherwise carry the surviving semantic tier forward never ran.

A code-only run cannot touch the semantic tier at all (no LLM dispatch), so
discarding the existing semantic layer is a destructive side effect with no
correctness justification. Re-enable the incremental merge when --force and
--code-only are combined and an existing graph.json is present; the AST tier
is still fully replaced (full re-scan, semantic cache reads skipped) while
doc/paper/image nodes are carried forward via build_merge / merge_raw_extraction.

graph_stale_sources still prunes semantic nodes for files deleted from disk
between the prior extract and this one, so the merge cannot resurrect
nodes for sources that no longer exist.

Adds two regression tests in test_extract_code_only_cli.py:
- test_code_only_force_preserves_existing_semantic_layer: seeded graph with
  AST + SEMANTIC nodes; verifies the SEMANTIC tier survives --code-only --force.
- test_code_only_force_prunes_removed_semantic_files: deletes NOTES.txt
  between seed and re-run; verifies its semantic nodes are pruned, not
  resurrected, by the merge.

Ref: Graphify-Labs#2923

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

Fixes --force --code-only in dispatch_command silently erasing the existing semantic layer: when a graph.json exists, it now forces incremental merge so doc/paper/image nodes and their hyperedges carry forward while the AST tier is fully re-scanned, and prints a line explaining the branch. Adds two regression tests in test_extract_code_only_cli.py covering surviving semantic nodes and pruning of semantic nodes for files deleted from disk.

Worth a look

  • --force --code-only no longer produces a code-only fresh graphgraphify/cli.py:3139 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • --force --code-only re-enables incremental_mode, silently reactivating manifest-gated AST cache reads that --force was meant to bypassgraphify/cli.py:3142 · Escalate · medium
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 262 functions depend on the 76 functions this change touches.

Health — this change adds coupling hotspots:

  • new: dispatch_command() — 2 callers, 119 callees
  • new: _stale_graph_sources() — 7 callers, 6 callees
  • new: _run_hook_guard() — 4 callers, 7 callees
  • new: test_poisoned_manifest_is_healed() — 0 callers, 6 callees

Verification — 262 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: 215 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify dispatch\_command.

The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)

· 4 more finding(s) on lines outside this diff (see the check run).

…rging the existing semantic layer

The previous fix set incremental_mode=True, which re-activated the manifest cache and skipped unchanged code files on a warm tree. This made --force no longer a full re-scan and could leave the AST tier unchanged.

The corrected path does a full _detect scan (incremental_mode stays False) and then merges the existing graph into the new output via build_merge / merge_raw_extraction, preserving doc/paper/image nodes and hyperedges. graph_stale_sources is computed on the full scan so deleted/excluded non-code files are still pruned.

Adds a regression test with a warm manifest that removes an unchanged AST node and seeds semantic nodes; the full re-scan must restore the AST node and keep the semantic layer.
@santhiprakash

Copy link
Copy Markdown
Contributor Author

Updated the PR with a corrected fix and an additional regression test.

What changed

  • The previous commit set incremental_mode = True for --force --code-only, which made --force consult the manifest cache. On a warm unchanged tree it dispatched zero code files, so the AST tier was not re-scanned (contrary to the --force contract).
  • The new code keeps incremental_mode false (full _detect scan) and introduces a preserve_semantic flag that triggers build_merge / merge_raw_extraction on the full-scan output, preserving doc/paper/image nodes and hyperedges.
  • Stale sources are still computed in the full-scan branch so deleted or excluded non-code files are pruned.

New regression test

  • test_code_only_force_rescans_unchanged_code_with_manifest_and_preserves_seeded_semantic_nodes verifies that, even when the manifest reports no code changes, --force --code-only still re-extracts all code files and restores an AST node while keeping seeded semantic nodes.

Verification run

  • uv run pytest tests/test_extract_code_only_cli.py -v — 13 passed
  • uv run pytest tests/test_extract_cli.py -v — 31 passed
  • uv run pytest tests/test_extract.py -v — 194 passed, 4 skipped
  • uv run pytest tests/test_incremental.py tests/test_no_dedup_flag.py tests/test_build_merge_hyperedges_and_prune.py tests/test_build_merge_shrink_guard.py -v — 37 passed
  • uv run --frozen python -m tools.skillgen --check — OK, 134 artifacts
  • uv run pytest tests/test_skillgen.py — 64 passed
  • uv run pytest tests/ -q — 4800 passed, 52 skipped, 8 failures unrelated to this change (pre-existing test_labeling, test_ollama* environment/API-key issues, and test_terraform assertion issues on this runner)

The two medium bot findings about fresh-code-only-graph and manifest-gated AST cache reads are addressed by the full-scan approach.

@graphify-labs graphify-labs Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Graphify reviewed this change.

Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.

Formal verification. No changes could be formally verified in this run.


Graphify review — findings

Fixes --force --code-only in dispatch_command so it no longer wipes the existing semantic layer: it still does a full AST re-scan but skips the semantic pass and merges the prior graph forward, pruning stale sources for deleted/excluded files (#2923). Adds a preserve_semantic guard that reuses the incremental merge/prune paths, plus a console line announcing the branch. Covers it with new test_extract_code_only_cli cases for layer preservation, deleted-file pruning, and full re-scan of unchanged code.

Worth a look

  • preserve_semantic merge may prune all non-code nodes since only code files are in _seen_filesgraphify/cli.py:3231 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
Analysis details — impact, health, verification

Impact & health

Graphify review

Impact — 264 functions depend on the 78 functions this change touches.

Health — this change adds coupling hotspots:

  • new: dispatch_command() — 2 callers, 119 callees
  • new: _stale_graph_sources() — 7 callers, 6 callees
  • new: _run_hook_guard() — 4 callers, 7 callees
  • new: test_poisoned_manifest_is_healed() — 0 callers, 6 callees

Verification — 264 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: 217 function(s) in the blast radius were not formally verified this run

Formal verification

Could not verify: Could not verify dispatch\_command.

The verifier did not have enough to check dispatch\_command, so it is saying so rather than guessing. No false assurance is the whole point.

Guarantee: No guarantee either way, this is an honest abstention, not a pass.

Note: Reason: not verifiable: all 23 sampled inputs raised on both versions — the function never executed, so 'no divergence' would be vacuous (mostly SystemExit — names the real obstacle, not a sampling gap)

· 4 more finding(s) on lines outside this diff (see the check run).

@santhiprakash

Copy link
Copy Markdown
Contributor Author

Re the high advisory at graphify/cli.py:3231:

preserve_semantic merge may prune all non-code nodes since only code files are in _seen_files

This does not reproduce. _seen_files is computed at line 3231 before the --code-only branch zeroes semantic_files at line 3248, and files_by_type comes from the full _detect scan, which contains code, document, paper, image, and unclassified files. I verified directly on a mixed repo (app.py, README.md, NOTES.txt):

  • _seen_files includes all three files (not just app.py).
  • _stale_graph_sources(..., _seen_files, ...) returns [] for a seeded graph with semantic nodes from README.md and NOTES.txt.

The fail-closed liveness guard in _stale_graph_sources keeps nodes whose source files still exist on disk and are not provably excluded; it only prunes genuinely deleted or excluded files.

Regression tests confirm the behavior:

  • test_code_only_force_preserves_existing_semantic_layer — seeded doc/text semantic nodes survive --force --code-only.
  • test_code_only_force_prunes_removed_semantic_files — deleted doc nodes are still pruned.
  • test_code_only_force_rescans_unchanged_code_with_manifest_and_preserves_seeded_semantic_nodes — full AST re-scan while preserving the seeded semantic layer.

Verification run on the current head (428a93d):

  • uv run pytest tests/test_extract_code_only_cli.py -v — 13 passed
  • uv run pytest tests/test_extract_cli.py -v — 31 passed
  • uv run pytest tests/test_extract.py -v — 194 passed, 4 skipped
  • uv run pytest tests/test_incremental.py tests/test_no_dedup_flag.py tests/test_build_merge_hyperedges_and_prune.py tests/test_build_merge_shrink_guard.py -v — 37 passed
  • uv run --frozen python -m tools.skillgen --check — OK
  • uv run --frozen python -m tools.skillgen --audit-coverage — OK
  • uv run --frozen python -m tools.skillgen --schema-singleton — OK
  • uv run --frozen python -m tools.skillgen --monolith-roundtrip — OK
  • uv run --frozen python -m tools.skillgen --always-on-roundtrip — OK
  • uv run ruff check . — all passed
  • uv run pytest tests/ -q — 4801 passed, 52 skipped, 7 unrelated pre-existing test_ollama* / test_ollama_retry_cap failures (environment/default-backend and missing openai module)

No further code change is needed for this advisory. The branch is ready for human review/merge.

@safishamsi

Copy link
Copy Markdown
Collaborator

Shipped in v0.9.49 via authorship-preserving cherry-pick so you keep contributor-graph credit. Thanks @santhiprakash! Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.49

@safishamsi safishamsi closed this Aug 24, 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.

2 participants