Repository navigation
Boot seeds permission sets and positions one at a time: 4 sequential DB round trips each, 2 of them an unconditional UPDATE #10946
Description
Activity
huangyiirene commented
on Aug 22, 2026 CollaboratorMore actionsTriage first-touch (bare-card sweep): queued
pm:queue, Task,domain:services(lands inplugin-security's bootstrap loops + the position binder). Measured to R²=1.000 with the fix direction stated and uncontroversial: one$inexistence read before each loop, and skip the UPDATE when the stored row equals the write — O(1) steady-state rebuild. Acceptance: the card's own rig readings re-run (slope 4.0 → ~0 on rebuild), or an equivalent counting-driver test in-repo if the cloud rig is out of reach (state which in the PR). Family note for the seat: #10945 (p0, engine — the quadratic registry scan) and #10979 (cli — inline seed) are the same boot-cost campaign from cloud#1555 — different packages/lanes, coordinate in PR bodies, no cross-lane fold.
Generated by Claude Code
Claim: dispatched.
- session: user-29 [974948]
- branch:
claude/issue-10946-batch-identity-seed-roundtrips - worktree:
../objectstack-10946 - Clause-②: no — boot-time reconciliation, no public surface, no accept/reject set.
⚠️ But see the write-skip caveat below; if anything observable depends on the unconditional write, that reading is wrong and you stop. - serial constraint:
packages/plugins/plugin-security/src/bootstrap-declared-permissions.tsand the position binder. Free — PR fix(plugin-security): bind the report sink above start()'s two bail-outs (#10706) #11055 merged.
The card's measurement is unusually strong — inherit it, don't redo it
Slope exactly 4.0000, R² = 1.000000 on both axes, with the per-statement histogram diff naming all four legs. And it carries its own negative control: on the same rig, objects/views/artifact seeds add 0.00 round trips each (0 → 400 objects all boot in 242), because schema sync is already batched through
TursoDriver.supports.batchSchemaSync. Identity content is the one content axis that is not.That control is what makes this a real finding rather than "loops are slow" — do not discard it, and say in your report whether it still holds.
⚠️ You cannot run the card's reproduction. Plan around it, don't fake it.The reproducing commands live in
objectstack-ai/cloud:node scripts/dev-local/bootstrap-curve.mjs --hosted --permission-sets 0,5,10,20,40This session's GitHub access does not include
cloud. So you cannot re-run the curve, and you must not report a number as if you had.What you can do, and what I want:
- measure round trips against a local driver double that counts calls — the card's own defect is a count, not a latency, and a count is measurable without the hosted rig;
- state the narrowing explicitly: the latency claim (171.7 ms per ms of RTT) is inherited, not reproduced.
⛔ Do not report an inherited number in a way that reads as measured. This lane has been wrong that way and it is the failure the whole report format exists against.
The direction — both halves, and the second one is where the risk is
- Batch the read: replace the per-item existence
SELECTwith onefind({ name: { $in: [...] } })before the loop. The set is known in full before the loop starts. - Skip the write when nothing changed: an idempotent reconcile that writes unconditionally pays two round trips per item forever to change nothing.
⚠️ Half 2 changes when writes happen. Before you ship it, establish what — if anything — observably depends on the unconditionalUPDATE.updated_atbumping on every boot is the obvious candidate; an audit trail or a change-feed consumer is the dangerous one. If something does depend on it, stop and report — that turns a performance fix into a behaviour change and it is not yours to make silently.What must be pinned — both directions
- steady-state rebuild is O(1) round trips for both loops: with N sets and N positions already seeded and unchanged, the count does not grow with N. Assert the count, not the wall time.
⚠️ load-bearing — drift still reconciles: a row whose stored value differs from what would be written still gets its UPDATE. An implementation that skipped all writes would show a beautiful curve and stop reconciling entirely — passing a round-trip-only suite while silently breaking the thing the loop exists to do. This is the leg that makes a vacuous pass impossible.- a genuinely new set/position is still created — the batched read must not turn "absent" into "present".
- ⛔
nullis not "not mine" (approvals: adepartmentapprover never resolves when the business unit hasorganization_id = null(every seeded BU) #3807): a batchedfindthat cannot answer must not be read as "none of them exist", which would make every boot re-create everything. That is the same conflation class service-package answers "no such package" / "no packages installed" over a driver it never queried — its own normalizeRows maps a non-answering seam onto zero rows #10965 just fixed one package over — judge the seam by whether it returned a result set.
Standing proof standard
Ablation with the signature predicted first; restore proved byte-identical by
git hash-object, and re-run the restore leg to a real verdict. Provesrc/vsdist/resolution argued from the files — prefer the falsifiable form (asrc-only mutation flipping the verdict whiledistbytes stay identical, marker absent fromdist); two seats produced that today. Zero-hit counter-check with the positive control run FIRST. Gate union derived on the final commit, clean tree,dispatch-gates.mjswith no path arguments, exit codes captured before any pipe. Changeset required.⚠️ Class #10309 today: the derivation was short on five services PRs and complete on four — the complete ones all re-derived on the final committed diff. Do that. One gate it can never name by construction:check:optional-error-sink.⚠️ A gate's refusal (PREREQUISITE NOT MET/--re-measure cannot run/Nothing was checked) is NOT MEASURED, never a pass — build the closure and re-run. ⛔ If a ratchet reddens, fix the code, never the baseline — a seat did exactly that oncheck:slot-lookuptoday. ⛔ And if a census gate reddens naming a new import of yours, read its message:check:driver-memory-censusreddened another PR of mine an hour ago and its own text says "a new arrival is not a bookkeeping chore, it is the defect itself". Apnpm --filtermatching no script exits 0 having run nothing — echo the script name and count the echoed lines. Nevergit stash(shared stack). Never report a grep hit count as a fact count.⛔ Do not touch
content/docs/releases/**orpackages/spec— zero ownership in this lane. ⛔ Do not reach intocloud— it is outside this session's access.Refs
objectstack-ai/cloud#1555(the 20 s request budget this competes for) ·bootstrap-declared-permissions.ts:166·TursoDriver.supports.batchSchemaSync(the already-batched contrast) · #3807
Generated by Claude Code
os-dev-report
{ "issue": 10946, "status": "done", "branch": "claude/issue-10946-batch-identity-seed-roundtrips", "pr": "https://github.com/objectstack-ai/objectstack/pull/11116", "premise_still_valid": true, "summary": "Both halves landed. Half 1: both loops now hoist ONE {name:{$in:[...]}} existence read out of the loop via a new seed-name-lookup.ts. Half 2: the UPDATE fires only when the stored row differs — recordDiffersFromBody for permission sets (the same predicate the ADR-0094 reconciler already trusts), a label/description comparison for positions. The write-skip caveat DID find one load-bearing consumer: security-plugin.ts:2840's publish materializer computed applied = seeded + updated and used it for the publish success/failure report AND for whether reconcileAudienceBindingSuggestions runs, so a converged re-publish would have reported success:false; it now reads seeded + updated + unchanged, and since all three refusal branches leave unchanged at 0, every case that reported 0 before still reports 0. NOT a needs_decision: nothing observably DEPENDS on the unconditional write (evidence below). Writing the test for the #3807 leg exposed a real weakness in my first implementation — it degraded to per-item reads which, against a double with no unique index, re-created all four rows; the lookup now reports three outcomes (present/absent/unknown) and declines rather than inserts on a read that could not answer, which is stricter than the code it replaces. Scope note for PM: the file face is one file wider than the claim named — security-plugin.ts got a one-line + comment change, which is the direct consumer of the contract I changed and is required for correctness, not a drive-by.", "tests": "MEASURED ROUND TRIPS (the card's defect is a count, and a count needs no hosted rig). Counter = a call-counting ObjectQL double; BOTH columns produced by running the same counter twice, once with the four touched files reverted to origin/main and once at head. Steady-state rebuild, identical for permission sets and positions — n=1: 2 -> 1 · n=5: 10 -> 1 · n=20: 40 -> 1 · n=40: 80 -> 1. Measured slope 2.0 facade calls per item going to 0. LAYER STATED EXPLICITLY: my counter sits at the ObjectQL facade (one find + one update per item); the card's 4.0000 sits at @libsql/client, where each facade call expands to more than one statement (its histogram shows the per-item UPDATE arriving with a SELECT beside it) — the two agree at a 2:1 expansion. REPRODUCED BY ME: the facade counts above. INHERITED, NOT REPRODUCED: the 171.7 ms per ms of RTT / R²=0.998 latency figure AND the driver-level slope 4.0000 / R²=1.000000 fit — both from scripts/dev-local/bootstrap-curve.mjs in objectstack-ai/cloud, outside this session's access; nothing here measures wall time. NEGATIVE CONTROL: still holds, but by READING not re-measurement — schema sync is untouched by this PR so the 0.00-per-object result is unchanged, yet I did not re-run the 0-to-400-objects leg, so 'still 0.00' is inherited too. || SUITES: pnpm --filter @objectstack/plugin-security test -> 'Test Files 71 passed (71) / Tests 1371 passed (1371)', exit 0 (run 4x across the ablation legs). typecheck -> exit 0, script name echoed exactly once. Downstream: --filter '...@objectstack/plugin-security' is the PREFIX form = DEPENDENTS, 26 packages; 23 ship a typecheck script and all 23 ran (23 echoed script lines, 23 'Done') with 0 errors; the other 3 (hono, cloud-connection, service-automation) ship no typecheck script — verified by reading each package.json. Their closure needed the both-directions filter first; the dependents-only filter gave cascading TS2307 from an unbuilt service-datasource (not findings). || ABLATIONS, signature predicted IN WRITING BEFORE MUTATING. A (restore the unconditional UPDATE): predicted steady-state RED / batched-read GREEN / first-boot GREEN / drift RED / positions GREEN — all five observed as predicted, but breadth UNDER-CALLED: 4 tests red, not 2 (two further counter assertions moved). B (skip every write — the 'beautiful curve, reconciles nothing' shape): predicted round-trips GREEN + drift RED; observed exactly that, plus two PRE-EXISTING tests red. Informative asymmetry: under A the pre-existing suite stayed 100% green (it could not see this defect at all); under B it reddens. Vacuous pass impossible from two independent directions. || ABLATION REBUILD: no per-leg rebuild was required, and that is itself the measurement rather than an omission — the suite resolves through src/, proven in the falsifiable form: a src-only mutation carrying marker OS_ABLATION_MARKER_10946 flipped the verdict (4 red) while dist/ bytes stayed IDENTICAL (md5 of all 6 files, diffed clean across BOTH ablations) and the marker had 0 occurrences in every dist file. Separately ablation-dist-preflight.mjs confirms the shipped predicate DID reach dist/ (2 built files) for artifact consumers. || RESTORES: byte-identical by git hash-object — bootstrap-declared-permissions.ts at 3ad017323f162701ed9d4a56baf0b20330db5980 before and after each leg; the measurement revert restored all four touched files to their committed hashes. EVERY restore leg re-run to a real verdict: 1371 passed, three times. || REVERSE VERIFICATION of the cross-package type change: a probe constructing the pre-change PermissionSeedOutcome literal was rejected with 'TS2739 ... is missing the following properties from type PermissionSeedOutcome: unchanged, unreadable' — direction as predicted (RED), proving rebuilt declarations were read, not a cache. Probe removed. || ZERO-HIT COUNTER-CHECK with the POSITIVE CONTROL RUN FIRST: the control-char pattern was first run against a file containing a bell byte (hit reported at line 2), then against all 10 changed files (no hits). || GATES — union derived on the FINAL commit 4568d6fff, clean tree, `node scripts/pm/dispatch-gates.mjs` with NO path arguments; exit codes captured before any pipe (cmd > file 2>&1; EXIT=$?). Re-deriving after the second commit ADDED two families the first derivation could not name — check:entry-guard and check:parse-guard, pulled in by the pinned-ledger path — the #10309 class, caught. All quoted by the gate's OWN verdict line: changeset-gate-self-tests (118+212+116 assertions) · cross-package-test-inputs ('OK: 13 package(s) read outside themselves, all declared') · entry-guard ('136 scripts/ file(s)') · objectui-changeset · parse-guard ('135 scripts/ file(s)') · slot-lookup ('ratchet holds: 107 unswept site(s), none new') · test-source-alias ('OK — 72 packages with tests scanned') · type-source-resolution ('OK — 77 packages') · check-adr-0087-registration ('adds no declared-breaking changeset') · check-changeset-no-major ('no major bump') · check-ci-filter-parity ('OK: all 83 declared cross-package glob(s)') · check-empty-changeset · check-plugin-teardown-shape ('baseline fully burned down') · check-affected-docs · query-options-erasure ('ratchet holds, none new') · i18n ('OK (9 package(s) — all bundles in sync)') · type-check-coverage ('OK — 65/78 packages type-checked') · nul-bytes ('OK (scanned 6417 text file(s) ... no raw ASCII control bytes)') · optional-error-sink (run because the card named it; no derivation can). THREE REDDENED, ALL REPAIRED IN CODE, NO BASELINE TOUCHED: (1) check:engine-double-contract — my new double's update() did not route through assertEngineUpdateDispatch; routed it through the real predicate from @objectstack/metadata-core; the follow-up RETAINED verdict asked the PINNED (tightening) ledger to learn the new coverage, --write added exactly 1 row, and engine-double-contract.baseline.json (the shrink-only one) is untouched — verified by git status on that file specifically. Now 'OK — 378 pinned, 133 in the DEBT ledger, 2 exempt'. (2) check:where-matcher — my double read a combinator as a field name; made it REFUSE the combinators it does not implement. Now '279 matcher(s) discovered, 279 answer the combinator battery correctly or refuse it loudly (168 refuse)'. (3) check:type-check-debt --re-measure — plugin-security TEST_DEBT drifted 11 -> 12; the +1 was MY fixture (TS2353, an inferred grant literal the upgrade fixture widens); fixed by typing the fixture, re-measured back to exactly 11, ledger entry NOT raised. Now 'OK — 33 ledger entr(ies) re-measured in 338.2s, 1908 raw tsc error(s) total, none above its recorded number' — run against a fully built workspace closure (70/70 tasks), so a measurement, not a refusal. || DECLARED NARROWING: the two dogfood real-engine seeding tests were NOT run locally — attempted twice and killed mid-boot by this session's 10-minute foreground ceiling behind a contended shared verify lock. CI runs the dogfood farm once regardless. Standing in for it: the membership query shape this PR now issues is ALREADY in production on both tables (explain-engine.ts:389 reads sys_position with a name membership predicate; security-plugin.ts:997 reads sys_permission_set with one), so the engine answering this shape on these two tables is a shipped fact, not a new assumption.", "open_questions": [ { "question": "The seeders write with context {isSystem:true} but do NOT set skipAutomations, so a metadata-bound automation on sys_position / sys_permission_set update fires today once per item per boot with an empty diff. After this PR it fires only on real drift. Should the seeders additionally declare skipAutomations, making boot reconciliation explicitly not an automation trigger?", "options": [ "A: leave as-is (what this PR does) — the skip already removes the per-boot noise, and nothing in the tree declares such an automation", "B: set skipAutomations on both seeders — declares boot reconciliation as a non-user event, so a real drift repair also stops firing tenant automations", "C: file it as its own card with a survey of whether any hosted environment actually binds an automation to these two system objects" ], "recommendation": "A for this PR, then C. I did not decide B: it would change which events tenant automations see on a genuine drift repair, which is a behaviour change beyond this card's remit and exactly the class the claim comment said to stop on. Nothing observably depends on the current firing (no in-repo automation targets these objects), so A is safe today; C answers B properly with data this session cannot reach." } ], "out_of_scope_findings": [ "filed as #11096: bootstrapDeclaredCapabilities seeds sys_capability with the identical per-item read + unconditional UPDATE — same defect class, third table, and its count is the union of every package's systemPermissions so it is typically larger than either axis the card measured. Searched open issues first; no duplicate. Filed unassigned and unlabelled for PM triage (a concrete defect, not an observation).", "filed as #11097: reconcilePermissionSetProjection projects env permission-set overlays one at a time and re-writes unconditionally (ADR-0094 door). Deliberately NOT folded in: the env door stamps `customized`, which is outside what recordDiffersFromBody compares, so a naive skip would stop maintaining a flag the Setup list badges on — a design step, not a mechanical copy. Its slope is unmeasured and I said so on the card. Filed unassigned and unlabelled." ] }
Generated by Claude Code
PM review of PR #11116. Accepted. Reviewed against the branch by content.
⭐ The write-skip caveat found a load-bearing consumer — this is the finding
"Skip the write when nothing changed" is the kind of optimisation that quietly breaks whoever was counting the writes. It did, and the seat found it rather than shipping past it.
security-plugin.ts:2840's publish materializer computedapplied = r.seeded + r.updatedand used that number twice: for the publish success/failure report, and for whetherreconcileAudienceBindingSuggestionsruns. Once the seeder stops re-writing converged rows, a re-publish of an identical body would have reportedsuccess: falseand skipped the audience reconcile.Verified in the diff; the comment states the reasoning exactly right:
The question this line asks is "did the record end up matching the published body", NOT "was a write issued" — and until the seeder stopped re-writing rows it had nothing to change, those two were accidentally the same number.
And the safety argument is the part that makes it reviewable rather than plausible: all three refusal branches (foreign package, env-authored name, no owning package) leave
unchangedat 0, so every case that reported 0 before still reports 0. That is a proof about the boundary, not an assurance.Correctly classified as not a
needs_decision: nothing observably depends on the unconditional write; one consumer depended on its count, and that consumer was fixed.⭐ Writing the #3807 test broke the seat's own first implementation
the lookup degraded to per-item reads which, against a double with no unique index, re-created all four rows
The fix is the one the principle actually demands: the lookup now reports three outcomes — present / absent / unknown — and declines rather than inserts on a read that could not answer.
nullis not "not mine" (#3807), and a seeder that treats an unanswerable read as "absent" duplicates rows on every boot.Worth stating plainly: this is stricter than the code it replaces. The card asked for fewer round trips; the seat also closed a latent correctness hole it only found because it wrote the negative-direction test first.
Two measurements at different layers, reconciled instead of contested
The card's figure is 4.0000 round trips/item, R² = 1.000000, measured at
@libsql/client. The seat's counter measured 2.0 facade calls/item → 0, at the ObjectQL facade (onefind+ oneupdateper item). Rather than presenting that as a correction, the report names the layer and shows the two agree at a 2:1 expansion.Both columns come from running the same counter twice — once with the four touched files reverted to
origin/main, once at head. Steady-state rebuild:n=1: 2→1 · n=5: 10→1 · n=20: 40→1 · n=40: 80→1.Inherited, not reproduced — declared: the
171.7 ms per ms of RTT (R²=0.998)latency figure and the driver-level4.0000fit both come frombootstrap-curve.mjsin the cloud repo, which is outside this session's scope. The negative control (schema sync still adds 0.00) is also inherited, by reading rather than re-measurement — the 0→400-objects leg was not re-run, and the report says so instead of letting "still 0.00" read as freshly measured.Ablation: under-called, and the asymmetry is the interesting half
Predictions were written first. Leg A (restore the unconditional UPDATE): all five correct in direction, but breadth under-called — 4 red, not 2. Reported as a miss rather than retro-fitted.
The asymmetry is what I want on the record: under leg A the pre-existing suite stayed 100% green. The suite that existed before this card could not see this defect at all. That is the strongest available statement of why the new pins are load-bearing rather than decorative.
Three ratchets reddened; all three repaired in code, no shrink-only baseline touched
check:engine-double-contract— routed the double throughassertEngineUpdateDispatch;--writeadded one row to the PINNED/tightening ledger only, withbaseline.jsonverified untouched. Confirmed in the diff: the only ratchet file in the surface isscripts/engine-double-contract.pinned.json(+5).check:where-matcher— made the double REFUSE combinators rather than pretend to answer them.check:type-check-debt --re-measure—TEST_DEBT 11→12was the seat's own fixture (TS2353); it typed the fixture and returned to exactly 11, entry not raised. Run against a fully built closure (70/70), so a measurement rather than a refusal.
⚠️ Class #10309, caught in the actRe-deriving the gate union after the second commit ADDED
check:entry-guardandcheck:parse-guard. So the union is not a property of the card — it is a property of the diff at a moment, and deriving it once and reusing it silently drops families. This is the cleanest demonstration of that class I have seen today, and it argues for the rule the lane already follows: derive on the final commit, every time.22 families green, each quoted by its own printed verdict line.
Declared narrowings and scope
- The two dogfood real-engine seeding tests were not run locally — killed mid-boot twice by the 10-minute foreground ceiling behind a contended verify lock. A stand-in is named (the membership query shape is already shipping on both tables at
explain-engine.ts:389andsecurity-plugin.ts:997), and the gap is stated rather than papered over. CI runs them. - Scope note reported, not hidden: the file face is one file wider than the claim named —
security-plugin.tstook a one-line + comment change. That is the direct consumer of the contract this PR changed, required for correctness, not a drive-by. Reporting it is the right call; a wider face discovered by a reviewer reads very differently from one the author declares.
Findings filed rather than ridden
bootstrapDeclaredCapabilitiesseedssys_capabilityone at a time too — the same per-item read + unconditional UPDATE #10946 removed from the two sibling loops #11096 —bootstrapDeclaredCapabilitiesseedssys_capabilitywith the identical per-item read + unconditional UPDATE. Same defect class, third table, and its count is the union of every package'ssystemPermissions, so typically larger than either axis this card measured. Searched for duplicates first.- Boot reconciliation projects env permission-set overlays one at a time, and re-writes every record unconditionally — the #10946 shape on the ADR-0094 door #11097 —
reconcilePermissionSetProjectionprojects env overlays one at a time and re-writes unconditionally. ⭐ Deliberately not folded in, with the reason measured: the env door stampscustomized, which is outside whatrecordDiffersFromBodycompares, so a naive skip would stop maintaining a flag the Setup list badges on. Its slope is unmeasured and the card says so.
That second one is the right kind of refusal — not "out of scope" as a shrug, but a specific mechanism explaining why the obvious transplant would break something.
Open question — escalating
Whether the seeders should additionally declare
skipAutomations. They write with{isSystem: true}but do not set it, so a metadata-bound automation onsys_position/sys_permission_setupdate fires today once per item per boot with an empty diff; after this PR it fires only on real drift.The seat recommends A for this PR, then C (file it with a survey of whether any hosted environment actually binds an automation to these two system objects), and declined B because it changes which events tenant automations see on a genuine drift repair — a behaviour change beyond this card's remit. Nothing in the tree declares such an automation, so A is safe today. Agreed, and escalating C.
CI convergence is mine to read; I am subscribed and will verify by job name.
Generated by Claude Code
- added a commit that references this issue
on Aug 22, 2026 - added a commit that references this issue
on Aug 23, 2026
Every permission set and every position an environment declares costs exactly 4 sequential database round trips on every kernel boot, and 2 of those 4 are an UPDATE that fires even when nothing changed.
Measured
Real per-environment kernel build (
ArtifactKernelFactory.create(), hosted capability slate, realTursoDriverin remote transport with every@libsql/clientcall counted), measuring a REBUILD — tables present, rows already seeded, nothing to change:Least squares: slope exactly 4.0000, R² = 1.000000 on both axes.
Diffing the per-statement round-trip histogram between 0 and 40 of each names the four:
For contrast, on the same rig objects, views and artifact seed records add 0.00 round trips each (0 → 400 objects all boot in 242): schema sync is already batched through
TursoDriver.supports.batchSchemaSync. Identity content is the one content axis that is not.Why it costs what it costs
bootstrapDeclaredPermissionSets(packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts:166) awaitsupsertPackagePermissionSetonce per set inside aforloop; the position binder has the same shape. On a local file database the loop is invisible. On a remote libsql/Turso database — every hosted environment — each leg is its own HTTP request, and the legs are awaited one after another.The same rig measures how much that matters: injecting a synthetic per-call latency grows the whole
bootstrapstep at 171.7 ms per ms of RTT (R² = 0.998), so ~172 of the 242 baseline round trips sit on the critical path. At that rate 40 permission sets plus 40 positions add 320 round trips — more than doubling a boot that a request will only wait 20 s for (objectstack-ai/cloud#1555).Direction
Both loops are read-then-write per item over a set known in full before the loop starts:
SELECTwith ONEfind({ name: { $in: [...] } })before the loop.UPDATEwhen the stored row already equals what would be written — an idempotent reconcile that writes unconditionally is paying two round trips per item forever to change nothing.That takes the steady-state rebuild cost of both loops to O(1) round trips, leaving writes only where something genuinely drifted.
Reproducing
objectstack-ai/cloud:Refs objectstack-ai/cloud#1555