fix(skill): --update no longer over-reports unchanged files (#2459) - #2560
fix(skill): --update no longer over-reports unchanged files (#2459)#2560SomSamantray wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Graphify review — findings
This PR addresses issue #2459 by threading kind="ast" through the --update skill flow's detect_incremental calls, changing the default semantic behavior that had been re-flagging AST-only files (with empty semantic_hash) as changed on every update. The change is made in three skillgen source fragments and propagated to the generated shipped artifacts (14 platform update.md files plus the aider/devin skill files), while the extract path is made to pass kind="semantic" explicitly so it no longer depends on the default. It also adds a planning doc, a CHANGELOG entry, guard tests ensuring runbooks pass kind="ast", and premise tests contrasting ast vs semantic empty-hash detection behavior. The surface area spans skill generation source/fixtures, the CLI extract path, and the detect/skillgen test suites.
No blocking issues surfaced.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1287 functions depend on the 1130 functions this change touches.
Health — grade A; 8 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):
dispatch_command()— 2 callers, 118 callees (high)render()— 13 callers, 5 callees (high)audit_coverage()— 8 callers, 6 callees (high)_stale_graph_sources()— 7 callers, 6 callees (high)main()— 3 callers, 11 callees (medium)_run_hook_guard()— 3 callers, 6 callees (medium)monolith_roundtrip()— 3 callers, 5 callees (medium)test_audit_catches_a_dropped_non_allowlisted_heading()— 0 callers, 6 callees (medium)
Verification — 1287 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: 1244 function(s) in the blast radius were not formally verified this run
Summary
graphify --updatereported far more changed files than were actually modified.detect_incrementaldefaults tokind="semantic", which treats every file whosesemantic_hashis empty (AST-extracted only, never semantically chunked) as changed on every run. On a real monorepo this flagged 811 files after ~25 merged PRs when only 173 had actually changed, and each false positive was re-dispatched to a semantic extraction sub-agent at real model/token cost.The update flow now passes
kind="ast"at all 16 call sites -- the 14 platform--updaterunbooks (generated from the sharedtools/skillgenfragment), plus the aider and devin monoliths -- so the changed set matches actual content changes. Thegraphify extractincremental scan keeps its semantic re-queue contract (files with emptysemantic_hashre-extract), now passed explicitly instead of relying on the default. A sanctioned-diff predicate keeps the monolith roundtrip guard in sync with the new call shape, and a guard test locks the runbooks tokind="ast".Fixes #2459
Validation
python -m tools.skillgen --check-- 134 artifacts match committed output andexpected/(no drift).uv run pytest tests/test_skillgen.py tests/test_detect.py tests/test_extract_cli.py-- 327 passed.uv run pytest tests/test_incremental.py tests/test_office_incremental.py tests/test_watch.py tests/test_watch_manifest_location.py tests/test_build.py-- 196 passed, 3 skipped.test_runbook_update_flows_pass_kind_astguard: every shipped--updaterunbook and fragment passeskind="ast"(red before the fix, green after).test_detect_incremental_kind_ast_ignores_empty_semantic_hashpremise test: ast mode reports unchanged files with emptysemantic_hashclean; semantic mode still re-queues them.