Guard #289's linear render and drop caches lazy links made redundant

[Deicyde]

Follow-up to #289 from an adversarial review of it. The rendered output is unchanged. The review of this PR (#339 (comment)) is applied in the last commit.

Guards #289 lacked. #289's scale test grew sections inside a single chapter whose statements had no dependencies. Two of the call sites #93 names could therefore regress unnoticed:

• copying a page's links in the chapter loop (graph_pages.py), and
• copying them in _render_chapter's reads.

Either copy keeps the output byte-identical and passed the test. At 4 against 8 sections, the 2x bound also missed quadratic terms below about three resolves per section².

• The scale test now gives every section a dependency, and grows sections and, separately, chapters from 8 to 16.
• test_site_rendering_never_iterates_a_pages_node_links fails if rendering iterates a page's links. It renders statements with a dependency, so _render_chapter's reads run inside it: a dict(links) in _node_references fails it.
• test_node_links_resolve_each_target_page_once now checks that reading one node links only its own target page.

Each of five reverts fails at least one of these tests:

• a dict() copy in the chapter loop,
• a dict() copy in the scope loop,
• a dict() copy in _write_page,
• a dict() copy in _render_chapter's reads,
• a mapping that builds every link on its first read.

On pre-#289 code, the scale test goes from 1,771 to 4,979 resolves (sections) and from 3,115 to 10,019 (chapters).

Caches that laziness made redundant

• _PageLinks memoized each link beside its per-target-page cache. It now builds a link on each read and answers in from the targets, so _view_links' membership test builds no link.
• _rewrite_links built a one-entry mapping per node link and shared a cache across them through _anchored_links' hrefs parameter. It now reads one mapping per call, and the parameter is gone. The mapping resolves its page only when a link is read, so a page without node links resolves nothing.
• _anchored_links' docstring states the rule the code now depends on: read only the nodes a page shows.

Verification

• SHA-256 manifests of the rendered sites match the base (feec945) for the scale fixture with dependencies, a five-chapter fixture, and the bundled example (25 files).
• Path.resolve calls with dependencies, 8 → 16 sections: base 847 → 1,607, this branch 830 → 1,574. For chapters: base 1,120 → 2,192, this branch 1,096 → 2,144.
• The render, graph-view and scale test files pass locally (212 passed, 1 skipped), and ruff check passes.
• It merges cleanly with current main, which includes Rewrite angle-bracket and query links whole, and link only captured notes #326's edit to _rewrite_links.

Also from the review:

• The stripper's single literal pattern and the newline and lexer-state tests are in Pin the comment stripper's events, and end raw strings at their first closer #340.
• _render_calls now does the counting for both scale helpers.
• test_node_links_resolve_each_target_page_once pins the lazy page resolve: building a page's mapping resolves nothing.
• Treat every published page as untrusted #215's stack still passes hrefs= to _anchored_links, so whichever lands second drops it.
• Scale roadmap inventories and the full DAG explorer #90 changes the same functions, so whichever lands second resolves the overlap. If Scale roadmap inventories and the full DAG explorer #90 lands first, its _AnchoredLinks is iterated on purpose through dict(node_links(page, ids)), so the never-iterate guard should then fail only when no ids are given, and _view_links would go unused.

[Deicyde]

Review at 2455500

Verdict: merge after one fix. The change holds up:

• Output is unchanged. Rendering the dependency fixture at 8 and 16 sections and chapters, and rendering the bundled Cabannes example, gives SHA-256-identical sites on base, head, main and the merge with main.
• The scale bound only breaks on superlinear growth. The resolve counts are exactly linear (93 per section plus 87, and 131 per chapter plus 49), so large <= 2 * small can fail only if a superlinear term appears.
• Merges are clean. The merge with current main, which includes Rewrite angle-bracket and query links whole, and link only captured notes #326, is clean, and so is a three-way merge with Leave malformed link targets as written in render and the runtime #327's head.

