Persist + resume indexing-job progress across retries (CS-11036)

[habdelra]

Summary

Closes CS-11036. Stops indexing job retries from re-doing work the previous attempt already finished. On staging this is 9.7 h / 7d (49% of incremental compute) for incremental-index retries and ~1648 worker-hours / 7d for from-scratch-index retries — all of it currently thrown away when an attempt dies and the next one starts from zero.

Linear: https://linear.app/cardstack/issue/CS-11036/indexer-persist-resume-worker-progress-across-job-retries

Mechanism

The indexer already writes per-URL rows to boxel_index_working as the visit loop progresses (index-writer.ts::Batch.updateEntry), and never deletes them on worker death — the rows physically survive a retry. The gap was on the read side: when a retry re-claimed the job, the visit loop had no concept of "URLs already processed" and walked the full seed plan again.

This PR closes that loop:

1. Stamp job_id on every working-table row. New job_id INTEGER column on boxel_index_working (migration 1778201534289). Stamped from JobInfo.jobId in Batch.updateEntry, Batch.tombstoneEntries, and Batch.copyFrom. The jobs row is immutable across reservation-expiry → re-claim cycles, so job_id is the natural stable handle that links all attempts of the same logical work.
2. Load resumed rows on Batch.ready. Batch queries boxel_index_working WHERE realm_url = $1 AND job_id = $2 AND is_deleted = false AND has_error = false and exposes the URL → last_modified map as Batch.resumedRows. The same set is pre-seeded into the in-memory #invalidations so applyBatchUpdates promotes the resumed rows even though no updateEntry call in this attempt added them. Errored rows are deliberately excluded — a retry exists precisely so a transient failure (renderer hang, network blip, OOM) gets re-attempted.
3. Skip already-processed URLs in the visit loop. IndexRunner.incremental skips visiting any URL in resumedRows (incremental's args.changes is the deterministic seed; if the file changed since, that's a different changeset enqueued as a separate job). IndexRunner.fromScratch additionally compares the resumed last_modified against the current EFS mtime — if mtime changed mid-attempt, fall through to a normal visit so the upsert overwrites the resumed row with current content.
4. Don't tombstone resumed real rows. Batch.tombstoneEntries filters invalidations against resumedRows before upserting tombstones. Without this, the retry's invalidate() would clobber the previous attempt's real content with a tombstone (PK is (url, realm_url); tombstones upsert).
5. Handle resumed-but-now-deleted URLs. discoverInvalidations consults batch.resumedRows alongside boxel_index when computing deletedUrls. For any resumed URL that's no longer on disk, it calls batch.forgetResumedRows([url]) to drop the resume protection so tombstoning lands and applyBatchUpdates promotes the deletion. Without this, the resumed real row would be preserved and a deleted file would resurrect.

The cumulative boxel_index_working table is preserved as-is — Batch.invalidate's reverse-dependency walk (via itemsThatReference) reads it as the source of truth for fan-out, so wiping cross-job rows would break error propagation. The job_id filter in loadResumedRows is what isolates the current job's resume hand-off from those rows.

Why job_id, not realm_version

realm_version looks like a candidate for keying retries but isn't:

• The flatten-index refactor (commit fe7bf4168be, migration 1735832183444) removed realm_version from the PK of both boxel_index and boxel_index_working. PK today is (url, realm_url). No read path filters by realm_version — it's load-bearing only for realm_meta row pruning.
• It's incremented per-Batch via realm_versions.current_version, so two Batches always get distinct numbers, but their rows still collide on (url, realm_url) and upsert each other — realm_version cannot segregate concurrent attempts.

job_id doesn't have those problems and is immutable across retries.

Concurrency safety

The CS-11036 design assumes resumed rows for realm X are never being concurrently written by another job. That's enforced upstream by the pg_advisory_xact_lock on concurrencyGroup = "indexing:{realmUrl}" (acquired in pg-queue.ts::acquireConcurrencyGroupLock, set in runtime-common/jobs/reindex-realm.ts:32). At most one indexing job per realm holds a reservation at a time. The resume key only needs to differentiate retries of the same logical job, not interleaved jobs.

Tests

Seven new unit tests in packages/host/tests/unit/index-writer-test.ts:

• Resumes URLs already processed by a previous attempt of the same job
• Does not resume rows with has_error=true so the retry can re-attempt them
• Does not resume rows from a different job
• Cumulative working state survives across jobs (only resumedRows is job-scoped)
• invalidate() preserves resumed real rows instead of overwriting with tombstones
• forgetResumedRows allows tombstoning a previously-resumed URL (deletion path)
• done() promotes resumed rows even though they were never visited in this attempt

Test plan

• Existing index-writer-test.ts suite passes (no regressions to baseline Batch behavior).
• Existing realm-server indexing-test.ts and queue-test.ts suites pass — error-propagation tests in particular exercise the cross-job working-table state.
• After deploy: spot-check a rejected incremental-index retry on staging — boxel_index_working has rows tagged with the rejected job's id, and the next claim's perf log line emits the skipped N URLs already processed by prior attempt line.
• After deploy: confirm rejected-job compute on staging drops materially over a 7-day window (target: the 9.7h baseline).

Migration

Migration 1778201534289_add-job-id-to-boxel-index-working adds the column + an index on (realm_url, job_id). Pure additive, NULL-default, no backfill — old rows simply have NULL job_id and never match a resume query.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 657bb2ebc5

