Skip to content

feat(pipeline): link markdown file references into the graph - #1832

Merged
DeusData merged 3 commits into
DeusData:mainfrom
cdeust:feat/doclinks-markdown
Sep 25, 2026
Merged

DeusData merged 3 commits into
DeusData:mainfrom
cdeust:feat/doclinks-markdown

Conversation

@cdeust

@cdeust cdeust commented Aug 25, 2026 •

Copy link
Copy Markdown
Contributor

Markdown docs reference other repo files constantly (a coding-standards doc links the modules it governs, a README points at entry points) but none of that surfaced as edges, so fan-in queries were blind to documentation hubs. In an A/B retrieval benchmark I ran in August on a docs-heavy repo, the graph's top fan-in answer was off by roughly 17x: the most-referenced file was a standards doc with 173 references and zero inbound edges.

New pre-dump pass, pass_doclinks.c, modeled on pass_configlink.c: three strategies emit REFERENCES_FILE edges between existing File nodes only (unresolvable targets are dropped, the pass never invents nodes): inline links, backtick paths, bare path mentions. Targets resolve against the referencing file's directory and the repo root; repeated references collapse into one edge carrying strategy, confidence and count. Registered in the pre-dump sequence after configlink and in the incremental post-passes; REFERENCES_FILE added to the skill's edge-type list and the structural/language contract tests.

Tests mirror test_configlink.c (real files in a tmpdir, File nodes in a gbuf, run the pass, assert edges): inline link, backtick, bare mention, http/anchor ignored, anchor-suffixed file link, dedupe with count, relative-vs-root resolution, unresolvable-target guard, NULL repo_path skip.

A companion change adds the same linking for shell files (source lines and script invocations); split out to keep each change reviewable.

Validation: scripts/build.sh clean; focused serial runner (doclinks, configlink, pipeline, edge_structural, lang_contract) 340 passed under ASan/UBSan; cppcheck clean on the new files with the repo's flags.

What does this PR do?

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)
  • New behavior is covered by a test (reproduce-first for bug fixes)

@cdeust
cdeust requested a review from DeusData as a code owner August 25, 2026 11:38
@github-actions

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

@cdeust
cdeust force-pushed the feat/doclinks-markdown branch 3 times, most recently from 3f48c01 to d72a5c7 Compare August 25, 2026 12:27
@DeusData DeusData added enhancement New feature or request parsing/quality Graph extraction bugs, false positives, missing edges priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Sep 1, 2026
@DeusData

DeusData commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Thank you for the concrete documentation-link benchmark, the dedicated pass, and the negative and deduplication controls. This introduces new graph edge semantics and several source-text heuristics, so we need more time to review precision, incremental parity, and security boundaries carefully. The contribution queue is quite full, but we will return with grounded feedback as soon as possible.

@DeusData

DeusData commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Maintainer decision: REFERENCES_FILE is accepted. A new edge type has to earn itself here — the default is to map a new construct onto an existing one — so it is worth saying what earned it.

None of the nine existing types means "mentions". DEPENDS_ON and CONFIGURES are causal claims, IMPORTS is language-level, USAGE is symbol-level. Forcing a documentation reference onto any of them would not just be imprecise — it would make that type mean less for everyone already querying it, and a fan-in query would silently mix two unlike relations. This is the case where a new type is cheaper than the alternative.

And the defect is measured rather than argued. A standards document with 173 references and zero inbound edges, and a top-fan-in answer off by roughly 17x, is a concrete way for the graph to be confidently wrong about which files matter. That is the kind of blindness that is invisible until someone benchmarks it.

What made this easy to accept

It emits edges between existing File nodes only. Dropping unresolvable targets rather than inventing nodes is the single most important property here — a doc-linking pass that minted File nodes for every path-shaped string would poison the graph rather than enrich it. Saying so explicitly in the description is what let me stop worrying about it early.

Modelling it on pass_configlink.c rather than inventing a pass shape means the next reader meets one pattern twice.

