Skip to content

Feature: s3-sink - #7

Merged
philcunliffe merged 3 commits into
masterfrom
integration/s3-sink
May 22, 2026
Merged

Feature: s3-sink#7
philcunliffe merged 3 commits into
masterfrom
integration/s3-sink

Conversation

@philcunliffe

Copy link
Copy Markdown
Contributor

Auto-generated by /feature-launch to host the integration branch for feature s3-sink.

Work beads file individual sub-PRs into this branch via the refinery feature-flow loop. When all work + review + ship beads complete, the ship formula flips this PR to ready-for-review.

See /feature-flow for the flow architecture.

feature-launchand others added 2 commits May 21, 2026 12:50
Implements hy-s3-1: a bundled first-party plugin providing
hypaware.blob-store@1.0.0 and contributing the `s3` sink.
Plugin tree under hypaware-core/plugins-workspace/s3/:
- manifest declares network + read_env permissions and the s3 sink
- src/config.js: validates bucket/prefix/region/profile/storage_class/
server_side_encryption + endpoint_url/force_path_style; emits stable
s3_config_invalid errors
- src/keys.js: renders <prefix>/<dataset>/<segment>/<filename> matching
local-fs layout; normalizes prefix slashes; rejects keys outside the
configured prefix
- src/client.js: AWS SDK v3 wiring with injectable client factory and
a credential_source_kind detector that never leaks secret material
- src/errors.js: AWS SDK error -> stable error_kind mapping
(s3_credentials_missing, s3_access_denied, s3_bucket_missing,
s3_region_mismatch, s3_throttled, s3_put_failed, encoder_failed)
- src/index.js: sink composes ctx.query schema + ctx.storage rows + the
paired encoder; emits s3.client.init / s3.put_object /
s3.put_object.failed telemetry; closes terminal errors as
status=failed and surfaces transient failures as status=partial
Bundled wiring + dependencies:
- @hypaware/s3 added to V1_BUNDLED_PLUGIN_ALLOWLIST
- @aws-sdk/client-s3 + @aws-sdk/credential-provider-ini added to root
dependencies; npm pack --dry-run ships the new plugin tree
Tests (test/plugins/s3-*.test.js):
- s3-keys: key composition, prefix normalization, path-separator
stripping, within-prefix guard
- s3-config: required/optional fields, storage_class allowlist,
endpoint_url URL parsing, S3-compatible (MinIO) shape
- s3-client: credential_source_kind precedence + redaction contract
- s3-errors: full AWS SDK error -> stable kind mapping table
Hermetic smoke (s3_sink_export_fixture):
- Activates real format-parquet + s3 plugin trees with a fake injected
S3 client; fires one driver tick; asserts PutObject bucket/key/body,
decodes the body back into Parquet rows, and verifies the
s3.client.init / s3.put_object / sink.encode_partition telemetry
carries the expected attributes with no credential-shaped substring
Smoke fix (cli_bundled_plugins_activated):
- The allowlist now has 8 entries; the smoke selects 6, so
plugins_skipped is 2 and both format-jsonl and s3 emit
plugin.skipped logs
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@philcunliffe

Copy link
Copy Markdown
ContributorAuthor

Dual-agent review — request_changes

  • Verdict:request_changes
  • Risk class:medium
  • Blast-radius bead:hy-k1j1

Risk capstone

Cross-reference: reviewer findings that intersect high-risk surfaces

SourceFinding (severity, evidence)Intersects
codex.mdBehavioral Correctness, major/high — terminal-failure return in exportBatch omits retryPartitions; on a terminal S3 error mid-batch the driver outboxes every partition including already-uploaded ones. Evidence: hypaware-core/plugins-workspace/s3/src/index.js:223, hypaware-core/plugins-workspace/s3/src/index.js:229, src/core/sinks/driver.js:146, src/core/sinks/driver.js:150.Targets (s3/src/index.jsactivate / sink build), Direct callers (src/core/sinks/driver.js:146-148 — named explicitly as the retryPartitions consumer), Concurrency surface (exportBatch serial-per-partition loop is the bug locus)
claude.md(no findings — "No issues found" template)

