fix(compile): repair wikilinks that name a page too briefly to resolve - #193
fix(compile): repair wikilinks that name a page too briefly to resolve#193LorenzoGentile wants to merge 1 commit into
Conversation
Generation writes a concept's short canonical name while the page it means carries a longer descriptive title, so `[[Argo CD]]` sits broken beside `argo-cd-image-update-ownership-model`. Extraction picks the titles and generation picks the link text, independently and in that order, and nothing reconciled them — the link stayed dead with its target on disk. A repair pass now runs after interlink resolution and repoints a link when its slug prefixes exactly one page. Only the target is rewritten: `[[Argo CD]]` becomes `[[argo-cd-image-update-ownership-model|Argo CD]]`, so rendered output is byte-identical and a bad match can only misdirect a link, never alter prose. It refuses to guess. Two candidate pages means no repair; zero means no repair either, because a link to a concept the wiki lacks is a signal about what extraction missed rather than noise to hide — lint still reports it. Targets awaiting review are skipped, since they resolve on approval. Measured on five compiles of a mixed corpus (two PDFs plus five prose documents): 21.3% of broken wikilinks repaired on a GPT-5-class model, 16.7% on gpt-4o-mini, with no page's prose changed.
…s $ (#197) `rewritePage` splices the linked body back into the page with `content.replace(body, linked)`. `String.replace` interprets `$&`, backtick-$, `$'` and `$$` in a STRING replacement even when the search argument is a plain string, and `linked` is page prose. Measured on a page reading "The PID is $$ and sed uses <backtick-$> for the prefix.": `$$` collapsed to a single `$`, and the backtick-$ spliced this page's own frontmatter into the middle of the sentence. `$&` duplicates the whole body; `$'` deletes the text after it. Shell, sed, awk and Makefile pages carry those sequences as ordinary prose, and `$$` for a process id is common, so this is reachable by compiling ordinary technical documentation. It was also silent: the link resolved correctly and the page was rewritten, so nothing failed — the prose around the link was just wrong afterwards. Passing a replacer FUNCTION suppresses the substitution entirely, so the rewritten body is inserted verbatim. Every other non-regex `.replace()` in `src/` was swept: the rest pass a replacer function or a literal with no `$`. The four regression cases assert EXACT whole-file equality rather than containment. An earlier draft used `toContain` and passed against the bug for `$&`, because duplicating the body leaves every substring of the original present — the page said everything twice and the assertions were satisfied. Only comparing the whole file tests the property this pass actually promises, which is that nothing but the link changes. Found while reviewing #193, which copies this line into a second pass.
There was a problem hiding this comment.
The heuristic here is right, and it is the hard part. I checked the rule directly rather than reading it:
argo-cd -> argo-cd-image-update-ownership-model (repairs)
test -> null (does NOT claim testing-guide)
python [2 pages] -> null (refuses ambiguity)
ab -> null (below MIN_REPAIRABLE_SLUG_LENGTH)
foo [foo] -> null (exact match is not a prefix match)
targetSlug + SLUG_SEPARATOR is what stops test claiming testing-guide: matching on a segment boundary rather than a raw string prefix. That is the difference between this working and it being a silent mis-linker, and it is easy to get wrong. Your similarity-scoring argument holds too, and leaving genuinely-missing pages broken because they inventory what extraction missed is a judgement I would not have thought to ask for.
Four things before it lands. Three are yours; one was ours and is already fixed.
Already fixed on main: the $ splice
page.content.replace(body, result.body) interprets $&, $`, $' and $$ in the replacement, even with a plain-string search argument. Since result.body is page prose, a page containing any of those is rewritten wrongly - $` splices the page's own frontmatter into the sentence, $& duplicates the body, $$ collapses to $.
You inherited this by copying resolver.ts, which is the right file to copy. It was broken there, live, on every compile that linked a page. That is fixed on main in #197, and rewritePage now reads:
body: content.replace(body, () => linked),Rebase and use that form. Nothing for you to work out; the fixed line is sitting in the file you modelled this on.
The read path should be compile's, not the linter's
repairLinks calls collectAllPages from src/linter/rules-shared.ts, whose readMarkdownFiles does a raw readFile: it follows symlinks, with no directory confinement and no no-follow.
Compile does not read wiki pages that way anywhere else. confined-wiki-read.ts states the contract: a wiki file that is a symlink escaping the project root is dropped, never read and never re-emitted. Six compile call sites honour it, and resolver.ts is one of them - linkPage reads through readWikiPageContentOrWarn. This is the one place the PR departs from the model it otherwise follows, and the consequence is what the contract exists to prevent: an escaping wiki/concepts/*.md symlink is read and its out-of-tree bytes reach a CompilePageWrite. It also reintroduces blocking reads on special files.
Read through the confined reader and skip dropped entries, the way resolver.ts does.
Fenced examples get rewritten
WIKILINK_PATTERN is a global regex over the whole body with no Markdown structure awareness, so a fenced example of [[Argo CD]] is rewritten to the long target. Markdown renders fenced content literally, so a reader sees the change.
The codebase has no fence-awareness anywhere, and addWikilinks in the resolver will equally wrap a bare title mention inside a fence, so the gap is not new. But the resolver never promised byte-identical rendering and this PR does. Either the handling or the claim has to give, and I would rather it be the handling.
Pending candidates do not participate in ambiguity
existing.has(targetSlug) || pending.has(targetSlug) ? null : resolveUniquePrefix(targetSlug, slugs)pending is consulted for an exact slug only, and resolveUniquePrefix searches live slugs. So with a live argo-cd-deployment-patterns, a pending argo-cd-image-update-ownership-model, and [[Argo CD]]: the pending candidate is invisible, the live page looks unique, and the link is repointed to it. That is the misdirection the comment above the check says it prevents, and the prefix case is exactly where the pass acts.
There is a second half worth deciding on. review-approve.ts:314 runs only resolveAndApplyLinks, so a link correctly left alone because its only match was pending does not repair when that candidate is approved. It waits for the next full compile.
Smaller
slugifyTarget reimplements slugify from src/utils/markdown.ts, the module you already import on line 24. I compared them over 15 adversarial inputs - curly quotes, CJK, C++, 50%, collapsing separators, leading and trailing dashes - and they are identical on all of them. Two copies of an algorithm that must agree is the drift this repo keeps removing; import it and delete the copy.
No CI has run on the branch, and it now conflicts with main. Worth a rebase and an approved run, since the $ issue is the sort of thing a test over realistic page bodies catches.
Sorry for the wait on this one, and on your other two, and thanks for the detailed contributions!
2 participants
What
Compile now repairs a wikilink whose target does not exist but unambiguously names a page that does:
[[Argo CD]]→[[argo-cd-image-update-ownership-model|Argo CD]].Why
Extraction chooses page titles; generation chooses link text. They run independently, in that order, and nothing reconciles them. Generation reaches for a concept's short canonical name while the page carries a longer descriptive title, so the link sits broken with its target already on disk:
[[Argo CD]]againstargo-cd-image-update-ownership-model[[Alembic]]againstalembic-database-migration-conventions[[genetic algorithm]]againstgenetic-algorithm-coupled-with-abaqus-finite-element-analysisThese are the same concept, so this is a naming-granularity mismatch rather than a hallucinated link. Similarity scoring is the wrong instrument for it — those pairs score 0.31 to 0.69 on
SequenceMatcher, so any threshold safe enough to use misses all of them. Prefix containment catches them exactly.What it will not do
It never rewrites prose. Only the link target changes; the displayed text is preserved, including an existing alias. Rendered output is byte-identical, so the worst a bad match can do is point a link at the wrong page.
It refuses to guess. A slug prefixing two pages is left alone. So is one prefixing none — a link to a concept the wiki genuinely lacks is a signal about what extraction missed, and
lintstill reports it asbroken-wikilink. Hiding those would trade a useful inventory for a clean metric.Targets awaiting review are skipped, since they resolve when the candidate is approved; repointing one now would redirect the link away from the page the author is about to publish.
Measured
Five compiles of a mixed corpus — a 182k-character PDF thesis truncated to 100k on ingest, a 1-page PDF, and five prose documents, none containing wikilinks:
gpt-4o-minir1gpt-4o-minir221.3% and 16.7% respectively, with no page's prose changed.
Design notes
Placed in
finalizeWikiimmediately afterresolveAndApplyLinks, inside the same already-held lock, routed through the same lock-freeapplyCompilePageWritesLockedseam. Ordered after resolution so a link resolution has just created is already valid and never looks repairable.It scans all pages rather than only changed ones. A link broken today becomes repairable the moment a later compile creates the page it names, and that page's arrival never touches the file holding the link. The pass reads files and calls no model, so it costs the same order as
llmwiki lint.Two things I'd like your call on:
--no-repair-wikilinksif you'd rather it be switchable.Testing
Full suite 4870 passed, 3 skipped.
fallow:ci0 above threshold.test/link-repair.test.tscovers the unique-prefix repair, alias preservation, byte-identical rendering, ambiguous and no-candidate refusal, already-resolving links, a too-short slug, idempotence, multiple occurrences across pages, and frontmatter being untouched.