feat(sdk)!: stream over fetch so SSE authenticates by header

[EricAndrechek]

Summary

Moves .stream() / .liveQuery() off EventSource and onto fetch, so the JWT travels as Authorization: Bearer instead of ?token= in the request URI — where every proxy, CDN, and load balancer in front of WaveHouse is free to log it.

Server untouched. bearerToken() has always preferred the header, and the CORS preflight has allow-listed Authorization + Last-Event-ID since #215 — internal/api/router_test.go:352 is an existing test naming #203 as its reason. ?token= stays accepted for clients that genuinely can't set headers.

Advances #203; does not close it. Tasks 1 and 2 of 3 are done (header auth, cURL flow). The third — retiring ?token= server-side — is #468, and closing that closes this.

This does not make streams authenticated, and (regarding #203) does not close it. /v1/stream stays ungated: an expired or missing token still resolves to default_role and gets a filtered 200, never a 401. What changes is that the JWT stops appearing in request URIs, and therefore in proxy, CDN, and load-balancer logs. Enforcing expiry on a live stream is #239 and is deliberately not delivered here.

Part of #194.

Decisions worth your eyes

1. Redirects are refused only when the request carries a credential.

Platforms strip Authorization on a cross-origin redirect (whatwg/fetch#1544 — the mitigation for the class behind CVE-2022-1650, which was this exact bug in the eventsource package) while forwarding other headers intact. A credentialed hop therefore either silently downgrades the stream to default_role — this endpoint answers an unauthenticated caller rather than rejecting them — or hands a configured proxy secret to whatever the redirect names. Refusing costs nothing, because following would never have produced an authenticated stream anyway.

With no credential there's nothing to protect, so redirects are followed and CDN canonicalization, geo/LB indirection, and http→https upgrades all work. options.fetch overrides redirect if you need the credentialed case followed regardless.

manual rather than error: error rejects with a bare TypeError indistinguishable from a connection failure, which the reconnect loop would retry forever against a redirect that will never stop happening.

2. FetchLike's URL parameter narrows to string. The design note on #269 said the type was narrow so hand-written (url: string, init?) => Promise<Response> middleware would assign. What shipped was string | URL | Request, which — parameters being contravariant — rejects exactly that. Purely additive for implementers; only code that imported FetchLike and called through it with a URL breaks.

3. The SDK gains its first runtime dependency: eventsource-parser@^3.1.0. MIT, zero transitive deps, npm provenance-attested (SLSA v1), no install scripts, 61.7M downloads/week, same maintainer as the canonical eventsource. Dual CJS/ESM, so it composes with our dist/index.cjs. Measured cost in the CDN IIFE bundle: 3402 B minified, 1430 B gzipped.

A caret rather than an exact pin because pinning in a published library duplicates the package in any consumer tree already resolving a 3.x and freezes them out of patch/security releases until we cut one — our lockfile still governs CI. The range stops below the ESM-only 4.0.0.

Why rent rather than hand-roll: the correct parser is ~440 lines, and the parts that matter — reassembling a frame split across chunk boundaries without quadratic recopying, disambiguating a trailing \r at a chunk boundary between a bare-CR terminator and half a split \r\n, and capping buffered input against a hostile stream — are what a naive version gets wrong, and are unavoidable even though we control the server, since HTTP/2 and intermediaries re-chunk freely. The buffer cap is set explicitly to 16 MiB; the parser defaults to unbounded.

This retires the "zero-dependency" claim everywhere it appeared — both READMEs, AGENTS.md invariant 14, four site pages, and an older Unreleased CHANGELOG entry that would otherwise have shipped in the same release notes as the entry introducing the dependency. The only surviving mentions are in released CHANGELOG history, which is a record rather than a claim.

What this fixes beyond the headline

• Expired-token silent downgrade. auth() was called once and baked into the URL; EventSource then reconnected forever with that token. Once expired the stream stayed open serving a reduced view. This is why feat(sdk): migrate SSE auth from ?token=JWT to Fetch EventSource #203 is a prerequisite for feat(stream): re-validate JWT / enforce token expiry on long-lived SSE connections #239.
• Blank id: clearing resumption. The hub emits id: for passthrough payloads; per spec an empty id clears the last-event-id. The transport retains the last non-empty id.
• Real errors. A rejection carries its actual status and message instead of EventSource's status-free onerror — which, in the old transport, meant a gateway 401 surfaced as a silent closed with no error callback at all. Note the limit: a browser stream going cross-origin only sees the status if the rejection passes CORS and the gateway answered the Authorization preflight; otherwise it degrades to a retryable network error.
• Non-SSE 200s refused (SSE_BAD_CONTENT_TYPE). An auth gateway's login page would otherwise feed HTML to the parser, which per the SSE grammar parses to nothing — leaving the stream live and permanently silent.
• No Node polyfill. polyfills.ts and the eventsource devDependency are deleted.
• options.fetch / headers / fetchOptions reach streams, closing the carve-out documented in feat(sdk)!: add options.headers, fetchOptions, and fetch #456.

Behavior changes to be aware of

• A credentialed cross-origin browser stream now preflights on the initial connect. EventSource never preflighted at all (its request isn't a fetch(), so Fetch's unsafe-request flag is never set). A proxy that answers CORS itself must allow Authorization on OPTIONS /v1/stream or the stream never opens. Documented in reverse-proxy.mdx.
• A proxy that strips Authorization on /v1/stream, or redirects it while credentialed, now breaks. Both previously "worked" by accident.
• Resumption is at-least-once and time-bounded — this was always true; the docs now say so. The last event you saw is certainly redelivered (the id is a received_timestamp and replay is inclusive), the SDK does not dedupe live frames, and replay is capped by mq.gap_window_minutes (15 default).

Testing

sse.test.ts is rewritten against an injected fetch returning a scripted ReadableStream. The old harness stubbed a global FakeEventSource and could only assert on URL strings — framing, reconnect, and resumption had no coverage at all. 204 tests now, with 14 behavioral fixes mutation-verified: reverting each one makes a test fail, and — after review caught a case where it didn't — fail on the assertion that names it.

The e2e auth test was rewritten to be discriminating: anon is denied payload on the events table, so dropping the Authorization header flips the assertion. The previous version could not fail.

Beyond the nominal scope

Two REST-path fixes ride along, both surfaced by review of the streaming work and both in clients/ts/src/http.ts. Flagging them because an SSE PR is not where you would look for them, and either can be split out on request.

• A cancelled request could throw instead of returning ABORTED. The network-error backoff is the one sleep inside request()'s catch, so its rejection had no handler and escaped as a raw DOMException. Nothing wraps request(), so it reached callers as an unhandled rejection — and the AbortController example in our own reference demonstrated a branch that could not be taken against an unreachable server.
• Abort is now classified from the signal, not the rejection's type. Keying off the error made the outcome depend on maxRetries: AbortSignal.timeout() raises a TimeoutError, so it reported NETWORK_ERROR at maxRetries: 0 and ABORTED at 2. It also mis-handled middleware — an options.fetch enforcing its own per-attempt deadline aborts an internal controller while the caller never cancelled, which is transient and should be retried, not reported as a terminal ABORTED.

The same rule then had to be applied to the stream transport, where the old error-type check was worse: an AbortError from auth() or a custom fetch ended the stream terminally and emitted nothing.

Review

Fifteen pre-push rounds against both gating reviewers, who verify by executing the code rather than reading it. Worth knowing what they caught, since none of it was reachable by CI:

Behavior bugs	SSE_CONNECT_ERROR never reaching a subscriber; a closed stream stuck reporting live; a consumed-body guard that didn't guard; an unhandled rejection that killed the host process; a stranded reconnect timer
Regression vs EventSource	close() from inside a handler no longer stopped delivery
Coverage holes	deleting the bearer half of the credentialed-redirect rule left the suite green
False claims in docs	EventSource "preflighted on reconnect" (it never preflights); "WaveHouse does not reject a stream" (it 400s on a missing table); "the SDK isolates a throwing handler" (true only inside the transport — several paths outside it are not, now enumerated in the SDK reference and filed as #473)

Two recurring shapes, both worth knowing before you read the diff.

In the code: a guard or cleanup applied to N−1 of N call sites. Six defects shared it. if (this._closed) return now appears eight times in sse.ts, several added a round apart. Assume any new early-exit path in this transport is the one that got missed.

In the prose: a true mechanism attached to a wider case set than it holds for. This accounts for essentially every documentation defect found here, and it recurred for eight consecutive rounds — three times inside the sentence written to fix the previous instance. The reviewers' diagnosis is the useful part: none of these were factual errors about the system, they were missing quantifiers. Nearly every claim in this area is a function of a variable the docs cannot name — the reader's token-provider latency, which origin, which credentials mode — so the domain lives only in the author's head at the moment of writing, and the next revision reaches for the deepest true mechanism and silently re-attaches it to the whole case set. The empirical tell was sharp: the rule-shaped sentences never needed correcting; the value-shaped ones were corrected every round.

The Live Queries failure section is written to that conclusion — it states rules and gives the reader a test, rather than reporting which outcome is typical. Two amplifiers were also removed: sentences that counted table rows (a ninth row would have silently falsified five of them) and facts restated independently in four or five files. If you are reviewing prose here, the question that finds bugs is "for which cases is this true?", not "is this true?" Four axes account for essentially every defect found on this branch, and a claim that is silent about which side it means is the shape to distrust:

1. Who rejected it — WaveHouse (a 400 on the stream route; a 404/405 off it) versus something in front. Never "the server said 401": /v1/stream is ungated.
2. Where the caller runs — server-side or same-origin (statuses visible) versus browser cross-origin, where CORS can make any rejection, including a rejected preflight, indistinguishable from a network drop.
3. Whether the request carries a credential — decides redirect: "manual" versus "follow", and the test is Authorization-or-headers, so cookies are not credentials by it (fix(sdk): cookie-authenticated streams bypass the redirect guard #478).
4. Whether the failure is semantically transient — the 4xx-is-terminal rule is a transport mechanism, not a claim about the world (fix(sdk): treat a stream 429 as retryable and honor Retry-After #469).

One structural note so it isn't rediscovered: CHANGELOG.md is denylisted from the docs-prose gate (scripts/docs-prose.sh:38), so that entry has never been read by the automated docs reviewer. It is worth reading at docs scrutiny rather than skimming as boilerplate — a false claim survived three rounds there for exactly that reason.

The Go diff is one comment, so this looks deployment-free. It isn't. A credentialed cross-origin browser stream now preflights where EventSource never did, so a proxy that answers CORS itself must allow Authorization on OPTIONS /v1/stream or streams stop opening — silently, in a retry loop, not with a visible error. That break is documented in reverse-proxy.mdx; a reviewer reading only internal/ will conclude nothing operational changed.

Follow-ups filed

Design work deferred out of this PR:

• perf(stream): evaluate gzip on /v1/stream with per-frame flush #465 — gzip on /v1/stream (per-frame flush; measure before adopting)
• feat(sdk): normalize a schemeless baseURL, with a loopback exception #466 — normalize a schemeless baseURL, with a loopback exception
• explore(stream): binary frame format negotiated via Accept, alongside SSE #467 — binary framing negotiated via Accept, sequenced behind perf(stream): evaluate gzip on /v1/stream with per-frame flush #465
• fix(auth): deprecate and retire the ?token= query parameter #468 — retire ?token= server-side; closes feat(sdk): migrate SSE auth from ?token=JWT to Fetch EventSource #203
• feat(sdk): SSE replay semantics across multiple subscriptions #204 — commented with the POST-body analysis that unblocks multiplexing

Defects found by review of this branch and left unfixed here, each because the
fix lands outside the transport or carries a design question I didn't want to
answer unilaterally in a PR about auth:

• fix(sdk): treat a stream 429 as retryable and honor Retry-After #469 — a stream 429 is terminal; should honor Retry-After
• fix(sdk): a rejected resumption preflight leaves a stream re-dialing forever #471 — a rejected resumption preflight leaves a stream re-dialing forever
• fix(sdk): a throwing subscriber silently stops delivery to the others #473 — a throwing subscriber silently stops delivery to the others. Widened during review to cover every path outside the transport's guard — .subscribe()'s initial unguarded status call (worse via liveQuery(), which returns no handle at all), the fan-out dropping the event for a concurrent for await and leaving it un-terminated on a terminal close, a throwing status handler making .connected() time out against a live stream, and liveQuery()'s backfill flush discarding its buffer. The docs and CHANGELOG now describe the real contract rather than the one I first wrote.
• fix(sdk): sleep() leaks an abort listener when the timer wins #476 — http.ts's sleep() leaks an abort listener when the timer wins
• fix(sdk): harden the stream transport against a non-conforming peer #484 — transport hardening against a non-conforming peer, raised as "what I'd watch" in the final review: a parser-buffer overflow re-dials at a flat rate forever because the backoff reset counts the overflowing connection as healthy, and FetchLike's contract never states that init.signal must be honored. Neither is reachable with a conforming peer; both are cheap now.
• fix(sdk): StreamController retains every event when nothing iterates #477 — StreamController retains every event when nothing iterates. The buffer's only drain is the async iterator's next(), so a .subscribe()-only consumer — the pattern the docs lead with — holds every event it has ever received, unbounded. Doubled on a filtered or live stream, since both controller layers buffer.
• fix(sdk): cookie-authenticated streams bypass the redirect guard #478 — a cookie-authenticated stream bypasses the redirect guard. credentialed tests for a bearer token or configured headers; cookies are neither, so the request follows a redirect and can arrive unauthenticated.
• sdk(ts): live-query dedup uses the last fetched row, not the newest — duplicate delivery on desc-ordered fetches #449 — pre-existing liveQuery dedup boundary, re-confirmed by review
• docs(sdk): wh.pipe(name).stream() documented as working but yields no events #445 — not touched here, but surfaced again while reviewing the streaming docs and worth a person's eye: wh.pipe(name).stream() is documented as working in three places while pipes.ts streams ?table=<pipeName>, so it subscribes to a topic nothing publishes and silently yields nothing. The DLQ variant carries an inline caveat for the same class of gap; the pipe one doesn't.

Reviewer notes

The interesting file is clients/ts/src/stream/sse.ts. controller.ts is untouched — the transport sits behind the same StreamTransport interface — so the diff is scoped to the transport, its tests, and the docs the change invalidated.

clients/ts/src/stream/live-query.test.ts is new and is the first test coverage LiveQuery has had. It pins one thing worth knowing about: the backfill, not the stream, spends the first auth() call, and that ordering is emergent from four independent details rather than declared anywhere. One added await on the REST path silently swaps the two failure modes the docs describe, so the test exists to make that a red build rather than a documentation drift.

Merged with main at 1064a4fe, which brought #381 (per-subscriber SSE row filtering) and #457. Both conflicted textually with this branch and both were resolved keeping each side; #381 adds no new status code, so this PR's claim that /v1/stream raises exactly one 4xx itself still holds.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Streaming now uses fetch and readable streams without an EventSource polyfill.
  • Added configurable headers, fetch options, custom fetch implementations, automatic reconnection, retry backoff, event resumption, and connected() readiness tracking.
  • Streaming authentication uses Bearer headers with refreshed credentials on reconnect.
  • Added a lightweight SSE parser dependency.

• Bug Fixes

  • Improved connection, authentication, parsing, network-error, and cancellation handling.

• Documentation

  • Updated streaming, authentication, CORS, redirects, reconnection, filtering, and dependency guidance.

Walkthrough

The TypeScript SDK replaces EventSource with fetch-based SSE. It adds header authentication, streaming configuration, parsing, reconnection, resumption, validation, and lifecycle handling. REST cancellation now returns standardized ABORTED results. Tests and documentation cover the new behavior.

Changes

SDK streaming and HTTP behavior

Layer / File(s)	Summary
Transport contracts and wiring clients/ts/package.json, clients/ts/src/types.ts, clients/ts/src/http.ts, clients/ts/src/client.ts, clients/ts/src/stream/sse.ts	The SDK adds eventsource-parser, supports custom fetch settings for streaming, exports mergeHeaders, narrows FetchLike URLs to strings, and removes the EventSource requirement.
Connection, parsing, and reconnect flow clients/ts/src/stream/sse.ts, clients/ts/src/stream/sse.test.ts, clients/ts/src/client.test.ts	SSETransport sends Bearer authentication, validates URLs and responses, parses SSE frames, tracks Last-Event-ID, applies retry hints and jittered backoff, and manages disconnects.
HTTP cancellation and live-query behavior clients/ts/src/http.ts, clients/ts/src/http.test.ts, clients/ts/src/stream/live-query.ts, clients/ts/src/stream/live-query.test.ts	Caller cancellation returns ABORTED during requests and retry backoff. Tests cover cancellation classification and live-query authentication ordering.
Transport validation and end-to-end coverage clients/ts/src/stream/sse.test.ts, tests/e2e/sdk/*, tests/e2e/sdk/vitest.config.ts	Tests cover fetch construction, framing, retries, resumption, validation, lifecycle races, asynchronous startup failures, header authentication, restricted payloads, and removal of EventSource polyfills.
SDK documentation and release notes AGENTS.md, README.md, CHANGELOG.md, clients/ts/README.md, docs/src/content/docs/*, internal/api/stream.go	Documentation describes fetch-based streaming, runtime requirements, Authorization headers, CORS, redirects, reconnect behavior, replay semantics, error handling, dependencies, and live-query behavior.

Estimated code review effort: 4 (Complex) | ~60 minutes

Merge Risk: 🔵 Low · up to 924d3

The SDK now authenticates streams through fetch headers, reducing token exposure in request URLs while changing redirect, preflight, and proxy behavior. The PR is mergeable with explicit owner follow-up for strict SSE validation, accurate proxy upload guidance, reconnect/close edge cases, and test diagnostics; these are bounded risks rather than release-blocking failures.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant SSETransport
  participant TokenProvider
  participant Fetch
  participant ReadableStream
  Client->>SSETransport: start stream
  SSETransport->>TokenProvider: request token
  TokenProvider-->>SSETransport: return token
  SSETransport->>Fetch: fetch SSE request with Authorization
  Fetch-->>SSETransport: return streaming response
  SSETransport->>ReadableStream: read response chunks
  SSETransport->>SSETransport: parse events and track Last-Event-ID
  SSETransport->>Fetch: reconnect after retryable failure

Loading

Possibly related issues

• feat(sdk): migrate SSE auth from ?token=JWT to Fetch EventSource #203 — Implements fetch-based SSE transport and moves authentication from ?token=JWT to Authorization: Bearer.
• fix(sdk): a rejected resumption preflight leaves a stream re-dialing forever #471 — Covers fetch-based SSE and Last-Event-ID resumption behavior.

Possibly related PRs

• Wave-RF/WaveHouse#174 — Shares TypeScript SSE transport and streaming authentication changes.
• Wave-RF/WaveHouse#448 — Modifies SSE transport URL and request construction.
• Wave-RF/WaveHouse#456 — Extends REST HTTP customization to the fetch-based SSE transport.

Suggested labels: area/streaming

Suggested reviewers: taitelee

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the primary change: replacing the SSE EventSource transport with fetch and header-based authentication.
Description check	✅ Passed	The description directly explains the fetch-based SSE transport, header authentication, behavior changes, testing, and documented follow-ups.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch sse-auth

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch sse-auth

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

📚 Docs preview is live → https://54d0207d-wavehouse-docs.wave-rf.workers.dev

• Commit — 924d30d: docs(changelog): scope the last pre-fetch polyfill claim, and credit queries.md
• Author — @EricAndrechek, Claude Opus 5 (1M context)
• Committed — 2026-08-18 16:09 (UTC-04:00)
• Deployed — 2026-08-18 16:25 EDT

[github-code-quality]

Code Coverage Overview

Languages: Go

Go

The overall coverage in commit 924d30d in the sse-auth branch remains at 91%, unchanged from commit 91ac2be in the main branch.

---

_{Updated August 18, 2026 20:25 UTC}

[coderabbitai]

Actionable comments posted: 10

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 574111d6-fbcd-496d-9aff-6224803cf733

📥 Commits

Reviewing files that changed from the base of the PR and between ea4fbf6 and 77feb9d.

⛔ Files ignored due to path filters (1)

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (23)

• AGENTS.md
• CHANGELOG.md
• README.md
• clients/ts/README.md
• clients/ts/package.json
• clients/ts/src/client.test.ts
• clients/ts/src/client.ts
• clients/ts/src/http.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/stream/sse.ts
• clients/ts/src/types.ts
• docs/src/content/docs/api.md
• docs/src/content/docs/getting-started.md
• docs/src/content/docs/index.mdx
• docs/src/content/docs/reverse-proxy.mdx
• docs/src/content/docs/sdk/index.mdx
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/sdk/streaming.md
• docs/src/content/docs/why-wavehouse.md
• tests/e2e/sdk/package.json
• tests/e2e/sdk/polyfills.ts
• tests/e2e/sdk/streaming.test.ts
• tests/e2e/sdk/vitest.config.ts

💤 Files with no reviewable changes (2)

• tests/e2e/sdk/package.json
• tests/e2e/sdk/polyfills.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (8)

• GitHub Check: Unit tests
• GitHub Check: Coverage
• GitHub Check: E2E tests
• GitHub Check: Integration tests
• GitHub Check: Docs build
• GitHub Check: Lint
• GitHub Check: Analyze (go)
• GitHub Check: Analyze (go)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

• Comment the why, not the what. Add a comment only when the reason isn't obvious from the code; a line that matches the surrounding pattern needs none. Keep comments to 1–2 lines and match the file's existing density.

Files:

• docs/src/content/docs/getting-started.md
• AGENTS.md
• README.md
• docs/src/content/docs/why-wavehouse.md
• clients/ts/README.md
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/api.md
• CHANGELOG.md
• docs/src/content/docs/sdk/streaming.md

tests/e2e/sdk/*.test.ts

📄 CodeRabbit inference engine (AGENTS.md)

tests/e2e/sdk/*.test.ts: Add new E2E scenarios as tests/e2e/sdk/*.test.ts files using helpers from tests/e2e/sdk/helpers.ts.
A new test file must (1) add its suite name to SUITES in tables.ts and (2) get its names via const T = suiteTables("<suite>"), then reference T.clicks etc. — never a bare clicks.

Files:

• tests/e2e/sdk/streaming.test.ts

🧠 Learnings (30)

📓 Common learnings

Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-12T21:55:01.697Z
Learning: Applies to clients/ts/**/*.ts : The TypeScript SDK (`wavehouse/sdk` in `clients/ts/`) is the canonical client and ships from this repo.

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:23.813Z
Learning: In the TypeScript SDK, `ClientConfig.baseURL` must be an absolute URL with a scheme and host. A relative `baseURL` causes `resolveURL` to throw a `TypeError` on the first request. REST requests reject because `resolveURL` runs outside `request()`'s retry `try` block. In `clients/ts/src/stream/sse.ts`, `SSETransport` catches this failure and reports `SSE_CONNECT_ERROR` through the optional `StreamSubscriber.error` callback, so a subscriber without that callback can observe no error.

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/client.test.ts:0-0
Timestamp: 2026-08-12T20:34:04.763Z
Learning: In the TypeScript SDK, `clients/ts/src/types.ts` defines `FetchLike` as `(input: string | URL | Request, init?: RequestInit) => Promise<Response>`. This explicit standard fetch-compatible signature avoids `typeof fetch` differences when DOM library types are absent and allows fetch wrappers to be shared with SDKs that use the standard wide input type.

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: docs/src/content/docs/sdk/index.mdx:0-0
Timestamp: 2026-08-12T15:28:20.891Z
Learning: For `docs/src/content/docs/sdk/index.mdx`, the documented workaround for the undici idle-event-loop keep-alive stall is to upgrade to undici 8.10.0 or later. If a consumer is pinned to an affected version, `new Agent({ pipelining: 0 })` must be merged as `dispatcher` into the SDK-provided `RequestInit`; this disables keep-alive reuse. Configuring `keepAliveTimeout` does not mitigate this stall because the socket retirement timer is starved by the same idle event loop.

📚 Learning: 2026-08-12T21:55:01.697Z

Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-12T21:55:01.697Z
Learning: Applies to clients/ts/**/*.ts : The TypeScript SDK (`wavehouse/sdk` in `clients/ts/`) is the canonical client and ships from this repo.

Applied to files:

• clients/ts/package.json
• docs/src/content/docs/getting-started.md
• AGENTS.md
• README.md
• tests/e2e/sdk/vitest.config.ts
• docs/src/content/docs/index.mdx
• docs/src/content/docs/why-wavehouse.md
• clients/ts/README.md
• clients/ts/src/client.ts
• docs/src/content/docs/sdk/index.mdx

📚 Learning: 2026-06-10T15:01:09.027Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.

Applied to files:

• docs/src/content/docs/getting-started.md
• AGENTS.md
• README.md
• docs/src/content/docs/why-wavehouse.md
• clients/ts/README.md
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/api.md
• CHANGELOG.md
• docs/src/content/docs/sdk/streaming.md

📚 Learning: 2026-08-12T15:28:20.891Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: docs/src/content/docs/sdk/index.mdx:0-0
Timestamp: 2026-08-12T15:28:20.891Z
Learning: For `docs/src/content/docs/sdk/index.mdx`, the documented workaround for the undici idle-event-loop keep-alive stall is to upgrade to undici 8.10.0 or later. If a consumer is pinned to an affected version, `new Agent({ pipelining: 0 })` must be merged as `dispatcher` into the SDK-provided `RequestInit`; this disables keep-alive reuse. Configuring `keepAliveTimeout` does not mitigate this stall because the socket retirement timer is starved by the same idle event loop.

Applied to files:

• AGENTS.md
• clients/ts/README.md
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/api.md
• docs/src/content/docs/reverse-proxy.mdx
• CHANGELOG.md
• docs/src/content/docs/sdk/streaming.md
• docs/src/content/docs/sdk/index.mdx

📚 Learning: 2026-08-12T21:55:01.697Z

Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-12T21:55:01.697Z
Learning: Applies to internal/auth/**/*.go : **Auth: always on, fail-loud, decoupled from authz (security)**

Applied to files:

• AGENTS.md
• CHANGELOG.md

📚 Learning: 2026-08-12T21:55:01.697Z

Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-12T21:55:01.697Z
Learning: Applies to internal/policy/**/*.go : **Hasura-style access control: fail-closed (security)**

Applied to files:

• AGENTS.md

📚 Learning: 2026-08-12T21:55:01.697Z

Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-12T21:55:01.697Z
Learning: Applies to internal/query/**/*.go : **Structured queries: column authz fail-closed (security)**

Applied to files:

• AGENTS.md

📚 Learning: 2026-05-13T20:41:09.256Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 125
File: internal/api/health_test.go:100-163
Timestamp: 2026-05-13T20:41:09.256Z
Learning: In `internal/api/health_test.go` (WaveHouse), every handler test explicitly asserts `Content-Type: application/json` and `X-Content-Type-Options: nosniff` headers, including on 503 responses. This is deliberate regression coverage: the comment in `TestHealth_Readiness_PingFails` explains that without the 503-path header test, a future refactor moving header setup into the success branch would silently drop headers on error responses. New boot-degraded tests should follow the same pattern.

Applied to files:

• AGENTS.md
• CHANGELOG.md

📚 Learning: 2026-08-11T15:22:47.380Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: docs/src/content/docs/sdk/index.mdx:330-334
Timestamp: 2026-08-11T15:22:47.380Z
Learning: In WaveHouse Go server authentication, `internal/auth/auth.go` `bearerToken` returns from the `Authorization` header path before modifying `r.URL`. It removes the `token` query parameter only when authentication uses the query parameter without an `Authorization` header. Documentation must state that this protects WaveHouse's own logs only; reverse proxies, CDNs, load balancers, and other upstream intermediaries require query-string redaction.

Applied to files:

• AGENTS.md
• docs/src/content/docs/api.md
• docs/src/content/docs/reverse-proxy.mdx
• CHANGELOG.md
• docs/src/content/docs/sdk/index.mdx

📚 Learning: 2026-05-20T20:30:22.556Z

Learnt from: taitelee
Repo: Wave-RF/WaveHouse PR: 172
File: internal/api/pipes_test.go:106-118
Timestamp: 2026-05-20T20:30:22.556Z
Learning: In WaveHouse's pipes authorization fix (PR `#172`), fixing the AllowedRoles fail-open bug requires two changes: (1) remove the outer `if role != ""` guard in PipesHandler.Execute so empty roles are evaluated against the allowlist, AND (2) add an `ar != ""` guard inside the allowlist scan (i.e., `ar != "" && ar == role`) so a malformed allowlist containing empty strings (e.g., `[""]`) cannot match an empty role via `"" == ""`. Doing only (1) is insufficient and would make `[""]` fail-open.

Applied to files:

• AGENTS.md
• CHANGELOG.md

📚 Learning: 2026-07-07T12:38:15.328Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 378
File: internal/auth/auth.go:119-132
Timestamp: 2026-07-07T12:38:15.328Z
Learning: Repo: Wave-RF/WaveHouse. WaveHouse deliberately does not log or trace any client IP address anywhere in the codebase. `middleware.RealIP` was removed in PR `#332` due to IP-spoofing GHSAs, and trusted-proxy-aware client-IP extraction for logs/traces is tracked as a future cross-cutting effort in issue `#333`. Do not suggest adding `r.RemoteAddr` or naive `X-Forwarded-For`-derived IPs to logs (e.g., audit logs in internal/auth/auth.go for the operator-key path) until `#333` lands with proper trusted-proxy handling.

Applied to files:

• AGENTS.md
• CHANGELOG.md

📚 Learning: 2026-08-11T21:56:06.521Z

Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: docs/src/content/docs/sdk/go/queries.md:356-362
Timestamp: 2026-08-11T21:56:06.521Z
Learning: In Wave-RF/WaveHouse Go SDK cursor pagination, `fetchNextTyped` uses only the first `QueryBuilder.OrderBy` column and a strict `gt` or `lt` filter. Duplicate values at a page boundary can skip rows. The Go SDK documentation must require a unique ordering column until the shared Go and TypeScript composite-cursor or tie-breaker implementation tracked in GitHub issue `#452` is available.

Applied to files:

• AGENTS.md
• CHANGELOG.md

📚 Learning: 2026-08-11T21:55:42.427Z

Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: clients/go/cmd/wavehouse-codegen/main.go:96-97
Timestamp: 2026-08-11T21:55:42.427Z
Learning: In `clients/go/cmd/wavehouse-codegen/main.go`, `fetchSchemas` is a self-contained CLI helper with one caller, `main`. The `AGENTS.md` convention to pass dependencies explicitly applies to package constructors, not to this type of CLI helper. Do not request HTTP-client injection unless HTTP-level test coverage or additional callers make that refactor necessary.

Applied to files:

• AGENTS.md

📚 Learning: 2026-05-25T11:25:14.412Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 180
File: internal/observability/instruments.go:22-38
Timestamp: 2026-05-25T11:25:14.412Z
Learning: In WaveHouse's `internal/observability/instruments.go`, the `mustFloat64Histogram` and `mustInt64Counter` helpers intentionally panic at package init time if OTel instrument registration fails. This follows the `regexp.MustCompile`/`template.Must` Go idiom for build-time-constant invariants. The "return errors, don't panic" coding guideline applies to runtime/request-response paths only, not to init-time instrument registration. Do not flag this pattern as a violation.

Applied to files:

• AGENTS.md

📚 Learning: 2026-08-12T21:55:01.697Z

Learnt from: CR
Repo: Wave-RF/WaveHouse PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-08-12T21:55:01.697Z
Learning: Applies to tests/e2e/sdk/*.test.ts : Add new E2E scenarios as `tests/e2e/sdk/*.test.ts` files using helpers from `tests/e2e/sdk/helpers.ts`.

Applied to files:

• tests/e2e/sdk/vitest.config.ts
• clients/ts/src/stream/sse.test.ts

📚 Learning: 2026-08-11T15:22:23.813Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:23.813Z
Learning: In the TypeScript SDK, `ClientConfig.baseURL` must be an absolute URL with a scheme and host. A relative `baseURL` causes `resolveURL` to throw a `TypeError` on the first request. REST requests reject because `resolveURL` runs outside `request()`'s retry `try` block. In `clients/ts/src/stream/sse.ts`, `SSETransport` catches this failure and reports `SSE_CONNECT_ERROR` through the optional `StreamSubscriber.error` callback, so a subscriber without that callback can observe no error.

Applied to files:

• tests/e2e/sdk/vitest.config.ts
• clients/ts/README.md
• docs/src/content/docs/sdk/reference.md
• CHANGELOG.md
• docs/src/content/docs/sdk/streaming.md
• docs/src/content/docs/sdk/index.mdx

📚 Learning: 2026-08-11T15:22:20.507Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:20.507Z
Learning: In the TypeScript SDK, ensure `ClientConfig.baseURL` is validated or documented as requiring an absolute URL with both a scheme and host. Relative URLs cause `resolveURL` to throw a `TypeError` before REST retry handling runs. Ensure SSE connection failures are surfaced through `StreamSubscriber.error` and are not silently lost when that callback is absent.

Applied to files:

• clients/ts/src/client.test.ts
• clients/ts/src/client.ts
• clients/ts/src/http.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/types.ts
• clients/ts/src/stream/sse.ts

📚 Learning: 2026-08-12T05:38:52.277Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 455
File: tests/e2e/sdk/helpers.ts:214-221
Timestamp: 2026-08-12T05:38:52.277Z
Learning: In `tests/e2e/sdk/helpers.ts`, `chQuery` must reclassify a caught error as a request timeout or caller abort only when the error is an abort error. On Node 22, `AbortSignal.timeout()` produces an error named `TimeoutError`, `AbortController.abort()` and caller cancellation through `AbortSignal.any()` produce `AbortError`, and `JSON.parse()` failures produce `SyntaxError`.

Applied to files:

• clients/ts/src/client.test.ts
• clients/ts/README.md
• CHANGELOG.md
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/types.ts

📚 Learning: 2026-08-12T21:45:38.018Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/pipes.ts:0-0
Timestamp: 2026-08-12T21:45:38.018Z
Learning: In the TypeScript SDK, use the exported `PipeRequestOptions` type for `PipeRef.fetch` and shared pipe, table, or query-builder fetch options rather than `Pick<RequestOptions, "signal">` or `RequestOptions`. `PipeRequestOptions` declares `limit?: never`, ensuring object literals and named `RequestOptions` values containing `limit` fail type checking instead of silently dropping it. When only a signal is needed, use `PipeRequestOptions` or an inferred `{ signal }` object; do not expect a `RequestOptions` value to be assignable to `PipeRequestOptions`.

Applied to files:

• clients/ts/src/client.test.ts
• clients/ts/src/client.ts
• clients/ts/src/http.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/types.ts
• clients/ts/src/stream/sse.ts

📚 Learning: 2026-08-11T21:55:44.607Z

Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: clients/go/go.mod:3-6
Timestamp: 2026-08-11T21:55:44.607Z
Learning: In the WaveHouse repository, `clients/go/go.mod` declares `go 1.24` as the deliberate minimum supported Go version for the published Go SDK. This SDK compatibility floor is independent of the server build toolchain declared by the root `go.mod` and referenced in `AGENTS.md`; do not require the Go SDK module to use the server toolchain version.

Applied to files:

• docs/src/content/docs/why-wavehouse.md

📚 Learning: 2026-06-10T19:54:03.032Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: CHANGELOG.md:0-0
Timestamp: 2026-06-10T19:54:03.032Z
Learning: In the Wave-RF/WaveHouse repository, CHANGELOG.md entries under `[Unreleased]` use descriptive Keep-a-Changelog leads (e.g. "The structured-query column allowlist is now a hard cap…"), NOT the Conventional Commit PR title verbatim. Do not flag CHANGELOG entry leads for not matching the PR title — that is not a rule in this repo. There is no `.coderabbit.yaml`, and neither `AGENTS.md` nor `CONTRIBUTING.md` requires CHANGELOG leads to match PR titles.

Applied to files:

• docs/src/content/docs/why-wavehouse.md
• CHANGELOG.md

📚 Learning: 2026-08-12T20:34:04.763Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/client.test.ts:0-0
Timestamp: 2026-08-12T20:34:04.763Z
Learning: In the TypeScript SDK, `clients/ts/src/types.ts` defines `FetchLike` as `(input: string | URL | Request, init?: RequestInit) => Promise<Response>`. This explicit standard fetch-compatible signature avoids `typeof fetch` differences when DOM library types are absent and allows fetch wrappers to be shared with SDKs that use the standard wide input type.

Applied to files:

• clients/ts/README.md
• CHANGELOG.md
• clients/ts/src/types.ts
• clients/ts/src/stream/sse.ts
• docs/src/content/docs/sdk/index.mdx

📚 Learning: 2026-08-11T16:02:20.914Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: internal/auth/auth.go:0-0
Timestamp: 2026-08-11T16:02:20.914Z
Learning: In `internal/auth/auth.go`, `Middleware` must call `bearerToken(r)` before any authentication branch that can return early, including operator-key authentication. `bearerToken` removes a non-empty `token` query parameter from `r.URL.RawQuery` before selecting the Bearer-header or query-token credential, so WaveHouse handlers and logs do not retain an unused query token.

Applied to files:

• docs/src/content/docs/api.md
• CHANGELOG.md

📚 Learning: 2026-06-26T15:07:28.749Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 0
File: :0-0
Timestamp: 2026-06-26T15:07:28.749Z
Learning: In the Go SSE implementation in `internal/api/stream.go`, keepalive frames from `internal/stream.Heartbeater` are only written from the post-replay select loop. The replay/gap-fill step is synchronous before entering that loop, so registering the `internal/stream.Subscriber` before replay does not materially improve idle-time coverage during replay; it can at most buffer one heartbeat in the subscriber's capacity-1 queue. Covering a genuinely long replay would require interleaving replay with the select loop and is tied to the broader delivery-path rework tracked by Issue `#294`.

Applied to files:

• docs/src/content/docs/reverse-proxy.mdx
• docs/src/content/docs/sdk/streaming.md

📚 Learning: 2026-05-20T01:02:03.228Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 164
File: internal/api/router_test.go:289-350
Timestamp: 2026-05-20T01:02:03.228Z
Learning: In the WaveHouse project (`internal/api/**/*_test.go`), the convention for testing `RequireRole` middleware is to inject `ContextKeyRole` directly into the request context rather than using `testutil.MakeJWT`. JWT token parsing is covered separately in `middleware_test.go` (17 dedicated tests). Do not suggest switching role-gate tests to JWT-driven tests — the separation of concerns is intentional to keep failure surfaces isolated.

Applied to files:

• CHANGELOG.md

📚 Learning: 2026-05-13T21:06:12.242Z

Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 135
File: deployments/signoz/clickhouse/users.xml:68-68
Timestamp: 2026-05-13T21:06:12.242Z
Learning: In the WaveHouse repository, `deployments/signoz/clickhouse/users.xml` intentionally configures the ClickHouse `default` user with an empty password and global network access (`::0`). This is acceptable because the `deployments/signoz/` stack is a local-dev-only SigNoz observability stack; ClickHouse ports are not published to the host in `deployments/signoz/compose.yaml`. Do not flag this as a security issue in future reviews.

Applied to files:

• CHANGELOG.md

📚 Learning: 2026-07-08T12:46:29.364Z

Learnt from: taitelee
Repo: Wave-RF/WaveHouse PR: 381
File: internal/stream/hub.go:142-178
Timestamp: 2026-07-08T12:46:29.364Z
Learning: In `internal/stream/hub.go`, the per-subscriber `policy.Evaluate(...)` call in `Broadcast` (on the row-filtered path, when `perms.HasRowFilter()` is true) is intentionally not memoized per distinct claim set. Rationale from maintainer taitelee: the claims-independent fast path (`!HasRowFilter()`) already ensures high-fanout public streams without a row-filter pay no per-subscriber cost; for topics that do carry a row-filter, visibility is inherently per-connection (different JWT claims → different rows) so the per-subscriber evaluation can't be hoisted without losing correctness, and memoization by claim set would rarely hit since subscribers in a row-filtered bucket typically have distinct tenant claims (plus `map[string]any` claims aren't cheaply hashable). This tradeoff is intentional; don't flag it as a perf issue unless profiling on a real filtered-high-fanout topic shows it matters.

Applied to files:

• CHANGELOG.md

📚 Learning: 2026-08-12T21:45:38.018Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/pipes.ts:0-0
Timestamp: 2026-08-12T21:45:38.018Z
Learning: In the TypeScript SDK, `PipeRef.fetch` uses the exported `PipeRequestOptions` type rather than `Pick<RequestOptions, "signal">`. `PipeRequestOptions` declares `limit?: never` so both object literals and named `RequestOptions` values that include `limit` fail type checking instead of silently dropping the limit. A value declared as `RequestOptions` is intentionally not assignable to `PipeRequestOptions`, even if it has no runtime `limit`; consumers can use `PipeRequestOptions` for shared pipe, table, and query-builder fetch options, or use an inferred `{ signal }` object.

Applied to files:

• CHANGELOG.md
• docs/src/content/docs/sdk/index.mdx

📚 Learning: 2026-08-12T20:33:30.744Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/pipes.ts:28-28
Timestamp: 2026-08-12T20:33:30.744Z
Learning: In the TypeScript SDK, `PipeRef.fetch` in `clients/ts/src/pipes.ts` accepts only a signal option. Pipe row limits are not generic request options. A pipe SQL definition can declare a `{{limit}}` parameter, and callers provide that parameter through `wh.pipe(name, { limit })`. The API binds the pipe request body as pipe parameters through `pipes.BindParams` in `internal/api/pipes.go`.

Applied to files:

• CHANGELOG.md
• docs/src/content/docs/sdk/streaming.md

📚 Learning: 2026-08-11T21:56:03.206Z

Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: clients/go/query_builder.go:278-291
Timestamp: 2026-08-11T21:56:03.206Z
Learning: In `clients/go/query_builder.go`, `fetchNextTyped` intentionally treats a failed JSON decode of a non-object typed `Row` as normal end-of-pagination. This behavior matches the existing “cursor column was not in the projection” path and TypeScript SDK parity. The broader behavior change is tracked in GitHub issue `#452`.

Applied to files:

• CHANGELOG.md

🪛 LanguageTool

docs/src/content/docs/sdk/reference.md

[style] ~40-~40: A comma is missing here.
Context: ...OR| No | Stream could not be started (e.g. a non-absolutebaseURL) | | 0 | SSE_...

(EG_NO_COMMA)

---

[style] ~50-~50: Since ownership is already implied, this phrasing may be redundant.
Context: ...retryable after the SDK has exhausted its own attempts, so acting on it further is yo...

(PRP_OWN)

---

[style] ~52-~52: Consider an alternative for the overused word “exactly”.
Context: ...roxy. That silent-downgrade behavior is exactly why auth is re-read on every connecti...

(EXACTLY_PRECISELY)

docs/src/content/docs/sdk/streaming.md

[style] ~147-~147: Since ownership is already implied, this phrasing may be redundant.
Context: ...kfill seam), so key on timestamp plus your own row identity if duplicates matter. Rep...

(PRP_OWN)

---

[style] ~171-~171: Since ownership is already implied, this phrasing may be redundant.
Context: ...ed more of on this path; see Supplying your own fetch. ...

(PRP_OWN)

docs/src/content/docs/sdk/index.mdx

[style] ~419-~419: Since ownership is already implied, this phrasing may be redundant.
Context: ...row if it is set at all. See Supplying your own fetch for w...

(PRP_OWN)

---

[style] ~468-~468: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...used with .stream() or .liveQuery() needs a different set: .ok, .status, `.ty...

(EN_REPEATEDWORDS_NEED)

---

[style] ~573-~573: Since ownership is already implied, this phrasing may be redundant.
Context: ... one underlying reason: undici declares its own request/response types, separate from t...

(PRP_OWN)

---

[style] ~575-~575: Consider using the typographical ellipsis character here instead.
Context: ...the two aren't structurally assignable. { ...init, dispatcher } as never covers the ...

(ELLIPSIS)

---

[style] ~577-~577: ‘whether or not’ might be wordy. Consider a shorter alternative.
Context: ...ither spelling, so one snippet compiles whether or not your lib includes DOM); and the retur...

(EN_WORDINESS_PREMIUM_WHETHER_OR_NOT)

🔇 Additional comments (26)

clients/ts/src/stream/sse.test.ts (5)

75-212: LGTM!

---

226-294: LGTM!

---

326-369: LGTM!

Also applies to: 397-538

---

542-751: LGTM!

---

753-886: LGTM!

clients/ts/src/client.test.ts (1)

361-384: LGTM!

AGENTS.md (1)

63-63: LGTM!

CHANGELOG.md (1)

14-14: LGTM!

Also applies to: 28-31

README.md (1)

63-63: LGTM!

clients/ts/README.md (1)

3-11: LGTM!

Also applies to: 47-47, 134-134

docs/src/content/docs/api.md (1)

28-28: LGTM!

Also applies to: 586-587

docs/src/content/docs/getting-started.md (1)

108-108: LGTM!

docs/src/content/docs/index.mdx (1)

99-103: LGTM!

docs/src/content/docs/reverse-proxy.mdx (1)

180-180: LGTM!

docs/src/content/docs/sdk/index.mdx (1)

3-8: LGTM!

Also applies to: 57-58, 145-153, 319-333, 346-352, 397-425, 465-484, 494-582

docs/src/content/docs/sdk/reference.md (1)

28-49: LGTM!

Also applies to: 52-56, 103-103

docs/src/content/docs/sdk/streaming.md (1)

14-18: LGTM!

Also applies to: 74-86, 125-125, 129-172, 222-222

docs/src/content/docs/why-wavehouse.md (1)

157-157: LGTM!

tests/e2e/sdk/streaming.test.ts (1)

26-33: LGTM!

Also applies to: 135-165, 174-192

tests/e2e/sdk/vitest.config.ts (1)

45-47: LGTM!

clients/ts/package.json (1)

43-45: LGTM!

clients/ts/src/stream/sse.ts (2)

1-4: LGTM!

Also applies to: 13-102, 113-192, 194-320, 322-354, 360-409, 432-455, 467-528

---

355-359: 🗄️ Data Integrity & Integration

No change is requested for the existing CORS allowance, Fetch/preflight explanation, or FetchLike documentation and intentional string narrowing; these compatibility details should remain as documented.

clients/ts/src/types.ts (1)

15-16: LGTM!

Also applies to: 146-157, 166-189

clients/ts/src/http.ts (1)

34-40: LGTM!

clients/ts/src/client.ts (1)

103-105: LGTM!

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

tests/e2e/sdk/streaming.test.ts (1)

250-267: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Assert the insert results in the two new tests.

These insert() calls discard their Result. If a write fails, the following waitForCondition calls run to their 10s deadline and report a stream problem instead of the write failure. The same fix already landed at Lines 188-197 for the payload test. Apply it to the row-filter inserts here (Lines 252-253, 266-267) and to the metered inserts at Lines 313-314.

🧪 Proposed fix for the row-filter inserts

         const base = { page: "/scoped", user_id: "u", session_id: "s", duration_ms: 1 };
-        await inserter.from(T.clicks).insert({ ...base, event_id: usId, country: "US" });
-        await inserter.from(T.clicks).insert({ ...base, event_id: caId, country: "CA" });
+        for (const row of [
+          { ...base, event_id: usId, country: "US" },
+          { ...base, event_id: caId, country: "CA" },
+        ]) {
+          // A failed write would otherwise surface as a stream timeout below.
+          expect((await inserter.from(T.clicks).insert(row)).error).toBeNull();
+        }

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: fc2fd988-5069-4ad0-be0c-57eec5daeb0b

📥 Commits

Reviewing files that changed from the base of the PR and between 77feb9d and 297a6b2.

📒 Files selected for processing (20)

• AGENTS.md
• CHANGELOG.md
• README.md
• clients/ts/README.md
• clients/ts/src/http.test.ts
• clients/ts/src/http.ts
• clients/ts/src/stream/live-query.test.ts
• clients/ts/src/stream/live-query.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/stream/sse.ts
• clients/ts/src/types.ts
• docs/src/content/docs/api.md
• docs/src/content/docs/index.mdx
• docs/src/content/docs/reverse-proxy.mdx
• docs/src/content/docs/sdk/index.mdx
• docs/src/content/docs/sdk/queries.md
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/sdk/streaming.md
• internal/api/stream.go
• tests/e2e/sdk/streaming.test.ts

Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review.

📜 Review details

⏰ Context from checks skipped due to timeout. (2)

• GitHub Check: E2E tests
• GitHub Check: Coverage

🧰 Additional context used

📓 Path-based instructions (6)

**/*.md

📄 CodeRabbit inference engine (AGENTS.md)

• Comment the why, not the what. Add a comment only when the reason isn't obvious from the code; a line that matches the surrounding pattern needs none.

Files:

• docs/src/content/docs/sdk/queries.md
• README.md
• AGENTS.md
• CHANGELOG.md
• docs/src/content/docs/api.md
• clients/ts/README.md
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/sdk/streaming.md

**/*.{md,mdx}

📄 CodeRabbit inference engine (AGENTS.md)

Every code change should update the corresponding docs in the same PR. A code change without its doc update is incomplete.

Files:

• docs/src/content/docs/sdk/queries.md
• README.md
• docs/src/content/docs/index.mdx
• AGENTS.md
• CHANGELOG.md
• docs/src/content/docs/api.md
• clients/ts/README.md
• docs/src/content/docs/reverse-proxy.mdx
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/sdk/index.mdx
• docs/src/content/docs/sdk/streaming.md

clients/ts/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

The TypeScript SDK (@wavehouse/sdk in clients/ts/) is the canonical client and ships from this repo.

Files:

• clients/ts/src/http.test.ts
• clients/ts/src/stream/live-query.test.ts
• clients/ts/src/stream/live-query.ts
• clients/ts/src/http.ts
• clients/ts/src/types.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/stream/sse.ts

tests/e2e/sdk/*.test.ts

📄 CodeRabbit inference engine (AGENTS.md)

• E2E tests via SDK: The TypeScript SDK is the primary E2E test harness. Tests in tests/e2e/sdk/ exercise the full pipeline (ingest → ClickHouse → query) and simultaneously validate backend behavior and SDK correctness. Use make test-e2e to run. Add new E2E scenarios as tests/e2e/sdk/*.test.ts files using helpers from tests/e2e/sdk/helpers.ts.

Files:

• tests/e2e/sdk/streaming.test.ts

**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: - Go 1.26, strict formatting (gofumpt, enforced by CI)

• No global state: Dependencies are passed explicitly (constructor injection).
• Package naming: Lowercase, single word (or abbreviated). internal/ enforces module privacy.
  Column-level access control is a hard cap on every read path. A role's allow_columns/deny_columns is enforced against every column a structured query references (projection, aggregations, filters, group_by, order_by, time_range) inside query.Build, and a select_all request expands to the role's allowed columns rather than SELECT *

Files:

• internal/api/stream.go

internal/**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

• DRY — one source of truth. Before adding logic, look for an existing helper, type, or constant to reuse; before duplicating a rule, factor it into one place every caller reads.

Files:

• internal/api/stream.go

🧠 Learnings (6)

📚 Learning: 2026-06-10T15:01:09.027Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.

Applied to files:

• docs/src/content/docs/sdk/queries.md
• README.md
• AGENTS.md
• CHANGELOG.md
• docs/src/content/docs/api.md
• clients/ts/README.md
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/sdk/streaming.md

📚 Learning: 2026-08-11T15:22:20.507Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:20.507Z
Learning: In the TypeScript SDK, ensure `ClientConfig.baseURL` is validated or documented as requiring an absolute URL with both a scheme and host. Relative URLs cause `resolveURL` to throw a `TypeError` before REST retry handling runs. Ensure SSE connection failures are surfaced through `StreamSubscriber.error` and are not silently lost when that callback is absent.

Applied to files:

• clients/ts/src/http.test.ts
• clients/ts/src/stream/live-query.test.ts
• clients/ts/src/stream/live-query.ts
• clients/ts/src/http.ts
• clients/ts/src/types.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/stream/sse.ts

📚 Learning: 2026-08-12T21:45:38.018Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/pipes.ts:0-0
Timestamp: 2026-08-12T21:45:38.018Z
Learning: In the TypeScript SDK, use the exported `PipeRequestOptions` type for `PipeRef.fetch` and shared pipe, table, or query-builder fetch options rather than `Pick<RequestOptions, "signal">` or `RequestOptions`. `PipeRequestOptions` declares `limit?: never`, ensuring object literals and named `RequestOptions` values containing `limit` fail type checking instead of silently dropping it. When only a signal is needed, use `PipeRequestOptions` or an inferred `{ signal }` object; do not expect a `RequestOptions` value to be assignable to `PipeRequestOptions`.

Applied to files:

• clients/ts/src/http.test.ts
• clients/ts/src/stream/live-query.test.ts
• clients/ts/src/stream/live-query.ts
• clients/ts/src/http.ts
• clients/ts/src/types.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/stream/sse.ts

📚 Learning: 2026-08-13T12:17:52.620Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 470
File: docs/src/content/docs/reverse-proxy.mdx:137-144
Timestamp: 2026-08-13T12:17:52.620Z
Learning: For Wave-RF/WaveHouse documentation, verify claims about implementation control flow against the authoritative implementation source (for example, internal/auth/auth.go) rather than relying solely on docs/** content. Documentation may lag behind or paraphrase behavior, so control-flow claims should be confirmed in source code.

Applied to files:

• docs/src/content/docs/index.mdx
• docs/src/content/docs/reverse-proxy.mdx
• docs/src/content/docs/sdk/index.mdx

📚 Learning: 2026-07-07T12:38:12.052Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 378
File: internal/auth/auth.go:119-132
Timestamp: 2026-07-07T12:38:12.052Z
Learning: In this repo, do not add or recommend logging/tracing client IP addresses using naive or untrusted sources (e.g., `r.RemoteAddr` or directly trusting/deriving `X-Forwarded-For`) anywhere in the Go codebase. `middleware.RealIP` was removed due to IP-spoofing risks, and proper trusted-proxy-aware client-IP handling is intentionally deferred to issue `#333`. During code review, if proposed changes would record client IPs (including in audit paths such as `internal/auth/auth.go`), reject/redirect until `#333` lands with correct trusted-proxy configuration and safeguards.

Applied to files:

• internal/api/stream.go

📚 Learning: 2026-08-11T21:55:53.726Z

Learnt from: jfwoods
Repo: Wave-RF/WaveHouse PR: 434
File: clients/go/stream_test.go:159-176
Timestamp: 2026-08-11T21:55:53.726Z
Learning: For Go files in this repository, do not report direct type assertions solely because the `forcetypeassert` rule is commented out in `.golangci.yml`. Only flag a type assertion when there is an independent correctness, safety, or maintainability issue.

Applied to files:

• internal/api/stream.go

🪛 LanguageTool

CHANGELOG.md

[style] ~14-~14: Since ownership is already implied, this phrasing may be redundant.
Context: ...stream report measured ~450–465 ms, and our own runs against an instant-answering serve...

(PRP_OWN)

---

[grammar] ~14-~14: A noun may be missing here.
Context: ...mer's own tests without monkey-patching a global. fetch stays optional all the way thr...

(DT_JJ_NO_NOUN)

---

[style] ~14-~14: Since ownership is already implied, this phrasing may be redundant.
Context: ...nned by tests. Implementations shipping their own request/response declarations (undici, ...

(PRP_OWN)

---

[style] ~28-~28: Consider using a more formal/concise alternative here.
Context: ...nt an implementation throwing something other than a DOMException named AbortError — `...

(OTHER_THAN)

---

[style] ~28-~28: Since ownership is already implied, this phrasing may be redundant.
Context: ...raises aTimeoutError, node-fetchits own class — was reported asNETWORK_ERROR`...

(PRP_OWN)

---

[style] ~32-~32: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ... are pinned by tests. ### Changed - BREAKING (SDK): streaming moves from EventSource to fetch, so the JWT rides in a header instead of ?token= (clients/ts/src/stream/sse.ts, clients/ts/src/stream/sse.test.ts, clients/ts/src/client.ts, clients/ts/src/http.ts, clients/ts/src/types.ts, clients/ts/package.json, tests/e2e/sdk/vitest.config.ts, tests/e2e/sdk/package.json, tests/e2e/sdk/polyfills.ts (deleted), tests/e2e/sdk/streaming.test.ts, clients/ts/src/stream/live-query.ts, clients/ts/src/stream/live-query.test.ts (new), clients/ts/src/client.test.ts, clients/ts/src/http.test.ts, pnpm-lock.yaml, internal/api/stream.go (comment only), docs/src/content/docs/sdk/index.mdx, docs/src/content/docs/sdk/streaming.md, docs/src/content/docs/sdk/reference.md, docs/src/content/docs/api.md, docs/src/content/docs/reverse-proxy.mdx, and the six places the zero-dependency claim was retired — README.md, clients/ts/README.md, AGENTS.md (invariant 14), docs/src/content/docs/index.mdx, docs/src/content/docs/getting-started.md, docs/src/content/docs/why-wavehouse.md): advances #203 — tasks 1 and 2 of 3; #468 tracks retiring ?token= server-side, which closes it. .stream() and .liveQuery()'s live c...

(TOO_LONG_SENTENCE)

---

[style] ~32-~32: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: because pinning in a published library duplicates the package in any consumer tree already resolving a 3.x and freezes them out of patch and security releases until we cut one, while our own lockfile still governs CI, the SDK's first runtime dependency, costing ~3.3 KB minified / ~1.4 KB gzipped in the CDN IIFE bundle (measured by bundling the parser alone; the SDK's own IIFE grew more than that, but the rest is the transport rewrite). Renting it rather than hand-rolling i...

(TOO_LONG_SENTENCE)

---

[style] ~32-~32: Consider an alternative for the overused word “exactly”.
Context: ...nst a malformed or hostile stream — are exactly what a naive implementation gets wrong,...

(EXACTLY_PRECISELY)

---

[style] ~32-~32: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...duced an authenticated stream anyway. Without either, redirects are followed normally, so CDN canonicalization, geo/LB indirection, and an http→https upgrade all work — note the test is that concrete pair rather than "is this authenticated", so cookies are not covered — a cookie is re-derived from the store at each hop rather than carried, so under the default same-origin credentials mode a redirect keeps the stream authenticated only while the hop stays inside both the request's origin and the cookie's Path, and a hop outside either (a subdomain redirect, an http→https upgrade, a same-origin rewrite off the cookie's Path) leaves it silently unauthenticated; with credentials: "include" the origin half is replaced rather than lifted — ordinary cookie scoping decides, so a cookie crosses a CORS-allowed cross-origin hop only if its own Domain covers the target, and a host-only cookie (the default) does not (#478); options.fetch can override redirect for anyone who needs the credentialed case followed regardless. manual rather than error so the out...

(TOO_LONG_SENTENCE)

---

[style] ~32-~32: Since ownership is already implied, this phrasing may be redundant.
Context: ...r` callback, so a handler that swallows its own failures fails silently. Four paths sit...

(PRP_OWN)

---

[style] ~32-~32: ‘by accident’ might be wordy. Consider a shorter alternative.
Context: ...fix the proxy; both previously "worked" by accident. - **BREAKING (SDK): options.fetch, ...

(EN_WORDINESS_PREMIUM_BY_ACCIDENT)

---

[style] ~42-~42: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: ...ly either omitted or quoted as 18. - Live SSE events are now projected and serialized once per role instead of once per subscriber (internal/stream/hub.go (new), internal/stream/{subscriber,bucket,heartbeat,metrics,doc}.go, internal/api/stream.go, internal/api/hub.go + internal/api/transform.go (both removed — the broadcast hub moves to internal/stream, and the orphaned test-only transformForClient is dropped), cmd/wavehouse/main.go, docs/src/content/docs/architecture.md, AGENTS.md, plus tests in internal/stream/{hub,filter,subscriber,bucket,heartbeat}_test.go and internal/api/{stream,transform,router,errors}_test.go): the first PR of the SSE delivery-path throughput epic (#294), building on the internal/stream primitives from #346. The broadcast hub moves into `int...

(TOO_LONG_SENTENCE)

---

[style] ~42-~42: Since ownership is already implied, this phrasing may be redundant.
Context: ...id: timestamp) on the same event in its own read loop — byte-identical work repeate...

(PRP_OWN)

---

[uncategorized] ~44-~44: The official name of this software platform is spelled with a capital “H”.
Context: ...Cloudflare token to PR-authored code** (.github/workflows/ci.yml, `.github/workflows/h...

(GITHUB)

---

[uncategorized] ~44-~44: The official name of this software platform is spelled with a capital “H”.
Context: ...red code** (.github/workflows/ci.yml, .github/workflows/housekeeping.yml, `.github/a...

(GITHUB)

---

[uncategorized] ~44-~44: The official name of this software platform is spelled with a capital “H”.
Context: ..., .github/workflows/housekeeping.yml, .github/actions/setup-env/action.yml, `Makefil...

(GITHUB)

---

[uncategorized] ~44-~44: The official name of this software platform is spelled with a capital “H”.
Context: ...The architecture is documented once, in .github/workflows/README.md (DAG diagram, desi...

(GITHUB)

---

[uncategorized] ~48-~48: The official name of this software platform is spelled with a capital “H”.
Context: ...KFILE CI failures on dependency PRs** (.github/dependabot.yml, docs/src/content/docs...

(GITHUB)

---

[typographical] ~56-~56: Consider using an em dash in dialogues and enumerations.
Context: - **Live SSE streams now apply a role's r...

(DASH_RULE)

---

[style] ~56-~56: This sentence is over 40 words long. Consider splitting it up, as shorter sentences make the text easier to read.
Context: - Live SSE streams now apply a role's row-filter per subscriber, closing a query/stream row-level-security drift (internal/stream/hub.go, internal/stream/subscriber.go, internal/stream/metrics.go, internal/policy/policy.go, internal/policy/{rowfilter,canonical,numeric}.go (new — predicate evaluation, operand rendering, and storage-domain comparison as three focused files), internal/discovery/validation.go, internal/discovery/timestamp.go, internal/api/stream.go, cmd/wavehouse/main.go, docs/src/content/docs/access-control.mdx, docs/src/content/docs/architecture.md, docs/src/content/docs/api.md, docs/src/content/docs/sdk/streaming.md, AGENTS.md, SECURITY.md, internal/stream/doc.go, plus tests in internal/policy/rowfilter_test.go (new), tests/integration/rowfilter_narrowing_test.go (new), internal/policy/policy_test.go, internal/discovery/validation_test.go, internal/discovery/timestamp_test.go, internal/stream/hub_test.go, tests/e2e/sdk/streaming.test.ts): closes #319. The SSE delivery path stripped de...

(TOO_LONG_SENTENCE)

docs/src/content/docs/sdk/reference.md

[style] ~28-~28: Since ownership is already implied, this phrasing may be redundant.
Context: ... or .liveQuery(), described under If your own callback throws below. | Status | Cod...

(PRP_OWN)

---

[style] ~61-~61: Consider an alternative for the overused word “exactly”.
Context: ...roxy. That silent-downgrade behavior is exactly why auth is re-read on every connecti...

(EXACTLY_PRECISELY)

---

[style] ~65-~65: Since ownership is already implied, this phrasing may be redundant.
Context: ...cts rather than feeding it again. If your own callback throws. For anything deliver...

(PRP_OWN)

---

[style] ~65-~65: Since ownership is already implied, this phrasing may be redundant.
Context: ...r` callback, so a handler that swallows its own failures fails silently. **Wrap your h...

(PRP_OWN)

docs/src/content/docs/sdk/streaming.md

[style] ~162-~162: Since ownership is already implied, this phrasing may be redundant.
Context: ...sues/449)) — so key on timestamp plus your own row identity if duplicates matter. Rep...

(PRP_OWN)

---

[style] ~199-~199: ‘On top of that’ might be wordy. Consider a shorter alternative.
Context: ...ect. ### Client-Side Stream Filtering On top of that, when a QueryBuilder with .where() ...

(EN_WORDINESS_PREMIUM_ON_TOP_OF_THAT)

---

[style] ~295-~295: Since ownership is already implied, this phrasing may be redundant.
Context: ...ers — treat initial() never firing as its own failure. Where auth rejects and the s...

(PRP_OWN)

---

[typographical] ~297-~297: The word ‘Where’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...es auth or the URL, never the backfill. Re-run the fetch then; you never have t...

(WRB_QUESTION_MARK)

🔇 Additional comments (19)

clients/ts/src/http.test.ts (1)

170-191: LGTM!

Also applies to: 193-214, 216-232

clients/ts/src/stream/live-query.test.ts (1)

1-120: LGTM!

clients/ts/src/stream/live-query.ts (1)

64-77: LGTM!

clients/ts/src/types.ts (1)

13-21: LGTM!

Also applies to: 105-109, 127-127, 133-159, 163-211

clients/ts/src/http.ts (1)

40-60: LGTM!

Also applies to: 63-70, 150-182

docs/src/content/docs/reverse-proxy.mdx (1)

140-144: LGTM!

Also applies to: 180-180

docs/src/content/docs/sdk/queries.md (1)

154-154: LGTM!

docs/src/content/docs/sdk/reference.md (1)

28-28: LGTM!

Also applies to: 30-74, 121-121

docs/src/content/docs/sdk/streaming.md (1)

51-57: LGTM!

Also applies to: 96-105, 159-162, 172-176, 194-213, 257-322

internal/api/stream.go (1)

45-51: LGTM!

Also applies to: 83-89, 106-108

docs/src/content/docs/api.md (1)

244-245: LGTM!

Also applies to: 286-286, 295-295, 586-591

clients/ts/src/stream/sse.test.ts (1)

11-23: LGTM!

Also applies to: 84-91, 405-411, 469-579, 788-814, 1021-1118

tests/e2e/sdk/streaming.test.ts (1)

268-287: LGTM!

Also applies to: 289-322

AGENTS.md (1)

63-63: LGTM!

CHANGELOG.md (1)

14-14: LGTM!

Also applies to: 26-35, 132-132, 474-474

README.md (1)

63-63: LGTM!

clients/ts/README.md (1)

3-3: LGTM!

Also applies to: 11-11, 47-47, 134-134

docs/src/content/docs/index.mdx (1)

99-99: LGTM!

Also applies to: 133-133

docs/src/content/docs/sdk/index.mdx (1)

8-8: LGTM!

Also applies to: 144-153, 464-535, 564-564, 595-604

[EricAndrechek]

@coderabbitai Addressing the outside-diff-range comment on tests/e2e/sdk/streaming.test.ts — taken in df20d7dc, and widened.

You listed three sites (the row-filter inserts at 252-253 and 266-267, and the metered pair at 313-314). The same shape was in five: the two you didn't list are the public-stream insert and the timestamp-canonicalization insert, both of which also feed a 10s waitForCondition, so a failed write there produced exactly the misleading timeout you describe. All five now assert; the payload test you cited as precedent was already the sixth.

Fixing three of five would have left the same defect in a subset — a pattern this branch has had six instances of, so it seemed worth closing completely rather than to the letter of the comment.

Two notes on the mechanics:

• The paired inserts are wrapped in for...of with the await in the body, which is strictly sequential — so the metered test still writes duration_ms: 100 before 250. That ordering is the point of that test (the high row arriving proves the low row was withheld rather than merely late), so it mattered that the refactor preserve it.
• The assertion has teeth here: every single-row /v1/ingest rejection path returns non-2xx, so a failure lands in the Result error arm rather than a 200 carrying ok: false.

Deliberately not changed: the admin.policy.set() results discarded in beforeAll/afterAll (lines 58 and 64). Same class, and arguably worse — a beforeAll failure yields five misleading timeouts rather than one — but every e2e suite in the repo does this, so it's a suite-wide cleanup rather than something to ride in on a transport PR. Worth its own issue if it ever bites.

🤖 Fixed by Claude Code

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git show --stat --oneline df20d7dc
git show --format=fuller --find-renames -- clients/ts/../tests/e2e/sdk/streaming.test.ts df20d7dc 2>/dev/null || true

file="$(fd -a '^streaming\.test\.ts$' tests/e2e/sdk | head -n 1)"
if [ -z "$file" ]; then
  echo "streaming.test.ts was not found under tests/e2e/sdk" >&2
  exit 1
fi

echo "--- $file: relevant structure ---"
ast-grep outline "$file" --items all --type function --view compact || true
echo "--- $file: ingest calls and surrounding assertions ---"
rg -n -C 5 '(/v1/ingest|for\s*\(\s*const\s+.*\s+of)' "$file"

Length of output: 3166

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="$(fd -a '^streaming\.test\.ts$' tests/e2e/sdk | head -n 1)"

echo "--- df20d7dc diff for $file ---"
git show --format=fuller --find-renames df20d7dc -- "$file"

echo "--- all click insert call sites with context ---"
rg -n -C 4 'inserter\.from\(T\.clicks\)\.insert\s*\(' "$file"

Length of output: 7887

---

@EricAndrechek Thanks for the complete fix and the detail.

Verified df20d7dc. All five affected insert paths now assert that Result.error is null. The for...of loops preserve the required sequential write order for the metered test.

The unchanged admin.policy.set() calls are outside the scope of this transport PR. The finding is addressed.

---

✏️ Learnings added

Learnt from: EricAndrechek
URL: https://github.com/Wave-RF/WaveHouse/pull/470

Timestamp: 2026-08-18T13:46:01.151Z
Learning: In `tests/e2e/sdk/streaming.test.ts`, direct `/v1/ingest` writes that precede `waitForCondition` must assert that `Result.error` is `null`. A rejected single-row ingest returns a non-2xx response and otherwise appears as a misleading stream timeout. For paired inserts where event order is part of the test contract, use a `for...of` loop with `await` in the loop body to preserve sequential writes.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

---

If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

_{You are interacting with an AI system.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

clients/ts/src/stream/sse.ts (1)

319-328: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Require an exact SSE media type.

Line 320 accepts values such as text/event-streaming. That is not an SSE media type. The transport can mark that response as live and silently consume a non-SSE body.

Compare the normalized media type with text/event-stream by equality.

Proposed fix

-    if (!contentType.toLowerCase().split(";")[0].trim().startsWith("text/event-stream")) {
+    if (contentType.toLowerCase().split(";")[0].trim() !== "text/event-stream") {

docs/src/content/docs/reverse-proxy.mdx (1)

239-241: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Align the proxy examples with the large-NDJSON guidance.

The body-limit section at Line 106 says NDJSON can carry batches larger than 16 MiB. These examples set a 16 MiB limit for every request, so copied configurations reject those uploads before WaveHouse receives them. Raise or remove the outer limit for deployments that use large NDJSON, or state that these examples intentionally cap all uploads at 16 MiB.

Also applies to: 274-276

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 085a8be9-0e0e-4549-a275-2eb142487255

📥 Commits

Reviewing files that changed from the base of the PR and between 297a6b2 and 924d30d.

📒 Files selected for processing (15)

• AGENTS.md
• CHANGELOG.md
• clients/ts/README.md
• clients/ts/src/client.test.ts
• clients/ts/src/client.ts
• clients/ts/src/http.test.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/stream/sse.ts
• docs/src/content/docs/api.md
• docs/src/content/docs/getting-started.md
• docs/src/content/docs/reverse-proxy.mdx
• docs/src/content/docs/sdk/queries.md
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/why-wavehouse.md
• tests/e2e/sdk/streaming.test.ts

💤 Files with no reviewable changes (1)

• CHANGELOG.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Review details

⏰ Context from checks skipped due to timeout. (5)

• GitHub Check: Unit tests
• GitHub Check: Docs build
• GitHub Check: Coverage
• GitHub Check: Integration tests
• GitHub Check: E2E tests

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

14. TypeScript SDK — @wavehouse/sdk: typed query builder, real-time SSE over fetch, live queries (incrementable/decomposable/poll aggregation), codegen CLI. Exactly one runtime dependency — eventsource-parser (SSE framing, itself dependency-free); adding a second needs the same scrutiny the first got. The canonical client (see §SDK Sync).

Files:

• clients/ts/src/client.ts
• clients/ts/src/http.test.ts
• clients/ts/src/client.test.ts
• tests/e2e/sdk/streaming.test.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/stream/sse.ts

tests/e2e/sdk/*.test.ts

📄 CodeRabbit inference engine (AGENTS.md)

• Per-suite table isolation: Each e2e test file owns its own ClickHouse tables — clicks_<suite> / events_<suite> / users_<suite>, generated from tests/e2e/sdk/tables.ts and created by setup.ts. A new test file must (1) add its suite name to SUITES in tables.ts and (2) get its names via const T = suiteTables("<suite>"), then reference T.clicks etc. — never a bare clicks.

Files:

• tests/e2e/sdk/streaming.test.ts

🧠 Learnings (6)

📚 Learning: 2026-08-11T15:22:20.507Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 448
File: clients/ts/src/types.ts:61-71
Timestamp: 2026-08-11T15:22:20.507Z
Learning: In the TypeScript SDK, ensure `ClientConfig.baseURL` is validated or documented as requiring an absolute URL with both a scheme and host. Relative URLs cause `resolveURL` to throw a `TypeError` before REST retry handling runs. Ensure SSE connection failures are surfaced through `StreamSubscriber.error` and are not silently lost when that callback is absent.

Applied to files:

• clients/ts/src/client.ts
• clients/ts/src/http.test.ts
• clients/ts/src/client.test.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/stream/sse.ts

📚 Learning: 2026-08-12T21:45:38.018Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 456
File: clients/ts/src/pipes.ts:0-0
Timestamp: 2026-08-12T21:45:38.018Z
Learning: In the TypeScript SDK, use the exported `PipeRequestOptions` type for `PipeRef.fetch` and shared pipe, table, or query-builder fetch options rather than `Pick<RequestOptions, "signal">` or `RequestOptions`. `PipeRequestOptions` declares `limit?: never`, ensuring object literals and named `RequestOptions` values containing `limit` fail type checking instead of silently dropping it. When only a signal is needed, use `PipeRequestOptions` or an inferred `{ signal }` object; do not expect a `RequestOptions` value to be assignable to `PipeRequestOptions`.

Applied to files:

• clients/ts/src/client.ts
• clients/ts/src/http.test.ts
• clients/ts/src/client.test.ts
• clients/ts/src/stream/sse.test.ts
• clients/ts/src/stream/sse.ts

📚 Learning: 2026-06-10T15:01:09.027Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 312
File: docs/src/content/docs/development.md:0-0
Timestamp: 2026-06-10T15:01:09.027Z
Learning: In this repo’s Markdown review (all .md files), do not flag capitalization/style issues for literal paths starting with ".github/" (or any substring that is a path beginning with ".github/"). Treat ".github" as the correct lowercase dotfile directory name, even when it appears inside prose or code spans; automated checks such as LanguageTool’s "(GITHUB)" rule commonly produce false positives for this literal filesystem path.

Applied to files:

• AGENTS.md
• docs/src/content/docs/why-wavehouse.md
• docs/src/content/docs/sdk/queries.md
• docs/src/content/docs/getting-started.md
• docs/src/content/docs/sdk/reference.md
• docs/src/content/docs/api.md
• clients/ts/README.md

📚 Learning: 2026-08-18T13:46:01.151Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 0
File: :0-0
Timestamp: 2026-08-18T13:46:01.151Z
Learning: In `tests/e2e/sdk/streaming.test.ts`, direct `/v1/ingest` writes that precede `waitForCondition` must assert that `Result.error` is `null`. A rejected single-row ingest returns a non-2xx response and otherwise appears as a misleading stream timeout. For paired inserts where event order is part of the test contract, use a `for...of` loop with `await` in the loop body to preserve sequential writes.

Applied to files:

• tests/e2e/sdk/streaming.test.ts

📚 Learning: 2026-08-13T12:17:52.620Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 470
File: docs/src/content/docs/reverse-proxy.mdx:137-144
Timestamp: 2026-08-13T12:17:52.620Z
Learning: For Wave-RF/WaveHouse documentation, verify claims about implementation control flow against the authoritative implementation source (for example, internal/auth/auth.go) rather than relying solely on docs/** content. Documentation may lag behind or paraphrase behavior, so control-flow claims should be confirmed in source code.

Applied to files:

• docs/src/content/docs/reverse-proxy.mdx

📚 Learning: 2026-08-18T13:45:38.941Z

Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 470
File: clients/ts/src/stream/sse.ts:0-0
Timestamp: 2026-08-18T13:45:38.941Z
Learning: In `clients/ts/src/stream/sse.ts`, `SSETransport.disconnect()` must set `_closed` and then directly invoke `onStatus?.("closed")` inside its `if (!this._closed)` state-transition guard. It must not use `SSETransport._emitStatus()` for this terminal status because `_emitStatus()` suppresses callbacks after `_closed` is set. `StreamController` deduplicates repeated status values, so repeated transport-level `"closed"` callbacks do not cause duplicate subscriber cleanup, but `SSETransport.disconnect()` must still be idempotent independently.

Applied to files:

• clients/ts/src/stream/sse.ts

🪛 LanguageTool

docs/src/content/docs/api.md

[style] ~522-~522: Redundant conjunctions can lead to confusion; consider removing a conjunction here.
Context: ...meters can be supplied via query string and/or JSON body. Results are cached in the sh...

(AND_OR)

🔇 Additional comments (14)

clients/ts/src/http.test.ts (1)

79-79: LGTM!

Also applies to: 91-91, 103-103, 239-239, 264-266, 269-269, 277-277

clients/ts/src/client.ts (1)

98-105: LGTM!

AGENTS.md (1)

63-63: LGTM!

clients/ts/src/stream/sse.test.ts (1)

756-771: LGTM!

clients/ts/src/client.test.ts (1)

147-154: LGTM!

clients/ts/README.md (1)

3-11: LGTM!

Also applies to: 47-47, 134-134, 153-153

docs/src/content/docs/api.md (1)

8-8: LGTM!

Also applies to: 26-28, 32-32, 190-195, 262-269, 286-295, 382-400, 448-451, 503-505, 522-539, 588-591, 606-610, 612-613, 633-635, 659-661, 685-687, 718-724, 758-760, 788-788, 834-834

docs/src/content/docs/getting-started.md (1)

50-50: LGTM!

Also applies to: 76-76, 98-98, 108-108

docs/src/content/docs/reverse-proxy.mdx (1)

21-21: LGTM!

Also applies to: 95-98, 135-144, 180-180, 203-205

docs/src/content/docs/sdk/queries.md (1)

72-76: LGTM!

Also applies to: 158-158

docs/src/content/docs/sdk/reference.md (1)

28-48: LGTM!

Also applies to: 50-61, 63-74, 121-121

docs/src/content/docs/why-wavehouse.md (1)

157-157: LGTM!

tests/e2e/sdk/streaming.test.ts (2)

87-95: LGTM!

Also applies to: 127-134, 159-219, 254-261, 274-279, 325-330

---

48-55: 🗄️ Data Integrity & Integration

No change needed: the policy uses T.events, and streaming is registered in SUITES.

			> Likely an incorrect or invalid review comment.