Carrying strategy alongside confidence and count is what de-risks the part I would otherwise have pushed back on. Bare path mentions in prose are the strategy most likely to produce a false edge, and because the strategy is on the edge, a consumer can filter to explicit links only without us having to decide that question up front. Keep that field — it is doing more work than it looks.

Splitting the shell-file companion out was the right call for reviewability.

Two things to do

Strip the Claude-Session: URL from the commit message. Attribution of tool use is welcome here and the Co-Authored-By style trailer is fine; the session URL is the part we do not carry in repository history. An amend and a force-push will do it.

Please rebase — this is DIRTY. main moved three times yesterday: broken by a duplicate-symbol merge, repaired by #1993, then #1703 landed.

Your checklist is entirely unticked, but your validation paragraph covers all of it — build clean, 340 passed under ASan/UBSan across the focused suites, cppcheck clean on the new files. Worth ticking so a reviewer does not have to reconcile the two.

Clearance is REVIEW(1) on Makefile.cbm — the established shape here for registering a new test file. It needs a maintainer marker, not a change from you.

@cdeust
cdeust force-pushed the feat/doclinks-markdown branch from d72a5c7 to 844a36b Compare September 2, 2026 01:44
@cdeust

cdeust commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor Author

Rebased on main (5fbab7b), one commit 844a36b, force-pushed. Conflicts were only in src/pipeline/pipeline.c and Makefile.cbm: kept main's version (sizeof-derived pass count, new ensemble_routing/importance passes) and inserted the doclinks pass / test_doclinks.c right after configlink. All other hunks are byte-identical to the previous commit d72a5c7 (checked by interdiff). The Claude-Session trailer is removed and the checklist is ticked. Locally under ASan/UBSan: doclinks, configlink, pipeline, edge_structural and lang_contract suites = 355 passed. clang-format reports no violation on the diff.

@DeusData ready for review on your side.

@DeusData DeusData left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eighteen days between "accepted" and this review is too long, and I am sorry for it — you did both things I asked within a day. Here is the detailed read. Short version: the design is right and safe, it still merges cleanly after 379 commits, and what stands between it and main is a handful of small, local changes — most of them caused by rules that landed after you opened the PR.

What is right, specifically

  • Safe by construction. There is exactly one file open and it goes through cbm_fopen; only File nodes ending in .md/.mdx are read. Paths found inside markdown are never opened or stat'ed — resolution is a pure string operation followed by a hash lookup on the file_path → File node map. ../../.., absolute paths, file://, drive letters and UNC paths can at worst form a key that misses, and doclink_normalize refuses to climb above the root. Over-long tokens, targets and paths are dropped, not truncated (with one exception, below).
  • No quadratic work. The File index is built once per pass, not once per markdown file — which is precisely the mistake this project keeps meeting — and nothing scans all nodes.
  • Disciplined about graph quality. Exact-path matching only: no basename or suffix guessing, so a prose README.md cannot bind to an arbitrary file; no invented nodes; self-references dropped; a consumed [text](target) span is blanked so one mention is not counted three times; strategy and confidence are on the edge so a consumer can filter to explicit links.
  • Incremental parity holds on today's closure route, which did not exist when you wrote this: cbm_delta_preseed loads every File node as a proxy, so your path map is built from the whole repository rather than from the changed files. I went looking for that specific bug because we found it in another PR today; it is not here.
  • The tests assert strategy and confidence, so each one binds; the suite is wired in both the Makefile and test_main.c; no sleeps, no wall clocks.

Changes needed

1. lint-ci will go red after a rebase: raw malloc/free in a new file (pass_doclinks.c:88, :447). scripts/lint-memory-core.py is a ratchet that landed after your PR: a file not in the baseline is allowed zero raw allocator sites. The function you modelled this on is grandfathered; a new file is not. Use cbm_alloc(CBM_MEM_CLASS_OTHER, …) / cbm_free from src/foundation/mem_core.h — today's pass_configlink.c shows the shape. This also puts a markdown buffer under the memory budget's accounting.

