refactor(service-i18n): collapse the four inline success builders behind a sendOk, retire the ratchet (#3973) - #4023
Merged
Conversation
…ind a sendOk, retire the ratchet (#3973) #3973 option 1. No wire shape changes — this is about the guard, not the bodies. #3636 put the declared `{ success: true, data }` envelope on all three i18n read routes but built it inline in each, so one envelope half had four call sites while the error half had already been consolidated behind `sendError` (#3675). Those bodies were never wrong, which is exactly why this needed saying out loud rather than being left alone. `scripts/check-route-envelope.mjs` counts response write sites per module: a consolidated module sits at a fixed two however many routes it grows. This one sat at five with a declared ratchet, because a fifth read route could have hand-rolled a fourth-dialect body and only a driven test would have caught it — and a driven test only covers the routes that existed when it was written. The four builders now collapse into one `sendOk` and the module declares the same 2 / 1 / 1 as the other five. Guard goes 5/2/1 → 6/1/1 conformant/ratcheted/exempt. Also corrects a claim #3843 left in six suite headers. They said the repo-wide scan "found two immediately (#3973, #3983)" — but #3973 was not one of them: i18n's unconsolidated builder was already known from #3843's own survey. The two the scan actually surfaced were share-link-routes.ts and the dev-only hmr-routes.ts. The guard header carried the same conflation and is fixed alongside. Verified: guard 6/1/1 + self-test, spec tsc, check:docs (251 in sync), check:skill-docs, check-console-sha, pnpm lint, and — the one that matters for "no wire change" — runtime 901 passed, which includes i18n-success-envelope.conformance.test.ts driving those exact three routes. Plus i18n 62, rest 491, storage 220, settings 189, datasource 154. Empty changeset: releases nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CYbS3kS8xzsHNXFTzp4e2z
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 11 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
marked this pull request as ready for review
July 30, 2026 04:55
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.
#3973 option 1. No wire shape changes — this is about the guard, not the bodies.
What was actually wrong
#3636 put the declared
{ success: true, data }envelope on all three i18n read routes, but built it inline in each — four call sites for one envelope half, while the error half had already been consolidated behindsendError(#3675).Those bodies were never wrong. That is precisely why this needed stating rather than being left alone:
scripts/check-route-envelope.mjscounts response write sites per module, so a consolidated module sits at a fixed two however many routes it grows. This one sat at five with a declared ratchet, because a fifth read route could have hand-rolled a fourth-dialect body and only a driven test would have caught it — and a driven test only covers the routes that existed when it was written.Four builders → one
sendOk. The module now declares the same2 / 1 / 1as the other five.The one remaining ratchet is
share-link-routes.ts(#3983), which needs its own consumer sweep and is not touched here.A claim #3843 got wrong, fixed in the same pass
Six suite headers (and the guard's own header) said the repo-wide scan "found two immediately (#3973, #3983)". #3973 was not one of them — i18n's unconsolidated builder was already known from #3843's own survey. The two the scan actually surfaced were
share-link-routes.tsand the dev-onlyhmr-routes.ts. That conflation was mine, in #3843; corrected in all seven places.Why "no wire change" is a checkable claim, not an assertion
packages/runtime/src/i18n-success-envelope.conformance.test.tsdrives those exact three routes and parses the bodies against the real spec schemas. It passes unchanged — 901 tests in@objectstack/runtime— which is what makes the refactor provably shape-preserving rather than merely intended to be.Verification
pnpm check:route-envelope+--self-testpnpm linttsc --noEmitcheck:docscheck:skill-docs·check-console-sha· changeset gates@objectstack/runtime@objectstack/rest@objectstack/service-i18n@objectstack/service-storage@objectstack/service-settings@objectstack/service-datasourceI ran the typecheck job's gates explicitly this time —
check:docslives there, not in the test job, and missing it cost a red CI round on #3972.Empty changeset: this releases nothing. Branch restarted from
mainper branch hygiene.Generated by Claude Code