Repository navigation
watch: keep hub fallback community labels out of the tracked file (#3334) - #3551
ayushcodes10 wants to merge 7 commits into
Conversation
![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.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Splits community label persistence so hub-fallback names (used when a community has no curated name) now write to a separate .graphify_labels.pending.json sidecar instead of the tracked .graphify_labels.json, keeping the tracked file to genuinely curated names only. Fallback names are still merged into labels for report and graph.json generation, reused across rebuilds when membership is unchanged, and a curated name promotes its cid out of the pending file; the pending sidecar is deleted when nothing remains pending. Count-based staleness detection compares against the tracked file alone so a from-empty rebuild's fresh pending entries don't skew the "unchanged" signal.
No blocking issues surfaced. 6 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 593 functions depend on the 407 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 119 callers, 51 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
watch()— 5 callers, 7 callees - new:
_reconcile_graph_html()— 6 callers, 5 callees - new:
_reconcile_existing_graph()— 1 callers, 8 callees - new:
_reconcile_markdown_links()— 1 callers, 6 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 593 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: 419 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
3 of 275 test file(s) selected (1%) via static blast radius.
tests/test_labeling.py— impacttests/test_watch.py— impact, changed-testtests/test_watch_manifest_location.py— impact
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.
Formal verification
Could not verify: Could not verify \_rebuild\_code.
The verifier did not have enough to check \_rebuild\_code, 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: parameter `watch_path` is annotated `Path` — outside the synthesizable primitive/collection set
· 7 more finding(s) on lines outside this diff (see the check run).
A community without a curated name gets a deterministic hub fallback so reports and graph.json always show something readable, but that fallback has always been written straight into the same tracked file as genuinely curated, manual or LLM, names. This is the read half of splitting the two apart: a new sidecar next to the tracked labels file is now also loaded and merged into the working labels dict, and every cid that came from it, or gets freshly hub filled this run rather than reused from the tracked file, is tracked in a separate set so the write step can route each entry back to the file it belongs in. Stale invalidation is extended to cover sidecar entries the same way it already covers tracked ones. Toward Graphify-Labs#3334. The write side, which is what actually stops fallback names from landing in the tracked file, follows in the next commit; this commit alone still writes the merged result into the tracked file as before, so existing behavior is unchanged and no test needs updating yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
The write half of splitting the two label kinds apart. Labels are split back into the tracked, curated set and the pending, hub fallback set right before serialising, using the cid set the read side built up. The tracked file gets only the curated names, exactly as before this change set began; the pending set goes to the new sidecar, or the sidecar is removed outright once nothing in it is still unpromoted, so a stale, now empty pending file never lingers once its last entry becomes a real label. Fixes Graphify-Labs#3334. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
This test simulates a naming pass by reading whatever cid list the first rebuild produced and overwriting each with a distinctive name. That list now comes from the pending sidecar rather than the tracked file, matching what a real naming pass does: promote a pending, hub fallback entry into a curated one, removing it from the sidecar in the same move. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
A from empty rebuild on a two file corpus must leave the tracked labels file empty while writing both communities' hub names to the pending sidecar, and every node's community_name in graph.json must still carry the right name regardless of which file it actually lives in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
A hand edited pending sidecar entry must survive an unrelated rebuild untouched, matching the reuse guarantee already given to curated labels, rather than getting silently regenerated from the hub every single run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
Simulates a real naming pass by promoting every pending entry into the tracked file and deleting the sidecar, then forces a real rebuild. The promoted names must survive, and the community must not reappear in the sidecar even though it did not change membership. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
13d8009 to
240b759
Compare
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).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Splits community naming into two files so unreviewed hub-fallback names stop polluting the tracked semantic layer: _rebuild_code now writes deterministic hub-fill names to a separate .graphify_labels.pending.json sidecar while .graphify_labels.json holds only genuinely curated (manual/LLM) names. Reports and graph.json still see the merged label set, so fallback names remain visible everywhere they were; on write, a community whose fallback got promoted to a curated name is dropped from the sidecar, and an empty sidecar is unlinked rather than left stale. Pending names are reused across rebuilds when membership is unchanged, and the "unchanged label count" heuristic now compares against the tracked file alone so a from-empty rebuild's fresh pending entries don't skew it.
No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 605 functions depend on the 415 functions this change touches.
Health — this change adds coupling hotspots:
- new:
_rebuild_code()— 126 callers, 53 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
watch()— 5 callers, 7 callees - new:
_reconcile_graph_html()— 6 callers, 5 callees - new:
_reconcile_existing_graph()— 1 callers, 8 callees - new:
_reconcile_markdown_links()— 1 callers, 6 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 605 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: 431 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
4 of 283 test file(s) selected (1%) via static blast radius.
tests/test_labeling.py— impacttests/test_terraform_modules.py— impacttests/test_watch.py— impact, changed-testtests/test_watch_manifest_location.py— impact
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.
Formal verification
Could not verify: Could not verify \_rebuild\_code.
The verifier did not have enough to check \_rebuild\_code, 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: no capturable inputs from the test suite; property tier: parameter `watch_path` is annotated `Path` — outside the synthesizable primitive/collection set
· 7 more finding(s) on lines outside this diff (see the check run).
Resolves a conflict in the rebuild's sidecar write block: upstream restructured it after this branch diverged, switching report_path and labels_file to atomic writes and relocating the labels_file write further down to sit right before a new signatures sidecar it added, with its own explanatory comment on why labels must land before signatures (a crash between them must look stale, never falsely current). Kept that structure and moved this branch's own pending labels sidecar write to sit right after the relocated labels_file write instead of its old position, using write_text_atomic for it too to match the surrounding writes, and verified the existing atomicity and label ordering regression test still passes with it inserted.
github-actions
Bot
added
the
needs:title
|
Thanks @ayushcodes10! One small thing: our changelog and release notes are built from PR titles, so we use the conventional-commits format, for example: Prefixes we use: |
This branch has not been deployed
1 participant
Summary
Fixes #3334 (option 1 from the issue's own preference-ordered list: keep unlabelled communities out of the tracked file).
graphify updatenames any community without a saved label after its deterministic hub (its highest-degree member — usually a filename likeREADME.md,vite.config.ts), so reports andgraph.jsonalways show something readable. That fallback name was written straight into.graphify_labels.jsonalongside genuinely curated (manual or LLM) names — the file the project's own "Migration and portability" guidance says to commit as the curated semantic layer that makes label reuse work. Every from-empty rebuild (a fresh clone, a new worktree, a CI job with no priorgraphify-out/) therefore appended fresh placeholder entries to that tracked file.Hub-fallback names now live in a new, separate sidecar (
.graphify_labels.pending.json) next to the tracked file. The tracked file only ever contains names someone actually set. A pending entry is still reused across rebuilds (not regenerated every run, matching the existing reuse guarantee curated labels already had) as long as the community's membership hasn't changed, and agraphify labelpass promoting a pending community to a real name removes it from the sidecar in the same move. Reports andgraph.jsonare unaffected — they already read from the full, merged label set, so a fallback-named community still shows its name exactly as before; only the on-disk tracked file's content changes.Scoped to the
graphify update/watch rebuild path only (graphify/watch.py), which is the issue's own reproduction.cli.py's equivalentcluster-only/labelcode paths carry a materially more complex reuse/--missing-only/LLM-dispatch flow; extending the same split there is a reasonable follow-up but is deliberately left out of this PR to keep the change bounded and reviewable.Test plan
test_rebuild_code_drops_labels_whose_community_changedtest, which had assumed all first-rebuild labels land in the tracked file.graph.json'scommunity_name); a pending name is reused, not regenerated, across an unrelated rebuild; and a promoted (curated) name correctly leaves the pending sidecar.python3 -m pytest -q— 5489 passed, only the pre-existing unrelated failures (test_ollama_retry_cap.pymissingopenaiin this env, one flaky timing assertion intest_ts_import_type_arguments.py).python3 -m tools.skillgen --check— OK.🤖 Generated with Claude Code
https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh