types/object: add structurallySame and assertStructurallySame - #1538

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 872348932e

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fb5f6450b0

Reviewed at cd1b73d1 against origin/main e7e1881a.

The design is right and the implementation matches the structurally-same.md proposal
clause-for-clause — leaf placement, the asserts -> object -> nullable -> asserts cycle
avoided, the re-export from types/object, the five semantic rules, the assert* naming and
the readonly [unknown, unknown, unknown?] shape. The README is unusually good; documenting
the sparse-array asymmetry as unreachable rather than handled is exactly the right call and
exactly the right place to say it. Two things to fix.

1. Deleting the todo strands a link

bin/linkcheck.mjs goes from 137 broken links on main to 138 here, and the diff of the
broken-link sets is one line:

fjs/media/json/todo/remove-native-json.md:148:
  [`fjs/types/object/todo/structurally-same.md`](../../../types/object/todo/structurally-same.md)
  — `fjs/cas/evo/proof.f.mjs:68` stringifies two values only to compare them;
    `structurallySame` is the better fix for that one site.

That bullet is now stale in substance as well as in target: this PR did fix
fjs/cas/evo/proof.f.mjs:68. It should either point at
fjs/types/object/structurally_same/README.md and be reworded to past tense, or be dropped.
fjs/bnf/todo/serialized-proof-expectations.md — the correct new home for the deferred audit
work — would be a reasonable target too.

2. Three surviving mutants in the co-located proof

I mutated structurallySame twelve ways and ran the co-located proof plus fjs/asserts/proof.f.mjs
(40 cases) against each. Nine mutants died; three survived, and each survivor is observably wrong
on ordinary FunctionalScript data, so these are genuine §3.2 gaps, not equivalent mutants:

Why the existing cases miss them:

• differ([], {}) does not exercise b instanceof Array — it is already killed by
  0 === undefined. Only an array-like object reaches the check. (differ({}, []) does pin
  the other b instanceof Array, the standalone one; that mutant died.)
• differ({ a: 1 }, { b: 1 }) is killed by the value comparison (1 vs undefined), not by
  bm.has. differ({ a: undefined }, {}) is killed by the count. Disjoint key names and
  undefined on both sides is the only combination that isolates has.
• differ([1], [1, 2]) fails >= anyway, and differ([1, 2], [1]) is killed by the elementwise
  recursion. Only a trailing undefined — the array analogue of the { a: undefined } vs {}
  case the proof already has for objects — reaches the length check on its own.

Three one-liners in the existing groups close all three:

// arrays
() => differ([1, 2], { 0: 1, 1: 2, length: 2 }),   // array-like is not an array
() => differ([undefined], []),                     // a trailing `undefined` is an element
// objects
() => differ({ a: undefined }, { b: undefined }),  // key *names*, not just count

The nine that died, for the record: Object.is → === (2 cases, NaN and -0), array length
check dropped, the standalone if (b instanceof Array), index-ordered key comparison (2 cases —
key order really is pinned), property-count check dropped, both === null guards, and both
non-recursive === substitutions for element and property values. So the coverage is good; it
is specifically the undefined-vs-missing and array-vs-array-like family that is thin — the
same family the proposal called out as must-cover.

What I checked and found clean

• No duplication. assertDeepEqual in fjs/types/rtti/parse/proof.f.mjs is fully removed,
  and grepping the tree finds no other deep/structural comparison left behind. The replacement is
  strictly stronger: the old helper used === on leaves, so it accepted NaN as unequal-to-itself
  and 0/-0 as equal, and it threw bare strings instead of going through assert.
• §6.2 _ prefix. File-aware surface diff after prepack in both trees. The exported type
  set is byte-identical to main (non-empty output, 46,326 bytes, so the diff is meaningful) —
  this PR adds no exported type at all, prefixed or otherwise. Const additions are three, all with
  precise signatures and no widening to any:
  assertStructurallySame: (...x: readonly [unknown, unknown, unknown?]) => void,
  structurallySame: (a: unknown, b: unknown) => boolean, and the new proof.
• §4 module headers survive declaration emit: @module present in
  structurally_same/module.f.d.mts, types/object/module.f.d.mts and asserts/module.f.d.mts,
  and the re-export lands as export { structurallySame } from './structurally_same/module.f.mjs'.
• §8.3 CHANGELOG — entry present under Unreleased, links only /pull/1538, no released
  section touched. Pure addition, so no **BREAKING CHANGES:** prefix needed.
