fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(docs): the review's four rendering defects, and the check that could see one

Browse Source 
Review of PR #62 found four live defects and four smaller ones. The most
valuable is the first, because of what it says about the rest.

A TOC entry wrapping a heading whose rendered text already contained a link
emitted <a href="#anchor"><a href="github...">...</a></a>. The parser closes
the outer anchor early, so the entry became an empty unclickable stub and
clicking "src/draw.rs" in the TOC navigated to GitHub. 19 occurrences over 10
pages. Both hrefs resolved perfectly, so the link check, the whole-site link
check and a screenshot pass all missed it -- it answers "does every href
resolve" and this is not that question. So the fix is two things: the TOC keeps
a tag-free variant of each heading, AND a new check asserts no <a> appears
inside another <a>. Proved by reverting the first half: 66 nested anchors
reported, assembly exits 1. Clicking that TOC entry now lands on #src-draw-rs.

esc() escaped & < > but not the quote it was interpolated into title="..."
with. Five shipped bead titles contain one, and Chromium parsed the resulting
tag as having three junk attributes. Now escapes " and ' as well. Today's
corpus being friendly is not a reason to interpolate tracker text unescaped.

Seven pages overflowed at 390px by up to 92%. The reviewer attributed it to the
TOC; the TOC was real but not sufficient, and the first fix measured clean by
getBoundingClientRect while the page still overflowed, because the heading text
overflows its box without widening it. Only per-element scrollWidth names the
culprit: hundreds of snake_case test names as headings. overflow-wrap on the
headings and on .toc a, plus dropping display:table from .tickets, which had
been cancelling the mobile overflow-x guard. 0 of 23 pages overflow at 390 and
1280 now, measured.

table() split rows on every pipe before inline parsing, so `|Time Period|` --
this repo's own quoting syntax, in a corpus whose subject is that syntax -- was
shredded into extra columns. A row that still parses, so no column count could
see it. Now walked, respecting code spans and \|. Proved with a fixture: 9 tds
with the fix, 12 with the old splitter reinstated.

The four smaller ones: the link check counted 48 links it never verified behind
a comment promising they were "checked below" when there was no below -- those
are the links leaving the docs tree, and they are now resolved against the
filesystem, which is why assemble.sh runs the generator LAST, once its siblings
exist. The gotcha jump bar's hardcoded 48 is derived from the emitted anchors,
because it failed closed on deletion and silently on addition, and the list only
grows. context/code/doc-site.md cited improvise-xxxx and so broke the audit the
surrounding paragraph describes; the repo-wide audit is now 189 ids, all
resolving. And the Conventions paragraph's own example path now goes through
inline(), so the sentence demonstrates the linking instead of asserting it.

The ticket index keeps all 180 rows, open bugs and titles included, by the
user's ruling: the data is already public in .beads/issues.jsonl, the page only
makes it legible, and filtering it while the lede links the raw JSONL would be
theatre.

Gates: 1378 rust tests, 26 node --test, 16 playwright (3 skipped, webkit OPFS),
all three check-comment-runs modes, 3460 site links / 0 broken, 0 nested
anchors, 189 bead ids all resolving.

