Repository navigation
fix(devx): price the read-seam wrapper depth bound and pin it from above - #13472
Merged
os-project-manager merged 1 commit intoAug 30, 2026
Merged
Conversation
The bound was documented from the day it was written; what it COSTS never was. Measured on 71627f7: 6 real read seams, all admitted at depth 3, unchanged to depth 50, all six carrying `no invented answer` — so the bound holds a denominator still, it does not buy a finding. Before this change the constant was constrained from BELOW only: lowering it to 1 reddens one fixture, raising it to 3/4/6/50 left all 51 read-seam fixtures and every gate in this file green while the census moved 66 to 72. Two fixtures now pin it from above, and `--depth-cost` re-derives the price on demand so it cannot go stale in a comment again. Zero-delta: the full `--list` output is byte-identical before and after. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pk26oZ12t5N1hwGW1m1MgC
This was referenced Aug 30, 2026
os-project-manager
marked this pull request as ready for review
August 30, 2026 14:14
os-project-manager
enabled auto-merge
August 30, 2026 14:14
os-project-manager
deleted the
claude/issue-12360-read-wrapper-depth-bound
branch
August 30, 2026 14:33
os-tesla
pushed a commit
that referenced
this pull request
Oct 25, 2026
…json, pnpm-lock.yaml and the root manifest resolve the workspace root upward (objectstack#25723) (#13484) Part of #25723 (sub-issue 2b of #25677, the objectui → objectstack `ui/` merge). Follow-up of #13472. ## What Adds `scripts/workspace-root.mjs` (Node builtins only, importable before `pnpm install`) and its `.d.mts` twin: `workspaceRoot()`, `workspaceGlobs()`, `lockfilePath()`, `rootManifest()`, `uiTurboConfigPath()`, `uiScopedTurboConfig()`. Every reader of `pnpm-workspace.yaml`, `pnpm-lock.yaml`, the root `package.json` and `turbo.json` under `scripts/`, `scripts/__tests__/`, two package tests and the five e2e specs resolves those files through it. The three ad-hoc `packages:` parsers are unified (union of their refusals). `check-ui-root-lookups` learns the `marker-walk` shape that let the e2e `REPO_ROOT` IIFEs escape #13472, with self-test cases. Standalone behaviour is byte-identical: every helper branch is the identity when `<uiRoot>/pnpm-workspace.yaml` exists. ## Why Once objectui is `ui/` of the objectstack monorepo, the workspace root, lockfile and overrides live one directory up and `turbo.json` is parked as `turbo.objectui.json` (a nested turbo.json must `extends` the root). Readers that assumed those files at `uiRoot()` fail with ENOENT (measured on the verification PR #25718: Type Check UI at `check:side-effects-array`, Build & E2E UI at the first e2e spec, 15 of 16 Test UI shards) or walk up to the wrong root (five e2e specs). This PR makes them layout-aware ahead of the import. ## Verification | command | standalone | merged layout (`ui/` in objectstack, scratch copy of the verification tree) | |---|---|---| | `pnpm check:ui-root-lookups` / `--self-test` | 0 / 0 (17 cases) | 0 | | `pnpm exec vitest run scripts/__tests__` | 0 (195 files, 5772 tests) | card's failing files + every touched test: all reader reds green; 10 remaining reds are the spec-pin gates (spec is `workspace:` post-import), the QUICK_REFERENCE script list, the vendor-chunk arm, one dom test — none a root-file read, all carried on objectstack#25697 | | `pnpm --filter @object-ui/cli test` | 0 (415) | `workspace-vite.test.ts` green | | `pnpm check:side-effects-array` | 0 | 0 | | `pnpm check:docs-route-closure` | 0 | 0 | | `pnpm check:lockfile-dedupe` / `check:lockfile-integrity` / `check:installed-pin-claims` | 0 / 0 / 0 | n/a (the pin gates presuppose the pin) | | `pnpm type-check:scripts` / `type-check:e2e` | 0 / 0 | 0 / 0 | | `pnpm lint:coverage`, `lint:root`, the pre-push gate list | 0 | — | | `playwright test --list --project=chromium` | 0 (78 tests) | 0 (78 tests, 0 ENOENT) | The same patch is on the verification branch as objectstack commit 1cade69 (run 3 of #25718). ## Landing Tooling and tests only; no product code (`packages/cli/src/utils/workspace-vite.ts`, `app-generator.ts` unchanged); no governed surface touched. Changeset `.changeset/25723-workspace-root-readers.md` with empty frontmatter — present because two package test files under `src/` changed; releases nothing. Draft until CI is green; then the epic PM asks for review. Clause-②: no Metadata-protocol: none 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01RSDL3SFtJaKzcymvCD8RsU --------- Co-authored-by: Claude <noreply@anthropic.com>
os-tesla
pushed a commit
that referenced
this pull request
Oct 25, 2026
…ssions groups, at an objectstack pin that declares them (objectui#13478) (#13485) <!-- claude-projects-attribution: {"github_login":"os-elon-musk"} --> _Requested by **Elon** · [project thread](https://claude.ai/code/project/chan_01WedDzV26nniwxv7jC88B9D?thread=cmsg_01WedDzV26nniwxv7jC88B9DSu9uoz6bLHskQmxnyBE86T)_ Fixes #13478 Clause-②: yes Metadata-protocol: none objectstack's `Console Pin Gate` is red on main because app-shell no longer type-checks against objectstack `47af3dde42` (#25701). That change added the `positions` and `permissions` groups to the per-app `TranslationDataSchema`, and `TranslationPreview`'s `CATEGORY_DEFS` is exhaustive over that schema's shape by `satisfies`. This PR adds the two rows and moves objectui's own pin to `47af3dde42`, so objectui type-checks against the same spec. Once it merges, objectstack can move its `.objectui-sha` past the merge commit (objectstack's own card names this one as its blocker). ## What changed - **`TranslationPreview.tsx`**: two `CATEGORY_DEFS` rows, in the schema's order (after `flows`, before `metadataForms`): `positions` with `Users` and `permissions` with `ShieldCheck`, the icons `PositionPreview` and `PermissionPreview` already draw. Both are nested groups (`flat: false`), so each card counts the group's top-level keys and samples them with their inner key count, as `objects` and `flows` do. - **`metadata-admin/i18n.ts`** (the designer table, the only pack these keys live in, `en` and `zh`): `engine.translationPreview.category.positions` = "Positions" / "岗位" and `engine.translationPreview.category.permissions` = "Permission Sets" / "权限集", the words the same table already uses for the two metadata types. - **Tests**: `TranslationPreview.categories-11765.test.tsx` gets a spec-valid fixture for each new group (its first case requires one per schema group). `adminPreviews.i18n-10862-s2.test.tsx` lists the two headings among the rendered locale sites and reads coverage over 13 groups (`2/13 (15%)`, was `2/11 (18%)`). - **Changeset** `.changeset/13478-translation-preview-positions-permissions.md`: `@object-ui/app-shell` patch. ## The pin - `.objectstack-sha` moves from `e649c2d91e` (the 18.1.0 release commit) to `47af3dde42bb4e77fd92025605b89b4a473295a4`. REST compare `e649c2d91e...47af3dd`: `status: ahead`, `ahead_by: 51`, `behind_by: 0`; the bump script also reported "(on origin/main)". It is pinned at the card's floor, not at objectstack main's head: the four commits after it on objectstack main are left for the next bump. - Path: `scripts/bump-objectstack.sh 47af3dd --no-commit` (built and packed the 7 packages), then `pnpm install`. - What moved besides the sha: three packed manifests in `.objectstack-pin/` (`core`, `lint`, `spec`), each only `zod` `^4.6.5` to `^4.6.6` (#25657). The lockfile already resolves `zod@4.6.7`, so **`pnpm-lock.yaml` is unchanged** and `pnpm install --frozen-lockfile` exits 0. - **Why the rows and the pin land together** (the seat's hypothesis; it held). At `e649c2d91e` the schema declares neither group: `grep -c '^ positions: z.record'` over that commit's `packages/spec/src/system/translation.zod.ts` reads 0, and 1 at `47af3dde42`; the same for `permissions`. Under `satisfies`, an extra key in an object literal is TS2353 "Object literal may only specify known properties" (probed with this repo's tsc 6.0.3 on a two-key `Record`). So the rows alone fail at the old pin, and the pin alone fails as the gate does. ## Clause-② reading (the claim asked for it) The claim declares `yes` because the seat could not tell whether `metadata-admin/i18n.ts` reaches app-shell's published entry, and the two lines above are copied from the claim. Measured on the built declaration closure (`packages/app-shell/dist`, walking the relative imports from `index.d.ts`, the only `exports` entry with types): - The closure holds 182 `.d.ts` files. `views/metadata-admin/i18n.d.ts` **is** one of them (the positive control), but its face is `t(key: string, …)`, `tFormat(key: string, …)` and the other helpers: the key table is not exported, and no key is a type. - Closure files that carry the string `translationPreview`: **0**. `previews/TranslationPreview.d.ts` is not in the closure at all. So the diff widens no published accept set and no public face of app-shell, and the reading is `no`. The declaration is the seat's, so the lines stay as claimed. ## Verification (local, this worktree, at the new pin) - `pnpm exec turbo run build --filter=@object-ui/console... --concurrency=2` (under the verify lock, 6144 MB heap): 35 of 35 tasks, including `@object-ui/app-shell`'s `tsc` and the console's vite build, SDUI manifest and plugin tsc. That is the build the Console Pin Gate runs, here at objectui's own pin. - `pnpm exec vitest run packages/app-shell/src/views/metadata-admin/ packages/types/` (under the lock): 980 files passed; 17091 tests passed, 1 skipped. - The three TranslationPreview test files: 63 of 63. Reverse check: with the two rows removed from `TranslationPreview.tsx` (on-disk count 0, back to 2 after the restore), the `categories-11765` file goes red on its order-and-coverage case and on every per-group case, the two new groups included. - Gates, each exit 0: `check:i18n-keys`, `check:i18n-designer-parity`, `check:i18n-dead-keys`, `check:i18n-drift`, `check:installed-pin-claims`, `check:spec-symbols`, `check:icon-record-names`, `check:control-bytes`, `check:new-line-citations` (0 new), `check-changeset-presence`, `check-changeset-no-major`, and `eslint` on the four changed source files. - `check:spec-floors`: exit 1 with the same two `floor-too-low` findings (`@object-ui/auth` `MEMBERSHIP_ROLE_EXTERNAL`, `@object-ui/types` `navItemVisibleDataNeedsDataSource`) that #13472 recorded as pre-existing on main. This diff touches neither package. The gate runs on push and schedule with `--cross-check`, which is its measurement, and that run is green on main `cd70b78`. NOT MEASURED locally (left to CI): the rest of the workspace `vitest` shards, `lint` and `type-check` across all packages, and e2e. ## Acceptance notes - objectui#13477 (the label render of the same objectstack change) is a different defect; it remains open and nothing here touches it. - [x] Adds no product-specific widget, dialog, hook or string (see AGENTS.md › Product-neutral renderer) Implemented by the ui lane card thread, session `https://claude.ai/code/session_01Usjyy1hT9FNxZHea9u7drp`; claim on the card, comment 6391374949. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Usjyy1hT9FNxZHea9u7drp --- _Generated by [Claude Code](https://claude.ai/code/session_01Usjyy1hT9FNxZHea9u7drp)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #12360
The card's framing is the whole job: the bound is documented, the number never was.
MAX_READ_WRAPPER_DEPTH = 2has carried an honest limitation notice since the day it was written — the walk "cannot DISCOVER a seam whose read runs outside the try block, or more thanMAX_READ_WRAPPER_DEPTHhops away. A ratchet, not a proof." What has never been recorded is what that ratchet costs, and a constant whose price nobody has measured is held in place by inertia rather than by a reason.This PR does not move the bound. It prices it, proves every seam behind the price is real, and makes the number impossible to raise in silence or to go stale in a comment.
Re-derived on the merged ref, not inherited
The card's figures (64 to 70 at depth 6, "+8/-2", "5 of the 8 deltas") were taken on
3ddad51b5c, five days and two recognizer changes ago. PR #13444 landed in this exact file hours before this branch was cut. Everything below was re-run onorigin/main@71627f7b4e.The headline number reproduces exactly: 6 real seams. The route to it does not, and both corrections matter to the next person who re-runs this census.
trylines re-attributed to a different first-matching callee. That still happens here (engine.ts:9741and:10906move fromresolveMasterDetailParent(s)tomediaValueShapeStrictFor) but at depth 4, one level above the level that admits anything, and it moves no count.Every admitted seam shown real before anything was proposed
The dispatch was explicit that a depth increase is the other direction of the risk #13444 closed, so each of the six was read at its call site — chain traced through the recognizer's own resolution, not assumed:
getMetaItemCachedengine.findOnesaveMetaItemengine.findOnemigrateStoredMetadataengine.findOnepublishPackageDraftsengine.findOneduplicatePackageengine.findOnesweephot.findAll six are unbroken
awaitchains on the caller's own tick ending in a genuine driver read. Two are not the tails the card predicted: 3 and 5 reach a read throughsaveMetaItem's own precondition check, not through thegetMetaItemtail assigned to them. Same verdict, and the difference is the argument for reading chains rather than assuming them — the walk stops at the first read it finds, which is not always the one a human tracing by hand would land on.What admitting them would do to the gate: nothing. All six carry
no invented answer, and every parenthetical in the rule's verdict line (8 type-discriminated, 1 pass-through, 1 answer-by-assignment, 1 baselined) is byte-identical at depth 2 and at depth 50. Only the denominator moves, 66 to 72 — and that denominator is what #5186, #6451, #9165, #8845 and #8901 are quoted against. The bound is not buying correctness here; it is holding a published number still.Which half of "5 of the 8" survived
Decomposed rather than reported as one number, with the
walkAllprobe run beside the depth probe and both compared to today's 66 by try line:publishPackageDraftsSo the number 5 survives and its denominator does not: read it as "5 of 7", the eighth having been the fake seam PR #13444 removed. And a correction the card could not have made —
walkAllis only a partial mask for this bound.publishPackageDraftsis admitted by depth and by depth alone: counting call hops reaches its read, descending lexically through nested declarations does not.The dispatch hypothesis, measured and falsified
It refuses none of them. The full checker output is byte-identical with that guard enabled and disabled at depths 2, 3, 6 and 50. Its subject is the walk, not the depth:
Two readings, and the second is the one to carry forward. The depth axis never arms the defect #13444 closed, so a depth raise is safe from that defect. But if the walk is ever widened as well, raising the depth doubles what that guard has to catch. The two widenings cost independently and risk multiplicatively.
And a resolution hazard the guard cannot see, found while verifying the six rather than looked for.
functionBodiesis keyed by bare name and is last-wins.protocol.tsdeclareslookupthree times (bodies at 6657, 6717, 7129), and the hop in seams 1 and 2 resolves the call at 6674 to the declaration at 7129 — not the one lexically enclosing it.contradictsWrapperResolutionadmits it and is right to: the receiver is a bare identifier and the arity matches, because the three declarations are near-copies of one another. Both seams are real anyway (all threelookupbodies readsys_metadatathroughthis.engine.findOne), so nothing is miscounted today. But the verdict is right by luck, and each additional hop multiplies the number of names that must be unique for it to stay right. That is the strongest argument on this tree for leaving the bound alone, and no seam count can make it.The finding that made this worth landing: the bound was pinned from below only
Measured by ablation, not assumed. Lowering
MAX_READ_WRAPPER_DEPTHto 1 reddens exactly one fixture (#13444'sthis-rooted chain, which needs two hops). Raising it to 3, 4, 6 or 50 left all 51 read-seam fixtures green and every gate in this file green — while the census denominator moved 66 to 72 with nothing anywhere saying so. A tunable five other cards quote must not be movable in silence. That is this card's own numbers going stale twice, one level down.What ships
THE DEPTH BOUND'S COST: the table, the six seams by name with their chains, thewalkAlldecomposition, the interaction matrix, thelookuphazard, and why the bound is nevertheless held.DRIVER_READ_CALLEES. Deliberately not moved; the new block names it and adds the half that was missing (the price), so the two read as a pair rather than a duplicate.--depth-cost— a diagnostic that re-derives the price on demand and always exits 0. It prints admitted and removed seams by name per step, re-attributions per step (invisible in any count, and reading one as a removal is what produced the card's "-2"), and states its own probe ceiling rather than claiming saturation it did not test. Both stale readings of this number were produced by hand-patching the constant and re-running; this is that step, kept.MAX_READ_WRAPPER_DEPTH + 1, which must find exactly one seam or the pin is asserting nothing.maxDepthis threaded throughisReadCall,analyzeReadSeamsand a newcollectReadSeams(split out ofrunReadSeamRule). Nothing in the audit path passes it, so the shipped verdict is the constant's.The deliverable is a measured zero-delta
--listoutput is byte-identical before and after on this tree — verdicts, counts and every seam line.diffof the complete run, not just the totals.Tests
Both legs, both quoted from the gate's own verdict lines, on final head
0f2bb1ce:--self-test:✓ self-test (log-level rule): 63 case(s) passedand✓ self-test (read-seam invention rule): 53 case(s) passed, and the baseline offer stays marked maintainer-only (#8435)✓ read-seam invention (#5186 + #6451 + #9165, 3 package roots, vocabulary find/findOne/count): 66 read seam(s), none invents an unreported answerand✓ durability-degradation log levels: 29 durability-critical catch seam(s), all loud, rethrowing or propagating to the callerBoth new fixtures proven non-vacuous by ablation. Predictions were written first and both held; each mutation was confirmed on disk by blob hash before the reading was taken, and each restore verified by blob hash against the HEAD blob plus an empty
git diff HEAD:MAX_READ_WRAPPER_DEPTH2 to 3expected violation=false seams=0, got violation=true count=1 seams=1, discrimination case still green, main prints 72analyzeReadSeamsignores the depth option[at maxReadWrapperDepth=3]: expected violation=true seams=1, got violation=false count=0 seams=0, pin still green, main prints 66Gate families derived on my own final head with
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack(13 families, from git's own change set, not a hand-written list). All green:check:agent-test-spelling,check:bash32-floor,check:cli-command-ids,check:cross-package-test-inputs,check:durability-log-level,check:entry-guard,check:parse-guard,check:pnpm-filter-targets,check:watch-hint-literal,check-ci-filter-parity.mjs,check-cross-package-test-inputs.mjs,check-shard-attestation.mjs. Convention obligations green:pnpm check:pm-dispatch-gates(944 cases),node scripts/pm/bare-root-worklist.mjs --self-test(51 live rows, none stale),node scripts/check-self-test-wired.mjs(151 scripts),pnpm check:nul-bytes(75 assertions), plus agrep -naPcontrol-byte sweep over the diff.The two pin tests that read this gate's own source —
sys-metadata-repository.draft-drain.test.tsandprotocol.metadata-store-outage.test.ts— pass:Test Files 2 passed (2) / Tests 44 passed (44), after building the dependency closure.Full-repo
pnpm lintunder the shared verify lock: 5494 files, 0 errors, 0 warnings, with the edited file confirmed present in the run (counts read from--format json, not from a summary line).Recorded as NOT MEASURED, not as a failure:
node scripts/check-test-completeness.mjsexits 3 withPREREQUISITE NOT MET— it grades a savedturbo run testlog that CI tees and this run has none. Its own text says the branch is unreachable in CI.Changeset
None. This is a CI-internal gate script that publishes nothing from any package, which
.github/workflows/lint.ymlcalls the textbookskip-changesetcase in as many words. Theskip-changesetlabel is applied. PR #13444, on this same file hours ago, took the same route.The decision this measurement does not take
Whether the bound should move is now a question with a price attached rather than an unknown, and it is deliberately left to the maintainer. The case for raising it: 6 verified-real seams are missing from a population that five cards quote. The case against, all measured here: admitting them changes no finding, only a number; it triggers the census re-run #8901's restart conjunct (b) reserves; and the last-wins name-resolution hazard grows with every hop. The card's own standing value is preserved either way — anyone re-running that census must read 66 as bound-dependent, and
--depth-costis now how they check the dependence instead of remembering it.Generated by Claude Code