• npx tsc --noEmit exit 0; npm run prepack from a cleaned tree exit 0 (both passes);
  npm test 2584 pass / 0 fail vs 2551 on origin/main. The +33 is exactly accounted for:
  30 cases in the new proof (13 primitives, 1 identity, 8 arrays, 8 objects) plus 3 in the asserts
  proof. No test lost in the assertDeepEqual removal.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e20a7b2155

Re-reviewed at e20a7b21 (previous review was at cd1b73d1). Baseline: origin/main = 87c58701.

The only change since the last review is the merge of main (git diff cd1b73d1 e20a7b21 touches exactly CHANGELOG.md, fjs/text/utf16/* and the todo removed by #1540). Nothing in fjs/types/object/structurally_same/ or fjs/asserts/ moved, so both open findings stand unchanged. Details below so the next round has everything in one place.

Still green

• npx tsc --noEmit — exit 0.
• npm run prepack from a freshly cleaned tree — exit 0.
• npm test — 2586 pass / 0 fail, vs 2553 on origin/main (+33; the new proof contributes 30 cases, consistent).
• CHANGELOG entry present and correctly links only /pull/1538.

1. Stranded link (unresolved)

Deleting fjs/types/object/todo/structurally-same.md leaves a dangling reference. Broken-link sets compared against origin/main (node bin/linkcheck.mjs, sorted, comm):

main: 137 broken   PR: 138 broken
only in PR:
  fjs/media/json/todo/remove-native-json.md: [`fjs/types/object/todo/structurally-same.md`](../../../types/object/todo/structurally-same.md)
only in main: (none)

One net new broken link, no pre-existing one repaired. fjs/media/json/todo/remove-native-json.md:148 needs to point at the new fjs/types/object/structurally_same/README.md (or drop the link).

2. Three surviving mutants (unresolved)

No proof cases were added, so all three still survive the co-located proof, and each is observably wrong. I rebuilt each mutant against e20a7b21 and ran the proof (30 cases) plus a direct probe:

differ([], {}) does not catch the first because [].length === ({}).length is 0 === undefined; it takes an array-like with a matching length. differ({ a: 1 }, { b: 1 }) does not catch the second because the mismatched value is not undefined. differ([1], [1, 2]) does not catch the third because that is the short-a direction, which >= still rejects.

Three one-line additions close all three:

// arrays:
() => differ([1, 2], { 0: 1, 1: 2, length: 2 }),
() => differ([undefined], []),
// objects:
() => differ({ a: undefined }, { b: undefined }),

Each of these is also documentation: the first two are exactly the "an array never matches a non-array" and "same length" clauses of the JSDoc, and the third is the "a property whose value is undefined is a property" clause in its hardest form.

Settled, not re-litigated

The implementation matches the structurally-same.md proposal clause-for-clause, assertDeepEqual is fully removed and the replacement is strictly stronger, and there are zero new exported types — all re-confirmed unchanged by the diff above.

Re-reviewed at 50734558 (previous review was at e20a7b21). Baseline: origin/main = 87c58701. Both open findings are resolved. Approving.

1. Stranded todo link — fixed

fjs/media/json/todo/remove-native-json.md no longer points at the deleted todo/structurally-same.md; it now links structurally_same/README.md and the new bnf/todo/serialized-proof-expectations.md, both of which exist. grep -rn 'structurally-same' fjs --include='*.md' returns nothing — no reference survives anywhere.

Broken-link sets compared against main, not counts:

PR: 155   main: 155
only in PR:   (none)
only in main: (none)

The sets are byte-identical. No net-new entry, and the two links added by this PR resolve.

2. The three surviving mutants — all now die

Three proof cases were added this round, so I did not take them at face value: I reconstructed each of the three mutants against this head and re-ran the co-located proof. Baseline first, to establish the harness discriminates: 33/0 (was 30 cases, now 33).

Each added case is the minimal one that reaches the clause, and the comments say why the obvious weaker case does not: differ([], {}) already fails on 0 === undefined and never reaches the b instanceof Array check, and differ({ a: 1 }, { b: 1 }) is settled by the values rather than by the key-set test. That reasoning is correct — I confirmed it by mutation, not by reading.

Two further mutants I built as controls also die: dropping the if (b instanceof Array) { return false } early return (1 fail) and dropping ae.length === bm.size (2 fails). Dropping the Object.is short-circuit fails 9. 12/12 of the mutants I have tried across the two rounds are now killed.

Battery

• npx tsc --noEmit — exit 0.
• npm run prepack from a freshly cleaned tree — exit 0 (both passes).
• npm test — 2589 pass / 0 fail, against 2553 / 0 on main at 87c58701. +36, consistent with the 33 new proof cases plus the asserts proof changes.
• Public surface, both axes, main vs PR. Exported type aliases: no diff at all — zero new exported types, so nothing to check against the _ convention. Exported consts: three additions and no removals or widenings:

  + /fjs/asserts::assertStructurallySame: (...x: readonly [unknown, unknown, unknown?]) => void
  + /fjs/types/object/structurally_same::structurallySame: (a: unknown, b: unknown) => boolean
  + /fjs/types/object/structurally_same::proof
  

  No signature collapsed to any; no elided in the emitted declarations.
• §4 header: @module survives into the emitted module.f.d.mts (blank line after the header block is present).
• §8.3 CHANGELOG: entry present under Unreleased, links only /pull/1538, no released section touched. The new bnf/todo/*.md is docs and correctly gets none.

Carried over from the last round and not re-derived: the implementation matches the structurally-same.md proposal clause-for-clause, assertDeepEqual is fully removed and the replacement strictly stronger.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90c638f7f0

Re-review at 90c638f7. Previously approved at 50734558; the head has moved twice since, both times by merging main (3b739360 merged 5f5c687f/#1530, 90c638f7 merged daf48b68/#1539). Still approving — the delta is merge-only.

What actually changed since the approved head

The PR's own delta against its base is unchanged. Every source file this PR touches is byte-identical to the approved head — same blob hashes at 50734558 and 90c638f7 for all nine:

fjs/types/object/structurally_same/{module.f.mjs,proof.f.mjs,README.md}, fjs/asserts/{module.f.mjs,proof.f.mjs}, fjs/types/object/module.f.mjs, fjs/types/rtti/parse/proof.f.mjs, fjs/bnf/proof.f.mjs, fjs/cas/evo/proof.f.mjs.

git diff daf48b68 90c638f7 is the same 14-file change as before (406+/269-).

Both merges were clean apart from CHANGELOG.md, and git diff-tree --cc shows the resolution is ordering only — this PR's entry plus #1530's and #1539's entries, all three intact under ## Unreleased, nothing rewritten and no released section touched. That is the whole conflict-resolution content of both merges.

Battery re-run at the merged head

• npx tsc --noEmit — exit 0.
• npm run prepack from a freshly cleaned tree — exit 0 (both passes).
• npm test — 2617 pass / 0 fail, against origin/main daf48b68 at 2581 / 0. +36, the same delta as the previous round; the merge of #1539 accounts for main's own move from 2553.
• Broken-link sets vs origin/main daf48b68 — byte-identical, empty diff over the sorted sets (not just equal counts). Nothing stranded by the merge.
• Public surface, dual axis, PR vs daf48b68 after prepack in both trees:
  • exported type aliases: no diff at all. Zero additions, so the _-prefix question in §6.2 does not arise.
  • exported consts: exactly three additions, none widened to any —
    • /fjs/asserts::assertStructurallySame :: (...x: readonly [unknown, unknown, unknown?]) => void
    • /fjs/types/object/structurally_same::structurallySame :: (a: unknown, b: unknown) => boolean
    • /fjs/types/object/structurally_same::proof
  • no deletions, so nothing that was public went away.
• @module survives declaration emit — 1 occurrence in each of structurally_same/module.f.d.mts and asserts/module.f.d.mts; zero elided or : any in the emitted structurally_same declaration.

Mutation coverage

The implementation and the proof are byte-identical to the head where I rebuilt the mutants, so the earlier result carries over by construction rather than by assumption. I spot-checked it anyway at this head rather than relying purely on that: dropping the bm.has(k) && guard on line 58 gives 2616 pass / 1 fail against the 2617 baseline, so differ({a: undefined}, {b: undefined}) still kills it after the merge. Mutation verified applied (grep for bm.has came back empty) and the file restored to a clean tree afterwards.

Not re-verified

I did not re-derive the clause-for-clause match against the structurally-same.md proposal, the assertDeepEqual removal, or the remove-native-json.md link retarget — those were settled at 50734558 and the files carrying them have not changed by a byte since.