fix(portal): PR #54 review round 2 — factory-alias hole, case-normalize, INV-17 candor

Fix round 2 on the INV-17 write-side idempotence guard (#53), per the gated review:

1. BLOCKER (factory-alias hole): the sync-store keys factory_addresses by STORE
   IDENTITY (factory minus id/sourceId; sync-store/index.ts insertChildAddresses
   ~:459 and getChildAddresses ~:502 both strip { id, sourceId } and upsert/select
   the factories row on the remaining value under UNIQUE (factory)). Two config
   sources aliasing the same on-chain factory (identical minus id/sourceId) each
   read empty then both insert -> duplicate child rows on the FIRST flush, breaking
   the PR's own idempotence claim. The guard now canonicalizes the flush by store
   identity (storeFactoryKey): ONE read + ONE dedupe + ONE insert per canonical
   group. The mock store is RE-KEYED the same way the real store keys (strip
   id/sourceId) — its prior factory.id keying is exactly why the hole was invisible.
   New regression test 'alias-hole' (two aliased sources -> exactly one store row).

2. Case normalization: getChildAddresses returns stored text verbatim and
   min-merges case-sensitively; children are lowercased at discovery. The persisted
   lookup is now lowercase-normalized so a checksummed pre-existing row is still
   deduped. New regression test 'case-normalization' (checksummed row -> no re-insert).

3. INV-17 doc (perf, candor): the guard is a live uncached getChildAddresses per
   canonical pending factory per flush-bearing interval (worst ~O(N^2) over an
   accreting backfill) — documented as a known cost with a per-process cache as the
   follow-up candidate. Not implemented here (correctness first).

4. INV-17 wording: (a) the ×2-children observation is re-attributed to the chaos
   DRIVER's two concurrent live instances (a fixed harness defect), NOT SIGKILL/
   resume itself — consistent with the SCOPE section. (b) The single-writer claim is
   upgraded from "sequential" to TRANSACTIONAL: read->dedupe->insert->insertIntervals
   run in ONE tx-scoped syncStore transaction per interval (runtime/historical.ts
   :1310-1352); residual risk is genuinely only the two-transactions/two-processes case.

Both gates green (0.16.6 and 0.15.17: 210/210, exit 0). Both mutations bite named
tests: revert canonicalization to factory.id -> alias-hole fails (expected 2 to be 1);
drop the lowercase-normalize -> case-normalization fails (length +0 but got 1). Biome
error-clean on the changed portal files.

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

Author: dzhelezov

portal/INVARIANTS.md

@@ -32,7 +32,7 @@ Legacy `C#` tags from the pre-refactor comments are mapped where they existed.
 | **INV-14** | **Config validity.** All `PORTAL_*` env is parsed ONCE into a frozen, validated `PortalConfig`; garbage values fail fast with actionable messages (previously `Number("abc") → NaN` silently poisoned the chunk grid). | A silent NaN in the chunk grid corrupts every downstream computation. | `loadPortalConfig` validates + `Object.freeze`s once. | `PortalConfigError` at parse. | `portal-config.test.ts` — "INV-14: garbage numeric → loud PortalConfigError (not silent NaN)" + the defaults/bounds/checks matrix. |
 | **INV-16** (C10) | **Backfill-floor genesis semantics.** `compileFetchSpec`'s `backfillStart` — the LOW-side clamp `chunkRange` (portal-chunks.ts) applies to every chunk fetch — is `0` whenever ANY source omits `fromBlock` (⇒ genesis), and only the `Math.min` of the DEFINED `fromBlock`s when they are ALL defined. Symmetric with `backfillEnd`, where any undefined `toBlock` ⇒ unbounded. | A source with no `fromBlock` starts at genesis (ponder's runtime reads `filter.fromBlock ?? 0`). Taking `Math.min` over only the DEFINED `fromBlock`s let a bounded source (e.g. 15M) raise the floor past an unbounded source's start; `chunkRange` then clamped every fetch to `≥ 15M` and the unbounded source's `[0, 15M)` intervals assembled EMPTY and were marked synced — a permanent silent gap on the PRODUCTION historical path. | `compileFetchSpec` sets `backfillStart = froms.every(defined) ? Math.min(froms) : 0` (mirrors `backfillEnd`); `portal.ts` closes over the frozen spec and `chunkRange(idx, chunkBlocks, backfillStart, end)` uses it as the low bound. | — (pure floor; the empty-assembly it prevents has no safe runtime signal, so it is proven by test). | `portal-filters.test.ts` — "INV-16/C10: an undefined fromBlock anchors backfillStart at genesis (0)" (undefined ⇒ 0, all-defined ⇒ min, all-undefined ⇒ 0); `portal.test.ts` — "regression: an undefined fromBlock is genesis — the [0, min) prefix of an unbounded source is NOT silently skipped (C10, ports #8 by @mo4islona)". Ported from PR #8 (@mo4islona). |
 | **INV-15** | **Factory-children persistence.** A factory interval marked cached ⇒ its children are persisted in the sync store: newly-discovered children are queued and flushed via `syncStore.insertChildAddresses` inside the SAME `syncBlockRangeData` whose transaction ponder commits with `insertIntervals`; the flush is scoped to children whose creation block ∈ the interval (cross-interval children stay queued for their owning interval); a failed flush restores the queue and fails the interval loud; restart-preloaded children re-discover at `prev ≤ bn` and are NEVER re-queued (no double-insert). Ported from PR #2 into this architecture. | Ponder's core marks `requiredFactoryIntervals` cached after every interval and on startup loads children ONLY from the store — an unpersisted child means every restart of a factory app silently drops that child's events. | `portal-discovery.ts` queues inside the min-merge guard (`prev === undefined \|\| prev > bn`) and exposes `takePendingInRange`/`restorePending`; `portal.ts` flushes after the interval's chunks resolve, before read-ahead/assemble/insert. The discovery floor is pinned from the spec at construction (`pinDiscoveryFloor`, FIX 2), so even a spanning-chunk fetch issued BEFORE `requiredFactoryIntervals` runs discovery — and thus queues + flushes — rather than fetching a factory chunk with an unset floor (which no-op'd discovery, missing the children entirely). | A flush failure rethrows (ponder rolls the interval back). | `portal.test.ts` — "regression: discovered factory children are persisted via insertChildAddresses in the SAME syncBlockRangeData call", "regression: a failing insertChildAddresses fails LOUD and the children re-flush on the next call…", "restart: a new sync seeded ONLY with the persisted children still fetches child logs…", "regression: each syncBlockRangeData persists ONLY its interval's children…", "regression: a post-flush insertLogs failure fails the interval LOUD…", "regression: a child already known from the store is NOT re-flushed when discovery re-runs live", "regression (FIX 2, BUG-A): an EARLY spanning-chunk fetch with requiredFactoryIntervals=[] still runs discovery — the child log is fetched AND its address flushed…", "regression (#21, INV-15 gate): a factory creation log BELOW factory.fromBlock is neither recorded in childAddresses nor queued/flushed (isLogFactoryMatched floor gate)". The `#21` gate test pins the SILENT dependence on `isLogFactoryMatched`'s `fromBlock` floor: a sub-`fromBlock` creation log delivered to `scanWindow` must be discarded by the matcher — else it would enter `pendingChildren` below the walk floor and the min-merge guard would suppress re-queueing at its in-range block (a restart loss). Mutation-verified: bypassing the matcher (`scanWindow` records the sub-floor child) fails the test (`expected true to be false` at the `.has(BELOW_FLOOR_CHILD)` assert). Test-only per #21 §1; #21 §2's FIX-2 floor-refinement overscan is intentionally left OPEN. |
