refactor(core): retire ActionEngine's zero-caller event-mapping API (#3368) - #4411
Merged
Merged
Conversation
…3368) Implements the maintainer ruling of 2026-08-11 on objectui#3368: retire `addMapping` under enforce-or-remove — remove the export and its dead registration path. Removed from `@object-ui/core`'s public surface: `ActionEngine.addMapping()`, `ActionEngine.dispatch()`, the private `mappings` registry they shared, and the exported `ActionMapping` interface. Re-measured at this branch point (f762f5b): definition plus four call sites, all four in the engine's own test file, zero production callers. Nothing ever registered a mapping, so `dispatch()` had no reachable caller either. The three recorded contract inconsistencies vs `visible` (truthy entry gate, string-only condition, fail-open evaluation) die with the path rather than being fixed on it. A retirement note at the survivor site records what left and why, per the convention measured on d9d3463 (#4328 / PR #4366). Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
ACCEPT — PM 复核 (session
Flipping ready + arming auto-merge. Generated by Claude Code |
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
2 participants
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 #3368
The ruling
Maintainer ruling of 2026-08-11, recorded on #3368 by the triage seat and quoted verbatim:
Zero-caller evidence, re-measured at this branch point (
f762f5bdf)The premise holds, unchanged from the four HOLD re-verifications on the card.
addMappingpackages/core/src/actions/ActionEngine.ts:181(definition) +__tests__/ActionEngine.test.ts:77, :113, :128, :211ActionEngine.dispatchActionEngine.ts:324(definition) +__tests__/ActionEngine.test.ts:115, :121, :134ActionMappingActionEngine.ts:48(interface),:96(the privatemappingsfield),:181(theaddMappingparameter type)Commands, word-boundary aware, whole tree with
node_modulesexcluded:dispatchis a common word in this repo (116 files match it), so it was scoped to themethod rather than the token. Three checks, all clean:
git grep -nE '\.dispatch\(' -- packages apps examples scripts(CHANGELOG excluded) returns exactly three hits, allengine.dispatch(...)inActionEngine.test.ts.git grep -nE "\[[\"'](dispatch|addMapping)[\"']\]"returns zero hits repo-wide, so nothing reached either method through a string key.packages/react/src/hooks/useActionEngine.ts— the one React wrapper around this class — contains zero occurrences ofdispatch,addMappingorActionMapping, so the hook never re-exposed the event-mapping surface to its consumers.That last check is what licenses deleting
dispatchoutright: it reads onlythis.mappings,which only
addMappingever wrote, so it serves the event-mapping path and nothing else. Theother four entry points (
executeActionby name,getActionsForLocationby location,handleShortcutby shortcut,executeBulkin bulk) are untouched and keep their callers.Export layers narrowed
ActionMappingwas public API of@object-ui/core, reaching consumers through two star re-exports:packages/core/src/actions/ActionEngine.tsexport interface ActionMappingdeclarationpackages/core/src/actions/index.ts:10export * from './ActionEngine.js'packages/core/src/index.ts:26export * from './actions/index.js'Because both barrels are
export *, the narrowing happens by deleting the declaration andthere is no barrel line to remove — recorded here so the absence of a barrel diff is not read
as an oversight.
packages/corecommits nodist/*.d.ts(0 tracked files underdist/), sono generated surface needed regenerating.
ActionMappingdoes not exist in@object-ui/types,which was verified before touching that package: it was never in scope.
Deleted
ActionEngine.addMapping()packages/core/src/actions/ActionEngine.tsActionEngine.dispatch()private mappings: ActionMapping[](the registry state)this.mappingsline inunregisterAction()this.mappingsline inclear()export interface ActionMappingaddMappingcall sites + the 3-testdescribe('dispatch')blockpackages/core/src/actions/__tests__/ActionEngine.test.tsTwo of the four
addMappingcall sites were in tests that cover something else, so thosetests were kept and only the mapping-specific lines removed —
unregisterAction's test(its assertions were already on
getAction/getShortcuts, never on mappings) andclear's test (same). Both titles lost their now-falsemappingsclause. The three testsinside
describe('dispatch')covered only the deleted path and went whole.The three contract inconsistencies die with the path
Per the ruling and the dispatch, the inconsistencies recorded on the card are not fixed
anywhere and are not ported into a doc note as live behaviour:
condition: falsedispatched anyway (08-09 addendum);conditiontyped asstringonly, so a{ dialect: 'cel', source }envelope could not reach the canonical@objectstack/formulaengine;throwOnError, so a throwing predicate failed OPEN — the opposite ofvisible's fail-closed posture ingetActionsForLocation.Aligning the contract of an API nobody calls would only have widened behaviour nobody uses,
which is the reasoning the card itself gave for grading this observation-class.
Retirement record — a doc note at the survivor site, per measured precedent
git log -Son the prior core-export retirements gives the convention. Commitd9d346307("retire four zero-consumer declared surfaces (#4328) (#4366)") contains all three shapes:
mergeViewsIntoObjects(core public export, whole file)RoleDefinition.permissions(member of a surviving exported type)RecordContextValue.loading/errorThis card is the middle shape: members removed from a surviving exported class. So it follows
RoleDefinition's convention — a retirement note on theActionEngineclass docblock namingthe retired methods, the registry behind them, the issue, and the three drifted behaviours as
history, plus a forward instruction that any future event-keyed entry point must be built
on the shared
hasDeclaredPredicate+toPredicateInputdefinitions rather than a fourthspelling. The file header's own "event-to-action mapping" clause was removed in the same pass.
No negative pin, and that is a measured decision rather than an omission: the pin in the
third row exists because dead wiring could reappear without any gate noticing. Here it cannot —
every residual reference to the deleted registry is a compile error, and the repo-wide
type-checkbelow is the gate that would catch a resurrection. That is the same reasoningthat left
mergeViewsIntoObjectsandRoleDefinition.permissionsunpinned in the precedentcommit.
Post-deletion sweep — each symbol individually
Every residual hit lands in the intentional retirement note; nothing else survives anywhere in the tree.
addMappingActionEngine.ts:89, inside the retirement docblockActionMapping.dispatch(packages/apps/examples/scriptsmappingsinpackages/coreActionEngine.ts:91, the same docblockevent-to-action/event mappingdrill-event mappingCHANGELOG rows)No README, guide, ADR or doc page ever documented this API, so nothing needed retargeting —
that absence was measured, not assumed.
Changeset grading
.changeset/retire-action-engine-event-mapping-3368.md—@object-ui/core: minor.Graded by the measured precedent, not by feel.
ActionMappingwas reachable as@object-ui/core's public export through the two star barrels above, andaddMapping/dispatchwere public methods of the exportedActionEngineclass, so the public surfacenarrows and code typing against or calling any of them stops compiling. That is exactly the
case
d9d346307graded'@object-ui/core': minorformergeViewsIntoObjects, and the samegrading #4403 applied to
common.searchfor narrowing an exported type. It is notpatch:patchwould have been right only if the symbols had turned out unexported from the packagebarrel, and the export-layer table above measures that they were not. Never
major, per theversion-alignment rule (the major tracks
@objectstack; breaking changes ship asminorwiththe semantics spelled out in the body).
No runtime behaviour changes, because no runtime path reached the deleted code.
Verification
pnpm --filter '@object-ui/core^...' build(build closure first, fresh worktree)pnpm exec vitest run packages/core/ --maxWorkers=2pnpm --filter @object-ui/core type-check— both tsc commands (tsc --noEmit && tsc -p tsconfig.test.json)pnpm exec turbo run type-check --concurrency=2turbo run lint --filter=@object-ui/coreno-explicit-any/ unused-directive in untouched files)node scripts/check-control-bytes.mjsgrep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'over the three changed filescheck-changeset-presence.mjs/check-changeset-no-major.mjsAll heavy runs were serialized through the shared
flockverification lock and capped at--max-old-space-size=4096.The repo-wide
type-checkis the load-bearing step for this change: an export removal isonly safe if no downstream package typed against it, and 78/78 green across every package,
app and example is that proof. Note the direction — this is the whole workspace, not a
filtered upstream closure, so it necessarily includes every consumer of
@object-ui/core.Reverse verification (direction predicted before running)
Per the #4365 retirement pattern, the zero-caller evidence plus the green ladder after
deletion is the verification, and it is what the table above records. Predicted direction
before running: green throughout, because a deletion with zero callers cannot turn a consumer
red — a red anywhere would have falsified the premise rather than revealed a bug in the patch.
That is what measured.
No negative pin was added (see the grading above), so there is no scratch re-introduction to
show red — the compile-time proof is the repo-wide
type-check, whose 78/78 green is the sameassertion a pin would have made, executed against every package rather than one.
Surface discipline
Touched exactly three files, all inside the dispatch's stated surface:
packages/core/src/actions/ActionEngine.ts, its test, and the changeset. No barrel fileneeded editing (both are
export *), andpackages/typeswas verified not to containActionMappingbefore being left alone. None of the parallel agents' reserved surfaces —scripts/**new gates (#4394), the defaults-map files and mirror suite (#4401),packages/fields/**(#4361),packages/plugin-charts/**(#4405),packages/app-shell/src/views/metadata-admin/**(#4406) — was read or modified.No out-of-scope findings.
Generated by Claude Code