Visit written URLs before their dependents, and record the write order in the diagnostics

[habdelra]

An incremental index job re-indexes the URLs the triggering write named — the targets — plus every card that depends on them, writing one row per card. This makes the targets' rows the first ones the pass writes, so the card a user actually changed is never written after cards that merely depend on it, and records the resulting order on both diagnostics channels so it can be read back in SQL.

Where the order comes from

The pass builds its visit list in three steps: sortInvalidations (realm config first, then modules, then instances lexically), then orderInvalidationsByDependencies (a topological order over the persisted boxel_index.deps rows). That produced a target-first order only where the dependency graph happened to say so. Three reachable cases produced the opposite:

• A dependent whose persisted deps row does not name the target — the state between a link being written and that row being reindexed, and also the steady state for a relationship only the format renders record (those edges live on prerendered_html.deps, which the invalidation walk reads but the ordering does not). With no edge to order the pair, lexical order alone decides.
• A target inside a dependency cycle with its dependents — two cards with mutual linksTo links. A cycle has no topological order, so the URLs it strands fall back to the order they arrived in, which is the lexical one.
• A target stranded in a cycle, competing with a dependent the graph left schedulable — the topological pass emitted every schedulable URL before appending a cycle's leftovers, so priority stopped applying the moment a URL was caught in a cycle.

In all three, a target that sorts late lexically was written last.

The ordering change

prioritizeWrittenURLs ranks the visit list by (visit class, target, position within group): the targets lead in the order they were written, then the dependents in the order they arrived in. orderInvalidationsByDependencies reads that rank as a priority rather than a fixed order, so exactly two things outrank being a target:

• A recorded dependency wins outright. A target that names another URL in the set in its deps row is visited after it.
• Visit class wins, whether or not the index has recorded the edge, and for dependents as well as targets: every module in the fan-out is written before every instance. An instance whose module has no file entry yet cannot render at all, so this is a correctness constraint rather than a preference. visitClassRank — the class rule extracted from sortInvalidations and now shared by both orderers — is why the guarantee survives the topological pass, which ranks a cycle's members on priority alone and would otherwise leave a module stuck behind an instance.
• A cycle does not win. #edgesWithinCyclesDropped drops exactly the edges whose endpoints share a strongly-connected component (iterative Tarjan), so a cycle's members compete on priority — no order satisfies a cycle anyway — while the edge that leaves a cycle survives and still holds back the URL sitting behind it. The condensation of a digraph by its components is a DAG, so one Kahn pass always schedules everything, and an acyclic set's ordering is byte-identical to before.

Nothing else about the job changes: one job per write, same priority, same invalidation set, same clientRequestId handling, same settle-then-broadcast, same response timing (the request still waits for the whole pass).

Recording the order — diagnostics.writeSeq

writeSeq is stamped on each row as it enters the writer's write path. indexedAt resolves only to the millisecond and a pass's rows drain through buffered multi-row upserts that share one timestamp, so it cannot order two rows of the same pass. Grouped with the existing invalidationId, writeSeq reconstructs a pass's write order. A card instance contributes two rows (file and instance), so reduce to one position per URL:

SELECT url, min((diagnostics->>'writeSeq')::int) AS seq
  FROM boxel_index
 WHERE diagnostics->>'invalidationId' = '<id>'
 GROUP BY url
 ORDER BY seq

The lowest sequences in an index fan-out are its targets; everything after them is dep closure.

invalidate() flushes the write buffer before rotating invalidationId and resets the sequence with it, so the two stamps always describe the same fan-out and each fan-out is 0-based. Without the flush, a row buffered before the rotation would be prepared after it — filed under the new fan-out while carrying the old one's sequence.