Blast radius

  • Default-on capability ambiguity: zero-arg hyp / hyp init
    boots in all-bundled mode and will now select both
    @hypaware/local-fs and @hypaware/s3, each providing
    hypaware.blob-store@1.0.0. The good news:
    checkCapabilityAmbiguity (src/core/config/validate.js:430) only
    inspects plugins listed in config.plugins[], so all-bundled
    boots that never write a plugins[] array escape the check. The
    bad news: any user who writes a config that enables both will hit
    capability_ambiguous and need to add
    disambiguate.hypaware.blob-store = "@hypaware/local-fs" (or s3).
    The cli_bundled_plugins_activated smoke already mitigates by
    deliberately skipping @hypaware/s3. No new in-tree config example
    exercises both plugins — worth one fixture asserting the
    disambiguation error message reads cleanly.
  • AWS SDK bundle size: package-lock grows by 722 lines for the
    @aws-sdk/* + @smithy/* + @aws-crypto/* transitive set. The
    lazy import() in defaultClientFactory defers the load cost to
    first instantiation, but the bytes still ship in the npm pack
    tarball. Worth a npm pack --dry-run check before merging to
    master to confirm the published size is within whatever bound the
    V1 release sets.
  • First plugin to declare permissions: ["network", "read_env"]:
    no kernel enforcement reads this field today; if/when enforcement
    lands, the s3 plugin is the only one that would fail an
    enforcement-on default.
  • __clientFactory injection seam: the resolver reads it from
    the raw sinkCtx.config (not the validated config). The PR's
    documentation rightly notes "production configs never carry this
    key — it lives outside the validated config shape." That contract
    depends on no future config code path treating __clientFactory
    as a passthrough value. A schema-level reject of __* keys would
    harden this.
  • Credential redaction: the smoke asserts no AKIA[A-Z0-9]{12+}
    / aws_secret_access_key / aws_session_token substring appears
    in any captured log or span (s3_sink_export_fixture.js). The
    s3.client.init log surfaces credential_source_kind only; the
    client factory never receives credentials in the smoke. The
    contract is good as written; the regression risk is a future log
    line that includes process.env or an SDK error's raw
    err.message, both of which can carry credential-shaped substrings.
    describeS3ErrorKind is the only error surface today and is
    enum-driven, so the contract holds — but every new log row in this
    plugin needs to keep the redaction discipline.
  • Memory characteristic for large partitions: materializeBytes
    buffers the entire encoded blob before PUT. A huge dataset
    partition (e.g., the durable cache spool added in 1c47926)
    could push significant resident memory on a tick. The PR's smoke
    uses 50 rows; no large-partition test exists.
Codex review

Fix Validations

No explicit bug-fix claims were included in the PR context.

Findings

Behavioral Correctness

  • Severity: major
  • Confidence: high
  • Evidence:hypaware-core/plugins-workspace/s3/src/index.js:223, hypaware-core/plugins-workspace/s3/src/index.js:229, src/core/sinks/driver.js:146, src/core/sinks/driver.js:150
  • Why it matters: If a terminal S3 error happens after one or more partitions already uploaded, the sink returns status: 'failed' without retryPartitions, and the driver falls back to persisting every batch partition to the outbox, including successful uploads.
  • Suggested fix: Return retryPartitions: failures on the terminal-failure path, or add an explicit no-retry contract if terminal failures should not be outboxed.

No Finding

Contract & Interface Fidelity
Change Impact / Blast Radius
Concurrency, Ordering & State Safety
Error Handling & Resilience
Security Surface
Resource Lifecycle & Cleanup
Release Safety
Test Evidence Quality
Architectural Consistency
Debuggability & Operability

Evidence Bundle

  • Changed hot paths: S3 sink activation/create/export path; S3 key rendering; S3 config validation; bundled plugin discovery allowlist; sink export driver interaction.
  • Impacted callers:src/core/registry/sinks.js:199 calls destination create; src/core/sinks/driver.js:112 calls exportBatch; src/core/sinks/driver.js:146 consumes retryPartitions.
  • Impacted tests:test/plugins/s3-config.test.js:8; test/plugins/s3-client.test.js:8; test/plugins/s3-errors.test.js:8; test/plugins/s3-keys.test.js:8; hypaware-core/smoke/flows/s3_sink_export_fixture.js:49.
  • Unresolved uncertainty: Tests were not run; review is based on the supplied diff plus targeted contract tracing.
Claude review

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

🤖 Generated with Claude Code


Reports: /Users/phil/testcity/.gc/pr-pipeline/reviews/pr-7 · Bead: hy-as5t · Blast-radius: hy-k1j1

Address the major review finding on PR #7: the s3 sink's `exportBatch`
terminal-failure return (`s3_credentials_missing`, `s3_access_denied`,
`s3_bucket_missing`, `s3_region_mismatch`, `s3_config_invalid`) omitted
`retryPartitions`. The sink driver at `src/core/sinks/driver.js:146`
falls back to the entire batch in that case, so a terminal error
landing on partition N caused the driver to outbox every partition in
the batch — including the N-1 partitions that had already been PUT to
S3. Subsequent retries would then double-export those objects.
Return `retryPartitions: failures` so the driver outboxes only the
partitions that actually failed. `failures` always has at least one
entry when `lastConfigFailure` is set because both are pushed in the
same catch block.
Adds `test/plugins/s3-export-batch.test.js` covering the terminal,
partial, and all-success exit paths, exercised through the real
`activate` registration path with an injected `__clientFactory`.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@philcunliffe
philcunliffe marked this pull request as ready for review May 22, 2026 00:46
@philcunliffe
philcunliffe merged commit 746061c into masterMay 22, 2026
6 checks passed
@platypii
platypii deleted the integration/s3-sink branch May 28, 2026 06:37
philcunliffe added a commit that referenced this pull request Jun 26, 2026
… sinks (#159 review)
Dual-review found a BLOCKER plus majors against the exactly-once claim. Fixes,
each with a regression test that fails before and passes after:
[BLOCKER] Forward sink advanced the watermark per acked chunk to the scan's
running-max `after`. Because the cache scan is NOT seq-ordered (interleaved
live+backfill spool; post-compaction sortOrder re-sort), an early acked chunk
could checkpoint past lower-seq rows still un-acked in a later chunk; a
between-chunk failure then dropped them forever (`seq <= since`). Fix: advance
the forward watermark ONCE at end-of-partition (as the blob sink already does),
so a partial partition never checkpoints — a failure re-reads the whole
partition and the server ledger dedupes the acked prefix (stable
chunkStartSeq batch ids).
test: central-forward-chunking "an unordered scan never skips a lower-seq row
when a later chunk fails".
[MAJOR] Legacy null-seq rows re-exported every tick (never reached steady
state). Add `includeLegacy` to the storage read API (default true). Both sinks
pass `includeLegacy = (no durable watermark)`: a fresh sink exports the
pre-upgrade backlog once, then excludes null-seq rows. Safe because no new
null-seq row can appear post-upgrade (decorateRow stamps every flushed row).
tests: sink-incremental-acceptance pure-legacy + mixed, forward + blob
(~0 bytes on 2nd tick, no row in two artifacts).
[MAJOR] Central forward sink scoped watermarks per-PLUGIN, not per-instance —
two @hypaware/central instances clobbered one watermark file. Switch to
createInstanceWatermarkStore({ paths, instanceName }) (matching local-fs/s3);
correct the @ref.
test: sink-incremental-acceptance "two instances keep independent
watermarks (no cross-instance skip)".
[MAJOR, doc] Bounded-reads constraint (C): the read is a full scan over the
surviving partition with a yielded-row filter, not the file/row-group skip the
design implied. Correct LLP 0040 §1(C)/§5 to state reads are bounded by
surviving-partition size, with O(N_new) reads pending null-aware icebird
pushdown.
[MINOR/NIT, doc+style] Correct LLP 0040 §4 (batchIdForChunk keys by
chunkStartSeq, not chunkIndex). Hoist inline import('...') types to @import in
sink-watermarks/acceptance tests and the compaction smoke.
[MAJOR, ESCALATED — left unfixed] Mid-retry duplication when a unit commits,
its watermark write is lost, AND new rows append before the retry: the resumed
in-flight unit grows past what committed and the dedup net no longer recognizes
it. The common in-flight retry (no new arrivals) IS covered. A correct fix needs
a pre-commit intent (read `until` upper bound + persisted intent record) across
both sinks — a design-level change to the watermark contract, deferred rather
than half-patched under the exactly-once claim. Documented as LLP 0040 §6 risk
#7 / §5 known gap.
LLP 0040/0042 updated in this commit to match the new behavior.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
philcunliffe added a commit that referenced this pull request Jun 30, 2026
…ob sinks (LLP 0039/0040) (#159)
* Design: incremental sink reads (LLP 0040)
Cover LLP 0039 with a neutral-minted design for a per-(sink, partition)
watermark so the central forward sink and the core blob sink read and ship
only rows added since their last successful export.
Recommends a monotonic per-row _hyp_ingest_seq column over snapshot ancestry
(does not survive a compaction generation swap) and a content-addressed
seen-set (cannot meet the bounded-read goal). Specifies the readRows since/
continuation extension, the persisted watermark contract keyed by the
generation-stable logical partition path, application to both sinks, and the
exactly-once argument across retention prunes and compaction swaps.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Plan: incremental sink reads (LLP 0042)
Refine LLP 0040 into six small, independently-mergeable tasks along the
producer -> read-API -> persistence -> consumer seam:
T1 stamp _hyp_ingest_seq at the decorateRow chokepoint (deps: [])
T2 readRows since/continuation + readRowsSince (deps: T1)
T3 per-(sink,partition) watermark store keyed by logical path (deps: T2)
T4 wire the central forward sink (deps: T2,T3)
T5 wire the core blob sink (deps: T2,T3)
T6 exactly-once tests across retention prune + compaction swap (deps: T4,T5)
Verified with `neutral ready incremental-sink-reads --json`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* T1: stamp internal monotonic _hyp_ingest_seq at the decorateRow flush chokepoint
Adds the row-resident, append-monotonic int64 watermark column that the
incremental-sink-reads design (LLP 0040, Candidate B) is built on. This is
the producer half of the seam: nothing reads the column yet, and it is
stripped by INTERNAL_FIELDS from every existing readRows consumer, so it
merges with zero behavioural change.
- New `createIngestSeqAllocator` (src/core/cache/ingest-seq.js): a crash-safe,
never-regressing monotonic int64 allocator. Reserve-before-stamp — a block
of seqs is durably persisted (nextSeq advanced via atomic write-rename)
before any seq in it is handed to a row, so a resumed flush never re-issues
a seq <= one already stamped/exported. Gaps are tolerated; regressions are
not. The counter is cache-global (<cacheRoot>/_hyp_ingest_seq.json), not a
per-partition cursor.json, because decorateRow runs before rows are grouped
into source= partitions and two spool paths (live + backfill) can feed one
partition — only a cache-wide counter keeps every partition's seq subsequence
strictly increasing. (LLP 0040 §7 records this refinement of risk #2.)
- streaming-reader.js: decorateRow stamps `_hyp_ingest_seq` (the cache_row_id
hash is still computed over the original row, so seq does not perturb dedup);
the chunk's columns gain the additive nullable INT64 column so it lands in
the Iceberg schema and rides a compaction generation swap verbatim; the field
joins INTERNAL_FIELDS.
- spool.js wires one cache-global allocator into the flush loop.
- Tests: allocator monotonicity / never-regress-across-restart / reserve-
before-stamp / concurrency; streamFlushFile stamping; and a storage round-trip
proving the seq persists in Iceberg, increases per row, and is stripped from
readRows. Verified separately that the column survives a real compaction swap.
Task-Id: T1
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* T2: extend storage read contract with cursor-aware incremental reads
Add a back-compatible `opts.since` to `readRows` and a cursor-aware
`readRowsSince` sibling that pairs each internal-stripped row with its
`after` continuation token, so the forward and blob sinks can read only
rows added since their last durable export.
- `scanRowsFromTable` gains an `opts.since` (bigint `_hyp_ingest_seq`
watermark) and applies a `seq > since` predicate as a yielded-row
filter. It is NOT pushed into icebird's `scan({ where })`: icebird
couples file/row-group pruning with a per-row match that drops nulls
(`null > since` is false), which would silently skip the legacy
null-seq rows the migration must preserve (LLP 0040 risk #1). The
design names this yielded-row filter as the fallback; a future
null-aware icebird filter can add the file-skip optimization on top.
- null-seq = new: a row whose `_hyp_ingest_seq` is null/absent
(pre-upgrade) is always yielded, so the one-time migration is at worst
a full re-export, never silent data loss. A table that never carried
the seq column yields everything.
- `after` is a monotonic high-water of real seqs, so a null-seq row
carries the prior watermark forward and progress never regresses even
when the scan visits seqs out of order (interleaved sources).
- `opts` absent ⇒ byte-for-byte the pre-existing full scan, so every
current caller is untouched until it opts in.
- Update the kernel-types decl: `SinkContinuation`, `ReadRowsOptions`,
the extended `readRows`, and the new `readRowsSince`.
Tests cover back-compat, after-token monotonicity, no-new-rows ≈0,
incremental new rows, the null-seq migration contract, the pure-legacy
(no seq column) table, and invalid-token rejection.
Task-Id: T2
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* T3: persisted per-(sink,partition) watermark store keyed by logical path
Add `src/core/sinks/watermarks.js`, the shared incremental-read watermark
store for the forward and blob sinks (LLP 0042 task T3). Files live at
`<stateDir>/watermarks/<dataset>/<partition-key>.json` and carry the
versioned `{ continuation, exportedRowCount, updatedAt }` record.
The key is derived from the partition's stable LOGICAL path
(`datasets/<dataset>/<partition...>` relative to cacheRoot), never the
physical `tableDir` inside it — the hinge of design constraint (B): the
watermark reads straight through a compaction generation swap and a
retention front-prune. `write` is atomic write-rename (the
`writeCursor`/`writeProgress`/`ingest-seq.js` idiom); a corrupt or absent
record reads back as null so a sink re-exports from the start
(at-least-once + downstream dedup) rather than silently skipping rows.
Adds `SinkWatermarkKey`/`SinkWatermarkRecord`/`SinkWatermarkStore` types,
anchors LLP 0040 §3 (`#watermark-contract`), and a unit suite covering key
derivation (logical-not-tableDir, nesting, sanitize, sentinel, escape
guard), round-trip, in-place advance, atomic-no-temp, malformed-token
rejection, and corrupt-file null.
@ref LLP 0040#watermark-contract
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task-Id: T3
* Wire central forward sink to incremental readRowsSince watermark
Switch `forwardPartition` from a full-partition `storage.readRows(tablePath)`
scan to the cursor-aware `storage.readRowsSince(tablePath, { since })`, driven
by a per-(sink instance, partition) watermark loaded from the sink's stateDir
watermark store (T3). Each acked chunk advances the watermark to that chunk's
last `after` token (ship first, advance second), so a crash re-sends at most one
chunk and the server idempotency ledger now backstops only a bounded in-flight
suffix instead of the whole partition. A tick with no new rows yields zero rows,
sends zero chunks, and writes zero bytes.
Chunking (MAX_CHUNK_ROWS/MAX_CHUNK_BYTES), the Retry-After backpressure loop,
and `batchIdForChunk` derivation are unchanged. A missing/unreadable watermark
or underivable key falls back to a full scan (at-least-once + server dedup),
never a silent skip.
Implements task T4 of incremental-sink-reads (LLP 0040 §3-4, 0042).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task-Id: T4
* T5: wire core blob sink (local-fs + s3) to incremental readRowsSince
Wire the local-fs and s3 blob destinations to the cursor-aware
readRowsSince surface (T2) and the persisted per-(sink instance,
partition) watermark store (T3), so each tick exports only rows added
since the sink's last durable PUT.
- New shared helpers in src/core/sinks/incremental.js (exported via
hypaware/core/sinks): openIncrementalRows (peek-to-decide-empty,
self-tracking rowCount + high-water lastAfter, feeds the unchanged
encoder.encodePartition contract), withSeqRangeFilename (embeds
[sinceSeq,lastSeq] before the extension), watermarkKeyFor, and
createInstanceWatermarkStore.
- Empty new-row set writes no blob (skip, 0 bytes).
- The output filename/object key embeds the [sinceSeq,lastSeq] range so a
crash-retry re-PUTs the same key (idempotent overwrite) — the blob
sink's stand-in for the central server ledger.
- The watermark advances only after the durable write/PUT.
- PluginPaths.stateDir is per-plugin, not per sink instance, so the
watermark store is scoped under the instance to honor the per-(sink
instance, partition) contract.
- Tests: helper unit tests, a local-fs end-to-end incremental test
(ranged filename, watermark advance, skip-empty, new range, cumulative
count), and rewritten s3-export-batch tests (skip-empty, ranged key,
watermark advance, idempotent re-PUT on lost watermark) preserving the
prior retry-semantics coverage.
- Updated the two local-fs blob-sink smokes to match the ranged filename
(note: both are pre-existing red on the integration branch for an
unrelated reason — the driver hands the sink the drained spool path).
Task-Id: T5
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* T6: exactly-once acceptance suite + smoke for incremental sink reads
Add the LLP 0040 exactly-once proof the T4/T5 unit suites can't give
(they stub storage): a deterministic acceptance test driving the REAL
kernel cache, retention enforcer, maintainCache compaction, BOTH sinks
(central forward + core local-fs blob), and the driver outbox respool.
Covers: ~0 bytes on a no-new-rows tick, ~N on an N-new tick, exactly-once
across a retention front-prune and a compaction generation swap for both
sinks, and watermark vs. driver-outbox respool composition (suffix-only
replay + idempotent batch-id / re-PUT). Adds a hermetic smoke proving the
blob sink reads straight through a compaction generation swap via the real
driver. Anchors LLP 0040 section 5 so the @ref resolves.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Task-Id: T6
* fix(central-forward): keep chunk batch-id stable across watermark advance
The forward sink derived its X-Hyp-Batch-Id from the per-tick chunk ordinal
(chunkIndex). After an earlier chunk was acked and the (sink, partition)
watermark advanced, a respool re-read only the un-acked suffix and re-numbered
it from 0 — minting a NEW batch-id for a chunk that may already have committed
on the server (ambiguous ack / commit-then-5xx). The server idempotency ledger
could not dedupe the redelivery, double-storing rows and breaking the spec's
'ledger covers mid-batch retries' guarantee.
Key the batch-id on the chunk's start seq (the watermark it resumes from)
instead. A respooled suffix reproduces the same [startSeq, body] and thus the
same id, so the ledger dedupes it; distinct chunks still differ because each
row's _hyp_ingest_seq is unique. Adds a cross-tick regression test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(incremental-sink-reads): close exactly-once holes in forward+blob sinks (#159 review)
Dual-review found a BLOCKER plus majors against the exactly-once claim. Fixes,
each with a regression test that fails before and passes after:
[BLOCKER] Forward sink advanced the watermark per acked chunk to the scan's
running-max `after`. Because the cache scan is NOT seq-ordered (interleaved
live+backfill spool; post-compaction sortOrder re-sort), an early acked chunk
could checkpoint past lower-seq rows still un-acked in a later chunk; a
between-chunk failure then dropped them forever (`seq <= since`). Fix: advance
the forward watermark ONCE at end-of-partition (as the blob sink already does),
so a partial partition never checkpoints — a failure re-reads the whole
partition and the server ledger dedupes the acked prefix (stable
chunkStartSeq batch ids).
test: central-forward-chunking "an unordered scan never skips a lower-seq row
when a later chunk fails".
[MAJOR] Legacy null-seq rows re-exported every tick (never reached steady
state). Add `includeLegacy` to the storage read API (default true). Both sinks
pass `includeLegacy = (no durable watermark)`: a fresh sink exports the
pre-upgrade backlog once, then excludes null-seq rows. Safe because no new
null-seq row can appear post-upgrade (decorateRow stamps every flushed row).
tests: sink-incremental-acceptance pure-legacy + mixed, forward + blob
(~0 bytes on 2nd tick, no row in two artifacts).
[MAJOR] Central forward sink scoped watermarks per-PLUGIN, not per-instance —
two @hypaware/central instances clobbered one watermark file. Switch to
createInstanceWatermarkStore({ paths, instanceName }) (matching local-fs/s3);
correct the @ref.
test: sink-incremental-acceptance "two instances keep independent
watermarks (no cross-instance skip)".
[MAJOR, doc] Bounded-reads constraint (C): the read is a full scan over the
surviving partition with a yielded-row filter, not the file/row-group skip the
design implied. Correct LLP 0040 §1(C)/§5 to state reads are bounded by
surviving-partition size, with O(N_new) reads pending null-aware icebird
pushdown.
[MINOR/NIT, doc+style] Correct LLP 0040 §4 (batchIdForChunk keys by
chunkStartSeq, not chunkIndex). Hoist inline import('...') types to @import in
sink-watermarks/acceptance tests and the compaction smoke.
[MAJOR, ESCALATED — left unfixed] Mid-retry duplication when a unit commits,
its watermark write is lost, AND new rows append before the retry: the resumed
in-flight unit grows past what committed and the dedup net no longer recognizes
it. The common in-flight retry (no new arrivals) IS covered. A correct fix needs
a pre-commit intent (read `until` upper bound + persisted intent record) across
both sinks — a design-level change to the watermark contract, deferred rather
than half-patched under the exactly-once claim. Documented as LLP 0040 §6 risk
#7 / §5 known gap.
LLP 0040/0042 updated in this commit to match the new behavior.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Fix typecheck: root-anchor type-module imports in feature .js files
The conflict-resolution merge of master (PR #196 type-surface cleanup)
left four feature .js files importing type modules via sibling-relative
paths (`./types.d.ts`). tsc copies the generated declaration into
`types/` but does not emit input `.d.ts` siblings there, so
`types/core/sinks/incremental.d.ts` had a dangling `import('./types.d.ts')`
→ TS2307 in `npm run typecheck` after `build:types` (CI-only; local
`npm test` does not run tsc emit).
Match master's convention (e.g. sinks/driver.js, cache/spool.js): .js
files reference type modules by a repo-root-anchored path into src/
(`../../../src/core/sinks/types.js`), which resolves identically from
both the source location and the emitted types/ location, plus `.js`
extension for the kernel-types barrel.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for freeto 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.

1 participant

@philcunliffe