fix(doc-gates): review findings — GitHub-slugger parity and the surviving old-rule prose
ds-review-bot round: slugs now come from RENDERED heading text (links, inline code, emphasis) with underscores kept and GitHub's occupied-set repeat suffixes; explicit <a id> anchors register only from real HTML flow (fences, inline code, and comments no longer produce phantoms); fragment matching is exact-case since element ids are. anchorCache is exported and the spec reuses it. The contradicting 'gate checks file existence, not #anchor validity' sentence in docs/AGENTS.md is gone; the dsh-doc-standards residual caveat names the real TS-string anchor homes; the 2026-06-18 cross-link note is updated to shipped behavior and cross-linked with the fragment-gate note.
This commit is contained in:
Tianyi Cui
@@ -5,11 +5,11 @@
|
||||
* external targets stay out of scope.
|
||||
*/
|
||||
|
||||
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { afterEach, describe, expect, it } from 'vitest'
|
||||
import { documentAnchors, findViolations, githubSlug } from './verify-md-links.ts'
|
||||
import { anchorCache, documentAnchors, findViolations, githubSlug } from './verify-md-links.ts'
|
||||
|
||||
const roots: string[] = []
|
||||
afterEach(() => {
|
||||
@@ -27,19 +27,11 @@ function layout(files: Record<string, string>): string {
|
||||
}
|
||||
|
||||
function violationsIn(root: string, rel: string): { url: string; reason: string }[] {
|
||||
const cache = new Map<string, Set<string>>()
|
||||
const anchorsOf = (abs: string): Set<string> => {
|
||||
const hit = cache.get(abs)
|
||||
if (hit) return hit
|
||||
const anchors = documentAnchors(readFileSync(abs, 'utf8'))
|
||||
cache.set(abs, anchors)
|
||||
return anchors
|
||||
}
|
||||
return findViolations(join(root, rel), anchorsOf, root).map(({ url, reason }) => ({ url, reason }))
|
||||
return findViolations(join(root, rel), anchorCache(), root).map(({ url, reason }) => ({ url, reason }))
|
||||
}
|
||||
|
||||
describe('documentAnchors', () => {
|
||||
it('slugs headings, suffixes repeats, and reads explicit <a id> anchors', () => {
|
||||
it('slugs rendered heading text, suffixes repeats, and reads explicit <a id> anchors', () => {
|
||||
const anchors = documentAnchors([
|
||||
'# My Doc',
|
||||
'## Live `events` — mode!',
|
||||
@@ -51,6 +43,34 @@ describe('documentAnchors', () => {
|
||||
expect(anchors).toEqual(new Set(['my-doc', 'live-events--mode', 'repeat', 'repeat-1', 'hand-anchor']))
|
||||
expect(githubSlug('Security and authority are non-goals')).toBe('security-and-authority-are-non-goals')
|
||||
})
|
||||
|
||||
it('keeps underscores the way GitHub does', () => {
|
||||
expect(githubSlug('Showcase: web_fetch')).toBe('showcase-web_fetch')
|
||||
expect(documentAnchors('## Showcase: web_fetch\n')).toEqual(new Set(['showcase-web_fetch']))
|
||||
})
|
||||
|
||||
it('slugs a heading containing a link from its rendered text', () => {
|
||||
expect(documentAnchors('## [Install](setup.md)\n')).toEqual(new Set(['install']))
|
||||
})
|
||||
|
||||
it('bumps repeat suffixes past occupied slugs, matching GitHub', () => {
|
||||
const anchors = documentAnchors(['## Repeat', '## Repeat-1', '## Repeat', ''].join('\n'))
|
||||
expect(anchors).toEqual(new Set(['repeat', 'repeat-1', 'repeat-2']))
|
||||
})
|
||||
|
||||
it('ignores <a id> inside code fences, inline code, and HTML comments', () => {
|
||||
const anchors = documentAnchors([
|
||||
'# Doc',
|
||||
'```md',
|
||||
'<a id="fenced"></a>',
|
||||
'```',
|
||||
'Inline `<a id="inline"></a>` sample.',
|
||||
'<!-- <a id="commented"></a> -->',
|
||||
'<a id="real"></a>',
|
||||
'',
|
||||
].join('\n'))
|
||||
expect(anchors).toEqual(new Set(['doc', 'real']))
|
||||
})
|
||||
})
|
||||
|
||||
describe('findViolations fragments', () => {
|
||||
@@ -68,6 +88,11 @@ describe('findViolations fragments', () => {
|
||||
expect(violationsIn(root, 'a.md')).toEqual([{ url: '#deferred-work', reason: 'anchor' }])
|
||||
})
|
||||
|
||||
it('rejects a case-variant fragment: element ids are case-sensitive', () => {
|
||||
const root = layout({ 'a.md': '# A\n\n## Default Loop\n\n[case](#Default-Loop)\n' })
|
||||
expect(violationsIn(root, 'a.md')).toEqual([{ url: '#Default-Loop', reason: 'anchor' }])
|
||||
})
|
||||
|
||||
it('rejects a cross-file fragment missing from the target document', () => {
|
||||
const root = layout({
|
||||
'a.md': '# A\n\n[stale](b.md#old-heading)\n',
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
import { existsSync, readFileSync } from 'node:fs'
|
||||
import { dirname, relative, resolve } from 'node:path'
|
||||
import type { Nodes } from 'mdast'
|
||||
import { parseMarkdown, visitMarkdown } from './markdown.ts'
|
||||
import { markdownHeadingLines, parseMarkdown, visitMarkdown } from './markdown.ts'
|
||||
import { isArchivedAgentNotePath, uniqueRepoFiles } from './repo-files.ts'
|
||||
|
||||
const root = resolve(import.meta.dirname, '..')
|
||||
@@ -88,45 +88,59 @@ function fragmentPart(url: string): string | null {
|
||||
|
||||
/**
|
||||
* GitHub's heading-slug algorithm (lowercase; drop everything but letters,
|
||||
* numbers, spaces, hyphens; spaces become hyphens) — the same rule
|
||||
* `gen-cordis-catalog`'s region anchors are built from, kept in sync by the
|
||||
* corpus passing this gate rather than by a shared import across the
|
||||
* script/package boundary.
|
||||
* @param heading - the rendered heading text.
|
||||
* numbers, underscores, spaces, hyphens; spaces become hyphens). Underscores
|
||||
* survive (`## Showcase: web_fetch` → `#showcase-web_fetch`), unlike
|
||||
* `gen-cordis-catalog`'s region-anchor slugs — the generator's headings are
|
||||
* always reachable through its explicit `<a id>` anchors, so the two need not
|
||||
* share one rule.
|
||||
* @param heading - the RENDERED heading text (Markdown syntax already gone).
|
||||
* @returns the anchor GitHub assigns the first occurrence of the heading.
|
||||
*/
|
||||
export function githubSlug(heading: string): string {
|
||||
return heading.toLowerCase().replace(/[^\p{L}\p{N} -]/gu, '').replaceAll(' ', '-')
|
||||
return heading.toLowerCase().replace(/[^\p{L}\p{N}_ -]/gu, '').replaceAll(' ', '-')
|
||||
}
|
||||
|
||||
/**
|
||||
* Every anchor one Markdown document exposes: each heading's GitHub slug
|
||||
* (repeated headings get the renderer's `-1`, `-2`, … suffixes) plus every
|
||||
* explicit `<a id="…">`. Lowercased for case-insensitive fragment matching.
|
||||
* Every anchor one Markdown document exposes: each heading's GitHub slug —
|
||||
* computed from the RENDERED heading text, so links, images, inline code, and
|
||||
* emphasis inside a heading slug the way GitHub renders them — plus every
|
||||
* explicit `<a id="…">` that appears in real HTML flow (a fenced or inline
|
||||
* code sample and a commented-out anchor register nothing). Repeated slugs
|
||||
* get GitHub's occupied-set `-1`, `-2`, … suffixes: each collision bumps the
|
||||
* ORIGINAL slug's counter until a free name is found, so `Repeat`, `Repeat-1`,
|
||||
* `Repeat` yields `repeat`, `repeat-1`, `repeat-2`. Matching is exact —
|
||||
* element ids are case-sensitive.
|
||||
* @param source - the document's full Markdown text.
|
||||
* @returns the set of valid fragments for links into this document.
|
||||
*/
|
||||
export function documentAnchors(source: string): Set<string> {
|
||||
const anchors = new Set<string>()
|
||||
const seen = new Map<string, number>()
|
||||
const tree = parseMarkdown(source)
|
||||
visitMarkdown(tree, (node: Nodes): void => {
|
||||
if (node.type === 'heading') {
|
||||
const text = source.slice(node.position?.start.offset ?? 0, node.position?.end.offset ?? 0)
|
||||
.replace(/^#{1,6}\s+/, '')
|
||||
.replace(/[`*_]/g, '')
|
||||
const base = githubSlug(text)
|
||||
const bump = seen.get(base) ?? 0
|
||||
seen.set(base, bump + 1)
|
||||
anchors.add(bump === 0 ? base : `${base}-${bump}`)
|
||||
const occurrences = new Map<string, number>()
|
||||
for (const heading of markdownHeadingLines(source)) {
|
||||
const base = githubSlug(heading.text)
|
||||
let result = base
|
||||
let bump = occurrences.get(base) ?? 0
|
||||
while (anchors.has(result)) {
|
||||
bump += 1
|
||||
result = `${base}-${bump}`
|
||||
}
|
||||
occurrences.set(base, bump)
|
||||
anchors.add(result)
|
||||
}
|
||||
visitMarkdown(parseMarkdown(source), (node: Nodes): void => {
|
||||
if (node.type !== 'html') return
|
||||
const html = node.value.replace(/<!--[\s\S]*?-->/g, '')
|
||||
for (const match of html.matchAll(/<a id="([^"]+)"/g)) anchors.add(match[1] ?? '')
|
||||
})
|
||||
for (const match of source.matchAll(/<a id="([^"]+)"/g)) anchors.add((match[1] ?? '').toLowerCase())
|
||||
return anchors
|
||||
}
|
||||
|
||||
/** Lazily collect and cache the anchor set of any existing Markdown file. */
|
||||
function anchorCache(): (absPath: string) => Set<string> {
|
||||
/**
|
||||
* Lazily collect and cache the anchor set of any existing Markdown file —
|
||||
* shared across all scanned sources so a target parses once.
|
||||
* @returns the memoized absolute-path → anchor-set lookup.
|
||||
*/
|
||||
export function anchorCache(): (absPath: string) => Set<string> {
|
||||
const cache = new Map<string, Set<string>>()
|
||||
return (absPath) => {
|
||||
const hit = cache.get(absPath)
|
||||
@@ -169,7 +183,7 @@ export function findViolations(
|
||||
}
|
||||
const fragment = fragmentPart(url)
|
||||
if (fragment === null || !resolved.endsWith('.md')) return
|
||||
if (!anchorsOf(resolved).has(fragment.toLowerCase())) {
|
||||
if (!anchorsOf(resolved).has(fragment)) {
|
||||
out.push({ file, line: node.position?.start.line ?? 0, url, reason: 'anchor' })
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user