refactor(app-shell): stop declaring 28 symbols under names the spec owns - #3169
Merged
Merged
Conversation
Batch 3 of the objectstack#4115 debt burn-down (objectui#3157), and the
ledger's largest single package. Twenty-eight app-shell symbols were declared
under names `@objectstack/spec` already exports. Twenty are now imported or
derived from the spec; eight are renamed because they model something the
spec's same-named export does not.
The comment justifying the largest cluster -- "kept local so app-shell does
not take a build dependency on the framework spec package" -- was already
false: `@objectstack/spec` is a direct dependency of this package.
Three live defects the copies were hiding, each fixed by importing the type:
* `SchemaDiffEntryKind` was missing `index_mismatch` / `unmapped_index`
(framework#3728). ValidationPanel labels diffs from a total map, so an
index divergence the server already emits arrived unlabelled. Widening
the union made the compiler demand the two missing labels.
* `ExplainLayer.contributors[].state` ('active' | 'expired') was absent, so
an EXPIRED position / permission-set contribution rendered as a live one.
* Several keys were optional locally and required in the spec
(`ExternalColumn.primaryKey`, `ExplainRecordAttribution.rules`,
`ExplainDecision.principal.positions` / `.permissionSets`), leaving
nullish branches that can never fire.
Renamed, with what the spec's same-named export actually is:
FieldInput -> ScreenFieldInput (an object FIELD's authoring shape)
ConversationSummary -> ConversationListItem (the AI context-compaction record)
RuntimeConfig -> AppShellRuntimeConfig (the ENGINE runtime config)
PageHeaderProps -> PageHeaderComponentProps (the authored SDUI header schema)
FlowNode / FlowEdge -> FlowDesignerNode / ...Edge (a COMPLETE authored node/edge)
PackageManifest -> PackageManifestRow (the full authored manifest)
InstalledPackage -> InstalledPackageRow (the full install record)
`FieldGroup` becomes `ObjectFieldGroup` -- the spec's own name for this exact
shape -- and is derived from `z.input<typeof ObjectFieldGroupSchema>`, not the
exported `z.infer` type: `collapse` carries `.default('none')`, so it is
optional to author and required after parsing, and this designer authors.
Two symbols are derived structurally with one pinned divergence each:
`ScreenSpec` keeps `fields` optional (#3528) and `DecisionOutputDef` adds
`required`, which the server enforces but the spec does not model yet.
Deriving the latter narrowed `type` from a bare string to the spec's closed
enum, so a typo'd picker kind fails to compile rather than degrading to a raw
record-id text box (objectui#2955).
FlowNode/FlowEdge were NOT derived on purpose: a canvas holds nodes the user
has dropped but not finished (no label yet, no edge id yet), so the spec's
complete-node type would make the editor's own intermediate state
unrepresentable. Two genuine findings there are recorded rather than silently
changed, because both alter what gets written to metadata: the designer
persists geometry as `ui: {x,y}` while the spec models it twice already
(`FlowNode.position`, `FlowCanvasNode`), and the edge inspector can build a
`condition` object missing ADR-0089's required `dialect`.
Ledger 115 -> 87 collisions; `@object-ui/app-shell` drops out entirely.
Mutation-tested in three directions: re-forking a burned symbol trips the
guard by name and file, reverting a rename trips it, and leaving a burned
name in DEBT trips the rot ratchet. The new parity test also checks
`isAggregatedViewContainer` by REFERENCE identity -- the copy it replaced was
line-for-line identical, so no value comparison could have caught it
(objectui#3003).
Refs objectui#3157, objectstack#4115
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Jdef6ZFhJmiNRhNCd4DW3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-zhuang
marked this pull request as ready for review
August 1, 2026 14:43
os-zhuang
pushed a commit
that referenced
this pull request
Aug 1, 2026
objectui#3169 (ledger batch 3, `@object-ui/app-shell`) landed first and edited the same DEBT block — the expected hot spot for parallel batches. Resolved by REGENERATING the whole block with `--ledger` against the merged working tree rather than picking conflict lines, so the guard's two-way ratchet (a missed deletion and an over-deletion both fail) certifies the result. Verified against the post-#3169 main: removed is exactly this batch's ten symbols, added is empty, and 17 of 18 packages are byte-identical — including `@object-ui/app-shell`, which stays fully burned down. 87 -> 77. The guard header's new case 2b (`unknown` erasure, objectui#3155) and the `@object-ui/types:ListViewSchema` ALLOW entry merged cleanly; #3169 touched neither, so nothing of either side was dropped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Jdef6ZFhJmiNRhNCd4DW3
This was referenced Aug 1, 2026
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.
Closes objectui#3157 (ledger burn-down batch 3/8). Refs objectstack#4115.
@object-ui/app-shellwas the ledger's largest single package: 28 symbols declared under names@objectstack/specalready exports. All 28 are dispositioned here — 20 derived, 8 renamed — and the package drops out of the ledger entirely.The comment justifying the largest cluster (
external/api.ts) — "kept local so app-shell does not take a build dependency on the framework spec package" — was already false.@objectstack/specis a direct dependency in this package's ownpackage.json. That is the objectstack#4115 failure mode in miniature: a stale reason, still authoritative-sounding, holding a copy in place long after the reason expired.Three live defects the copies were hiding
Not "one spec release away from drifting" — already drifted, already user-visible:
SchemaDiffEntryKindwas missingindex_mismatchandunmapped_index(framework#3728).ValidationPanellabels diff rows from aRecord<SchemaDiffEntryKind, string>— a total map. The local union had 6 of the server's 8 kinds, so an index divergence the validate route already emits arrived as a row this UI could not name. Importing the real union made the compiler demand the two missing labels; they are added in this PR.ExplainLayer.contributors[].state('active' | 'expired') did not exist locally. An expired position or permission-set contribution rendered identically to a live one in the access-explain panel.ExternalColumn.primaryKey,ExplainRecordAttribution.rules,ExplainDecision.principal.positions/.permissionSets— optional here, required in the spec, so every reader carried a nullish branch that can never fire.Disposition of all 28
Derived — plain re-export (18)
RemoteTable,GenerateDraftOpts,ObjectDraft,SchemaValidationResultspec/contractsSchemaDiffEntry,SchemaDiffEntryKindspec/sharedExternalCatalog,ExternalColumn,ExternalTablespec/dataprimaryKey)ExplainDecision,ExplainLayer,ExplainMatchedRule,ExplainRecordAttributionspec/securityScreenFieldSpecspec/contractsvisibleWhenPluginPermissionsspec/kernelFilterConditionspec/dataRecord<string, any>— a local declaration under the spec's name carrying none of its structureisAggregatedViewContainerspecFieldGroup→ObjectFieldGroupis rename and derive, and is the most interesting one.@objectstack/spec/studioexports aFieldGroup— the Studio field-editor's group config (order, nocollapse). The object designer's type is key-for-key the spec'sObjectFieldGroup(spec/data), a different export. The local doc comment claimeddescriptionandcollapsewere "spec-defined" — true ofObjectFieldGroup, false of theFieldGroupthe name actually resolved to. A correct sentence filed under a name pointing somewhere else.It derives from
z.input<typeof ObjectFieldGroupSchema>, not the exportedz.infertype — the zod trap the guard's header warns about.collapsecarries.default('none'), so it is optional to author and required after parsing. This designer authors (addGroupemits{key, label}), so the output type would make the editor's own new-group shape unrepresentable.tsccaught this: thez.inferversion failed ataddGroup.Derived — structural, one documented divergence each (2)
ScreenSpec=Omit<SpecScreenSpec, 'fields'> & { fields?: ScreenFieldSpec[] }. Anobject-formstep, or a message-only screen from a third-party node executor, legitimately sends nofields; an absent array used to throw the moment the dialog opened (#3528).DecisionOutputDefextends the spec's, addingrequired— enforced by the server'sdecide(), not yet modelled by the spec. Deriving also narrowedtypefrom a barestringto the spec's closed enum, so a typo'd picker kind now fails to compile instead of degrading to a raw record-id text box, which is exactly the objectui#2955 failure this module exists to prevent.Both pinned so the divergence collapses to a plain re-export the day the spec catches up.
Renamed — genuine collisions (8)
Breaking for importers of
@object-ui/app-shell.FieldInputScreenFieldInputOmit<Partial<Field>, 'type'>— an object FIELD's authoring shape (local was a React component)ConversationSummaryConversationListItemkeyPoints,tokensSaved,messageRange). Zero keys in commonRuntimeConfigAppShellRuntimeConfigengine,engineConfig,resourceLimits). Zero keys in commonPageHeaderPropsPageHeaderComponentPropsReactNode, elements)FlowNode/FlowEdgeFlowDesignerNode/FlowDesignerEdgePackageManifestPackageManifestRowInstalledPackageInstalledPackageRowinstalledAt,upgradeHistory, …)Every new name was run through the guard before being adopted — the objectui#3074 lesson. That check paid off immediately:
ObjectFieldGroup,FlowCanvasNodeandFlowCanvasEdgewere all candidates, and all three are already spec exports.FlowCanvas*is especially treacherous: despite the name it is the pure visual overlay ({nodeId, x, y, collapsed, …}), not the node. The parity test pins those two as taken so a future rename cannot walk into them.Why Flow* was renamed rather than derived
Deriving was tried first and
tscrejected it, correctly. A canvas holds nodes the user has dropped but not finished — no label yet, no edge id until committed — so the spec's complete-node type makes the editor's own intermediate state unrepresentable. This is the two-layer split objectui#3090 named, not drift.Two genuine findings there are recorded, not silently changed, because both alter what gets written to metadata and that is a behaviour change with no place in a symbol burn-down:
ui: {x, y}while the spec models the same thing twice already —FlowNode.position(spec/automation) andFlowCanvasNode(spec/studio). Three spellings of one concept.conditionobject missing ADR-0089's requireddialectdiscriminant — a shape the server's ownFlowEdgeSchemawould reject.PackageManifestRow/InstalledPackageRoware likewise deliberate: lenient read projections over wire payloads from runtimes of different vintages, which must stay assignable-from anything the server sends — the opposite of what the spec type is for.Ledger
115 → 87 untriaged collisions; 19 → 18 packages.
--ledgerregenerated and diffed line by line: removed is exactly the 28 app-shell symbols, added is empty, every other package's entries byte-identical (parallel batches are running).Verification
check-spec-symbol-derivation.mjs✅ greenturbo type-check --force✅ 78/78 — afterturbo build --filter=@object-ui/app-shell, since per-packagetscresolves to stale dist and false-greens (objectstack#4115 lesson). Coversapps/console, which is a consumer.Mutation tests — all three fire
isAggregatedViewContainer→ guard reports by name and by file.FlowDesignerNode→FlowNode→ guard reports it.DEBT→ rot ratchet reports it.Plus the one that matters most here: the new parity test checks
isAggregatedViewContainerby reference identity, and mutation 1 failed it. The copy it replaced was line-for-line identical to the spec's implementation, so no value or behavioural comparison could ever have caught it — objectui#3003's argument, now with a live instance.The tripwire reads the spec's export names through the TypeScript checker, not
import * as. A runtime namespace import sees values only, and almost every symbol here is a type; a namespace-based tripwire would have passed for all of them while proving nothing. It also asserts in both directions — a rename fails if the spec ever claims the new name, and also if the spec ever drops the old one, so the workaround cannot outlive its reason.Cross-package decisions (for sibling batches)
FilterCondition— app-shell's was not the FilterBuilder row, unlike@object-ui/typesand@object-ui/components. That concept already lives here under its own name,BuilderCondition. app-shell's was the spec's ObjectQL AST, loosely typed, and is now re-exported. Do not fold it into theFilterBuilderConditionrename (batch 5).PageHeaderProps→PageHeaderComponentProps—@object-ui/layoutcarries the same collision (batch 7, objectui#3161). Suggest adopting this name so the two packages do not invent two dialects.Open questions for a maintainer
conditionmissingdialect) — worth their own issue?spec/automation'sDecisionOutputDefhas norequired, though the server enforces it. Should the spec adopt it?Generated by Claude Code