wiki: escape brackets in raw article labels (#3547) - #3550
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. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"''","nodes":"\[1, 2, 3\]","label":"'h\\u00e9llo w\\u00f6rld'","labels":"\{'a': 1, 'b': 2\}","cohesion":"100\.0","node\_community":"\{'a': None\}","resolver":"\{'a':…, the old code produced '\# héllo wörld\\n\\n\> 3 nodes · cohesion 100\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\- \*\*3\*\* \(2 connections\)\\n\\n\#\# Relationships… but the new code produces raises AttributeError. Paste that input straight into a regression test.
Graphify review — findings
Escapes [/] in every label printed directly into a generated wiki article — community titles, "Key Concepts" node listings, and god node titles — so source content that literally contains [[...]] renders as plain text instead of a dead wikilink. Factors the escaping already inlined in _md_link into a shared _escape_md_brackets helper and routes the three previously-unescaped print sites through it. Adds regression tests covering each site plus an end-to-end export check that no page retains a literal [[...]].
No blocking issues surfaced. 2 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 249 functions depend on the 23 functions this change touches.
Health — this change adds coupling hotspots:
- new:
to_wiki()— 43 callers, 7 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
_god_node_article()— 8 callers, 3 callees - new:
_community_article()— 5 callers, 3 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 249 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: 75 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
5 of 276 test file(s) selected (2%) via static blast radius.
tests/test_export_path_length.py— impacttests/test_wiki.py— impacttests/test_wiki_label_bracket_escaping.py— impact, changed-testtests/test_wiki_link_filename_parity.py— impacttests/test_wiki_truncation_indicator.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
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"''","nodes":"\[1, 2, 3\]","label":"'h\\u00e9llo w\\u00f6rld'","labels":"\{'a': 1, 'b': 2\}","cohesion":"100\.0","node\_community":"\{'a': None\}","resolver":"\{'a':…, the old code produced '\# héllo wörld\\n\\n\> 3 nodes · cohesion 100\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\- \*\*3\*\* \(2 connections\)\\n\\n\#\# Relationships… but the new code produces raises AttributeError. Paste that input straight into a regression test.
Could not verify: Could not verify \_god\_node\_article.
The verifier did not have enough to check \_god\_node\_article, 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: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
No difference found (not proven): No behavior difference found in \_md\_link (not a proof).
The verifier ran both versions of \_md\_link on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 4 more finding(s) on lines outside this diff (see the check run).
| @@ -185,7 +199,7 @@ def _community_article( | |||
| def _god_node_article(G: nx.Graph, nid: str, labels: dict[int, str], node_community: dict[str, int] | None = None, resolver: dict[str, str] | None = None) -> str: | |||
There was a problem hiding this comment.
_god_node_article()
8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Thanks for the catch — the formal verifier's finding on Fixed in 5d79e51 by coercing with |
There was a problem hiding this comment.
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. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Graphify review — findings
Escapes [/] in every label printed directly into a generated wiki article — community titles, "Key Concepts" node listings, and god-node titles — via a new _escape_md_brackets helper, so a label whose source content contains literal [[...]] renders as plain text rather than a dead Obsidian wikilink (#3547). The helper stringifies its input first, so the id-fallback path used when a node has no label handles non-string node ids (ints, tuples) without raising. Adds regression tests covering each site plus an end-to-end export check.
Worth a look
- Backslash can bypass bracket escaping in Markdown links —
graphify/wiki.py:102· 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 — 251 functions depend on the 25 functions this change touches.
Health — this change adds coupling hotspots:
- new:
to_wiki()— 43 callers, 7 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
_god_node_article()— 8 callers, 3 callees - new:
_community_article()— 6 callers, 3 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 251 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: 77 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
5 of 276 test file(s) selected (2%) via static blast radius.
tests/test_export_path_length.py— impacttests/test_wiki.py— impacttests/test_wiki_label_bracket_escaping.py— impact, changed-testtests/test_wiki_link_filename_parity.py— impacttests/test_wiki_truncation_indicator.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
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Could not verify: Could not verify \_god\_node\_article.
The verifier did not have enough to check \_god\_node\_article, 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: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
No difference found (not proven): No behavior difference found in \_md\_link (not a proof).
The verifier ran both versions of \_md\_link on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 4 more finding(s) on lines outside this diff (see the check run).
| @@ -185,7 +204,7 @@ def _community_article( | |||
| def _god_node_article(G: nx.Graph, nid: str, labels: dict[int, str], node_community: dict[str, int] | None = None, resolver: dict[str, str] | None = None) -> str: | |||
There was a problem hiding this comment.
_god_node_article()
8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Addressed the advisory finding on backslash bypassing the escaping in 64bfc3a. Source content can already contain a literal backslash right before a bracket (a doc excerpt showing a regex character class, Added a regression test with a small simulator of CommonMark's own left-to-right backslash-escape unwinding, so it checks the actual syntactic property (no bracket reachable as bare/unescaped) rather than just comparing output strings. |
There was a problem hiding this comment.
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. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Graphify review — findings
Escapes [/] in node labels printed directly into generated wiki articles — community titles, "Key Concepts" node listings, and god-node titles — via a new _escape_md_brackets helper, so a label whose source content literally contains [[...]] renders as plain text rather than a dead wikilink. The helper escapes backslashes before brackets (so a pre-existing \] can't collapse the bracket's escape under CommonMark) and stringifies its input first, covering the non-string node ids callers fall back to when a node has no label.
Worth a look
- Backslash doubling can reactivate escaped Markdown punctuation —
graphify/wiki.py:82· 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 — 254 functions depend on the 28 functions this change touches.
Health — this change adds coupling hotspots:
- new:
to_wiki()— 43 callers, 7 callees - new:
dispatch_command()— 2 callers, 124 callees - new:
_god_node_article()— 8 callers, 3 callees - new:
_community_article()— 6 callers, 3 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 254 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: 80 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
5 of 276 test file(s) selected (2%) via static blast radius.
tests/test_export_path_length.py— impacttests/test_wiki.py— impacttests/test_wiki_label_bracket_escaping.py— impact, changed-testtests/test_wiki_link_filename_parity.py— impacttests/test_wiki_truncation_indicator.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
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Could not verify: Could not verify \_god\_node\_article.
The verifier did not have enough to check \_god\_node\_article, 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: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
No difference found (not proven): No behavior difference found in \_md\_link (not a proof).
The verifier ran both versions of \_md\_link on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 4 more finding(s) on lines outside this diff (see the check run).
| @@ -185,7 +218,7 @@ def _community_article( | |||
| def _god_node_article(G: nx.Graph, nid: str, labels: dict[int, str], node_community: dict[str, int] | None = None, resolver: dict[str, str] | None = None) -> str: | |||
There was a problem hiding this comment.
_god_node_article()
8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Following up on the newest review pass's advisory ("Backslash doubling can reactivate escaped Markdown punctuation") — this is unverified/no reproducing input was given, unlike the earlier findings which did include a concrete counterexample. I checked it directly: the fix escapes every backslash in the source text first (single -> double) before escaping brackets, which is provably correct by induction — doubling always leaves an EVEN run of backslashes immediately before any bracket (0, 2, 4, ...), so the bracket-escaping step's one added backslash always makes an odd/pairing count that resolves as an escaped, inert bracket, regardless of how many backslashes preceded it or what surrounds them. Verified empirically against several adversarial cases (runs of 1-4 backslashes before/after/between brackets, mixed content) with a script that simulates CommonMark's left-to-right backslash-escape pairing — no case leaves a bare, live bracket. I believe this is the same concern the earlier finding already flagged and that 64bfc3a already fixed, not a new distinct gap; happy to add more test cases if a specific failing input turns up. |
_md_link already escapes a label before it ever becomes link text, because a node label is extracted source content and can literally contain a bracketed substring, a doc that discusses or demonstrates wikilink syntax being the obvious source. Three other places print a label straight into an article body without going through that escaping at all, a community's own title, its Key Concepts node listing, and a god node's own title, so a label like double bracket wikilink double bracket rendered as a real, and always dead since the wiki export never writes that link form, wikilink instead of the plain text it actually is. Pulls the escaping out of _md_link into a small shared helper and applies it at all three sites. Fixes the first of two bugs in Graphify-Labs#3547. The second bug the report describes, a wikilink target left percent encoded while the file written for it is not, was already fixed on this branch's base by number 2597, confirmed directly: _safe_filename strips everything that would need encoding from a slug before it is ever used as either the link target or the filename, so raw emission and the on disk name are the same string by construction. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
Covers the helper directly, each of the three fixed print sites, and an end to end export using the exact placeholder text the report listed, double bracket link double bracket and double bracket new underscore stem double bracket among them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
Addresses review feedback on PR 3550: the graphify review bot's
formal verifier found a concrete input on which _community_article
started raising AttributeError after this PR's escaping change.
_escape_md_brackets called replace directly on its argument. Every
caller passes d.get("label", nid), so a node whose dict has no
label attribute falls back to its own networkx node id, which is not
always a string (int and tuple ids are legal). The previous code
interpolated that fallback into an f string, which stringifies
anything; wrapping it in _escape_md_brackets turned that implicit
coercion into a hard requirement the fallback does not always meet.
Coercing with str() first restores the old behavior for a non string
fallback while keeping the escaping for genuine string labels.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
Addresses a review finding on PR 3550: the graphify review bot flagged that a pre existing backslash right before a bracket in source content can bypass the bracket escaping. _escape_md_brackets escaped only the two bracket characters. Source content occasionally already contains a literal backslash right before a bracket, a doc excerpt showing a regex character class like backslash close bracket plus, for example. Escaping the bracket alone turned that into two backslashes then a bare bracket, and CommonMark reads a doubled backslash as one literal backslash, which consumes the pair and leaves the following bracket unescaped and live as link syntax again. Escaping every backslash in the source text first, before the brackets, keeps the bracket's own escape intact regardless of what already preceded it. Added a regression test with a small simulator of CommonMark's own left to right backslash escape unwinding, so the assertion checks the actual syntactic property (no bracket reachable as bare, unescaped) rather than just comparing strings. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017qfdzgbA5KedGEjD1AayNh
64bfc3a to
85d2122
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. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Graphify review — findings
Escapes [/] (and backslashes ahead of them) in every label printed directly into a generated wiki article — community titles, "Key Concepts" node listings, and god-node titles — via a new _escape_md_brackets, so source content that literally contains [[...]] renders as plain text rather than a dead Obsidian wikilink (#3547). Backslashes are escaped before brackets so a pre-existing \] in source doesn't un-escape back into live link syntax, and non-string node ids used as label fallbacks are stringified rather than raised on. _md_link now routes its own escaping through the same helper.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 254 functions depend on the 28 functions this change touches.
Health — this change adds coupling hotspots:
- new:
to_wiki()— 43 callers, 7 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
_god_node_article()— 8 callers, 3 callees - new:
_community_article()— 6 callers, 3 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 254 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: 80 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
5 of 284 test file(s) selected (2%) via static blast radius.
tests/test_export_path_length.py— impacttests/test_wiki.py— impacttests/test_wiki_label_bracket_escaping.py— impact, changed-testtests/test_wiki_link_filename_parity.py— impacttests/test_wiki_truncation_indicator.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
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Could not verify: Could not verify \_god\_node\_article.
The verifier did not have enough to check \_god\_node\_article, 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: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
No difference found (not proven): No behavior difference found in \_md\_link (not a proof).
The verifier ran both versions of \_md\_link on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 4 more finding(s) on lines outside this diff (see the check run).
| @@ -185,7 +218,7 @@ def _community_article( | |||
| def _god_node_article(G: nx.Graph, nid: str, labels: dict[int, str], node_community: dict[str, int] | None = None, resolver: dict[str, str] | None = None) -> str: | |||
There was a problem hiding this comment.
_god_node_article()
8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
A community label that is just a bracket character exercises _community_article's own title heading, not a link target the existing bracketed label test already covers, and must render escaped there too or the lone bracket opens markdown link or image syntax the rest of the line never closes. Matches the exact shape a formal verification pass on this PR reproduced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Re: the reproduced formal-verification counterexample on `_community_article` (old code: `# [`, new code: `# [`) — this is the intended behavior change, not a bug. Escaping a community title that is (or starts with) a literal bracket is exactly what this PR's `_escape_md_brackets` is for; the review itself confirms "Looks safe to merge" with no blocking issues. Took the verifier's suggestion and added a regression test locking this in — `test_community_article_title_escapes_a_bare_bracket_label`, calling `_community_article` directly with a bare `"["` label and asserting the title heading comes out escaped. The existing bracket test in this file (`test_wiki_link_with_bracketed_label_resolves`) only covers a bracket inside a link target built via `_md_link`; this one covers the community's own `# {label}` title heading specifically, which is the exact site the verifier's input exercised. Full suite (5602 passed, no regressions) and `skillgen --check` green. |
There was a problem hiding this comment.
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. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Graphify review — findings
Escapes [/] in raw node labels wherever they're printed directly into a generated wiki article — the community title, its Key Concepts listing, and the god-node title — via a new _escape_md_brackets, so source content that literally contains [[...]] renders as plain text instead of a dead wikilink. The helper escapes backslashes before brackets (so a pre-existing \] in source can't un-escape back into live link syntax) and stringifies its input, matching the id fallback for nodes without a label attribute (int/tuple ids included). _md_link now routes through the same helper.
Worth a look
- Backslash doubling can unescape non-bracket Markdown punctuation —
graphify/wiki.py:84· 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 — 280 functions depend on the 85 functions this change touches.
Health — this change adds coupling hotspots:
- new:
to_wiki()— 43 callers, 7 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
_god_node_article()— 8 callers, 3 callees - new:
_community_article()— 7 callers, 3 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 280 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: 106 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
5 of 284 test file(s) selected (2%) via static blast radius.
tests/test_export_path_length.py— impacttests/test_wiki.py— impact, changed-testtests/test_wiki_label_bracket_escaping.py— impact, changed-testtests/test_wiki_link_filename_parity.py— impacttests/test_wiki_truncation_indicator.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
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Could not verify: Could not verify \_god\_node\_article.
The verifier did not have enough to check \_god\_node\_article, 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: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
No difference found (not proven): No behavior difference found in \_md\_link (not a proof).
The verifier ran both versions of \_md\_link on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 4 more finding(s) on lines outside this diff (see the check run).
| @@ -185,7 +218,7 @@ def _community_article( | |||
| def _god_node_article(G: nx.Graph, nid: str, labels: dict[int, str], node_community: dict[str, int] | None = None, resolver: dict[str, str] | None = None) -> str: | |||
There was a problem hiding this comment.
_god_node_article()
8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Doubling every backslash in the source text, not just ones right before a bracket, un-escaped any OTHER pre-existing markdown escape already in that text. A source string like a literal asterisk written as backslash asterisk survived as backslash asterisk before this change; doubled unconditionally it became two backslashes then a bare asterisk, and CommonMark reads two backslashes as one literal backslash followed by a now live, unescaped asterisk. Only doubling a backslash that precedes a bracket keeps the one case this handling exists for intact without touching anything else in the text. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Re: "Backslash doubling can unescape non-bracket Markdown punctuation" — confirmed real. Reproduced directly: ```python before: '\\bold\\ ' -- the doubled backslash un-escapes into a bare, live *after: '\bold\ ' -- unchanged``` The unconditional `.replace("\\", "\\\\")` doubled every backslash in the source text, not just ones adjacent to a bracket — so any OTHER pre-existing escape already in that text (`\*` meaning a literal asterisk, for instance) got un-escaped the exact same way the function exists to prevent for brackets. Switched to only doubling a backslash that's immediately followed by `[` or `]` (`re.sub(r"\\(?=[\[\]])", r"\\\\", ...)`), which keeps the one case this handling exists for (`\]+`, a regex character class in source) intact while leaving everything else in the text untouched. Also re: the reproduced formal-verification note on `_community_article` in this same review — that's the same confirmed-intended `# [` → `# [` change from the earlier round, already covered by the regression test I added in the previous commit. Full suite (5604 passed, no regressions) and `skillgen --check` green. |
There was a problem hiding this comment.
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. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Graphify review — findings
Escapes literal [/] in node and community labels wherever they're printed directly into a generated article — a community's title heading and "Key Concepts" listing, and a god node's title — via a new _escape_md_brackets helper, so source content that contains [[...]] renders as plain text instead of a dead wikilink. The helper doubles a backslash that already precedes a bracket before escaping the bracket, so a pre-existing escape like \] in a regex excerpt round-trips instead of un-escaping back into live syntax, and it str()s its input so a non-string node id used as a label fallback stringifies rather than raising.
Worth a look
- _escape_md_brackets order of replacement may un-escape brackets —
graphify/wiki.py· 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 — 284 functions depend on the 89 functions this change touches.
Health — this change adds coupling hotspots:
- new:
to_wiki()— 43 callers, 7 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
_god_node_article()— 8 callers, 3 callees - new:
_community_article()— 7 callers, 3 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 284 functions in the blast radius were not formally verified this run (proofs are advisory here).
Health delta baseline: last indexed commit eaaec1a (diverged from this PR's base — delta is approximate).
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: 110 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
5 of 284 test file(s) selected (2%) via static blast radius.
tests/test_export_path_length.py— impacttests/test_wiki.py— impact, changed-testtests/test_wiki_label_bracket_escaping.py— impact, changed-testtests/test_wiki_link_filename_parity.py— impacttests/test_wiki_truncation_indicator.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
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Could not verify: Could not verify \_god\_node\_article.
The verifier did not have enough to check \_god\_node\_article, 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: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
No difference found (not proven): No behavior difference found in \_md\_link (not a proof).
The verifier ran both versions of \_md\_link on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 4 more finding(s) on lines outside this diff (see the check run).
| @@ -185,7 +221,7 @@ def _community_article( | |||
| def _god_node_article(G: nx.Graph, nid: str, labels: dict[int, str], node_community: dict[str, int] | None = None, resolver: dict[str, str] | None = None) -> str: | |||
There was a problem hiding this comment.
_god_node_article()
8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Doubling only the single backslash immediately adjacent to a bracket left an earlier backslash in a longer run unpaired. Two source backslashes right before a bracket produced four escaped ones with nothing left to escape the bracket itself, so CommonMark read that as two literal backslashes followed by a bare, newly live bracket, un-escaping it right back into link syntax, exactly the failure this whole helper exists to prevent. A single regex pass now matches the entire run of backslashes ahead of a bracket, doubles all of it, then adds one more backslash for the bracket's own escape, so any run length still leaves the bracket correctly escaped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Re: "`_escape_md_brackets` order of replacement may un-escape brackets" — confirmed real, and the high severity call was right. The previous fix only doubled the SINGLE backslash immediately adjacent to a bracket, not a longer run of them. Reproduced directly: ```python before: 4 backslashes then a bare bracket -- CommonMark reads two escaped-backslashpairs (two literal backslashes) with nothing left to escape the bracket,so it un-escapes right back into live link syntaxafter: 5 backslashes then the bracket -- two pairs (two literal backslashes) plusone leftover that correctly escapes the bracket``` Rewrote it as a single regex pass that matches the entire backslash run ahead of a bracket (not just the last character of it), doubles the whole run, then adds one more backslash for the bracket's own escape — verified this now round-trips correctly for runs of 0, 1, 2, 3, and 4 source backslashes before a bracket, by simulating CommonMark's own backslash-pair-collapse rule against the output. Full suite (5605 passed, no regressions) and `skillgen --check` green. |
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. 1 change(s) alter behavior, breaking input(s) attached.
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Graphify review — findings
Escapes [/] in raw label text at the three sites that print a label directly into a generated article — a community title, its Key Concepts listing, and a god-node title — via a new _escape_md_brackets, so a label like [[wikilink]] renders as plain text instead of a dead wikilink; _md_link now routes its own bracket escaping through the same helper. The helper doubles a whole run of pre-existing backslashes before a bracket in a single regex pass, so source content like \]+ or [\\] round-trips as literal text without un-escaping the bracket back into live syntax, and leaves unrelated escapes like \* untouched. It also str()s its input so a non-string node id used as a label fallback stringifies rather than raising.
No blocking issues surfaced. 1 lower-confidence candidate did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 287 functions depend on the 92 functions this change touches.
Health — this change adds coupling hotspots:
- new:
to_wiki()— 43 callers, 7 callees - new:
dispatch_command()— 2 callers, 125 callees - new:
_god_node_article()— 8 callers, 3 callees - new:
_community_article()— 7 callers, 3 callees - new:
test_poisoned_manifest_is_healed()— 0 callers, 6 callees
Verification — 287 functions in the blast radius were not formally verified this run (proofs are advisory here).
Health delta baseline: last indexed commit eaaec1a (diverged from this PR's base — delta is approximate).
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: 113 function(s) in the blast radius were not formally verified this run
Test selection
Test selection
5 of 284 test file(s) selected (2%) via static blast radius.
tests/test_export_path_length.py— impacttests/test_wiki.py— impact, changed-testtests/test_wiki_label_bracket_escaping.py— impact, changed-testtests/test_wiki_link_filename_parity.py— impacttests/test_wiki_truncation_indicator.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
Behavior changes: \_community\_article changes behavior, here is the input that shows it.
The verifier found a concrete input on which \_community\_article behaves differently before and after the change. If that change is intended, ship it; if not, this is your bug.
Guarantee: This difference was REPRODUCED, the verifier actually ran both versions on that input and saw them disagree. It is real, not an artifact.
Evidence: On input \{"G":"\(lambda \_g: \(\_g\.add\_nodes\_from\(\[\(1, \{\}\), \(2, \{\}\), \(3, \{\}\)\]\), \_g\.add\_edges\_from\(\[\(1, 2, \{\}\), \(1, 3, \{\}\), \(2, 3, \{\}\)\]\), \_g\)\[\-1\]\)\(\_\_import\_\_\('networkx'\)\.Graph\(\)\)","cid":"'/'","nodes":"\[3, 1, 2\]","label":"'\['","labels":"\{'k': 'v'\}","cohesion":"1\.0","node\_community":"\{'a': None\}","resolver":"\{'a': 1, 'b': 2\}"\}, the old code produced '\# \[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\- No… but the new code produces '\# \\\\\[\\n\\n\> 3 nodes · cohesion 1\.00\\n\\n\#\# Key Concepts\\n\\n\- \*\*3\*\* \(2 connections\)\\n\- \*\*1\*\* \(2 connections\)\\n\- \*\*2\*\* \(2 connections\)\\n\\n\#\# Relationships\\n\\n\…. Paste that input straight into a regression test.
Could not verify: Could not verify \_god\_node\_article.
The verifier did not have enough to check \_god\_node\_article, 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: non-vacuity: domain too small (only 2 distinct inputs exercised, need 3) — 'no divergence' would be near-vacuous
No difference found (not proven): No behavior difference found in \_md\_link (not a proof).
The verifier ran both versions of \_md\_link on many inputs and saw identical behavior every time. Strong evidence the change is safe, but evidence, not a proof.
Guarantee: Empirical: differential testing (both versions run on many generated inputs). A divergence on an untested input remains possible, so this is 'no counterexample found', not 'proven equivalent'.
Note: An input the sampler did not try could still differ.
· 1 grounded finding(s) anchored inline below; 4 more finding(s) on lines outside this diff (see the check run).
| @@ -185,7 +227,7 @@ def _community_article( | |||
| def _god_node_article(G: nx.Graph, nid: str, labels: dict[int, str], node_community: dict[str, int] | None = None, resolver: dict[str, str] | None = None) -> str: | |||
There was a problem hiding this comment.
_god_node_article()
8 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
1 participant
Summary
Fixes the first of the two bugs in #3547 (literal placeholder/mis-encoded
[[wikilinks]]in generatedCommunity_N.mdpages).Bug 1 (literal placeholder wikilinks) — fixed here.
_md_linkalready escapes[/]before a label becomes link text, because a node label is extracted source content and can literally contain a bracketed substring — most plausibly from a doc that discusses or demonstrates wikilink syntax. Three other sites print a label straight into an article body without going through that escaping at all: a community's own title, its "Key Concepts" node listing, and a god node's own title. Printed unescaped, a label like[[wikilink]]renders as a real (and always dead — the wiki export never writes bracket-style links) wikilink instead of the plain text it actually is. Pulled the escaping logic out of_md_linkinto a small shared helper (_escape_md_brackets) and applied it at all three sites.Bug 2 (percent-encoded target vs. raw filename) — already fixed on
v8. Traced this directly:_md_link's own docstring already documents removingquote()-based percent-encoding as issue #2597, and confirmed_safe_filenamestrips every character that would need encoding from a slug before it's used as either the link target or the on-disk filename — so raw emission and the actual filename are the same string by construction. Verified with the issue's own exact example (CMDS.md_(System Philosophy & Context Guide).md): no%28/%29/%26anywhere in the current output, andtests/test_wiki_link_filename_parity.py(already in the suite, all passing) pins exactly this invariant for the same character classes the issue reports ((,),&, em dash,%, CJK).Test plan
tests/test_wiki_label_bracket_escaping.py: the helper directly, each of the three fixed print sites individually, and an end-to-endto_wiki()export using the issue's own example placeholder text ([[link]],[[new_stem]],[[]]) confirmed absent from every generated page.test_wiki.py,test_wiki_truncation_indicator.py,test_wiki_link_filename_parity.py) — 49 tests, all pass, no regressions.python3 -m pytest -q— 5491 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