feat(common): opt-in order-field fold in datawarehouse combine — 4.x line (PMT-4302)

[jgrantr]

What

The 4.x-line twin of #251: an opt-in, default-off ordering mode for the datawarehouse combine.js in-batch fold. A field flagged "combineOrder": true in the table's dw_fields schema makes the fold keep the row with the highest value of that field per natural key (e.g. source_eid) instead of the last row by arrival; the arrival counter remains as tie-breaker. Tables without the flag keep the arrival-order fold byte-for-byte (locked by test).

Why this branch

Audit of the general service (2026-08-31): its runtime is the awsv3 layer line (Node 22, leo-sdk 7.x), but webpack.config.js deliberately filters leo-connector-common/leo-connector-postgres out of the layer externals, so the offloader executes the npm-pinned, development-cut 4.0.13-rc/4.0.24-rc bundled at build time — the 5.x copies inside the layers are shadowed dead weight. The active 4.1.0-rc.<run_id> release stream publishes from this branch, so landing here is the shortest consumable path: next rc → bump general's two pins. No layer republish needed.

#251 carries the identical change for the v3 line (feature/aws-sdk-v3-again); general benefits from that copy only at the real 5.x cutover.

Stacked on #249

• Extends feat: combine collapse and delete-marker correctness, plus per-package release automation (RPL-6000, RPL-6780) #249's combineOpts plumbing (orderFields rides alongside emitSequence).
• Composes with combineRecords (RPL-5795): this change orders the rows the collapse sees; it never touches the collapse. Its delete/reactivate rules are order-relative and stay correct under source-EID order.
• Interplay note for RPL-6780 (__leo_seq__): on an ordered table the surviving row's sequence is the winning (highest-order-key) row's sequence, not the batch-latest arrival — which is the semantically right sequence for that row. Tables using emitSequence without combineOrder are unaffected.
• Test file is combine-order.test.js (no collision with this branch's combine.test.js); both DW suites pass together — 15 tests.

Mechanics (same as #251)

• Schema-driven opt-in: load.js scans structure for a combineOrder field flag (like nk/sk/scd) → combine() orderFields. The flag rides in the shared dw_fields entry, so every loader that reads the schema folds identically.
• Sort line for ordered tables: {md5(nk)}-{48-char order key}-{9-digit arrival}{json}; sort already runs under LC_ALL=C; a row with no order value (backfill) pads with spaces and sorts before every row that has one — asserted by test, per the Zero-Inventory design's testing plan.

Consumption plan

1. Merge into feat: combine collapse and delete-marker correctness, plus per-package release automation (RPL-6000, RPL-6780) #249's branch (or rebase onto development if feat: combine collapse and delete-marker correctness, plus per-package release automation (RPL-6000, RPL-6780) #249 lands first — the change applies cleanly either way).
2. Next 4.1.0-rc cut picks it up.
3. item-dw adds "combineOrder": true to source_eid in dw_fields/f_current_inventory_state.json (one line on item-dw#23).
4. general bumps leo-connector-common + leo-connector-postgres pins.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DhcouH67CgpNGpBZ29ujbk

[ch-snyk-sa]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[jgrantr]

Verified your truth table against this PR's head before touching anything — reproduced exactly, including the write→delete drop and the unordered control. Agree on all points, including both rejected repairs; the checkpoint-deadlock analysis on the refusal option matches how RStreams actually behaves.

Pushed 7f849d1 addressing everything actionable: the interaction is documented on the orderFields option, your warn-on-marker-for-ordered-table is in (once per table per batch), the truth table is pinned as characterization tests (18 passing across both DW suites), and aws-sdk is in devDependencies for the clean-install issue — good catch, my runs were masked by a pre-populated node_modules.

One addition to your latency analysis for f_current_inventory_state specifically: there's a third independent barrier — the group-scoped guarded importFact in Chub-Engineering/general#62 throws on any __leo_delete__, so a marker can't reach that table's merge even if (b) regressed.

Your composite-NK census deserves to outlive this thread — deleteMarkerData's composite branch being unreachable-until-armed is a real latent issue independent of combineOrder. Filed separately (link to follow in this thread).

🤖 Posted with Claude Code on Grant's behalf

[jgrantr]

Composite-NK issue filed: #255

[pmogren]

I read this against the delete-marker work that just landed (#252, #253). The base is clean — #254 branches off the tip that already includes both, preserves SEQUENCE_FIELD, emitSequence, the opts restore and deleteMarkerData, and merges combineOpts correctly. The sort-line formats do not collide either, because the arrival counter lives inside the JSON values object rather than in the prefix. I went looking for a sequencing problem and there isn't one.

This is not a request for changes. There is one semantic interaction between combineOrder and delete markers that I want to call out, because it is latent for reasons that are not obvious from either PR, and I would rather have it written down than rediscovered later.

A delete marker carries no value for the combineOrder field. encodeOrderKey(undefined) pads to 48 spaces, and under LC_ALL=C a space sorts below every character an eid contains, so the marker sorts first in its group and combineRecords takes the reactivation branch: the write clears the delete and the row stays open. No error, no metric. Measured against this PR's head, with combineOrder set on a varchar order field:

Case	Delete survives the fold?
write, then natural-key delete (delete genuinely last)	No — dropped
natural-key delete, then write (write genuinely last)	No — right answer, wrong reason
natural-key delete alone, no write for that key	Yes
write, then parent-FK delete (marker is its own group)	Yes

Control with combineOrder unset: the marker survives and the row ends deleted. #252's arrival counter does not rescue it either — a consumer's guard can only compare records that reach importFact, and here the delete is gone a layer earlier, in the fold.

Four conditions have to hold for that to bite, and in the configuration I checked, none of them does:

(a) A table sets combineOrder. Nothing does yet.
(b) Delete events reach the loader for that table. For the first intended consumer they do not, by design in two independent places: its producer returns before fan-out on deletion, so no delete is emitted at all, and its reconciliation sweep soft-deletes by direct UPDATE outside the merge path — precisely because a staging row with a null order value can never fire WHEN MATCHED.
(c) The table has a single-column natural key. A delete names one column, so on a composite key the marker can never carry every component, never lands in a write's combine group, and the reactivation branch cannot fire. The first intended consumer has a composite key.
(d) A write for the same key is in the same batch as the delete. An order-less delete with no competing write is alone in its group and applies correctly.

(b) and (c) fail independently there, so the composite key is not what is protecting that consumer — (b) already does it alone. (c) matters for whatever sets combineOrder next: a single-natural-key table that does receive queue deletes has one fewer barrier.

Worth flagging why (c) holds broadly today: in the configuration set I checked, every table has a single-column natural key, so a composite one would be the first. That also means deleteMarkerData's composite branch has never executed in production, and when it does become reachable it drops deletes outright — with no natural-key column populated, every id in one delete event hashes to the same combine group and the fold keeps one marker. Three ids in, one marker out on a synthetic composite table. Same shape as the bug #253 just fixed for single keys not named id, and it arms whenever a composite-key table receives a queue delete, which is not something this PR introduces.

Two repairs look obvious and are not. Sorting order-less markers last makes the delete win the batch, which settles delete-versus-write by a fixed structural rule instead of by event order — that is the defect rather than a fix for it. Sorting them first is not a bug to be corrected either: a data row with no order value sorting below every row that has one is load-bearing and separately tested, because it is how a backfill row loses the fold against a live one.

I did build the middle path and then threw it away: resolve the order value off the delete event (payload.data.<orderField>, the same name and shape the writes carry it under) and refuse the batch when an ordered table's delete supplies none. Two problems, either one disqualifying.

It deadlocks. A consuming bot that records the error without advancing its checkpoint re-reads the same event and fails the same way. The offending event is already on the queue and the event stream is append-only, so fixing the producer changes future events, not that one. The exits are an operator advancing the checkpoint past it — which discards the delete, the loss the refusal existed to prevent — or an emergency release of this library to downgrade the refusal. Until one of those lands, that consumer's whole load is stopped.

It also over-triggers, firing on every order-less delete including the majority that would have applied correctly, because the silent drop needs (d). The net trade is a narrow silent bug for a broad outage with no clean recovery.

What the interaction is worth instead is README guidance in this repo, plus the ordering guard as the default posture for new merge-type groups. If you want a runtime signal, a warn-level log when a marker gets built for an ordered table costs nothing and cannot stop a queue.

Separately and unrelated: combine-order.test.js does not run from a clean install, because leo-sdk pulls aws-sdk and nothing in this repo declares it — and there is no test CI here that would have surfaced that. Adding the devDependency works. So does the pattern already in that directory, where combine-records.js and delete-marker.js were split out of combine.js specifically so they could be tested without loading leo-sdk.