Skip to content

feat(core)!: the token grammars become parse/format pairs - #753

Merged
rejifald merged 2 commits into
mainfrom
claude/helper-utils-public-contract-7b77b2
Aug 16, 2026
Merged

feat(core)!: the token grammars become parse/format pairs#753
rejifald merged 2 commits into
mainfrom
claude/helper-utils-public-contract-7b77b2

Conversation

@rejifald

Copy link
Copy Markdown
Owner

The three house token grammars only ever decoded. #746 made them public contract, which changed who else needs them: a peer package that parses an authored value the way core does will eventually want to write one back — a CLI printing the cap it enforced, a config round-trip, an error message quoting a limit in the grammar its author used — and had nothing to call. Every such caller hand-rolls an encoder, which is the drift the export exists to prevent, running in the opposite direction.

Each dimension is now one namespace carrying both directions, following bytes's shape (bytes.parse / bytes.format) rather than adding three more verb-prefixed names to the barrel:

Was Now Gained
parseDuration(d) duration.parse(d) duration.format(90_000)'1.5m'
parseBytes(s) size.parse(s) size.format(1_048_576)'1mb'
parseRate(r) rate.parse(r) rate.format({ count: 2, per: 1000 })'2/s'

The decode direction is byte-for-byte what it was — same grammars, same 1024-based size units, same undefined fallback for a bad duration or size token, same throw for a bad rate. Only the spelling moved. A Rate type is now exported for the { count, per } pair.

Why not the ms(…) one-function overload it resembles

ms and bytes switch on the argument's type — a string decodes, a number encodes. That inversion is structurally unavailable here, and the reason is this repo's own rule: P17/P25 widen every authored field to number | string, and every read site funnels the value through the parser. So parse(5_000) must return 5_000:

// packages/core/src/util.ts
if (typeof d === 'number') return d; // load-bearing: `timeout.total: 5_000` reaches here

An overload would have broken every call site core makes of its own rule. The explicit pair is the half of those libraries' contract that survives the constraint — and it is the half bytes itself exposes as .parse / .format.

format is exact where ms rounds

ms(90_000) is '2m', which parses back to 120 000. A 33% widening is harmless in a log line and disqualifying in anything that writes a value back — and P25's "a typo can never widen a cap" only holds if the encode direction cannot widen one either.

So parse(format(v)) returns v unchanged for every value parse can produce. Exactness alone would still allow unreadable output (1537 / 1024 is 1.5009765625, which multiplies back perfectly and helps nobody), so a unit is used only when the quotient is both exact and short; otherwise the next smaller unit is tried, down to the base unit where both tests always pass:

duration.format(90_000); // '1.5m'
duration.format(90_001); // '90001ms' — no unit divides it cleanly, so not a rounded '2m'
size.format(1536); // '1.5kb'
size.format(1537); // '1537b'  — not '1.5009765625kb'

Pinned as a property over the whole numeric range in parsers-properties.spec.ts, not a table of pretty cases, and the expectations assert parse(format(n)) === n against the independently-tested decoder rather than re-running the encoder's own arithmetic. Verified non-vacuous by reintroducing ms-style rounding and watching all three round-trip properties plus both readability pins fail.

Two edge cases worth flagging for review, both found by the property run rather than reasoned about:

  • Number('0') || undefined is undefined, so a bare '0' is the one integer that does not survive a round-trip. Zero therefore carries a unit ('0ms' / '0b') — the single case where "largest exact unit" is not what makes the round-trip work.
  • rate.format does not reduce a rate to an equivalent one. '2/500ms''4/s' is the design (ADR 0023), but { count: 2, per: 500 } comes back as '2/500ms' — the numbers the author wrote, so the round-trip is equality rather than equivalence.

Bundle: +0.41 / +0.20 KB gzip, and the advertised figure moves

Measured against a main at 24.18 / 21.62 / 5.22. Two thirds of the naive cost was recovered rather than budgeted for, both measured:

  • the internals call the plain parseDuration/parseBytes/parseRate functions and the public objects are a thin facade over them, so a consumer's bundle is not routed through the namespace and the encoder can shake out (stitchapi/auth 5.44 → 5.22, back to its own baseline and inside the unchanged 5.35 ceiling);
  • each format's unit table lives inside the function. At module scope the minifier merges adjacent tables into one var statement, where the parse-side lookup being live pinned the encoder's table for every consumer of stitch. Deriving one table from the other with Object.fromEntries measured worse still, for the same reason.

What is left is ~0.1 KB and is a real cost of the namespace form, not an oversight: formatDuration is referenced by the duration facade object, whose other half is live on the stitch path, and esbuild will not split an object literal to drop the dead half. A consumer who never formats a duration still carries the formatter. Shaking it would mean not shipping the pair as an object — which is the shape of the API. Recorded in the budget note rather than left for someone to rediscover.

