Skip to content

fix(skill): clean up needs_update, not .needs_update - #2677

Open
rohit-jsfreaky wants to merge 1 commit into
Graphify-Labs:v8from
rohit-jsfreaky:fix/skill-needs-update-flag-name
Open

fix(skill): clean up needs_update, not .needs_update#2677
rohit-jsfreaky wants to merge 1 commit into
Graphify-Labs:v8from
rohit-jsfreaky:fix/skill-needs-update-flag-name

Conversation

@rohit-jsfreaky

Copy link
Copy Markdown
Contributor

Fixes #2440.

Summary

--watch writes graphify-out/needs_update when it sees a doc/paper/image change it cannot re-extract without an LLM, and the read hook then treats the graph as stale for every source read while that file exists. The documented escape hatch is a full rebuild through the skill, whose step 9 cleans the flag up.

Step 9 deleted graphify-out/.needs_update — with a leading dot, which nothing writes. The rm matched nothing, so the real flag survived a complete, successful rebuild and every later Read kept getting the stale nudge. graphify update does clear it (watch.py), so the CLI path worked while the documented skill path silently did not.

The dot looks accidental rather than a second flag: the line directly above cleans dotted temp files (.graphify_detect.json, .graphify_extract.json, …), and the same fragment's own prose already writes graphify-out/needs_update without one.

Changes

  • Three skillgen fragments (core/core.md, core/aider.md, core/devin.md) drop the dot. The 32 rendered artifacts follow from python -m tools.skillgen and --bless; all 35 generated-line changes are that one line, and the two Windows ones are the PowerShell form skillgen derives itself.
  • gen.py gains _is_needs_update_flag_fix_line, registered in _SANCTIONED_MONOLITH_DIFFS. The aider/devin monoliths are diffed against the pinned pristine v8 blob and every deviation has to be an enumerated change-class, so the round-trip guard fails without it. Same shape as the existing predicates.
  • The PowerShell translator test used the old dotted line as its "translates exactly" example; updated so it still mirrors a real corpus line.

Tests

test_skill_cleanup_targets_the_flag_the_code_actually_writes asserts the two halves agree: watch.py and cli.py name the undotted flag, and no rendered artifact references a dotted one. Confirmed it fails with the fragments reverted:

AssertionError: graphify/skill-agents.md cleans up '.needs_update', which nothing ever writes

uv run --no-sync pytest tests/test_skillgen.py -q — 65 passed.

uv run --no-sync python -m tools.skillgen --check — 134 artifacts match committed output and expected/. --audit-coverage, --schema-singleton, --monolith-roundtrip and --always-on-roundtrip all OK (the git-show validators need origin/v8 fetched locally or they skip).

uv run --no-sync ruff check tools/skillgen/gen.py tests/test_skillgen.py — passed.

Full suite: 22 failed, 4300 passed, 13 skipped, against a clean v8 baseline of 22 failed, 4299 passed, 13 skipped — the same 22 in both runs, all pre-existing and platform-sensitive on this Windows machine. The +1 is the new test.

I did not run the --watch → touch a doc → rebuild sequence end to end; the evidence here is the filename mismatch between what the code writes and reads and what the skill deletes, established from the source.

@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.


Graphify review — findings

This PR renames the cleanup target from .needs_update (dotfile) to needs_update (non-dotfile) in the final cleanup step of the graphify skill definitions. The change is applied consistently across all host-specific skill files (agents, aider, amp, claw, codex, copilot, devin, droid, kilo, kiro, opencode, pi, trae, vscode, windows, and the base skill.md), including the PowerShell variant for the Windows host. Corresponding updates also appear in the skillgen generator/fragment sources and expected-output test fixtures.

Worth a look

  • Windows path uses \needs_update but the file is named .needs_updategraphify/skill-windows.md:645 · Escalate · high
    • agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review
  • Cleanup no longer removes the documented hidden update sentinelgraphify/skill.md:620 · 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 — 1168 functions depend on the 1168 functions this change touches.

Health — grade A; 5 existing hotspot(s) in the area this change touches (pre-existing, not introduced here):

  • render() — 13 callers, 5 callees (high)
  • audit_coverage() — 8 callers, 6 callees (high)
  • main() — 3 callers, 11 callees (medium)
  • monolith_roundtrip() — 3 callers, 5 callees (medium)
  • test_audit_catches_a_dropped_non_allowlisted_heading() — 0 callers, 6 callees (medium)

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

@rohit-jsfreaky

Copy link
Copy Markdown
Contributor Author

Both findings assume the flag file is named .needs_update. Nothing writes that name: on v8 before this PR, .needs_update appears only in the skill files themselves and in one test fixture quoting them — no Python file reads or writes it.

The flag is undotted on both sides: written at watch.py:1499, 1546, 1750, 1776, 1785 (out / "needs_update"), read at cli.py:692 (out_path("needs_update").exists()), and described at watch.py:1827 as "writes a needs_update flag". The fragments' own prose already documented graphify-out/needs_update too.

So the Windows line is consistent — graphify-out\needs_update is the same path as graphify-out/needs_update — and no sentinel is left behind. The old rm matched nothing, which is exactly #2440.

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.

SKILL.md step 9 clears .needs_update but everything else uses needs_update, so a full rebuild never clears the stale flag

1 participant