A position is taken once per row and reused by every later write of it (#positionOf, keyed by url and row type, held for the fan-out and cleared with the id), so a row written twice in one pass keeps the position of its first write while its contents are the last write's. Without that, the buffer's dedupe — one multi-row upsert cannot touch a conflict target twice — would move the row behind URLs the visit loop only reached afterwards, and would only cover a rewrite that landed inside the same drain. A rewrite consumes no position, so a fan-out's sequences are gapless.

Both channels, not just the index one

prerendered_html.diagnostics carried only what the render itself reported — no invalidationId, no indexedAt, no order — and a row with a quiet render carried nothing at all, so a prerender_html job's fan-out could not be grouped, ordered, or even attributed to a pass. Both channels are written by a Batch, so that asymmetry had nothing behind it. Both now route their stamps through one #writeSideStamps helper, and every live rendering carries all three, merged over the render's own diagnostics.

The render channel's error_doc.diagnostics mirror — the blob operator mode renders verbatim in "send error to AI assistant" — is unchanged by this PR on either pipeline: it mirrors the entry's own diagnostics and adds nothing. What that amounts to differs by pipeline, as it did before: a split pipeline's render entry carries render-produced fields only, while a fused visit's entry is built from its index half's blob (prerenderedHtmlEntryFrom) and so arrives with that row's stamps already merged in.

Positions come from the caller rather than the counter, so a fused visit's two rows share one writeSeq — its boxel_index half and its prerendered_html half are one position, not two. A prerenderHtmlOnly batch has no index half and numbers its own writes.

The two channels' invalidationIds are deliberately different: the id is minted per Batch and refreshed by each invalidate() call, and an index pass and the prerender_html job it spawns are separate batches. Each id groups its own channel; the channels join on url (plus generation). That's documented on the field and in the skill rather than papered over.

Tombstones take no position and so carry no writeSeq. The index channel's still carry the pass's invalidationId and indexedAt (they are written by invalidate() and overwritten by the visit that follows); the render channel's clear diagnostics outright. A NULL writeSeq on an is_deleted row is therefore what identifies a URL a pass never reached.

Rows a realm copy produced are outside the contract, and the field doc says so: copyFrom / copyPrerenderedHtmlFrom clone the source realm's rows rather than rendering them, so a copied row keeps whatever its source carried — that realm's pass, or nothing when the row predates these stamps. A copy performs no visit, so it has no write order of its own to record.

Promotion stays atomic on both channels — done() swaps each working table in one transaction — so this is a within-pass write-order guarantee, not a change to what a reader can observe mid-pass.

Known limit

A retried job's promoted generation holds rows from two batches: loadResumedRows keeps the previous attempt's rows as they are, so they retain that attempt's invalidationId and sequences and only the URLs this attempt visits carry the current pair. Ordering within one id stays sound, but a query that wants the whole generation has to union the attempts' ids. That predates writeSeq (resumed rows already kept the prior attempt's invalidationId); restamping them would defeat the resume. Documented on the field and in the skill's Mode C, with a query that enumerates every id at the generation.

Skill update

.claude/skills/indexing-diagnostics/SKILL.md gains a "Reconstructing a pass's write order" section, documents the stamps on both channels plus the cross-channel invalidationId caveat and the retried-job case, and corrects the stalled-job reading in Mode C that ordered partial progress by indexedAt — which cannot separate rows a single buffered upsert stamped with one millisecond — in both the query and the confidence rubric that reads it. The planned-visit-order description now names all three ordering steps instead of just sortInvalidations. Drive-by: the working-table queries now name the generation column they actually have, in place of a realm_version that has not existed for some time and made them error out on paste.

Tests

packages/realm-server/tests/index-visit-order-test.ts — pins the composed ordering at the seam where it is decided, with no services: prioritizeWrittenURLs on its own, then fed through orderInvalidationsByDependencies over a stubbed deps graph. Covers a recorded dependency overruling the priority, a module never losing its class to an instance (whether or not the write named the instance, and with the deps graph empty so nothing downstream can restore the edge), a target realm config keeping the head of the target group, a dependent with no deps row, a target in a cycle, the mixed cycle-plus-dropped-edge graph, a URL behind a cycle still waiting for the member it depends on, the acyclic no-op, and a multi-target batch. Where the change alters an outcome, the test asserts both the un-prioritized and prioritized orders so it states the difference rather than just the result.

packages/realm-server/tests/prerender-html-split-test.ts — two new tests pin the stamps' contract per channel: on the render channel, one grouping id per pass, a 0-based writeSeq in render order rather than lexical order, and the stamps merging around a rendering's own diagnostics; on the index channel, the buffered write order surviving onto the rows, and a row rewritten both inside a drain and after an explicit flush keeping its first position while taking the last write's contents. The three existing tests that deepEqual'd a whole diagnostics blob now assert the stamps are present and compare the render-produced remainder.

packages/realm-server/tests/target-first-index-ordering-test.ts — end-to-end, next to the indexing tests: pushes cards through /_atomic, drains both queues, then reads the pass's write order back off boxel_index.diagnostics. Asserts a single write's row is written before its two dependents, and that a batch writes every target before any dependent. Each target depends on nothing in its own fan-out and sorts last lexically, so only the ordering under test can put it first, and each scenario asserts the fan-out precondition — that the dependents really do record the target, across both channels' deps — before it asserts an order.

Verification

• 142 local tests pass across index-visit-order-test.ts, prerender-html-split-test.ts, dependency-normalization-test.ts, canonical-url-memo-test.ts and the diagnostics-persistence suites, against a real Postgres.
• Every ordering claim was measured against orderInvalidationsByDependencies directly, before and after: with a cycle, with no deps rows, or with a module written second, the target (or module) landed last beforehand; only the clean acyclic case was already correct.
• The composed order was checked against randomized graphs: with no persisted deps rows it is exactly (class, target, position) over 2,000 random sets, and with class-respecting edges (cycles included) no class inversion appears over another 2,000. The topological pass itself was checked against a brute-force reference over several thousand random digraphs, byte-identical to main on acyclic ones, and timed at 50k URLs.
• writeSeq persistence checked against real Postgres on both channels: buffer order survives onto the promoted rows; a fused visit's two halves share one position; a rewrite keeps its first position across a drain boundary; two invalidate() calls on one batch produce two ids each numbered from 0; a tombstone-only row keeps its invalidationId but has a NULL writeSeq on both the working and production tables.
• ember-tsc --noEmit and eslint clean for every changed file in both packages.
• The end-to-end target-first-index-ordering-test.ts cannot run in this environment (the realm-server suite's full service stack needs mise and the docker-hub blob CDN, both blocked by the network policy), so it runs in CI.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DmkPknpuQyb6uvura5Aj4U

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-10T14:01:29.157292Z	25e6968	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 25e69685cc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

🟡 Changes recommended

Unresolved critical and moderate findings remain in ordering and write-sequence handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR prioritizes written URLs before their dependents during incremental indexing and records per-row visit order with diagnostics.writeSeq.

Changes:

• Adds target-first dependency ordering.
• Persists write-sequence diagnostics.
• Adds unit and end-to-end ordering tests.

Review findings:

• index-runner.ts:491 — critical, 2 votes: module-before-instance ordering can still fail with missing or stale dependency data.
• index-runner.ts:1194 — moderate, 1 vote: mixed cycles can still place a target after a dependent.
• index-writer.ts:312 — moderate, 2 votes: sequence state and invalidation IDs can become misaligned across reused batches.
• index.ts:740 — nit, 2 votes: update outdated write-stamp documentation.
• index.ts:747 — nit, 1 vote: clarify that the diagnostic query returns rows, not unique visited URLs.

File summaries

File	Summary
packages/runtime-common/index.ts	Defines write-sequence diagnostic metadata.
packages/runtime-common/index-writer.ts	Assigns and persists write sequences.
packages/runtime-common/index-runner/dependency-resolver.ts	Documents priority-based ordering.
packages/runtime-common/index-runner.ts	Prioritizes written URLs during dependency ordering.
packages/realm-server/tests/target-first-index-ordering-test.ts	Tests persisted ordering end to end.
packages/realm-server/tests/index-visit-order-test.ts	Tests composed ordering and edge cases.

Review details

Suppressed comments (2)

packages/runtime-common/index-runner.ts:1198

• [Claude Code 🤖] Leading the input with targets does not guarantee target-first order when a target is trapped in a dependency cycle alongside one dependent and another fan-out dependent has no persisted deps row. The topological pass emits the zero-indegree dependent first and appends the cyclic leftovers afterward, so the target can still be written after a dependent despite this hoist. Please make target priority span the queue/cycle boundary or add a regression for this mixed graph.

// The result is the input to `orderInvalidationsByDependencies`, which reads
// position as a priority rather than as a fixed order: a topological edge
// still wins, so a target that depends on another URL in the same set (an
// instance written alongside the module it adopts from) is visited after it,
// and the module's file entry exists before the instance renders. What the

packages/runtime-common/index.ts:748

• [Claude Code 🤖] This example returns one row for every index row, not one position per visited URL: a card visit writes both file and instance rows, as the new test accounts for. Operators will therefore see duplicate URLs and cannot directly use this query as the promised visit order; group by URL and order by its minimum writeSeq, or describe the result as row order.

  // `SELECT url FROM boxel_index WHERE diagnostics->>'invalidationId' = '<id>'
  //  ORDER BY (diagnostics->>'writeSeq')::int`. An incremental pass writes

• Files reviewed: 6/6 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[habdelra]

[Claude Code 🤖] 8e176bad addresses the review. The per-thread replies cover four of the five findings; this one had no thread (it came in as a suppressed comment on index-runner.ts:1194) and it was the most consequential, so answering it here.

A target stranded in a dependency cycle lost to any schedulable dependent. Reproduced exactly as described: target zzz in a cycle with dependent aaa, plus dependent ddd whose persisted edge doesn't resolve to anything in the set, gave ddd, zzz, aaa — the target second, behind a dependent. The topological pass emitted every schedulable URL first and appended the cycle's leftovers afterwards, so priority stopped applying the moment a URL was caught in a cycle.

Fixed by splitting the scheduler into #kahnByPriority and running it twice. Whatever the first pass can't schedule is exactly the set of URLs sitting in or behind a cycle, and every edge out of such a URL leads to another one of them (a URL reachable from a cycle can never clear its indegree either). So dropping their out-edges removes at least one edge from every cycle, leaves every other edge untouched, and the second pass is acyclic — it always completes, and it schedules the freed URLs on priority. Same probe now returns zzz, aaa, ddd.

Dropping those edges costs a cycle nothing it hadn't already cost: no visit order satisfies every edge in a cycle, so some member was always going to render against a stale sibling. And an acyclic set is untouched — the second pass only runs when the first strands something, which I pinned with a test rather than left to inspection.

That makes the ordering claim in the PR description true as written, which it wasn't before. Two regression tests added in index-visit-order-test.ts: the mixed cycle-plus-dropped-edge graph, and the acyclic no-op.

Verification for the round: 139 local tests pass across index-visit-order-test.ts, prerender-html-split-test.ts, dependency-normalization-test.ts, canonical-url-memo-test.ts, and the diagnostics-persistence suites, against a real Postgres. Each of the three code defects was reproduced first and re-probed after, and the invalidate() reset was verified end-to-end on disk. ember-tsc --noEmit and eslint clean on every changed file.

---

Generated by Claude Code

[github-actions]

Host Test Results

    1 files  ±0      1 suites  ±0   2h 37m 27s ⏱️ + 5m 8s
4 739 tests +5  4 725 ✅ +5  14 💤 ±0  0 ❌ ±0 
4 754 runs  +5  4 740 ✅ +5  14 💤 ±0  0 ❌ ±0 

Results for commit 817bf2c. ± Comparison against earlier commit cd7bc2b.

Realm Server Test Results

    1 files  ± 0    214 suites  +2   1h 22m 21s ⏱️ + 3m 22s
2 806 tests +23  2 806 ✅ +23  0 💤 ±0  0 ❌ ±0 
2 845 runs  +23  2 845 ✅ +23  0 💤 ±0  0 ❌ ±0 

Results for commit 817bf2c. ± Comparison against earlier commit cd7bc2b.

[Copilot]

🟡 Changes recommended

Unresolved issues remain in copied-row stamping, duplicate write sequencing, error diagnostics, and the documentation rubric.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

.claude/skills/indexing-diagnostics/SKILL.md:596

• [Claude Code 🤖] This new Mode C guidance uses writeSeq to identify the last completed file, but the “High confidence” rubric below still says to use the row with max indexedAt (line 689). Since one buffered upsert can give several rows the same millisecond, the rubric can select the wrong row; update that remaining rubric to use the per-invalidationId writeSeq query as well.

The bottom row of the per-`invalidationId` query (max `writeSeq`) is **the most recently completed file**; the file the worker stalled on is most likely the _next_ one in the planned visit order. That order is: `index-runner.ts::sortInvalidations` (realm config first, then non-`.json` files before the `.json` ones that depend on them, otherwise lexical by href), then `prioritizeWrittenURLs` hoists the URLs the triggering write named ahead of the dependents the fan-out found, then `orderInvalidationsByDependencies` topologically orders the result — treating the position it was handed as a priority, so a dependency edge wins and everything else keeps the order above. Combine three signals to pin it down:

1. The bottom row's `url` (max `writeSeq`) is the last-completed file.

packages/runtime-common/index-writer.ts:1522

• [Claude Code 🤖] In fused mode, prerenderedHtmlEntryFrom passes this already-stamped diagnostics object into the error entry, and the error path copies it into error_doc.diagnostics. That puts invalidationId, indexedAt, and writeSeq in the operator-facing error mirror, contradicting the stated contract that it contains only render-produced diagnostics while the canonical diagnostics column keeps write-side stamps. Strip the write-side fields when constructing the error-doc mirror.

    let diagnostics: Diagnostics = {
      ...(entry.diagnostics ?? {}),
      ...this.#writeSideStamps(seq),
    };

packages/runtime-common/index-writer.ts:1059

• [Claude Code 🤖] The first-position rule is implemented only in the split branch above. In fused mode, every duplicate (url, type) remains in buffered and this loop writes them all, so the final upsert replaces the earlier writeSeq with the later one. A repeated visit can therefore appear after URLs reached later, violating the documented order contract; apply the same last-content/first-sequence deduplication here.

      } else {
        for (let { url, entry, seq } of buffered) {
          await this.#writeEntryNow(url, entry, seq);
        }

packages/runtime-common/index.ts:719

• [Claude Code 🤖] This guarantee is bypassed by Batch.copyFrom() and copyPrerenderedHtmlFrom(), which insert source rows' diagnostics directly instead of applying #writeSideStamps. A copied row can therefore have no stamps (if its source predates this change) or retain another batch's invalidationId/writeSeq, so queries can attribute it to the wrong pass. Either stamp copied rows with the destination batch's stamps or explicitly exclude copy batches from this contract.

// `RenderTimeoutDiagnostics` (which already carries `requestId`) with three
// write-side stamps. Every live row on either channel carries all three,
// stamped as the row enters the IndexWriter's write path — so a row is
// always attributable to the pass that wrote it, whether or not its render

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Lite

[backspace]

[Claude Code 🤖] Convergence pass over the head commit: each finding the automated reviews raised, checked as a change in the code rather than from the reply, plus the ordering and write-sequence paths read end to end — the strongly-connected-component edge drop, the priority Kahn pass, #positionOf across drains and across channels, the invalidate() flush-and-reset, both channels' stamping and the error-doc mirror.

No blocking issues remain. Approving.

Dispositions of the automated-review findings, each verified on the head commit:

• Target instance hoisted ahead of the module it adopts from — resolved. prioritizeWrittenURLs ranks by visitClassRank before target-ness; the empty-deps regression test pins it.
• Target stranded in a cycle losing to a schedulable dependent — resolved. #edgesWithinCyclesDropped removes only intra-component edges, so a cycle member competes on priority while a URL behind the cycle still waits; both halves have a test.
• writeSeq continuing across invalidate() calls, and buffered rows filed under the new id — resolved by the flush before the id rotates plus the counter and position reset.
• A rewrite losing its first position across a flush — resolved by #positionOf, keyed per row and held for the fan-out; the split-pipeline test rewrites across an explicit flush.
• Copied rows outside the stamping contract, the fused error-doc mirror, the Mode C rubric ordering by indexedAt, the row-vs-URL example query, and the stale two-stamps doc sentence — resolved in the field doc and the skill as the threads describe.
• A retried job mixing two attempts' ids — documented rather than changed. The thread's reasoning holds (restamping would defeat the resume), and the skill's Mode C query enumerates the ids per generation.

One non-blocking question, inline on target-first-index-ordering-test.ts: whether the end-to-end test can fail without the hoist. The unit suite already pins the mechanism, so it only affects what that test proves.