types/sorted_list: name the two tail policies keepTail and dropTail - #1546

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb2c2adbf0

Approving — clean. Baseline is origin/main at 6684bef2.

Is this a public surface change?

No. mergeTail → keepTail and the module-level tailReduce → dropTail are both
module-private consts, and the shadowing local inside merge was private by construction. I
checked rather than assumed, with npm run prepack run in both trees and the dual-axis
extractors over the emitted declarations:

• bin/extract.mjs (exported type aliases) — byte-identical to origin/main.
• bin/consts.mjs (exported const signatures) — byte-identical.

So §8.4 does not apply: no **BREAKING CHANGES:** prefix, and the CHANGELOG's "internal only"
is literally true at the emitted-declaration level. This sits with #1524 (representation-only,
correctly unprefixed), not #1520.

TailReduce itself is not orphaned by dropping it from this module's @import — it is still
declared in sorted_list/types.ts, still part of _MergeReduce, and still @imported and used
by types/range_map/module.f.mjs:73.

Behaviour: is the split policy-preserving?

merge was one function with a local annotated alias resolving to mergeTail; intersect used
a differently-named module-level const. They are now two named consts passed explicitly. I swept
both against origin/main over 60,000 random sorted-list pairs (values 0–11, lengths 0–6, so
empty inputs, disjoint pairs, identical pairs, prefixes and one-side-exhausted-first cases all
occur densely — the last is the only shape where the tail policy is observable at all):

PR head 7376d128:      cases 60000, merged elements 273841, intersected 33662, errors 0
                       sha256 7f48d689d3de824b8e5a6ececc826e62f753bc502544d34cda7ddde7e043681e
origin/main 6684bef2:  cases 60000, merged elements 273841, intersected 33662, errors 0
                       sha256 7f48d689d3de824b8e5a6ececc826e62f753bc502544d34cda7ddde7e043681e

Identical hash over [a, b, merge(a,b), intersect(a,b)] for every case, and every case also
checked against an independent oracle (sorted set union / filter-by-membership) — 0 mismatches
on both sides.

Negative control: swapping the two policies at the call sites
(merge gets dropTail, intersect gets keepTail) breaks on the 2nd case with both a
MERGE and an INTERSECT mismatch and a different hash. The sweep distinguishes the two policies,
so the pass above is meaningful.

§3.2 — is the coverage real, or just case names?

Mutation-tested rather than read: with the same policy swap applied, the repository's own
suite fails 13 of 2646 (pass: 2633, fail: 13). The existing proofs genuinely discriminate
the two tail policies, so the rename does not need a new co-located proof — nothing new is
public, and what changed is already killed by existing tests.

The comment's inference claim

The new block comment asserts that annotating keepTail with
() => <T>(tail: List<T>) => List<T> introduces a second inference candidate, widens T to
unknown, and breaks both call sites. I applied exactly that annotation and re-ran tsc:

module.f.mjs(56,12): TS2322: Type '(a: List<unknown>) => (b: List<unknown>) => List<unknown>'
  is not assignable to type '(a: SortedList<T>) => (b: SortedList<T>) => SortedList<T>'
    … 'unknown' is not assignable to type 'T'
module.f.mjs(56,27): TS2322: Type '_CmpReduceOp<T>' is not assignable to
  type 'ReduceOp<unknown, null>' …

The claim reproduces, including the mechanism (unknown as the widened T). Documenting a
non-obvious "why there is no annotation here" is worth the lines.

It is also consistent with AGENTS.md "Prefer inference", which happens to use
const f: TailReduce<unknown, unknown> = () => () => null as its example of the annotation not
to write — this change removes precisely that shape — and with §6.3, which the deleted
tail-reduce-shadowing.md cited for hoisting the non-capturing local.

Battery

• npx tsc --noEmit — exit 0. npm run prepack from a cleaned tree — exit 0.
• npm test — pass: 2646, fail: 0 on the PR head; pass: 2646, fail: 0 on origin/main.
  Exact match.
• bin/linkcheck.mjs — broken-link sets identical to main (diff empty). Deleting
  fjs/types/sorted_list/todo/tail-reduce-shadowing.md strands nothing: git grep tail-reduce-shadowing returns no hits at the head, and the two prose references to
  mergeTail (uncurry-accumulator-types.md, todo/migrate-typescript-to-mjs.md) are both
  updated in the same PR rather than left stale.
• Rust gates skipped: nanvm-lib/ untouched.
• §8.3: one entry, code change, PR link only, released sections untouched, and correctly no
  separate entry for the todo/ deletion or the two doc touch-ups.
• §6.2: no new or renamed exported types, so the _-prefix question does not arise here.

Nothing to change.

Re-reviewed at 053c7ed3. The delta since my approval at 7376d128 is merge-only.

git log 7376d128..053c7ed3 is one merge commit plus main's #1544. Comparing each head against its own merge-base with main, the PR's own diff is byte-identical apart from two lines of CHANGELOG context (the neighbouring entry is now #1544's instead of #1543's) — fjs/types/sorted_list/module.f.mjs, the two todo files and the entry itself are unchanged. So there is no new implementation to re-derive against, and I have not carried the previous sweep forward as evidence for changed code; there is no changed code.

Re-run on the merged tree, baseline origin/main = e2199caf:

• npx tsc --noEmit — exit 0.
• npm run prepack from a cleaned tree — exit 0 (both passes).
• npm test — 2646 pass / 0 fail, exactly the baseline count.
• Broken-link set from linkcheck — identical to main, not just equal in count.
• Public surface — the diff adds no export type or export const; keepTail and dropTail remain module-private, so §6.2's _ rule is not engaged.
• CHANGELOG — one ## Unreleased entry, links only /pull/1546, correctly marked "internal only" and correctly without a **BREAKING CHANGES:** prefix.

Still clean. Approving.

Re-reviewed and still approving. The head moved twice while I was working, so to be
explicit about what I actually tested: I verified 2dac8206, then #1548 merged to
main as 2a1ee02f and this branch merged it as 71b5f40d, so I reset my baseline to
the new origin/main (2a1ee02f) and re-ran the whole battery. Everything below is
measured at 71b5f40d against origin/main = 2a1ee02f.

What moved since 053c7ed3: two merges of main, and one real one-line commit.
#1545's @import sweep hoisted the tags into the module header, and the merge-queue
resolution kept TailReduce there even though this branch had removed its last use
when the shadowing local in merge went away. 2dac8206 drops it. I checked the
claim rather than taking it from the message: TailReduce no longer appears anywhere
in sorted_list/module.f.mjs, and it is still exported from sorted_list/types.ts
and still imported by range_map/module.f.mjs (line 38, used at line 70), so nothing
was orphaned. fjs/types/sorted_list/ is byte-identical between 2dac8206 and
71b5f40d — the last merge is pure #1548.

Because sorted_list/module.f.mjs moved, I re-ran both load-bearing experiments at
this head instead of carrying them forward.

60,000-pair behaviour sweep — randomized sorted-list pairs, merge and
intersect against an independent set-union / filter-based-intersection oracle:

{"cases":60000,"mergedTotal":273841,"interTotal":33662,"errs":0,
 "hash":"7f48d689d3de824b8e5a6ececc826e62f753bc502544d34cda7ddde7e043681e"}

Zero mismatches, and the digest is identical to the one from 2dac8206, as it should
be for byte-identical source. Swap negative control, verified to land on code (I
re-grepped both call sites after the edit; it is a three-way rename, so it cannot
silently no-op): exchanging keepTail and dropTail at the two genericMerge call
sites breaks on the first sampled case — merge([5,9,11],[3,9]) gives [3,5,9]
instead of [3,5,9,11], intersect gives [9,11] instead of [9]. The sweep is
therefore capable of failing, and the policy split really is behaviour-preserving.

§3.2 is still satisfied without new proofs: the same verified swap fails
13 of 2652 existing tests, so both policies are already exercised along both paths.

Rest of the battery at 71b5f40dcaab86baa2af9267f4dd2131de3e023f:

• npx tsc --noEmit — exit 0, including with the now-unused @import gone.
• npm run prepack from a cleaned tree — exit 0.
• npm test — 2652 pass / 0 fail, exactly origin/main 2a1ee02f's count.
• Public surface, dual-axis with both trees prepacked — extract.mjs and
  consts.mjs output are byte-identical to main's on both axes. keepTail and
  dropTail are module-private consts, so the rename is invisible outside the file;
  nothing widened to any; no new unprefixed export type.
• @module survives declaration emit in sorted_list/module.f.d.mts after the
  @import hoist.
• Link check — broken-link set identical to main's; deleting the completed
  todo/tail-reduce-shadowing.md stranded no references.
• CHANGELOG — one Unreleased entry for this PR, links only /pull/1546, correctly
  marked internal-only; no **BREAKING CHANGES:** needed given the identical surface.

The comment explaining why both policies are left to inference — an explicit
() => <T>(tail: List<T>) => List<T> introduces a second inference candidate that
widens T to unknown — matches what the compiler actually does.