2. The 4 KiB line buffer can create a false edge (:400-409). A line longer than CBM_SZ_4K is cut at 4095 bytes and the rest is skipped. Paragraphs written on one line are common in markdown, so: if src/foo.cpp straddles byte 4095 it becomes src/foo.c, and if that file exists you emit a wrong edge (same for .json→.js, .tsx→.ts). Everything after the cut is lost, and on a final line without \n the remainder is re-scanned as a new line, splitting one token in two. source is already your private mutable copy — terminate lines in place (\n → \0) and pass pointers: no copy, no cap, 4 KiB less stack.

3. DOCLINK_MAX_REFS = 256 decides graph content (:53, :210). A generated SUMMARY.md or file index with more than 256 distinct targets silently loses everything after the 256th, in document order. This project has an explicit ruling that no work cap may decide what is in the graph. Collect matches in a growable vector, sort by target id, collapse — O(M log M), deterministic, and it removes the linear dedupe scan along with the cap.

4. A rooted link still tries the directory first (doclink_resolve, :166-185). The comment says /docs/x.md is repo-root-relative, but the leading / is only stripped and the directory join still runs first. With docs/guide.md containing [home](/README.md) and both docs/README.md and README.md present, the edge goes to docs/README.md at 0.95. Remember that the reference was rooted and skip the join.

5. An explicit ./x falls back to the repo root (same function). [cfg](./config.md) in docs/a.md, with no docs/config.md but a root config.md, produces an edge to the root file at 0.95 — for what is really a broken link. No root fallback when the reference began with ./. Related and optional: a root-fallback hit on an inline link arguably should not carry the same 0.95 as a directory-relative one; recording "resolved":"dir"|"root" in the properties would let consumers tell them apart.

6. Silent skips and no cancellation check. Unreadable or oversized markdown files are dropped without a log line (:442-445) — the convention here is that a skip is reported, never silent. And the per-file loop (:432) never calls cbm_pipeline_check_cancel, so on a docs-heavy repository cancel latency is the whole pass.

7. Tests. One structural gap: every test calls the pass directly, so deleting the {predump_doclinks, …} registration line leaves all nine green. Please add one pipeline-level test that indexes a tiny repo and finds a REFERENCES_FILE edge. And negative controls for the cases above, since each is a way to get a false edge: the rooted /x and the ./x precedence cases, a line longer than 4 KiB with a straddling token, more than 256 targets, .. escaping the root, a bare http://host/path/file.c in prose, backtick main.c in docs/ not binding to src/main.c, and a CRLF file.

Smaller: a few comments still mention the shell strategy you split out (:461, pipeline_internal.h, the test_main.c comment, the test_doclinks.c header); README.md's edge-type list (~line 704) and the edge table in docs/index.html should gain the new type. Fenced code blocks are not tracked, so gcc src/main.c inside a fence links at 0.70 — defensible, but worth either a test that pins it or its own strategy label so consumers can filter it.

Two things that are ours, not yours

  • Closure inflation. The incremental planner treats every edge type except the containment ones as a dependency, so with this type every markdown file that references a changed file joins the re-parse closure and counts against its budget — docs-heavy repositories may see more full rebuilds. That same behaviour is also what keeps one parity corner correct, so excluding the type is not free. I would rather keep it and measure than guess; I will do that measurement on a docs-heavy corpus once this is rebased, alongside the before/after edge counts and a hand-sampled precision figure per strategy. Nothing for you to do.
  • Build clearance. Your Makefile.cbm hunk (registering the test file) needs a maintainer marker before I can build the branch on our machines — the established shape, needing nothing from you. That is why everything above comes from reading rather than running, and I have said so where it matters.

