fix(wiki): make a link target the article filename verbatim, not its percent-encoded twin (#2597) - #2657
Conversation
…percent-encoded twin
`_md_link` emitted `quote(f{slug}.md)` while `to_wiki` wrote the file under
the raw slug, so the two agreed only when the label happened to be URL-safe.
Every article whose label contained `(`, `)`, `&` or a non-ASCII character was
linked at a path that does not exist: `_make_id%28%29.md` for a file named
`_make_id().md`. Renderers hid it by decoding before resolving, but the wiki
exists to be agent-crawlable, and an agent that reads the target off disk
verbatim gets a FileNotFoundError. Every callable node is labelled `foo()`, so
this hit any code repo -- 27 of 1141 links on a 2247-node graph.
Rather than encode on one side and hope the other side decodes, keep the slug
free of anything that would need encoding, and emit it raw. `_safe_filename`
now also removes `(` `)` `#` `%` and control characters, so the link and the
filename are the same string by construction.
Parentheses are dropped rather than substituted: substituting would leave a
trailing `foo__` on every callable, and collapsing the resulting runs would
mangle Python dunders (`__init__()` -> `_init_`). Dropping keeps `__init__`
intact; labels that collapse to one slug are still separated by `_unique_slug`.
Non-ASCII is deliberately left alone. The report suggested stripping it, but it
is legal raw in a CommonMark link destination and resolves on every filesystem
graphify targets -- stripping it would reduce a CJK, Cyrillic or accented wiki
to a wall of underscores. Leaving it raw is what fixes the em-dash case.
test_wiki_links_resolve_to_real_files already asserted this invariant, but
`_inline_links` unquoted the target before checking it, so the guard passed
straight through the bug. It now compares the target as written.
Fixes Graphify-Labs#2597
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. 2 change(s) tested, no difference found (not proven).
Graphify review — findings
This PR changes how the wiki generator produces markdown link targets, moving from percent-encoding the slug via urllib.parse.quote to emitting the on-disk filename verbatim, with the stated intent of keeping a link's target identical to the file it names (referencing issue #2597). To support this, _safe_filename is expanded to strip parentheses and substitute an additional set of characters (#, %, control chars, etc.) so slugs never contain anything that would require encoding, while non-ASCII characters are deliberately preserved. The surface area includes graphify/wiki.py (_safe_filename, _md_link), updates to existing assertions and the _inline_links helper in tests/test_wiki.py, and a new regression test file tests/test_wiki_link_filename_parity.py asserting link/filename parity across various label shapes.
No blocking issues surfaced. 4 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 222 functions depend on the 78 functions this change touches.
Health — this change adds coupling hotspots:
- worse:
to_wiki()— 36 callers, 6 callees
Verification — 222 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: 83 function(s) in the blast radius were not formally verified this run
Formal verification
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.
No difference found (not proven): No behavior difference found in \_safe\_filename (not a proof).
The verifier ran both versions of \_safe\_filename 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 more finding(s) on lines outside this diff (see the check run).
Fixes #2597.
Thanks @ericziko — the report was precise enough to reproduce first try, and the root cause is exactly where you pointed.
Reproduced
graphify's own package, 2247 nodes / 4492 edges / 152 communities,
export wiki:Same signature as your run: every failure decodes to a real file, nothing is genuinely missing.
Change
_md_linkemittedquote(f"{slug}.md")whileto_wikiwrote the file under the raw slug, so the two agreed only when the label happened to be URL-safe. Rather than encode on one side and hope the other side decodes, this keeps the slug free of anything that would need encoding and emits it raw — one string, not two spellings._safe_filenamenow also removes()#%and control characters, on top of the Windows-reserved set it already handled._md_linkdropsquote().Two deliberate departures from the suggested fix, both to avoid losing information:
Parentheses are dropped, not substituted. Substituting with
_leaves a trailingfoo__on every callable node, and collapsing the resulting runs to tidy that up would mangle Python dunders —__init__()becomes_init_. Dropping keeps__init__intact. Labels that then collapse to one slug (parse()andparse) are still separated by the existing_unique_slug, and there is a test for it.Non-ASCII is left alone. The report suggested replacing it, but it is legal raw in a CommonMark link destination and resolves on every filesystem graphify targets. Stripping it would reduce a CJK, Cyrillic or accented wiki to a wall of underscores — and leaving it raw is precisely what fixes your
Tailscale HTTPS endpoints — how services get their URLscase, since the em dash was only ever broken by the encoding step.I did not take the angle-bracket option.
[x](<_make_id().md>)is valid CommonMark, but the naive\]\(([^)]+\.md)\)scan an agent is likely to use finds no link at all in that form, so it trades one agent-crawlability problem for another.The guard that should have caught this
test_wiki_links_resolve_to_real_filesalready asserted "every link target exists on disk" — the exact invariant. It passed all the way through this bug because the_inline_linkshelper ranurllib.parse.unquoteon the target before checking it, papering over the mismatch it exists to catch. That helper now compares the target as written, which is what turns the existing guard into a real one.That felt worth fixing in place rather than only adding new tests beside it.
Tests
tests/test_wiki_link_filename_parity.py(15 tests) pins the invariant: for every emitted link,(wiki_dir / target).exists()with no unquoting step. Parametrized over the character classes from the report — callables, dunders,&,#,%, em dash, CJK, brackets — plus the collision case and a whole-wiki crawl.Two existing tests in
test_wiki.pyasserted the old encoded targets (C%23_%26_Auth_%28v2%29.md,Array%5BT%5D_Models.md) while asserting the raw filename on disk in the next line — they were encoding the mismatch itself. Updated to assert byte-identity. Flagging that explicitly since changing existing assertions deserves a second look.Reverting
graphify/wiki.pyand keeping the tests fails 14 of them, including all the character classes from the report.Validation
Windows 11, Python 3.12, branched off
26128ab.47 failed, 4247 passed→47 failed, 4262 passed. Identical failure set — no regressions; the +15 are the new tests. (The 47 are pre-existing Windows failures — symlink privileges,os.geteuid, and friends — unrelated to this change.)tools.skillgenvalidators pass._make_id.mdrather than_make_id%28%29.md.Note on #2656
My other open PR (#2656, Windows
MAX_PATH) also touches_safe_filename— it adds alimitparameter to the signature, this one changes the body. They are independent bugs and the changes do not interact, but they will conflict textually. Happy to rebase whichever lands second; no preference on ordering.