feat: typed command results, batch 4 (keyboard) — Phase 2#938
Merged
Conversation
Size Report
Startup median (7 runs, lower is better):
Top changed chunks: no changes in the largest emitted chunks. |
Wire `keyboard` into the CommandResultMap spine, grounded in the dispatch handlers' literal returns (src/core/dispatch.ts handleAndroidKeyboardCommand / handleIosKeyboardCommand). Modeled as a flat closed shape (platform + action always present; the remaining keyboard-state / message fields appear per branch) rather than a five-way platform×action union — the per-branch field sets overlap heavily and the underlying Android keyboard-state types live in the platform layer (below the public contract). The Record index signature of the previous hand-written mirror is dropped and the spurious `| null`s removed (the handler never returns null). Moves from the open client-types.ts mirror into src/contracts/keyboard.ts, wired through CommandResult<'keyboard'>; public export name preserved (re-exported via client-types.ts -> index.ts), so no API break. Parity test pins all 13 migrated commands. Stacked on the batch 3 PR. Verified: tsc --noEmit, oxfmt + oxlint --deny-warnings, fallow audit clean, Layering Guard empty, 379 tests across core/contracts/client/commands/system pass.
thymikee
force-pushed
the
refactor/phase2-typed-results-batch4
branch
from
b8fef04 to
68351fd
Compare
Member
Author
|
Reviewed current head 68351fd. I did not find actionable code blockers: the new KeyboardCommandResult shape matches the Android and iOS keyboard dispatch returns, keeps the existing public export name, and the client now routes through CommandResult consistently with the typed-result migration. Static checks, integration, unit, coverage, fallow, layering, and three smoke jobs are green; one Smoke Tests job is still pending, so I am leaving this reviewed but not marking ready-for-human until that finishes. |
thymikee
added
the
ready-for-human
|
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
1 participant
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.
What
Phase 2 (typed results) batch 4: wire
keyboardinto theCommandResultMapspine, grounded insrc/core/dispatch.tshandleAndroidKeyboardCommand/handleIosKeyboardCommand.Shape
Modeled as a flat closed shape —
platformandactionalways present; the remaining keyboard-state /messagefields appear per branch — rather than a five-wayplatform×actionunion. The per-branch field sets overlap heavily and the underlying Android keyboard-state types live in the platform layer (below the public contract), so a flat closed shape is the accurate, lower-risk migration. The previous mirror'sRecordindex signature is dropped and the spurious| nulls removed (the handler never returnsnullfor these).Mechanics
src/contracts/keyboard.ts(moved out of the openclient-types.tsmirror);CommandResultMapgainskeyboard; client method returnsCommandResult<'keyboard'>.KeyboardCommandResultpreserved (re-exported viaclient-types.ts→index.ts) — no API break. ThekeyboardCliOutputformatter type-checks unchanged.Verification
tsc --noEmitexit 0oxfmt+oxlint --deny-warningscleanfallow audit --base origin/maincleanvitestcore/contracts/client/commands/system → 379 tests passRemaining for Phase 2
This finishes the cleanly-closeable typed results. Left: (c) the mirror cleanup —
wait/alertare genuinely open (waitalso has an internal name-collision;alert's iOS path returns a genericRecord), so their client methods becomeCommandRequestResultand the last hand-written result types are deleted fromclient-types.ts; and (b) theTypedErrorgraft.