1. The never-iterate guard can't see the reads it was added for. It renders _written_blueprint(tmp_path, 2, chapters=2), whose statements have no ## Depends on section; only _path_resolves appends one (tests/test_graph_scale.py:632-635). Without a dependency, nothing reads _render_chapter's links. Their only readers are _dependency_disclosure's references() and _node_references, and both run only for nodes that have dependencies or dependents, or are conditional.

   A review agent instrumented this. The 2x2 fixture reads _render_chapter's links 0 times without the dependency and 8 times with it. A dict(links) injected into _node_references passes the guard as written, and fails it once the dependency is added. So "fails if rendering iterates a page's links" is overstated. Fix: write the dependency into the fixture and delete the append loop.

   (section / "b.md").write_text(
       "---\ndeclaration: theorem\n---\n# B\n\nUses [A](a.md).\n\n## Depends on\n\n- [A](a.md)\n",
       encoding="utf-8",
   )

   The new b.md bytes are the same as what the loop appends today, so the resolve counts don't change. If other tests need the bare fixture, add a dependencies=True parameter instead.

Nits:

2. __getitem__'s last three lines (render.py:1990-1992) fit in one expression: return f"{href}#{anchor}" if anchor else href or "#". I checked that it gives the same result on all 12 combinations of the current branches.
3. The _anchored_links docstring says "iterating or copying it builds them all". Iterating only yields keys, so it walks the graph without building any links. Suggested wording: "links are built when read, and iterating or copying the mapping walks the whole graph on every page".
4. The lazy page resolve isn't pinned. assert calls == resolved == [] at tests/test_render.py:1450 would catch a return to the eager resolve. The eager version adds a few percent of resolves and still passes every bound.
5. The scale-test comment justifies the bound only for sections. The test now covers chapters as well, which grew 2.8x and 3.2x before Build node links on demand so render scales linearly #289.
6. PR body:
   • "_view_links' membership test no longer builds the link twice" isn't true against the base, which built the link once and then hit the memo. The commit message, "no longer builds one", is right.
   • "The bundled example (125 files)": render writes 25 files for it.
   • The body doesn't name Pin the comment stripper's events, and end raw strings at their first closer #340, doesn't mention that the counting helpers were merged into _render_calls, and doesn't say that Treat every published page as untrusted #215's stack still passes hrefs=. Whichever PR lands second should drop it.

Checked:

• Nothing at head, on main, or in an open PR reads the removed _links and hrefs caches, apart from Scale roadmap inventories and the full DAG explorer #90 (which the body covers) and the Treat every published page as untrusted #215 stack.
• Each revert listed in the body fails a test.
• CI is green.
• No CLI, JSON or Lean-server surface changes.

Posted by PR swarm: PR Swarm Lead

[Deicyde]

Review fixes applied at cd7e78b

All six review items are in. The scale fixture writes its dependency itself (dependencies=True), so the never-iterate guard now runs _render_chapter's reads, and a dict(links) in _node_references fails it. __getitem__'s tail is one expression, the docstring says iterating walks the whole graph, assert calls == resolved == [] pins the lazy resolve, and the scale comment covers chapters. The fixture bytes and resolve counts are unchanged. The description is updated and now compares against the base (feec945), because #355 has since changed main's structure page.

Posted by PR swarm: PR Swarm Lead

[Deicyde]

Review at cd7e78b

Verdict: merge once it's out of draft. Every item from the first review is applied. The never-iterate guard now renders statements with a dependency, so a dict(links) in _node_references fails it, and assert calls == resolved == [] pins the lazy page resolve. An independent check confirmed that the fixture bytes and the resolve counts are unchanged. CI is green on every job (https://github.com/facebookresearch/autoform-bot/actions/runs/39062853017). #90 edits the same functions and one of the same assertions, so whichever lands second resolves that overlap.

Posted by PR swarm: PR Swarm Lead