improvise-djoo

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Edward Langley
2026-09-20 13:56:56 -07:00
parent f30d512169
commit 1a0f04856e
3 changed files with 180 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/doc-site.md
+75 -13
View File
@@ -54,7 +54,10 @@ a narrative.
## 2. Ticket ids resolve to a page, never to a URL that does not exist
The prose cites `improvise-xxxx` roughly 190 times. On a public site those are
The prose cites bead ids roughly 190 times — this paragraph deliberately spells
none, because a document that gives one as an example makes itself a source of
audit noise, and a placeholder is worse than a real id: it is ID-shaped and
resolves to nothing. On a public site those citations are
meaningless strings, and the obvious repair — link them to an issue tracker —
is unavailable: beads keeps its issues in a Dolt database on a git ref, and
there is no public issue URL to point at. Inventing one would produce 190
@@ -71,10 +74,26 @@ question without a navigation.
defect to work around but a fact to render honestly: an id the export does not
carry is emitted as plain text with a dotted underline and no link, and the
ticket page lists it as "not exported". The site therefore never links to an
anchor it did not generate, whatever state the export is in. At the time of
writing 8 of 178 linkified ids are in that state. A generator that instead
linked every id it saw would produce a page that is right today and broken the
first time somebody cites a fresh ticket.
anchor it did not generate, whatever state the export is in. A generator that
instead linked every id it saw would produce a page that is right today and
broken the first time somebody cites a fresh ticket.
**The page publishes every cited id, open bugs and their titles included, and
that is a decision rather than a default** (user ruling). The data is already
public in `.beads/issues.jsonl` in a repository anyone can clone; the page only
makes it legible. An honest open-bug list is a normal thing for an open-source
project to have, and filtering one while the page's own lede links the raw
JSONL would be theatre. Note the second-order reason to keep it unfiltered:
a citation whose row was suppressed would be a link into a page that does not
answer it, which is the failure this whole section exists to avoid.
**Titles are tracker data and are escaped as attribute values.** Five shipped
bead titles contain a double quote; interpolated into `title="…"` by an `esc`
that handled only `&`, `<` and `>`, they broke out of the attribute and the
browser parsed the remains as three junk attributes. `esc` now escapes `"` and
`'` as well. Today's corpus being friendly is not a reason to interpolate
untrusted-shaped text unescaped — the tracker is written by whoever files a
bug.
## 3. Source paths link only where the file exists
@@ -110,10 +129,29 @@ name that is never generated makes it report 763 missing pages — two different
diagnoses, because "the page is absent" and "the page is there but the anchor
is not" call for different repairs.
What it does **not** cover is links that leave the `docs/` subtree — the ones
to `../index.html` and `../app/`. Those are checked against the assembled
`site/` tree by hand at the moment, and the honest statement is that a rename
of the app directory would not be caught here.
Links that **leave** the `docs/` subtree — `../index.html`, `../app/` — name
files this script does not generate, so they are resolved against the
filesystem instead. That is the whole reason `assemble.sh` runs the generator
**last**: its siblings have to exist before they can be checked. An earlier
version skipped those 48 links while still counting them as checked, behind a
comment promising they were "checked below" when there was no below.
**And the link check is not sufficient, which is the more important lesson.**
It answers one question — does every href resolve — and a defect outside that
question is invisible to it however carefully it is run. The worked example is
a nested `<a>`: a TOC entry wrapping a heading whose rendered text already
contained a link produced `<a href="#anchor"><a href="github…">…</a></a>`, and
the HTML parser closes the outer anchor early, so the entry became an empty
unclickable stub and clicking it navigated to GitHub. Both hrefs resolved
perfectly. It shipped past the link check, past the whole-site link check, and
past a screenshot pass, and was caught by a reviewer *clicking a link*. The
repair is therefore two things, not one: the TOC keeps a tag-free variant of
each heading, **and** a separate check asserts no `<a>` appears inside another
`<a>` — proved by reverting the first half and watching the second report 66.
The general form: when a check misses a defect, add the check that could have
seen it rather than only fixing the defect, and ask what *question* the
existing check was answering.
## 5. Why a node script and not a generator
@@ -142,13 +180,37 @@ may contain a single `*` pair**, because `[^*]+` cannot match
`**a *b* c**` and the leftover `*` then pairs across the bold — which is how a
single sentence in gotcha 33 came out as three italic paragraphs.
A third, added later and for the same reason: **a table cell boundary is an
unescaped pipe outside a code span**, so the row is walked rather than
`split("|")`. This corpus documents a file format whose own quoting syntax is
`|Time Period|`, and a sentence containing one inside a table cell would have
been shredded into extra columns — a row that still parses, so no column count
could detect it. It was latent when fixed, which is the point: the trigger is
one ordinary sentence away in a document whose subject is pipe quoting.
## 6. Navigation
`repo-map.md` is one long document whose gotcha list is numbered 1 to 48, and
splitting it per gotcha was rejected: the gotchas cross-reference each other
constantly ("see gotcha 35", "for the reason gotcha 39 watches…"), and a reader
following those wants a scroll, not 48 page loads. It stays one page, with an
id on every list item and a compact strip of 48 numbered links under the
heading. The ids come from the list item's **own number**, not from a counter,
so an inserted gotcha renumbers the anchors exactly as it renumbers the prose
that cites them.
id on every list item and a compact strip of numbered links under the heading.
The ids come from the list item's **own number**, not from a counter, so an
inserted gotcha renumbers the anchors exactly as it renumbers the prose that
cites them.
The strip's contents are **derived from the anchors actually emitted**, never
from a count. A hardcoded 48 failed in only one direction: deleting a gotcha
left a link to a missing anchor, which the link check catches, while *adding*
one produced an unlinked gotcha and a clean build. The list only ever grows, so
the hardcoded form was silent in precisely the direction it would be used —
the same shape as an exclusion list that over-matches, failing in the direction
that looks like success.
**Long unbreakable headings need `overflow-wrap`.** The test-evidence pages are
hundreds of `snake_case` test names as headings; at phone width seven of them
overflowed the viewport by up to 92%. The diagnosis is worth keeping because
the first guess was wrong: `getBoundingClientRect` reported every element
inside the viewport, because the text overflows its box without widening it.
Only `scrollWidth` per element names the culprit. Measure the property that
actually overflowed rather than the one that usually does.
scripts/build-doc-site.mjs
+98 -20
View File
@@ -110,7 +110,18 @@ const NON_TICKET = new Set(
// ---------------------------------------------------------------------------
// Markdown. A deliberately small subset, covering exactly what the corpus uses;
// anything unrecognised becomes paragraph text, so no input line is ever lost.
const esc = (s) => s.replace(/&/g, "&amp;").replace(/</g, "&lt;").replace(/>/g, "&gt;");
const esc = (s) =>
s
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;")
.replace(/'/g, "&#39;");
// A TOC entry must not nest an anchor inside the one wrapping it: a heading
// whose text is a path or a ticket id already carries an <a>, and the parser
// closes the outer one early, leaving an empty unclickable stub.
const stripTags = (html) => html.replace(/<[^>]+>/g, "");
function slugify(s) {
return (
@@ -212,7 +223,7 @@ function renderMarkdown(src, ctx) {
seen.set(id, n);
if (n > 1) id = `${id}-${n}`;
ctx.anchors.add(id);
if (level <= 3) toc.push({ level, id, text: rendered });
if (level <= 3) toc.push({ level, id, text: stripTags(rendered) });
const mark = level > 1 ? `<a class="anchor" href="#${id}">#</a>` : "";
return `<h${level} id="${id}">${mark}${rendered}</h${level}>`;
}
@@ -350,14 +361,44 @@ function renderMarkdown(src, ctx) {
return [`<${tag}${attr}>\n${rendered}\n</${tag}>`, i];
}
// A cell boundary is an UNESCAPED pipe outside a code span. Splitting the raw
// row on every `|` shreds `\|` and this repo's own `|Time Period|` syntax into
// extra columns, which no column count can detect because the row still parses.
function splitRow(r) {
const out = [];
let cur = "";
let tick = 0;
for (let i = 0; i < r.length; i++) {
const c = r[i];
if (c === "\\" && r[i + 1] === "|" && !tick) {
cur += "|";
i++;
continue;
}
if (c === "`") {
let n = 0;
while (r[i + n] === "`") n++;
if (!tick) tick = n;
else if (tick === n) tick = 0;
cur += "`".repeat(n);
i += n - 1;
continue;
}
if (c === "|" && !tick) {
out.push(cur);
cur = "";
continue;
}
cur += c;
}
out.push(cur);
if (out.length && !out[0].trim()) out.shift();
if (out.length && !out[out.length - 1].trim()) out.pop();
return out;
}
function table(rows) {
const cells = (r) =>
r
.trim()
.replace(/^\|/, "")
.replace(/\|$/, "")
.split("|")
.map((c) => inline(c.trim(), ctx));
const cells = (r) => splitRow(r.trim()).map((c) => inline(c.trim(), ctx));
const head = cells(rows[0]);
const body = rows.slice(2).map(cells);
return (
@@ -383,7 +424,7 @@ nav.side a{display:block;padding:.2rem .45rem;border-radius:4px;color:var(--fg);
nav.side a:hover{background:var(--panel)}
nav.side a.here{background:var(--panel);color:var(--accent);font-weight:600}
main{flex:1 1 auto;min-width:0;max-width:60rem;padding:2.5rem 2rem 6rem}
h1,h2,h3,h4,h5{font-family:var(--mono);font-weight:600;letter-spacing:-.01em;line-height:1.3}
h1,h2,h3,h4,h5{font-family:var(--mono);font-weight:600;letter-spacing:-.01em;line-height:1.3;overflow-wrap:anywhere}
h1{font-size:2rem;margin:0 0 1.2rem;color:var(--accent)}
h2{font-size:1.25rem;margin:2.6rem 0 .6rem;padding-top:1.2rem;border-top:1px solid var(--rule)}
h3{font-size:1.02rem;margin:1.8rem 0 .4rem;color:#c8cdd6}
@@ -407,9 +448,9 @@ hr{border:0;border-top:1px solid var(--rule);margin:2rem 0}
.toc{background:var(--panel);border:1px solid var(--rule);border-radius:6px;padding:.8rem 1.1rem;margin:0 0 2rem;font-size:.92rem}
.toc summary{cursor:pointer;color:var(--dim);font-family:var(--mono)}
.toc ul{list-style:none;margin:.6rem 0 0;padding:0;columns:2;column-gap:2rem}
.toc li{break-inside:avoid;margin:.12rem 0}
.toc li{break-inside:avoid;margin:.12rem 0;overflow-wrap:anywhere}
.toc li.l3{padding-left:1rem;font-size:.92em}
.toc a{text-decoration:none}
.toc a{text-decoration:none;overflow-wrap:anywhere}
.toc a:hover{text-decoration:underline}
.jump{margin:0 0 2rem;font-size:.85rem;font-family:var(--mono)}
.jump a{display:inline-block;min-width:2.1rem;text-align:center;padding:.12rem .3rem;margin:.1rem;border:1px solid var(--rule);border-radius:4px;text-decoration:none}
@@ -420,8 +461,9 @@ hr{border:0;border-top:1px solid var(--rule);margin:2rem 0}
.card{background:var(--panel);border:1px solid var(--rule);border-radius:6px;padding:.9rem 1.1rem}
.card a{font-family:var(--mono);text-decoration:none}
.card p{color:var(--dim);font-size:.88rem;margin:.3rem 0 0}
.tickets{width:100%;display:table}
.tickets{width:100%}
.tickets td:first-child{white-space:nowrap;font-family:var(--mono);font-size:.86rem}
.tickets td:last-child{overflow-wrap:anywhere}
.st{font-size:.78rem;color:var(--dim);text-transform:uppercase;letter-spacing:.05em}
@media(max-width:900px){.shell{display:block}nav.side{position:static;flex:none;max-height:none;border-right:0;border-bottom:1px solid var(--rule)}main{padding:1.5rem 1rem 4rem}.toc ul{columns:1}}`;
@@ -497,10 +539,17 @@ for (const section of SECTIONS) {
.join("")}</ul></details>`
: "";
const jump =
slug === "repo-map"
? `<p class="jump">Gotchas: ${Array.from({ length: 48 }, (_, k) => `<a href="#gotcha-${k + 1}">${k + 1}</a>`).join("")}</p>`
: "";
// Derived from the anchors actually emitted. A hardcoded count fails closed
// on a deleted gotcha (the link check catches it) and SILENTLY on an added
// one, which is the direction the list moves in.
const gotchas = [...ctx.anchors]
.map((a) => /^gotcha-(\d+)$/.exec(a))
.filter(Boolean)
.map((m) => Number(m[1]))
.sort((a, b) => a - b);
const jump = gotchas.length
? `<p class="jump">Gotchas: ${gotchas.map((n) => `<a href="#gotcha-${n}">${n}</a>`).join("")}</p>`
: "";
const srcLine = `<p class="src">Source: <a href="${REPO}/blob/${ref}/${src}">${src}</a></p>`;
written.push({
@@ -538,6 +587,15 @@ for (const section of SECTIONS) {
// Landing page for the docs section.
{
// The two conventions go through `inline` rather than being hand-written, so
// the sentence describing the linking demonstrates it instead of asserting it.
const conventions = inline(
"Ids like `improvise-9c8c` are tracker issues and link to the ticket index below. " +
"Paths like `crates/improvise-core/src/model/types.rs` link to the source on GitHub at `" +
ref +
"`.",
{ depth: 0, cited, unresolved, anchors: new Set() },
);
const cards = SECTIONS.map(
(s) =>
`<h2 id="${s.id}"><a class="anchor" href="#${s.id}">#</a>${s.title}</h2><p class="lede">${s.blurb}</p><div class="cards">${s.docs
@@ -549,13 +607,22 @@ for (const section of SECTIONS) {
<p class="lede">Nothing here is a tutorial. For what the program does, see the <a href="../index.html">project home</a> or run the <a href="../app/">browser demo</a>.</p>
${cards}
<h2 id="conventions"><a class="anchor" href="#conventions">#</a>Conventions</h2>
<p>Ids like <code>improvise-9c8c</code> are tracker issues and link to the <a href="tickets.html">ticket index</a>. Paths like <code>crates/improvise-core/src/model/types.rs</code> link to the source on GitHub at <code>${ref}</code>.</p>`;
<p>${conventions}</p>`;
written.push({ rel: "index.html", html: page({ slug: "", title: "Documentation", depth: 0, body }), anchors: new Set(["conventions", ...SECTIONS.map((s) => s.id)]) });
}
for (const w of written) write(w.rel, w.html);
// ---------------------------------------------------------------------------
// An <a> nested in an <a> is closed early by the parser, so the outer link
// becomes an empty unclickable stub and the inner one wins. The link check
// cannot see it: both hrefs resolve. It asks a different question.
const nested = [];
for (const w of written) {
for (const m of w.html.matchAll(/<a\b[^>]*>((?:(?!<\/a>)[\s\S])*?)<a\b/g))
nested.push(`${w.rel}: <a> inside <a> near ${JSON.stringify(m[1].slice(0, 60))}`);
}
// Link check over the generated output. Every internal href must resolve to a
// generated file, and every fragment to an id that page really carries.
const anchorsByPage = new Map(written.map((w) => [w.rel, w.anchors]));
@@ -574,7 +641,16 @@ for (const w of written) {
continue;
}
const resolved = relative(".", join(base === "." ? "" : base, pathPart));
if (resolved.startsWith("..")) continue; // leaves the docs tree: checked below
// A link out of the docs tree (../index.html, ../app/) names a file this
// script does not generate, so it is checked on disk instead — which is why
// assemble.sh builds the docs LAST, once its siblings exist.
if (resolved.startsWith("..")) {
const onDisk = join(out, base === "." ? "" : base, pathPart);
const withIndex = raw.endsWith("/") ? join(onDisk, "index.html") : onDisk;
if (!existsSync(withIndex) && !existsSync(onDisk))
broken.push(`${w.rel} -> ${raw} (no such file outside the docs tree: ${withIndex})`);
continue;
}
const target = resolved.endsWith("/") ? `${resolved}index.html` : resolved;
if (!files.has(target)) broken.push(`${w.rel} -> ${raw} (no such page: ${target})`);
else if (frag && !anchorsByPage.get(target)?.has(frag))
@@ -585,7 +661,9 @@ for (const w of written) {
console.log(`doc-site: ${written.length} pages -> ${out}`);
console.log(`doc-site: ${cited.size} ticket ids cited, ${cited.size - unresolved.size} resolve, ${unresolved.size} not in the export`);
console.log(`doc-site: ${checked} internal links checked, ${broken.length} broken`);
if (broken.length) {
console.log(`doc-site: ${nested.length} nested anchors`);
for (const n of nested) console.error(`doc-site: nested anchor: ${n}`);
if (broken.length || nested.length) {
for (const b of broken) console.error(`doc-site: broken link: ${b}`);
process.exit(1);
}
web/assemble.sh
+7 -5
View File
@@ -42,11 +42,6 @@ mkdir -p "$out"
cp -f "$root/docs/index.html" "$out/index.html"
cp -rf "$root/docs/casts" "$out/casts"
# The prose site under `docs/`, generated from context/ by the same script the
# harness runs. It link-checks its own output and exits non-zero on a break, so
# a dead cross-reference fails the assembly rather than shipping.
node "$root/scripts/build-doc-site.mjs" "$out/docs" "$token"
app="$out/app"
mkdir -p "$app"
# shell.js and serial.js are imported by app.js and worker.js: leaving either
@@ -103,3 +98,10 @@ for entry in app.js worker.js; do
}
done
done
# The prose site at `docs/`, generated from context/ by the same script the
# harness serves. It runs LAST because its link check verifies the links that
# leave its own tree (`../index.html`, `../app/`) against the filesystem, which
# means their targets must already be here. It exits non-zero on a break, so a
# dead cross-reference fails the assembly rather than shipping.
node "$root/scripts/build-doc-site.mjs" "$out/docs" "$token"