-| **INV-17** | **Factory-children write-side idempotence.** Re-flushing an already-persisted child set does NOT durably duplicate rows: before every `insertChildAddresses`, the flush is deduped against the store's persisted rows (`getChildAddresses`) and a child is inserted only if it is not yet persisted OR is re-discovered at a STRICTLY LOWER creation block — the write-side analogue of the read-side min-merge (LEAST semantics). So a resumed or re-running writer that re-queues the same children from a fresh in-memory discovery queue inserts nothing; a lower re-discovery updates the effective min; equal/higher re-flushes are no-ops. Addresses #53 (the fork-side hardening it proposes). | Upstream's `insertChildAddresses` is a plain `INSERT` with no `ON CONFLICT`, and `ponder_sync.factory_addresses` has no `UNIQUE (factory_id, chain_id, address)` (the only sync-store table with no uniqueness story) — so a second writer on the same store durably duplicates the whole child set (a SIGKILL/resume chaos run ended with every child ×2). App behavior is unaffected (the only consumer, `getChildAddresses`, min-merges to a set), but store-identity/digest tooling breaks and the table grows unbounded under repeated violations. The in-memory min-merge guard (INV-15) suppresses re-queueing WITHIN one process; a second live process has a fresh queue and needs a store-side guard. **SCOPE (candor):** the portal-layer read-then-write dedupe fully closes the resumed/sequential single-writer case (the realistic production shape — production holds a single-writer app lock). It does NOT close a true two-concurrent-writers TOCTOU race (both read absence, both `INSERT`); only a DB `UNIQUE` + `ON CONFLICT DO UPDATE … LEAST` would, and the fork cannot add that cleanly — `factory_addresses` is created by a hash-keyed kysely migration (`2025_02_26_0_factories`), so a real constraint needs a NEW duplicate-collapsing migration in upstream `migrations.ts` for both pinned versions, materially larger blast radius. The #53 concurrent writer was a fixed harness-driver defect; the residual concurrent-writer gap is documented and left to a future DB-constraint change. | `portal.ts` reads `syncStore.getChildAddresses({ factory })` per pending factory before the flush and keeps a child only when `persisted.get(address) === undefined \|\| persisted.get(address) > block`; the store method is upstream, so the guard lives at the fork's own call site. | — (idempotence property; the duplicate it prevents has no safe runtime signal on the app path, so it is proven by test). | `portal.test.ts` — "regression (#53, write-side idempotence): a SECOND writer re-flushing an already-persisted child set does NOT re-insert it — the store keeps exactly one row at the min block" (mutation-verified: reverting the dedupe to a plain flush fails it — `expected […] to have a length of 1 but got 2`), "regression (#53, write-side idempotence): a re-discovery at a STRICTLY LOWER creation block is re-inserted so read-side LEAST resolves to the min (equal/higher blocks stay deduped)" (mutation-verified: a blanket `DO NOTHING` predicate `prev !== undefined` fails it — `expected [] to have a length of 1 but got +0`). |
+| **INV-17** | **Factory-children write-side idempotence.** Re-flushing an already-persisted child set does NOT durably duplicate rows: before every `insertChildAddresses`, the flush is deduped against the store's persisted rows (`getChildAddresses`) and a child is inserted only if it is not yet persisted OR is re-discovered at a STRICTLY LOWER creation block — the write-side analogue of the read-side min-merge (LEAST semantics). So a resumed or re-running writer that re-queues the same children from a fresh in-memory discovery queue inserts nothing; a lower re-discovery updates the effective min; equal/higher re-flushes are no-ops. The flush is FIRST grouped by STORE IDENTITY (factory minus `id`/`sourceId`, matching how the sync-store keys `factory_addresses`) so two config sources aliasing the same on-chain factory (identical factory minus `id`/`sourceId`) collapse to ONE read + ONE dedupe + ONE insert — otherwise both would read absence and each insert the same children (a first-flush duplicate the read-side min-merge cannot repair). The persisted lookup is LOWERCASE-normalized (children are lowercased at discovery) so a checksummed pre-existing row is still deduped. Addresses #53 (the fork-side hardening it proposes). **Known cost (candor):** the guard is a live UNCACHED `getChildAddresses` (SELECT of all rows) per canonical pending factory per flush-bearing interval — over a long accreting backfill this is worst-case ~O(N²) in the child count. It is left uncached in this PR (correctness first); a per-process read-through cache of the persisted min-map is the documented follow-up candidate. | Upstream's `insertChildAddresses` is a plain `INSERT` with no `ON CONFLICT`, and `ponder_sync.factory_addresses` has no `UNIQUE (factory_id, chain_id, address)` (the only sync-store table with no uniqueness story) — so a second writer on the same store durably duplicates the whole child set (the ×2-children observation came from the chaos DRIVER running two concurrent live instances — a fixed harness-driver defect — NOT from SIGKILL/resume itself, which is single-writer; see SCOPE below). App behavior is unaffected (the only consumer, `getChildAddresses`, min-merges to a set), but store-identity/digest tooling breaks and the table grows unbounded under repeated violations. The in-memory min-merge guard (INV-15) suppresses re-queueing WITHIN one process; a second live process has a fresh queue and needs a store-side guard. **SCOPE (candor):** the read→dedupe→insert→`insertIntervals` sequence all runs inside ONE store transaction per interval (the `syncStore` is tx-scoped — created from the tx handle in `runtime/historical.ts:1310-1352`), so for a single writer the guard is TRANSACTIONAL, not merely sequential: no interleaving read/insert can slip a duplicate past it, and a failed flush rolls the whole interval back. This fully closes the resumed single-writer case (the realistic production shape — production holds a single-writer app lock). The residual risk is genuinely ONLY the two-transactions / two-concurrent-processes case: two writers in two transactions can both read absence and both `INSERT`. That TOCTOU race the portal-layer dedupe does NOT close; only a DB `UNIQUE` + `ON CONFLICT DO UPDATE … LEAST` would, and the fork cannot add that cleanly — `factory_addresses` is created by a hash-keyed kysely migration (`2025_02_26_0_factories`), so a real constraint needs a NEW duplicate-collapsing migration in upstream `migrations.ts` for both pinned versions, materially larger blast radius. The #53 concurrent writer was a fixed harness-driver defect; the residual concurrent-writer gap is documented and left to a future DB-constraint change. | `portal.ts` reads `syncStore.getChildAddresses({ factory })` per pending factory before the flush and keeps a child only when `persisted.get(address) === undefined \|\| persisted.get(address) > block`; the store method is upstream, so the guard lives at the fork's own call site. | — (idempotence property; the duplicate it prevents has no safe runtime signal on the app path, so it is proven by test). | `portal.test.ts` — "regression (#53, write-side idempotence): a SECOND writer re-flushing an already-persisted child set does NOT re-insert it — the store keeps exactly one row at the min block" (mutation-verified: reverting the dedupe to a plain flush fails it — `expected […] to have a length of 1 but got 2`), "regression (#53, write-side idempotence): a re-discovery at a STRICTLY LOWER creation block is re-inserted so read-side LEAST resolves to the min (equal/higher blocks stay deduped)" (mutation-verified: a blanket `DO NOTHING` predicate `prev !== undefined` fails it — `expected [] to have a length of 1 but got +0`), "regression (#53, alias-hole): two sources whose factories differ ONLY in id/sourceId (store aliases) discovering the same child in one interval end with EXACTLY ONE store row — not two" (mutation-verified: reverting the canonicalization to key by `factory.id` fails it — the store ends with the child ×2), "regression (#53, case-normalization): a CHECKSUMMED pre-existing store row is deduped against the lowercased discovered child" (mutation-verified: dropping the lowercase-normalize of the persisted map fails it — the checksummed row no longer matches so the child is re-inserted). |
 
 ## The fixed gaps