Scenario main this PR budget headroom
whole entry 24.18 24.59 24.25 → 24.80 0.21
import { stitch } 21.62 21.82 21.70 → 22.00 0.18
stitchapi/auth 5.22 5.22 5.35 (unchanged) 0.13

The conventional ~0.2 KB step, not a minimum one — this is a new capability on the public surface, not a fix squeezing past a ceiling.

The advertised whole-entry figure moves ~24 → ~25 kB (25183 B is 24.59 KB, which rounds up where main's 24.18 rounded down). Propagated across all six bundle-advertised-size sites — both READMEs, the installation and principles pages, the home-page metrics component, and the docs' own source blurb. import { stitch } is unchanged at 22. Verified by the yakir tether in the pre-commit hook (3 tethers, 0 drift), not assumed.

Migration

A rename at every call site; no alias (P19 scopes that obligation to the GA channel and this is rc). The old names are pinned absent from the barrel, so a stale import fails at build rather than resolving to something else.

- import { parseDuration, parseBytes, parseRate } from 'stitchapi';
- const ttl = parseDuration(opts.ttl);
- const cap = parseBytes(opts.max);
- const { count, per } = parseRate(opts.rate);
+ import { duration, size, rate } from 'stitchapi';
+ const ttl = duration.parse(opts.ttl);
+ const cap = size.parse(opts.max);
+ const { count, per } = rate.parse(opts.rate);

Internal churn is smaller than the rename suggests: because the implementations keep their original names as module functions, auth.ts, cache.ts, test-mock.ts and testing.ts are byte-identical to main. resilience.ts and store.ts change only because they declared a local const rate = parseRate(opts.rate) that the new import would shadow — renamed to paced, which the compiler caught rather than a reviewer.

Reviewer notes

  • The Bytes/Chars hazard got quieter. parseBytes said "bytes" at the call site; size does not. stream.buffer.chars and trace.body.chars still reject a token at compile time, so the type still holds the line — but the name no longer warns first. Stated on the namespace's JSDoc and in P25, and it is the change here I would most want a second opinion on.
  • format must not touch emitted values. P17's complement is unchanged: an emitted duration is raw ms. Formatting one for a CLI table is display; writing one into an emitted field is a violation this pair cannot see and will not stop. Called out in the JSDoc, in P17, and as an anti-pattern callout in the docs.
  • CONTRACT.md P17/P25 gain the pairing clause; the §6 migration record carries the full reasoning. ADR 0023 is annotated, not rewritten — every decision in it stands, only the spelling moved, so the body stays as the 2026-08-04 record with a note in its Status block.

Docs

Reference docs land in #746, which documented the parsers this replaces and is stacked on this branch. That PR is where the pair is written up for readers; merge this one first.

Gates

Gate Result
pnpm -r test all packages pass — core 1593 passed
pnpm check:types / check:types-d clean across 39 projects
pnpm check:lint clean across 37 packages
pnpm check:size ✓ within budget (raised, documented above)
pnpm check:contract ✓ no new violations (0 baselined)
pnpm check:changelog ✓ 23 subsections, Keep a Changelog order
pnpm check:docs-links ✓ 117 routes resolve
pnpm check:exports / check:unknown-keys / check:format clean
yakir tethers (pre-commit) 3 ok, 0 drift

Round-trip additionally fuzzed outside the suite over 220k magnitudes × 2 and 100k rates — no failures.

🤖 Generated with Claude Code

`parseDuration`, `parseBytes` and `parseRate` are replaced by `duration`, `size`
and `rate` — one namespace per dimension, each carrying both directions, in the
shape `bytes` uses (`bytes.parse` / `bytes.format`).

The grammars only ever decoded. Making them public (#746) gave them callers who
need to write a token back — a CLI printing the cap it enforced, a config
round-trip, an error message quoting a limit in the grammar its author wrote —
and each would have hand-rolled an encoder, which is the drift the export exists
to prevent, running backwards.

`format` is the EXACT inverse of `parse`: `parse(format(v))` returns `v`
unchanged for every value `parse` can produce, pinned as a property over the
whole numeric range rather than a table of cases. This is where the pair departs
from `ms`, whose `ms(90_000)` is `'2m'` and reads back as 120_000 — a lossy
encode is fine in a log line and disqualifying in anything that writes a value
back, and P25's "a typo can never widen a cap" only holds if the encode
direction cannot widen one either. Where no unit divides cleanly the base unit
wins: `90_001` is `'90001ms'`, `1537` is `'1537b'` and not the exact-but-
unreadable `'1.5009765625kb'`.

The `ms`/`bytes` one-function overload is structurally unavailable here: P17/P25
widen every authored field to `number | string` and each read site funnels
through the parser, so `parse(5_000)` must return `5_000`. Overloading the
number arm would have broken every call site core makes of its own rule.

Hard break, no alias (P19 scopes that obligation to the GA channel; this is rc).
The old names are pinned absent from the barrel so they cannot drift back.

Bundle: +0.41 / +0.20 KB gzip (entry / `import { stitch }`). Two thirds of the
naive cost was recovered first — internals call plain functions with the
namespaces as a thin facade, and each `format`'s unit table lives inside its
function, since at module scope the minifier merges adjacent tables into one
`var` statement that a live parse-side declarator then pins. What remains is
~0.1 KB: esbuild will not split an object literal to drop a dead property, so
`format` ships wherever `parse` is live. The advertised whole-entry figure
crosses 24 → 25 kB; propagated across the six `bundle-advertised-size` sites.

Reference docs for the pair land in #746, which documented the parsers this
replaces.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@rejifald
rejifald merged commit 827f27a into main Aug 16, 2026
12 checks passed
@rejifald
rejifald deleted the claude/helper-utils-public-contract-7b77b2 branch August 16, 2026 13:23
rejifald added a commit that referenced this pull request Aug 16, 2026
#753 replaced `parseDuration`/`parseBytes`/`parseRate` with the `duration`/
`size`/`rate` parse/format namespaces. This branch bound the old three, so after
rebasing onto it the playground would have offered three names core no longer
exports and hidden three it now does.

Caught by the coverage guard this same branch adds, one commit after the rename
landed:

    ✗ 3 core export(s) are neither bound in the playground nor listed as
      deliberately absent:
        duration
        rate
        size

That is precisely the class it was written for — a core export growing or
changing without the hand-curated browser surface following. The old binding
test could not have caught the new names (they were never in its list), and it
would have reported the three dead ones only as "unbound", with no hint that a
rename was the cause.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rejifald added a commit that referenced this pull request Aug 16, 2026
…d guard the gap (#751)

* fix(sandbox): bind the 20 core exports the playground was missing, and guard the gap

The snippet scope is the spread of whatever `stitch-browser.ts` re-exports
(worker-main.ts), and that list is hand-curated. Anything core exports but this
file forgets is not a binding at all — so a snippet naming it dies with
`X is not defined`.

This already bit us once: the four auth strategies were re-exported from the
wrong entry, resolved to nothing, and `bearer('…')` threw for anyone copying the
auth guide (#545 / ADR 0021). That was fixed as a one-off; the class was not
swept. Probing the real Worker for every core export missing from the surface
found 20 more unbound — and ten of them appear in snippets we ship:
`StitchError` in nine doc/blog files, `xhrAdapter` in five, `axiosAdapter` and
`fileSink` in four each, plus `RateLimitError`, `consoleSink`, `loggerSink`,
`graphqlSurface`, `isStitch`, `verdictOf`. Doc snippets are not executable in
place, so the failure path is a reader copying one into /playground and hitting
a ReferenceError on our own documented API.

Eighteen are re-exported verbatim: core is browser-isomorphic (no static `node:*`
imports, guarded `nodeFs()`), so they were never unsafe — just forgotten.
`xhrAdapter` is browser-NATIVE, and `axiosAdapter` takes a caller-supplied client
(core never imports axios), so both belong here.

Two could not be raw:
- `consoleSink()` is `createTrace({ console: true })`, which re-enables core's
  console path — the exact thing the R1 must-know forbids in the browser.
- `fileSink(path)` resolves no `node:fs` in a Worker and writes nowhere. Silent
  is the problem: a trace-sinks snippet would look like it worked.
Both now route through the browser `createTrace` and emit a RunNotice, matching
`env` / `cookieSession` / the OTLP pair.

The durable half is playground-surface-coverage.test.ts: it asserts the DECLARED
surface COVERS core's barrel, the half nothing tested. The existing binding test
checks that names in the list resolve — it could never catch the list itself
drifting, which is what both this and #545 were. New core exports must now be
bound or given a reason in INTENTIONALLY_ABSENT.

Also loosens both binding assertions from `=== 'function'` to `!== 'undefined'`.
They coincided only while every surface name was callable; the surface now also
carries plain objects (`httpSurface`, `graphqlSurface`, `systemClock`), which a
function-only check rejects for being exactly right.

Verified: node 13/13, browser 4/4, and a direct Worker probe showing all 20
previously-unbound names resolve plus a real errors-doc snippet running end to
end (`e instanceof StitchError` true, `compact({a:1,b:undefined})` → `{"a":1}`).
Guard confirmed to fail when a name is dropped from the surface list.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(sandbox): follow #753's rename — bind `duration`/`size`/`rate`

#753 replaced `parseDuration`/`parseBytes`/`parseRate` with the `duration`/
`size`/`rate` parse/format namespaces. This branch bound the old three, so after
rebasing onto it the playground would have offered three names core no longer
exports and hidden three it now does.

Caught by the coverage guard this same branch adds, one commit after the rename
landed:

    ✗ 3 core export(s) are neither bound in the playground nor listed as
      deliberately absent:
        duration
        rate
        size

That is precisely the class it was written for — a core export growing or
changing without the hand-curated browser surface following. The old binding
test could not have caught the new names (they were never in its list), and it
would have reported the three dead ones only as "unbound", with no hint that a
rename was the cause.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant