Skip to content

rtti: option is a nullary schema denoting absence — stage 2 of option-as-omission - #1748

Merged
sergey-shandar merged 13 commits into
mainfrom
claude/rtti-option-flag-x87c8h
Aug 28, 2026
Merged

rtti: option is a nullary schema denoting absence — stage 2 of option-as-omission#1748
sergey-shandar merged 13 commits into
mainfrom
claude/rtti-option-flag-x87c8h

Conversation

@sergey-shandar

@sergey-shandar sergey-shandar commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Absence stops being a spelling of undefined and becomes a member of the
set: a member that may be omitted is or(option, t), {} and
{ a: undefined } are distinct sets, and every reader, subset, both
renderers and the JSON Schema derivation agree on which is which.

  • option is a new nullary tag; visit gains the case and both
    schema-form readers an option handler that rejects normally, so
    orVisit tries the other members of or(option, t) for a present value.
  • Absence is decided by the container loops before dispatch: a declared
    member with no own or inherited key succeeds iff its schema admits
    absence (admitsAbsence, traversing nested ors with a visited set);
    a prototype-supplied member is still held to the present part.
  • parse omits an absent member: the struct kind drops the key, the array
    kind keeps a hole a hole and shortens a trailing absent run — the old
    JSON round-trip defect of materialized undefined is gone. The rebuilds
    never consult the value and dispatch nothing overridable: they place the
    parsed entries with Object.defineProperty captured at module load,
    since reading a member can run an accessor that patches any
    dynamically-looked-up operation before the rebuild runs. All three
    readers re-ask every declared member's presence last
    (presenceUnchanged) and refuse identically when an accessor flipped a
    decided member mid-read — pollution making an omitted key present, or a
    delete taking a checked one — the one deliberate residual split being a
    null-prototype value, where the hands-back readers return the still
    faithful value and parse, whose every plain container inherits the
    polluted key, refuses (pinned in host.proof.mjs; the verdict path's
    remaining exposure is todo/hostile-accessor-hermetic-read-path.md).
  • Data form: absentBit as the fifth unit bit, excluded from unknown;
    toData(option) via an explicit thunkUnion case; inline rests are
    stripped of the bit (referenced ones exempt — subset resolves them
    rather than masking, which the absence-only cycle shows would be
    unsound); trimPrefix drops a trailing declared position restating its
    rest; the declared-member top is or(option, unknown); objectMayOmit,
    objectPresentSet and a split arraySetSubset move to the bit; the
    data reader gets the same before-dispatch absence test.
  • Renderers: Ts<> renders an omittable member optional with absence
    stripped (readonly a?: number, readonly [1, number?] — exact under
    exactOptionalPropertyTypes), an interior tuple position as
    T | undefined, array(option) as readonly []; the runtime printer
    matches; toJsonSchema derives required/minItems from the bit while
    stripUndefined stays keyed on undefined. A Phantom annotation
    carries absence in the new AbsentOr wrapper — a union-carried
    marker drowns when the present part renders as the top, Absent | unknown being unknown — and the new CheckRaw pins the flag half
    separately against the schema, so the unwrapped spelling fails the pin
    even at the top.
  • Every option(t) call site migrated to or(option, t) — deliberately
    narrowing: a present undefined is no longer accepted at those members —
    and the direct or(…, undefined) optionality spellings audited
    (mcp/cas, media/json/schema); protocol/mcp's _noParams checks a
    read top-level value, so it carries undefined the value.
  • Docs and JSDoc swept; parse-omits-undefined-members dissolved and the
    option-as-omission todo completed, both deleted.

Changelog:

  • BREAKING CHANGES: rtti: option is a nullary schema denoting
    absence: an omittable member is or(option, t), which rejects a present
    undefined — the old option(t) set is or(option, t, undefined).
  • rtti: parse omits an absent member — the struct kind drops the key,
    the array kind keeps holes and shortens a trailing absent run — so an
    optional member survives a JSON round-trip.
  • rtti: unknown excludes absence, so the omittable top is
    or(option, unknown); Ts<>, the runtime printer and toJsonSchema derive
    optionality (?, required, minItems) from absence.
  • rtti: a Phantom annotation on a schema whose root admits absence wraps
    its present part in the new AbsentOr, pinned with the new CheckRaw.
  • rtti: parse builds its result without dispatching any overridable
    operation, and all three readers refuse a value whose accessors flip a
    decided member's presence mid-read, instead of answering wrongly.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp

…on-as-omission

Absence stops being a spelling of `undefined` and becomes a member of the
set: a member that may be omitted is `or(option, t)`, `{}` and
`{ a: undefined }` are distinct sets, and every reader, `subset`, both
renderers and the JSON Schema derivation agree on which is which.

- `option` is a new nullary tag; `visit` gains the case and both
  schema-form readers an `option` handler that rejects normally, so
  `orVisit` tries the other members of `or(option, t)` for a present value.
- Absence is decided by the container loops before dispatch: a declared
  member with no own or inherited key succeeds iff its schema admits
  absence (`admitsAbsence`, traversing nested `or`s with a visited set);
  a prototype-supplied member is still held to the present part.
- `parse` omits an absent member: the struct kind drops the key, the array
  kind rebuilds by slice-then-map, so a trailing absent run shortens the
  result and an interior hole survives — the old JSON round-trip defect of
  materialized `undefined` is gone.
- Data form: `absentBit` as the fifth unit bit, excluded from `unknown`;
  `toData(option)` via an explicit `thunkUnion` case; inline rests are
  stripped of the bit (referenced ones exempt — `subset` resolves them
  rather than masking, which the absence-only cycle shows would be
  unsound); `trimPrefix` drops a trailing declared position restating its
  rest; the declared-member top is `or(option, unknown)`; `objectMayOmit`,
  `objectPresentSet` and a split `arraySetSubset` move to the bit; the
  data reader gets the same before-dispatch absence test.
- Renderers: `Ts<>` renders an omittable member optional with absence
  stripped (`readonly a?: number`, `readonly [1, number?]` — exact under
  `exactOptionalPropertyTypes`), an interior tuple position as
  `T | undefined`, `array(option)` as `readonly []`; the runtime printer
  matches; `toJsonSchema` derives `required`/`minItems` from the bit while
  `stripUndefined` stays keyed on `undefined`. `Absent`, `_TsRaw` and
  `CheckRaw` are the raw-shape surface `Phantom` annotations need.
- Every `option(t)` call site migrated to `or(option, t)` — deliberately
  narrowing: a present `undefined` is no longer accepted at those members —
  and the direct `or(…, undefined)` optionality spellings audited
  (`mcp/cas`, `media/json/schema`); `protocol/mcp`'s `_noParams` checks a
  read top-level value, so it carries `undefined` the value.
- Docs and JSDoc swept; `parse-omits-undefined-members` dissolved and the
  `option-as-omission` todo completed, both deleted.

Changelog:

- **BREAKING CHANGES:** rtti's `option` is a nullary schema denoting
  absence; `option(t)` becomes `or(option, t)`, which also narrows —
  the faithful translation of the old set is `or(option, t, undefined)`.
  `parse` no longer materializes an absent member.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
functionalscript 1491876 Commit Preview URL

Branch Preview URL
Aug 28 2026, 09:20 AM

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for security reviews. Please try again later.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 18a235581b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fjs/rtti/parse/module.f.mjs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 596b6d90f9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fjs/rtti/ts/types.ts

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Measured at 596b6d90f on node v23.11.0. The change itself is in good shape: npm test 3500/3500 exit 0 and tsc --noEmit exit 0 (main 39b4724ef: 3478/3478). Despite the 1375 deleted lines the leaf diff is one rename (shortArrayFillsAnOptionalPositionshortArrayLeavesAnOptionalPositionOut) plus 23 additions — +22 net, matching the count exactly, so nothing is hiding.

I checked the three readers separately, since validate, parse and the data form have diverged in this codebase before: they agree with each other at both revisions. option(t) = or(t, undefined) accepted an absent key and a present undefined; the nullary option accepts absence and rejects present-undefined. Both mutants die — forcing the option arm to ok fails 9 proofs, and making the shared admitsAbsence gate always succeed (so required members tolerate absence) fails 33.

Consumer migration is behaviour-preserving where I spot-checked it (media/json/schema, mcp/evo, protocol/mcp): the mechanical option(t)or(option, t) rewrites are inert for real JSON, which has no undefined literal. protocol/mcp's _noParams is deliberately not mechanical — option(record(unknown))or(record(unknown), undefined) — because it validates a read top-level value rather than a container position, and the comment says so. Right call.

Two convention breaks to fix:

  1. The changelog file is misnamed. changelog/unreleased/option-as-omission.md should be 1748.mdchangelog/README.md:20 says "named by that number" and CONTRIBUTING.md:166-167 says "named by the real pull request number". Every sibling in that directory follows it (1734.md, 1736.md, 1738.md, 1741.md, 1742.md).
  2. The body's Changelog: section does not hold the file's items. CONTRIBUTING.md requires exactly the list items of the entry; the body carries a 4-line condensed summary while the file is ~20 lines / 1428 characters. The entry is also about 5× the "~250 characters, about three wrapped lines" guideline in changelog/README.md1734.md is 276 and 1741.md is 366.

On the breaking marker: it is correct, and worth stating why, since the absent exports map cuts the other way — with no exports boundary every file under fjs/** is externally importable, so an option signature change is breaking for the whole tree rather than internal.

…m` short-circuits `_IsAbsentOnly`

Two review findings on #1748:

- `parse`'s tuple rebuild called `slice`/`map` on the input, so an
  accepted `Array` subclass overriding either could hand back a result
  that fails the schema it was parsed against, or throw past the `Result`
  API. The rebuild is now built from the parsed entries alone — hole-run
  segments folded with `concat` on a trusted plain array, which appends a
  spreadable argument element by present element and so keeps the holes —
  and the value is never consulted. Pinned in `host.proof.mjs` with an
  array whose prototype supplies a hostile `slice` and a throwing `map`.
- `_IsAbsentOnly` (behind `ArrayTs`) walked a phantom-wrapped schema's
  thunk, re-expanding a recursive union into itself — TS2589 where the
  annotation exists to prevent it. It now reads the `Phantom` annotation
  first, as `_AdmitsAbsence` and `Ts` do: the annotation is
  `_TsRaw`-shaped, so absent-only is `Exclude<O, undefined | Absent>`
  being `never`. Pinned with a recursive `X = or(option, number, X)`
  wrapped and used as an array element, and an absence-only annotation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0d297672f6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fjs/rtti/parse/module.f.mjs Outdated
…argument spread

Review finding on #1748, on the previous fix: accumulating segments with a
per-entry spread was quadratic — about 13.5 s for a 15,000-position tuple
where the parent took 0.37 s — and the one `concat(...segments)` call
overflowed the engine's argument limit on a large enough prefix, throwing
past the `Result` API.

The segments are now collected on the module's O(1)-prepend list and joined
pairwise: each round halves the segment count with one-argument `concat`
calls, so the rebuild is one linear pass plus a logarithmic number of
halving rounds and no call takes a variable argument list at all. Pinned
with an odd segment count and with thousands of alternating present/absent
positions, every hole and member asserted. (The remaining cost at that
scale sits in `undeclaredMembers`' pre-existing linear `declared.some`
scan, which predates this PR and matches the parent's 0.37 s.)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Measured at 9fc4fd93b on node v23.11.0: npm test 3503/3503 exit 0, tsc --noEmit exit 0 (main 39b4724ef: 3478/3478); every leaf difference is the 25 new option proofs plus the one rename.

The filename is fixed — changelog/unreleased/1748.md. Both new code commits check out, and the hardening one turned out to be understated: I built an Array subclass overriding slice/map/concat/at/Symbol.iterator, and at 596b6d90f parsing a [number, string] tuple called slice and map on the value and returned an instance of the subclass — species-preserving, which is worse than "runs a method". At 9fc4fd93b none of those are invoked and the result is a plain Array. Reintroducing the old value.slice(0, end).map(...) is KILLED — 3502/1, the single failure being host.proof.mjs:hostileArrayMethodsDoNotReachTheRebuild.

The spread claim holds too: a 150 000-element all-present tuple throws RangeError: Maximum call stack size exceeded at 0d297672f, and a 200 000-element one parses at 9fc4fd93b. Two notes on that. Wall clock is not linear in practice — ~17-31 s at 100 k and ~48 s at 200 k, at both commits — so something else in parse outside tupleRebuild is superlinear; not this PR's claim to defend, but worth a look. And ts/types.ts +35 touches only the non-exported _IsAbsentOnly/_IsAbsentOnly1: an emitted-declaration diff shows no export type line changed anywhere in the three commits, so nothing consumer-visible.

Still open: the body's Changelog: section does not hold the entry's items. The body is a 4-line, ~262-character summary; the file is 1428 characters. Missing from the body: parse's round-trip/hole behaviour, the data-form absentBit/unknown exclusion, the Ts<>/printer rendering rules, toJsonSchema's required/minItems derivation, and the Phantom/Absent/CheckRaw requirement. CONTRIBUTING.md wants exactly the file's list items there, and the squash lands the body. The entry is also still ~5× the changelog/README.md length guideline, which is the easier half to fix — shortening the file would make both problems go away at once.

…lared` scan is quadratic

The entry is rewritten as four short list items — what changed and why, at
release-note altitude — so the pull request body's `Changelog:` section can
hold exactly the same items, as CONTRIBUTING.md requires. The rationale,
measurements and migration detail stay in the pull request description and
the module docs, where the changelog README sends them.

Review of #1748 also measured a superlinear end-to-end parse at 100k+ tuple
positions on both heads. Root-caused to `undeclaredMembers`' per-member
`declared.some` linear scan — quadratic on dense tuples, predating this
branch — and recorded as `fjs/rtti/todo/undeclared-members-declared-scan.md`
with measurements and the O(1)-membership fix sketch, rather than widening
this pull request.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp

Copy link
Copy Markdown
Contributor Author

@o2alexanderfedin Thanks for the review — both points are addressed in 465499e:

  • Changelog length / body mismatch: changelog/unreleased/1748.md is rewritten to four items within the changelog/README.md length guideline, and the PR body's Changelog: section now holds exactly the file's items, as CONTRIBUTING.md requires.
  • Superlinear parse at large tuple sizes: reproduced and root-caused. The rebuild itself is linear (pairwise segment join); the quadratic term is the pre-existing declared.some(d => d === k) scan in undeclaredMembers (fjs/rtti/common/module.f.mjs), which every reader routes undeclared members through — a dense n-position tuple pays ~n²/2 string comparisons. Measured 0.7 s at 25 000 positions and 3.3 s at 50 000 (4× per doubling), matching your 17–31 s at 100 000; it reproduces identically on the parent commit, so it predates this PR. Filed as fjs/rtti/todo/undeclared-members-declared-scan.md (fix sketch: an O(1) membership test built once per schema) rather than widening this PR.

Generated by Claude Code

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The body's Changelog: section now matches changelog/unreleased/1748.md item-for-item — that half is fixed. Gates at 465499e52: npm test 3503/3503 exit 0, tsc --noEmit exit 0.

The quadratic claim in the new todo is real, and I measured it rather than taking the file's word: timing undeclaredMembers directly on a dense tuple gives 37.5 ms at n=5 000, 169.4 ms at 10 000 (4.5×), 768 ms at 25 000, 3 697 ms at 50 000 (4.81× per near-doubling) — O(n²), and within a few percent of the figures the todo itself cites. Status: open, three unchecked tasks, both links resolve.

One thing the commit message claims but the file does not quite reach: the entry is 933 characters / 14 lines, down from 1428. changelog/README.md asks for "about three wrapped lines, ~250 characters", so it is still ~3.7× the guideline. Not worth another round on its own — but "hold #1748's entry to the length guideline" is not yet true.

`changelog/README.md` bounds an *entry* — one list item; a pull request
with several entries puts them all in its one file — at about three
wrapped lines, ~250 characters. Two of the four ran past that; now every
item is within it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp

Copy link
Copy Markdown
Contributor Author

@o2alexanderfedin On the length point: changelog/README.md bounds an entry — one list item — at "about three wrapped lines, ~250 characters", and separately says "A pull request with several entries puts them all in its one file", so the 933-character figure measures the file, not an entry. Released precedent matches the per-item reading: multi-entry files of the same shape are common (0.46.0/1573.md — 4 items, 939 chars; 1581.md, 1608.md similar).

That said, two of the four items did run past ~250 characters on their own (306 and 287), so 293926f trims them — every item is now 137–268 chars / ≤4 lines, and the PR body's Changelog: section is re-synced to the file verbatim.

Thanks for independently confirming the undeclaredMembers quadratic — your per-n figures landing within a few percent of the todo's is good evidence the fix sketch (per-schema O(1) membership) targets the right walk.


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 293926ffa1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fjs/rtti/parse/module.f.mjs Outdated

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gates at 293926ffa: npm test 3503/3503 exit 0, tsc --noEmit exit 0; the body's Changelog: section matches the file item-for-item.

Correcting my own reading first: the guideline in changelog/README.md is per entry, not per file — "an entry is at most a few lines (about three wrapped lines, ~250 characters)". So my earlier "933 characters against ~250" compared the wrong things, and your commit message's "per-entry" framing was the right one.

On that correct reading it is close but not quite there: two of the four entries are still 4 lines rather than "about three" — entry 1 (BREAKING CHANGES) at 270 characters and entry 3 (unknown) at 242. Entry 3 is within the character figure and only over on line count. A small trim finishes it; not worth blocking.

claude added 2 commits August 28, 2026 07:47
Reading a member of the value can run arbitrary code — an accessor — and
the rebuild runs after every read, so by then that code may have replaced
anything reached by dynamic lookup: an `Array.prototype` method, the
array iterator, `Object.fromEntries`, the `Array` binding, or the
species lookup inside every array method — even a captured `concat`
builds through the receiver's species. A getter that patched
`Array.prototype.concat` steered the tuple rebuild into `['ok', []]`
for `[1, 2]` against `[number, number]`.

The rebuilds now walk the entry cons list the loop already builds —
plain literals, property reads only — and place members with
`Object.defineProperty` captured at module load, which consults nothing
patchable; `eachEntry` walks by index for the same reason, `for..of`
dispatching the patchable iterator. The intermediate array and the
pairwise `concat` join are gone with the dispatch. Pinned in
`host.proof.mjs` across all three container kinds.

The verdict path still dispatches overridable operations after a read —
recorded as `fjs/rtti/todo/hostile-accessor-hermetic-read-path.md`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5a5825f21a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fjs/rtti/parse/module.f.mjs
A getter on a later declared member can install an earlier, already
omitted declared key on `Object.prototype` (or an omitted position on
`Array.prototype`), and then every fresh container inherits it: the
member is present by the same HasProperty rule the readers dispatch on,
so no plain container the rebuild could hand back denotes the value that
was checked — `parse` returned `['ok', { b: 2 }]` whose payload fails
the very schema it was parsed against.

The declared-member kinds now check the omission's postcondition after
the rebuild: every declared key the entries omitted must still be absent
from what was built — `in` plus `Object.hasOwn` captured at module
load, both internal operations, so the check itself dispatches nothing
overridable — and refuse otherwise, per DESIGN.md §10. Pinned in
`host.proof.mjs` for the struct, tuple and rest kinds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fed8c16e26

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread fjs/rtti/ts/types.ts Outdated
…n member

`Absent | unknown` *is* `unknown`: a union-carried marker drowns when
the annotation's present part renders as the top — `Ts<{}>` is native
`unknown`, `StructTs`'s intersection identity — so
`or(option, {})` behind a `Phantom` silently rendered its member
required while the readers accept `{}`, and the union-shaped `CheckRaw`
passed anyway, `_TsRaw` collapsing identically on both sides.

The annotation now spells absence as the new `AbsentOr<Present>` wrapper
— the same shape the runtime keeps, the data form's absent bit riding
beside the union rather than in it — which survives any present type.
`_AdmitsAbsence`, `_IsAbsentOnly`, `Ts` and `_TsRaw` read the
wrapper first, and `CheckRaw` pins the flag half separately against the
schema's structural answer, so the unwrapped spelling now fails the pin
even at the top instead of passing silently. An absent-only root is
`AbsentOr<never>`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified the delta since 293926ffa. Gates on this head: tsc --noEmit exit 0, npm test exit 0, 3505/3505.

Both central claims hold, and I checked them by watching the opposite fail rather than by reading:

  • Rebuild dispatches nothing overridable — genuinely pinned. Reverting recordRebuild to route through Object.fromEntries makes host.proof.mjs's hostileIntrinsicPatchesDoNotReachTheRebuild fail. The claim is not merely asserted.
  • AbsentOr fixes a real, silent hole — on fed8c16e2 the old union spelling Absent | unknown type-checked while StructTs rendered the member required; on this head the same spelling fails CheckRaw with TS2344. Confirmed the probe wasn't vacuous by flipping an assertion and getting a real error.
  • No over-broad refusal in either change: empty/nested/interior-absent/rest/open/100-element cases all still round-trip, and a root that excludes absence is not forced to wrap.

Three things worth your judgement:

1. The three readers now disagree, on exactly this PR's input class. Same input, fresh value and prototype reset between calls, at fed8c16e2:

validate: ok {"b":2}
parse:    error {"path":[],"message":"unexpected value"}
data:     ok {"b":2}

At 293926ffa all three agreed (all ok, all silently wrong). parse alone gained the omittedStillAbsent recheck; validate and the data form check k in value once per member in schema order and hand back the original reference, so a later member's accessor that retroactively re-adds an earlier cleared member is never re-checked. readersAgree's own comment says a value one reader accepts and another rejects is a bug in whichever walk differs. The new fixtures call only parse, so nothing catches this. I can see the other reading — validate/data stay within their "hands back what it was given" contract, and closing it there needs a post-read recheck rather than a rebuild guard, which is what the new todo is scoped to. But right now the divergence is undocumented and untested either way.

2. Missing changelog entry. All four entries describe earlier commits in this PR (option-as-absence, omission, unknown, Phantom); none describes the rebuild hardening. Yet parse's observable behaviour changed — at 293926ffa the tuple case returned ok ["bad",2], materialising a polluted string into a number position; now it refuses. changelog/README.md asks for an entry for anything affecting behaviour, and this repo squash-merges, so the omission lands in history permanently. Lengths of the existing four are fine (137–219 chars).

3. Minor: CheckRaw has no production call site — only its own doc comments and self-tests — and no Phantom<> in the tree wraps an absence-admitting root. The fix is real, but nothing in the codebase currently exercises it.

Todo hygiene checks out: Status: open is valid, and I spot-checked its concrete claims against fjs/rtti/common/module.f.mjs — the spreads, Object.entries/.filter/.map/.toSorted uses and the rtti() destructuring it names are all still present as described. Coverage: one leaf name vanished, traced to a rename in 18a235581 outside this delta.

The pollution refusal lived in `parse` alone, so the readers disagreed
on exactly that input class: a getter installing an omitted key on
`Object.prototype` was refused by `parse` while `validate` and the
data form handed back a value that no longer denotes what was checked —
and a value one reader accepts and another rejects is a bug in whichever
walk differs, by `host.proof.mjs`'s own table.

Every declared-member walk now records each member's presence and re-asks
it last (`presenceUnchanged` in the shared kernel), after everything
that reads the value, refusing on any flip in either direction — an
omitted key made present by pollution, or a checked own key deleted by a
later accessor. Pinned across all three readers, both directions, all
declared-member kinds.

The one deliberate residual split is a value with **no prototype**:
pollution cannot flip its own absence, so the hands-back readers return
it — still a faithful member of the set — while `parse`, whose every
plain container now inherits the omitted key, refuses per DESIGN.md §10.
Pinned as such. The changelog entry the hardening was missing is added.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y8fxzY1KUThQy8GMWNoGWp

Copy link
Copy Markdown
Contributor Author

@o2alexanderfedin All three points addressed in 1491876:

  • 1. Reader divergence — you were right to put it to judgement, and the readersAgree contract decides it: a verdict two readers give and one refuses is a bug in whichever walk differs, so the fix went into all three rather than into documentation. Every declared-member walk now records each member's presence and re-asks it last (presenceUnchanged in the shared kernel), after everything that reads the value — refusing a flip in either direction, since a later accessor can also delete a checked own key, which would have left validate returning a value missing a required member. Pinned across validate, parse and the data form, both directions, all declared-member kinds. The one deliberate residual split is a null-prototype value: pollution cannot flip its own absence, so the hands-back readers return it — still a faithful member of the set — while parse, whose every plain container inherits the polluted key, refuses per DESIGN.md §10. That split is now documented and pinned as such (nullPrototypePollutionSplitsByContract), each reader honest to its own contract.
  • 2. Missing changelog entry — added as the fifth item (212 chars / 3 lines), covering both the dispatch-free rebuild and the presence recheck; the PR body's section is re-synced.
  • 3. CheckRaw call sites — correct: the tree currently has no Phantom over an absence-admitting root, so the API's only exercise is its own pins (including the or(option, {}) collapse case). It ships ahead of its first consumer deliberately — the prescription exists so the next recursive absence-admitting schema doesn't rediscover the TS2589/collapse pair — and the pins are what hold it until then.

Gates at 1491876: tsc 0 errors, 3506/3506 tests, coverage 100/100/100 on every module.f.mjs.


Generated by Claude Code

@o2alexanderfedin o2alexanderfedin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

The three readers now agree, and the agreement is pinned — neutralising presenceUnchanged in validate alone fails presenceFlipsAreRefusedByAllReaders. Checked 21 ordinary valid inputs through all three readers: none newly refused. Coverage is strictly broader (the replaced leaf asserted only parse), and the changelog gap is closed. Gates: tsc exit 0, npm test 3506/3506.

@sergey-shandar
sergey-shandar added this pull request to the merge queue Aug 28, 2026
Merged via the queue into main with commit f990e2b Aug 28, 2026
19 checks passed
@sergey-shandar
sergey-shandar deleted the claude/rtti-option-flag-x87c8h branch August 28, 2026 15:19
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.

3 participants