ℹ️ 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".

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files  ±    0      1 suites  ±0   1h 57m 44s ⏱️ - 2h 7m 28s
2 571 tests ±    0  2 556 ✅ +    1  15 💤 ± 0  0 ❌ ±0 
2 590 runs   - 2 590  2 575 ✅  - 2 573  15 💤  - 15  0 ❌  - 1 

Results for commit 38bfa4a. ± Comparison against earlier commit af7e8f4.

Realm Server Test Results

    1 files  ±0      1 suites  ±0   16m 33s ⏱️ -28s
1 256 tests ±0  1 256 ✅ ±0  0 💤 ±0  0 ❌ ±0 
1 334 runs  ±0  1 334 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 38bfa4a. ± Comparison against earlier commit af7e8f4.

[Copilot]

Pull request overview

This PR adds “resume across retries” support to the indexing worker by tagging working-table rows with the originating job_id, reloading previously-completed rows when a job is re-claimed, and skipping re-visits for already-processed URLs (with mtime checks for from-scratch runs). This reduces wasted compute on retry attempts while preserving existing cross-job working-table behavior needed for dependency invalidation.

Changes:

• Add job_id column + (realm_url, job_id) index to boxel_index_working, and stamp job_id on all working-table writes (including tombstones and copyFrom).
• Load “resumable” rows (non-deleted, non-error) on Batch.ready, expose them via Batch.resumedRows, and pre-seed them into the batch invalidation set so done() promotes them.
• Teach from-scratch and incremental visit loops to skip already-processed URLs (from-scratch also validates filesystem mtime), and ensure deletions discovered during from-scratch invalidate forget resume protection so tombstones can land.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
packages/runtime-common/index-writer.ts	Adds resumed-row loading, exposes resumedRows/forgetResumedRows, stamps job_id, and prevents tombstoning resumed real rows.
packages/runtime-common/index-structure.ts	Extends BoxelIndexTable typing with optional job_id (working-table only).
packages/runtime-common/index-runner/discover-invalidations.ts	Returns filesystem mtimes alongside invalidation URLs and includes resumed rows in deletion detection (with forgetResumedRows).
packages/runtime-common/index-runner.ts	Skips visits for resumed URLs (mtime-gated for from-scratch; direct skip for incremental) and logs skip counts.
packages/postgres/migrations/1778201534289_add-job-id-to-boxel-index-working.js	Adds job_id column and supporting index; includes down migration.
packages/host/tests/unit/index-writer-test.ts	Adds unit tests covering resumedRows loading, error/different-job exclusion, tombstone protection, forgetResumedRows, and promotion on done().
packages/host/config/schema/1778201534289_schema.sql	Updates the generated SQLite schema snapshot to include job_id on boxel_index_working.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[habdelra]

TODO remove be73283 after #4672 lands and merge upstream to get this fix.

[habdelra]

Acknowledged. Tracking — once #4672 lands on main, I'll merge main into this branch and drop be73283.

[habdelra]

packages/boxel-cli/src/commands/search.ts
Outdated
Comment on lines +54 to +57
let body: Record<string, unknown> = {
realms,
include: [],
...query,
@chatgpt-codex-connector
chatgpt-codex-connector Bot
4 hours ago
P2 Badge Preserve relationship data for CLI search results

When the CLI searches cards that have linked relationships, defaulting every request to include: [] changes the returned data[] shape even though SearchResult only exposes data: the server's link-loading path does not just populate top-level included, it also mutates each root resource's relationships to add relationship.data identifiers after resolving links.self. With this default, CLI/API clients that read linked card ids from result.data[*].relationships now get only relationship links unless they know to opt back in by passing an include key inside the query.

[habdelra]

The comment text references boxel-cli/src/commands/search.ts from PR #4673 — that's not in this PR's diff. Did you mean to post on #4673, or is there something specific on this PR you want me to look at?

[habdelra]

The comment text references boxel-cli/src/commands/search.ts from PR #4673 — that's not in this PR's diff. Did you mean to post on #4673, or is there something specific on this PR you want me to look at?

yeah, but the comment really was intended for this PR. your response was:

Thanks Codex — flagging this for Hassan: the comment is on parent-branch content. This PR (#4673) is a test-flake diagnostics fix that contains only packages/host/app/commands/update-room-skills.ts and packages/host/tests/helpers/index.gts; the packages/boxel-cli/src/commands/search.ts change comes from the parent branch cs-11037-jsonapi-federated-search (PR #4659). The substantive concern — that defaulting CLI requests to include: [] strips relationship.data identifiers from each root resource even though the CLI surface only exposes data[] — should be addressed on #4659 where the change was actually introduced.

[habdelra]

Got it. One thing to flag in the response template — the PR-number citation looks off:

the change comes from the parent branch cs-11037-jsonapi-federated-search (PR #4659)

cs-11037-jsonapi-federated-search is PR #4667, not #4659. PR #4659 is this one (CS-11036 indexer resume). Verified: git branch -r --contains 58a754a1a6 returns only origin/cs-11037-jsonapi-federated-search, and neither this branch nor main has the include: [] change in packages/boxel-cli/src/commands/search.ts.

Want me to:

• (a) Post the redirect comment on host tests: stabilize addSkillToAiAssistant + log silent skill-load skips #4673 with the corrected number (4667), OR
• (b) Address the substantive concern directly on Realm-server: make _federated-search JSON:API-compliant via include #4667 (i.e., open that worktree, fix the include: [] default that strips relationship.data, push)?