Closes #539: clarify --update prune message (split drift vs no-drift)#544
Closes #539: clarify --update prune message (split drift vs no-drift)#544saxster wants to merge 1 commit intosafishamsi:v5from
Conversation
Closes safishamsi#539. The current prune message in the --update flow is ambiguous: Pruned 0 ghost nodes from 13 deleted file(s) A reader can't tell whether (a) 13 files deleted, 0 of them had nodes worth pruning (benign — graph already clean), or (b) 13 ghost nodes still exist and only 0 got pruned (bug). The denominator is opaque. Split into two messages so the semantics are explicit: - Drift case: "Pruned N ghost node(s) from M deleted file(s) — drift detected and corrected." - No-drift case: "M file(s) deleted since last run, but no ghost nodes were present in the graph — no drift." Applied to both skill.md and skill-copilot.md (the only two skill files carrying the --update merge step verbatim).
There was a problem hiding this comment.
Pull request overview
Updates the --update merge-step logging in the skill templates to remove ambiguity when deleted files are detected, distinguishing between cases where ghost nodes are actually pruned vs. where no graph nodes existed for the deleted files.
Changes:
- Split the prior ambiguous “Pruned 0 … from M deleted file(s)” output into two explicit messages (drift vs. no-drift).
- Apply the same message split to both the canonical skill (
skill.md) and the Copilot variant (skill-copilot.md).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| graphify/skill.md | Updates the merge-step snippet to print distinct drift vs. no-drift messages when pruning deleted-file nodes. |
| graphify/skill-copilot.md | Mirrors the same merge-step logging changes in the Copilot skill variant. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| print(f'Pruned {len(to_remove)} ghost node(s) from {len(deleted)} deleted file(s) — drift detected and corrected.') | ||
| else: | ||
| print(f'{len(deleted)} file(s) deleted since last run, but no ghost nodes were present in the graph — no drift.') |
There was a problem hiding this comment.
The messages claim “drift detected and corrected” / “no drift”, but this code only checks/removes nodes with node['source_file'] in the deleted set; it doesn’t verify that all drift is corrected or that no other drift exists. Consider tightening the wording to the specific condition (e.g., “ghost-node drift from deleted files detected; pruned …” / “no ghost-node drift from deleted files”).
| print(f'Pruned {len(to_remove)} ghost node(s) from {len(deleted)} deleted file(s) — drift detected and corrected.') | |
| else: | |
| print(f'{len(deleted)} file(s) deleted since last run, but no ghost nodes were present in the graph — no drift.') | |
| print(f'Ghost-node drift from deleted file(s) detected; pruned {len(to_remove)} ghost node(s) from {len(deleted)} deleted file(s).') | |
| else: | |
| print(f'{len(deleted)} file(s) deleted since last run, but no ghost-node drift from deleted files was present in the graph.') |
| print(f'Pruned {len(to_remove)} ghost node(s) from {len(deleted)} deleted file(s) — drift detected and corrected.') | ||
| else: | ||
| print(f'{len(deleted)} file(s) deleted since last run, but no ghost nodes were present in the graph — no drift.') |
There was a problem hiding this comment.
The messages claim “drift detected and corrected” / “no drift”, but this code only checks/removes nodes with node['source_file'] in the deleted set; it doesn’t verify that all drift is corrected or that no other drift exists. Consider tightening the wording to the specific condition (e.g., “ghost-node drift from deleted files detected; pruned …” / “no ghost-node drift from deleted files”).
| print(f'Pruned {len(to_remove)} ghost node(s) from {len(deleted)} deleted file(s) — drift detected and corrected.') | |
| else: | |
| print(f'{len(deleted)} file(s) deleted since last run, but no ghost nodes were present in the graph — no drift.') | |
| print(f'Pruned {len(to_remove)} ghost node(s) from {len(deleted)} deleted file(s) — ghost-node drift from deleted files detected and pruned.') | |
| else: | |
| print(f'{len(deleted)} file(s) deleted since last run, but no ghost-node drift from deleted files was present in the graph.') |
Closes #539.
Summary
Splits the ambiguous
Pruned 0 ghost nodes from 13 deleted file(s)message into two unambiguous variants:Pruned N ghost node(s) from M deleted file(s) — drift detected and corrected.M file(s) deleted since last run, but no ghost nodes were present in the graph — no drift.Why
The current message reads
Pruned 0 ghost nodes from 13 deleted file(s). A reader can't tell whether:The two interpretations have very different operational implications. Splitting the cases removes the ambiguity.
Files changed
graphify/skill.md(line 918) — primary skill templategraphify/skill-copilot.md(line 821) — Copilot variant carries the same merge step verbatimThe other 9 skill-*.md files don't include the
--updatemerge step inline (they reference it via the canonicalskill.md).Test plan
--updateafter a deletion (drift) — verify drift message--updateafter deleting a file with no graph nodes — verify no-drift message