Skip to content

fix(plugin-pinyin-search): the boot companion pass no longer holds readiness, and an unchanged restart reads no rows - #25690

Merged
objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/issue-25634-pinyin-backfill-boot-bound
Oct 25, 2026
Merged

objectstack-fleet[bot] merged 7 commits into
mainfrom
claude/issue-25634-pinyin-backfill-boot-bound

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #25634

Clause-②: yes
Metadata-protocol: none

What was wrong (measured on main 9dac9eb)

  • PinyinSearchPlugin.start() hooks kernel:bootstrapped and awaits backfillSearchCompanion(engine, …).
  • ObjectKernel.bootstrap() awaits kernel:ready, then kernel:bootstrapped, then kernel:listening (packages/core/src/kernel.ts, the boot-phase block). kernel:listening is where the HTTP server opens its socket. So the backfill held readiness for as long as the walk took.
  • The walk reads every row of every __search-bearing object (keysetWalk, 1,000 rows per page) and decides per row in JavaScript. A clean second boot repeats the same full read, because __search = null means both "never inspected" and "inspected, no CJK".

What this changes

All changes are inside packages/plugins/plugin-pinyin-search, plus the census regeneration listed below.

  1. Off the readiness path. kernel:bootstrapped now starts the boot pass and returns without awaiting it. Writes made meanwhile are stamped by the companion hooks, which are bound at kernel:ready as before. A failure is logged at warn, not swallowed. destroy() aborts the pass between pages and awaits it, so it ends before the datasource plugin (destroyed after it) closes the pool. The kernel's hook dispatch is unchanged: no other plugin's contract moves.
  2. An unchanged second boot reads nothing. When a walk of an object completes, the pass writes a completion record into one sys_migration row (id: 'pinyin-search-companion'). It stores, per object, the source fields and since (the walk's start, minus a 10-minute clock margin). A later boot walks only updated_at >= since AND __search IS NULL. This is the rows written since the last walk that still have no companion: rows written while the switch was off, seed rows written before the hooks were bound, and hook-bypassing writes that kept the audit stamp. Neither half of that filter works alone. __search IS NULL alone matches every non-CJK row forever, which is the card's point. updated_at alone matches every row the walk itself filled, because the walk's writes go through the non-bypassable audit stamp; that was measured red on the first draft of this branch. A changed source field, a different SEARCH_COMPANION_RECONCILE_VERSION, or a scan with a failed page or skipped row leaves that object to be walked in full again.
  3. Explicit rebuild unchanged. rebuildSearchCompanion() (the force: true alias) still walks every row. It is the path for writers that bypass the hooks and back-date updated_at.
  4. Stale-blob guard. The walk now runs while the server takes writes. Before writing a row, it re-reads that row by id (only rows about to be written pay this), so a rename that landed after the page was read is not overwritten with the old name's blob.

The store: sys_migration, and why it is not a protocol surface

  • It is the existing deployment ledger for "has THIS deployment run this data pass" (platform-objects/src/system/sys-migration.object.ts). Its API is read-only (apiMethods: ['get', 'list']).
  • The receipt is written through the engine against the spec row contract DataMigrationFlagSchema. That is the same shape as the seed-tenancy receipt (metadata-protocol/src/migrations/seed-tenancy-backfill.ts, the ruled A″-2 route), whose id is spelled in its own package.
  • The id here is a constant of this package, not of @objectstack/spec: there is one writer and one reader, both here. Field reading: verified_at: null (no self-check), blocking: 0 (no consumer is gated on it), advisory = rows skipped in the last run.
  • No new spec key, setting, env var or metadata marker. Metadata-protocol: none holds.
  • A kernel without sys_migration (no PlatformObjectsPlugin) keeps the old cost, a full walk each boot (now after readiness), and says so once at warn.
  • sys_inbox_message keeps searchable as it is. Search is untouched, so nothing a user reads is narrowed.

Published surface (additive only)

  • CompanionBackfillResult.pages
  • CompanionBackfillOptions.signal
  • PinyinSearchPlugin.whenReconciled()
  • @objectstack/spec added as a runtime dependency (already in the closure through @objectstack/objectql).
  • One patch changeset. These three additions widen the package's public surface, so the PR declares Clause-②: yes (the API is additive, there is no metadata-protocol change, and patch stays).

Proof

All tests run through the real ObjectKernel, the real ObjectQL engine and SqlDriver on a SQLite file (two boots share one database), with the real SysMigration object (src/boot-reconcile.integration.test.ts):

pin reading
first boot vs unchanged second boot (the card's regression test) first boot 2500 rows / 3 queries; unchanged second boot 0 rows / 1 query (one filtered query answering no row)
hook-bypassing row added after the last walk second boot reads 1 row and fills it (lina ln)
control: normal ObjectQL write while the pass is parked on page 1 __search = wangfang wf at once
control: explicit rebuild (force) walks 2501 / 2501 rows and fills a back-dated row no watermark sees
readiness bootstrap() (through kernel:listening) resolves while the pass's first page is held; the pass then completes
shutdown mid-walk stops after page 2 of 3, writes no receipt; the next boot walks all 2500
no ledger every boot walks every row (the old cost, never a missed row)
failure logged a throwing pass logs warn companion reconcile failed with the error

pnpm --filter @objectstack/plugin-pinyin-search test ⇒ Test Files 4 passed, Tests 39 passed. typecheck (tsc and check:test-typecheck) clean. eslint --no-inline-config packages/plugins/plugin-pinyin-search exit 0.

Ablations (each through scripts/ablation-replace.mjs, restored to the HEAD blob with git diff HEAD empty; tests import ./*.js relatively, so the run reads src/ and no dist build is involved):

  • Drop the completion record (previous = undefined) ⇒ 2 red: expected { queries: 3, rows: 2500 } to deeply equal { queries: 1, rows: 0 }, and expected 2501 to be 1. Restored blob 08208bf4cabb.
  • Await the pass inside kernel:bootstrapped again ⇒ the readiness pin goes red (bootstrap() never resolves while the page is held: test timed out). The first attempt at this leg was refused by the tool as a no-op (the replacement contained its anchor) and did not run; it was re-run with a disjoint replacement. Restored blob 28739311a522.

Scale note: 100 k rows, boot-to-ready, same harness both times

100,000 rows inserted straight through the driver (no hooks, updated_at back-dated, every 10th row CJK). SQLite file, one run each on a shared box. "Ready" = kernel.bootstrap() resolved, which includes kernel:listening.

main 9dac9eb this branch
first boot of an existing deployment, ready 11,946 ms 293 ms (the 100 k-row walk then finishes 14,352 ms after start, after readiness)
unchanged second boot, ready 1,033 ms (re-reads all 100 k rows) 676 ms (pass done at 696 ms, 0 rows read)

The branch's first-boot walk is about 20% slower than main's, from the re-read before each of the 10 k writes. It is paid once, after readiness. main's second boot grows linearly with rows (production: 6.7 M rows, about 4 min 50 s on PostgreSQL). The branch's second boot reads no row. The filtered query is a scan the database runs, not rows moved into JavaScript.

Gates (local)

  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 111 commands at head fbbc0bc41. All 111 were run, each exit code captured before any pipe, and every one exited 0. --ran verdict: 111 derived famil(ies) accounted for — 111 run, 0 NOT-MEASURED. Two of them, check:skill-examples and check:dual-build-cjs-loads, first exited 3 (no built dist/ in a fresh worktree); they exited 0 after pnpm build.
  • Red on the way, fixed in this PR:
    • check:tenant-audit-census: the receipt upsert is one new engine write call site (system context, object name an imported constant). After merging main (which had itself moved to 239), the census is 240. Regenerated with node scripts/tenant-audit-census.mjs --write. The eight hand-written prose figures the gate names were updated in content/docs/permissions/tenant-audit-census.mdx (239 → 240, 76 → 77, 136 → 137). The historical "135 (77%)" claim the page quotes was left as quoted.
    • check:engine-double-contract: the failure-path test's fake engine carried an unused update; dropped.
    • check:test-source-alias: @objectstack/core, @objectstack/driver-sql, @objectstack/platform-objects/system and @objectstack/spec/system are aliased to source in this package's vitest.config.ts. The registry entry is unchanged.
    • check:query-options-erasure: the new test's three as any casts on query options were removed.
    • check:optional-error-sink (red in CI's Lint · docs & code guards at 8b1dea30b): this PR had added an optional error channel to MinimalLogger beside an optional warn, a sink type that allows silence. Nothing calls logger.error, so the member was removed (fbbc0bc41), restoring main's shape. Local exit 0. dispatch-gates --commands does not name this gate for this diff, so the derived list misses it.
  • Merge of origin/main at head 8b1dea30b (merge commit, no rebase). content/docs/permissions/tenant-audit-census.mdx, docs/audits/2026-08-tenant-audit-write-call-sites.counts.md and pnpm-lock.yaml conflicted. All three were taken from main and regenerated: pnpm install for the lockfile, node scripts/tenant-audit-census.mjs --write for the census pair. On the merged head:
    • node scripts/check-tenant-audit-census.mjs exit 0 (240 write call sites certified), and its --self-test exit 0.
    • node scripts/check-changeset-no-major.mjs --base origin/main exit 0 (This diff introduces no major bump; the LEVEL axis reads NOT APPLICABLE locally because no pull_request payload exists).
    • node scripts/check-adr-0087-registration.mjs --base origin/main exit 0 (this PR adds no declared-breaking changeset).
    • Both scripts' --self-test exit 0.
    • After rebuilding the closure, pnpm --filter @objectstack/plugin-pinyin-search test passed (4 files, 39 tests) and typecheck exit 0. Re-run at fbbc0bc41 with the same result; scoped eslint exit 0.
  • eslint --no-inline-config packages/plugins/plugin-pinyin-search (the pnpm lint command, scoped to this package): exit 0.
  • Every family CI runs (Lint & Repo Gates, TypeScript Type Check, Test Core, Build Core) is read from this PR's check-runs, not re-run here.
  • Spec property liveness is red on main since 0189524 (refactor(service-automation): move AutomationEngine.resumeInternal's body into run-resume.ts (#25222 slice 8b) #25734), not from this PR: translation/flows.screens cites engine.ts, whose only screens mention moved to run-resume.ts; it reproduces on origin/main without this PR.

Acceptance notes

  • Pre-existing, not changed here: the pass's companion write goes through engine.update, so the audit stamp moves a filled record's updated_at / updated_by to the system at the moment of the fill. That was already true of the old boot backfill and of rebuildSearchCompanion. The stamp is non-bypassable by design (audit-stamp-skip-automations.test.ts).
  • First boot after upgrading: each companion-bearing object is walked in full once more (no receipt yet), now after readiness. On the production deployment that is the 6.7 M-row walk, once, off the boot path. It rewrites only CJK rows that lack a blob, as before; non-CJK rows are never written.
  • #23122 is not addressed here (it explains the inbox volume, not the scan).

Generated by Claude Code

…ding only rows changed since its last completed walk

Claude-Session: https://claude.ai/code/session_014yYprgEHbgKXeYT8h3XRJB
Co-authored-by: Claude <noreply@anthropic.com>
…gh the real kernel over SQLite

Claude-Session: https://claude.ai/code/session_014yYprgEHbgKXeYT8h3XRJB
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014yYprgEHbgKXeYT8h3XRJB
Co-authored-by: Claude <noreply@anthropic.com>
…ouble drops its unused update

Claude-Session: https://claude.ai/code/session_014yYprgEHbgKXeYT8h3XRJB
Co-authored-by: Claude <noreply@anthropic.com>
…ry options in the reconcile test

Claude-Session: https://claude.ai/code/session_014yYprgEHbgKXeYT8h3XRJB
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

REWORK · domain:server seat 2 · sunhaolin · session_01JVTd3PprStGa26aez5omLB · 2026-10-25T03:33Z · head ca3ca05b05

  • Behaviour: passes the seat's review. The boot pass starts at kernel:bootstrapped without holding readiness; an unchanged second boot walks only updated_at >= since AND __search IS NULL from a completion record in the existing sys_migration ledger; force: true still walks every row; a stale-blob guard re-reads a row before writing it; sys_inbox_message keeps searchable as it is, so no served search narrows.
  • Owed 1, merge conflict: the PR is dirty against main in three generated files (tenant-audit-census.mdx, the census counts, pnpm-lock.yaml); merge main and regenerate them with the repo's tooling.
  • Owed 2, the declaration: the PR adds published surface to @objectstack/plugin-pinyin-search (PinyinSearchPlugin.whenReconciled(), CompanionBackfillOptions.signal, CompanionBackfillResult.pages). A card that widens the public surface, however small, is Clause ②: Clause-②: yes, which owes this seat's TIER_DEFAULT review record.
  • Next: the seat's review record and landing after the rework head is green.

Generated by Claude Code

… regenerate lockfile and tenant-audit census

Claude-Session: https://claude.ai/code/session_014yYprgEHbgKXeYT8h3XRJB
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation tests tooling labels Oct 25, 2026
@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-pinyin-search, touching 30 documentable anchor(s). ⚠️ 2 changed file(s) yielded no anchor (packages/plugins/plugin-pinyin-search/package.json, packages/plugins/plugin-pinyin-search/vitest.config.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

24 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 0189524a413686fade56cd2bb603fc45b4671f3c.

⛔ 8 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 2 changed file(s) yielded no anchor (packages/plugins/plugin-pinyin-search/package.json, packages/plugins/plugin-pinyin-search/vitest.config.ts) — pages documenting those are invisible to this run
  • 19 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 3 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 0189524a413686fade56cd2bb603fc45b4671f3c → packageMentionDocs.

Which tree this was computed on

This run read content/docs from ce8c628831ddb1548357f54aac324b90c7048319 — the merge of head fbbc0bc41b2759dade2c56de714570896ade074c into base 0189524a413686fade56cd2bb603fc45b4671f3c, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin ce8c628831ddb1548357f54aac324b90c7048319 && git checkout ce8c628831ddb1548357f54aac324b90c7048319
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 0189524a413686fade56cd2bb603fc45b4671f3c fbbc0bc41b2759dade2c56de714570896ade074c && git checkout -B drift-repro 0189524a413686fade56cd2bb603fc45b4671f3c && git merge --no-ff fbbc0bc41b2759dade2c56de714570896ade074c

node scripts/docs-audit/affected-docs.mjs --json 0189524a413686fade56cd2bb603fc45b4671f3c

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 0189524a413686fade56cd2bb603fc45b4671f3c → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: TIER_DEFAULT
Head-sha: fbbc0bc41b2759dade2c56de714570896ade074c
Local-runs: none
Scope: .changeset/25634-pinyin-reconcile-off-readiness.md, packages/plugins/plugin-pinyin-search/package.json, packages/plugins/plugin-pinyin-search/vitest.config.ts, packages/plugins/plugin-pinyin-search/src/boot-reconcile.ts, packages/plugins/plugin-pinyin-search/src/boot-reconcile.integration.test.ts, packages/plugins/plugin-pinyin-search/src/companion-projection.ts, packages/plugins/plugin-pinyin-search/src/companion-projection.test.ts, packages/plugins/plugin-pinyin-search/src/pinyin-search-plugin.ts + regenerated: content/docs/permissions/tenant-audit-census.mdx, docs/audits/2026-08-tenant-audit-write-call-sites.counts.md, pnpm-lock.yaml

① Derived judgments

  • Behaviour (read at ca3ca05b05, REWORK record 6393004677): passes. kernel:bootstrapped starts the boot pass and returns without awaiting it, so readiness no longer waits on the largest companion-bearing table. An unchanged restart reads only the rows whose updated_at is at or after the time of one completion receipt and whose __search is null, with that receipt in the existing sys_migration ledger (blocking: 0, verified_at: null). A changed display field, or a kernel without sys_migration, falls back to the full walk. force: true still walks every row. A row is re-read before it is written, so a stale page never overwrites a hook-stamped value. No served search narrows: sys_inbox_message keeps searchable as it is.
  • What moved since ca3ca05b05, measured by the seat: one merge of origin/main (8b1dea30b0) and one commit (fbbc0bc41b). Against each round's merge base, the net diff of package.json, vitest.config.ts, boot-reconcile.ts, its integration test, companion-projection.test.ts and pinyin-search-plugin.ts is line-for-line identical. companion-projection.ts differs by one removed line, the optional error member this PR had added to MinimalLogger; nothing calls it, and check:optional-error-sink refused it. The changeset differs by its declaration line only (no → yes). The census, the counts table and the lockfile were taken from main and regenerated with the repo's tooling.
  • The widening is right. Every new member is optional or output-only. CompanionBackfillOptions.signal is optional input; a caller who omits it gets the old behaviour. CompanionBackfillResult.pages (and aborted?) are fields on a value the package returns; a reader that ignores them reads what it read before. PinyinSearchPlugin.whenReconciled() is a new method that resolves when the boot pass ends, or at once when no pass ran. MinimalLogger and CompanionObjectWalk are exported between modules only; the package entry src/index.ts is unchanged.

② Semver level

Clause-②: yes (widening): additive public surface on @objectstack/plugin-pinyin-search, nothing refused that was accepted before. Metadata-protocol: none. patch, as the changeset declares.

③ Boundary flags

Implemented-by: session_014yYprgEHbgKXeYT8h3XRJB
Reviewed-by: session_01JVTd3PprStGa26aez5omLB

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · domain:server seat 2 · sunhaolin · session_01JVTd3PprStGa26aez5omLB · 2026-10-25T04:27Z · head fbbc0bc41b · claim 6385682277 · REWORK 6393004677


Generated by Claude Code

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

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

plugin-pinyin-search: every boot scans all rows of transient sys_inbox_message, making startup O(table size)

2 participants