main currently carries two reds that are not yours (a linter ratchet and a worker-policy script, both fixed by #2257, about to land) — rebase after that and your CI will be showing only your own results. The Co-Authored-By trailer is fine as it is.

Thank you for the care in this one. It is a good pass, and the list above is what careful work looks like after the ground moved under it, not a judgement of it.

cdeust and others added 2 commits September 22, 2026 08:06
Markdown docs reference other repo files constantly (a coding-standards
doc links the modules it governs, a README points at entry points) but
none of that surfaced as edges, so fan-in queries were blind to
documentation hubs. In an A/B retrieval benchmark I ran in August on a
docs-heavy repo, the graph's top fan-in answer was off by roughly 17x:
the most-referenced file was a standards doc with 173 references and
zero inbound edges.

New pre-dump pass, pass_doclinks.c, modeled on pass_configlink.c: three
strategies emit REFERENCES_FILE edges between existing File nodes only
(unresolvable targets are dropped, the pass never invents nodes):
inline links, backtick paths, bare path mentions. Targets resolve
against the referencing file's directory and the repo root; repeated
references collapse into one edge carrying strategy, confidence and
count. Registered in the pre-dump sequence after configlink and in the
incremental post-passes; REFERENCES_FILE added to the skill's edge-type
list and the structural/language contract tests.

Tests mirror test_configlink.c (real files in a tmpdir, File nodes in a
gbuf, run the pass, assert edges): inline link, backtick, bare mention,
http/anchor ignored, anchor-suffixed file link, dedupe with count,
relative-vs-root resolution, unresolvable-target guard, NULL repo_path
skip.

A companion change adds the same linking for shell files (source lines
and script invocations); split out to keep each change reviewable.

Validation: scripts/build.sh clean; focused serial runner (doclinks,
configlink, pipeline, edge_structural, lang_contract) 340 passed under
ASan/UBSan; cppcheck clean on the new files with the repo's flags.

Signed-off-by: Clément Deust <clement.deust@gmail.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace raw malloc/free with cbm_alloc/cbm_free (CBM_MEM_CLASS_EXTRACT)
and a growable CBM_DYN_ARRAY for matches. Remove the 4KiB line-copy
cap; lines are split in place instead of copied. Remove the
DOCLINK_MAX_REFS(256) cap; the ref vector now grows, is sorted by
target id, then collapsed. Track reference kind (bare, relative,
rooted) so /x never attempts a directory join and ./x never falls
back to root. Report per-file skips via cbm_pipeline_add_file_error
and cbm_log_warn, and check cbm_pipeline_check_cancel in the per-file
loop.

Add negative-control tests for all of the above, plus CRLF handling,
fenced-code-block pinning, bare URLs, backtick-basename exactness,
and ".." escaping.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Clément Deust <clement.deust@gmail.com>
@cdeust
cdeust force-pushed the feat/doclinks-markdown branch from 69a9c90 to 6ae6cd4 Compare September 22, 2026 06:51
@cdeust

cdeust commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main (e783f73), one commit 6ae6cd4, force-pushed. Addressing each item:

  1. Raw malloc/free replaced with cbm_alloc/cbm_free from mem_core.h at both sites (:114, :523 in the new diff). Used CBM_MEM_CLASS_EXTRACT rather than CBM_MEM_CLASS_OTHER since this pass extracts references from file content, matching the naming used elsewhere for extraction buffers. scripts/lint-memory-core.py passes: 3805 raw sites across 85 files, none grew.

  2. The 4 KiB line-copy cap is gone. source is the private mutable copy, so lines are now terminated in place (\n to \0, CRLF stripped) and scanned by pointer instead of being copied into a fixed buffer. No cap, no truncation, no stack buffer.

  3. DOCLINK_MAX_REFS(256) is gone. Matches are collected into a growable CBM_DYN_ARRAY, sorted by target id, then collapsed in one pass, so ordering no longer decides what survives past the 256th match.

  4. doclink_resolve now tracks whether a reference was bare, relative, or rooted. A rooted /x reference skips the directory join entirely and resolves against the root only.

  5. An explicit ./x reference no longer falls back to the root when the directory-relative path misses; only bare and relative-without-./ references get the root fallback.

  6. Unreadable and oversized files are now reported through cbm_pipeline_add_file_error plus cbm_log_warn instead of being dropped silently, and the per-file loop calls cbm_pipeline_check_cancel each iteration.

  7. Added a pipeline-level test (pipeline_doclinks_edge_lands_in_store) that indexes a small repo and asserts the REFERENCES_FILE edge lands in the store, plus direct-pass negative controls for: rooted /x vs ./x precedence, a line past the old 4 KiB cap with a straddling token, more than 256 distinct targets, ".." escaping the root, a bare URL in prose, backtick main.c not fuzzy-matching src/main.c, and CRLF line endings. Fenced code blocks are pinned by a dedicated test (doclinks_fenced_code_block_bare_mention_pinned) rather than given a separate strategy label, since the 0.70 confidence already lets consumers filter it.

Smaller items: the stale shell-strategy comments in pipeline_internal.h:668 and test_main.c are fixed. Checked README.md and docs/index.html: README.md already lists REFERENCES_FILE, and docs/index.html has no edge-type table to update.

Validation: build clean (make -f Makefile.cbm cbm), and the full ASan/UBSan suite (make -f Makefile.cbm build/c/test-runner) passed end to end: 8182 passed, 10 skipped, 0 failed. clang-format-20 reports no drift on any of the 5 changed files.

@DeusData ready for another look.

The full Edge Types list near the graph schema section still missed
the new REFERENCES_FILE type added by the doclinks pass. The
"selected" list earlier in the file is explicitly a subset and is
left as is.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: Clément Deust <clement.deust@gmail.com>
@cdeust

cdeust commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Correction to my previous comment: README.md did not already list REFERENCES_FILE. The "selected" list near the top is intentionally a subset, but the full Edge Types table near the graph schema section (line 705) was missing it. Fixed in 9a9534c, pushed. docs/index.html genuinely has no edge-type table, that part of the previous comment stands.

@cdeust
cdeust requested a review from DeusData September 22, 2026 21:10
@DeusData
DeusData merged commit da3ea36 into DeusData:main Sep 25, 2026
39 checks passed
@DeusData

Copy link
Copy Markdown
Owner

Merged as da3ea36. Thank you, and thank you for how you handled the second round: all seven points fixed properly, and your own README claim corrected before anyone asked.

The decision I said was queued came back yes, including the new REFERENCES_FILE edge type. We add labels to the schema reluctantly — a new edge type has to earn its place — and "this document mentions that file" is a relationship none of the existing types carries, so this one does.

What I verified on 9a9534cb before merging, reading the code rather than the summary: no raw malloc/free left in the new pass, both sites on the memory core with CBM_MEM_CLASS_EXTRACT; the 4 KiB copy buffer gone, lines terminated in place and scanned by pointer; DOCLINK_MAX_REFS replaced by an append log sorted by target id and collapsed in one pass, so the order matches were found in no longer decides which survive; three explicit reference kinds, so a rooted /docs/x.md resolves against the repo root only, ./x against the referencing directory only, and a bare x tries the directory then the root; one cancellation check per file, and the two skips that used to be silent now say why; and pipeline_doclinks_edge_lands_in_store closing the registration gap, with a comment that explains itself.

Local verification on the merge result with main:

check result
build (-Werror, ASan + UBSan) clean
doclinks 18 passed
pipeline 288 passed
lang_contract 41 passed
cli 319 passed
incremental 163 passed
memory-core linter, clang-format clean

I also indexed this repository with the merged build: 127 REFERENCES_FILE edges, and of ten I checked by hand against the Markdown, nine are exactly right. The tenth shows one narrow false class worth a small follow-up: a bare name in backticks inside a document that talks about another project resolves to a same-named file here — docs/EVALUATION_PLAN.md's zsh section names that project's install.sh, and it linked to this repository's root install.sh. Giving the bare-name root fallback a lower confidence than an explicit path, or requiring a path separator for backtick matches outside the document's own directory, would close it; happy to take that on our side if you would rather not.

It ships in the next patch release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request parsing/quality Graph extraction bugs, false positives, missing edges priority/normal Standard review queue; useful PR with ordinary maintainer urgency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants