feat(config): hot-reload whitelisted fields via SIGHUP or admin endpoint - #414
feat(config): hot-reload whitelisted fields via SIGHUP or admin endpoint#414taitelee wants to merge 13 commits into
Conversation
github-actions
Bot
added
documentation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughWaveHouse adds a SQLite control-plane database for policies and pipes. It adds persistent runtime dedupe settings with an authenticated admin API, validation, revisions, reset support, and live ingest application. Boot configuration rejects retired dedupe keys. ChangesControl-plane persistence
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant AdminAPI
participant SettingsStore
participant Ingest
Operator->>AdminAPI: Update dedupe settings
AdminAPI->>SettingsStore: Validate and persist revision
SettingsStore-->>AdminAPI: Return updated settings
Ingest->>SettingsStore: Read request snapshot
SettingsStore-->>Ingest: Apply dedupe configuration
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
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. Comment |
|
📚 Docs preview is live → https://b7d3fe7c-wavehouse-docs.wave-rf.workers.dev |
Code Coverage OverviewLanguages: Go GoThe overall coverage in commit 4ae84cb in the Show a code coverage summary of the most impacted files.
Updated |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0331ca52-0303-49a0-a1ee-a4b2f2142706
📒 Files selected for processing (13)
CHANGELOG.mdcmd/wavehouse/main.goconfig.yamldocs/src/content/docs/api.mddocs/src/content/docs/configuration.mdxinternal/api/config_reload.gointernal/api/config_reload_test.gointernal/api/ingest.gointernal/api/ingest_test.gointernal/api/router.gointernal/api/router_test.gointernal/config/reload.gointernal/config/reload_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: E2E tests
- GitHub Check: Unit tests
- GitHub Check: Coverage
- GitHub Check: Integration tests
- GitHub Check: Docs build
- GitHub Check: Lint
🧰 Additional context used
📓 Path-based instructions (4)
config.yaml
📄 CodeRabbit inference engine (AGENTS.md)
Keep the sample configuration file in sync with configuration fields and defaults.
Files:
config.yaml
internal/api/router.go
📄 CodeRabbit inference engine (AGENTS.md)
Route registrations in
router.gomust stay in sync with the API docs and middleware/dependency wiring.
Files:
internal/api/router.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Write tests in table-driven form witht.Run(tt.name, ...)for multiple cases.
Use shared mocks frominternal/testutil/instead of ad-hoc mocks in tests.
Use the repo’s JWT, schema, policy, pipes, and JSON response test helpers (testutil.MakeJWT,testutil.MakeExpiredJWT,NewTestSchemaRegistry,policy.NewMemoryStore,pipes.NewMemoryStore,AssertJSONResponse,AssertJSONContains) where applicable.
Every new function should have corresponding test cases, and new code should aim for 80%+ coverage.
Files:
internal/api/router_test.gointernal/api/config_reload_test.gointernal/config/reload_test.gointernal/api/ingest_test.go
docs/src/content/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Documentation prose under the Starlight docs site must stay accurate against code, include runnable examples where relevant, and reflect code↔docs sync for changed behavior.
Files:
docs/src/content/docs/configuration.mdxdocs/src/content/docs/api.md
🧠 Learnings (5)
📚 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/config_reload.gointernal/config/reload.gointernal/api/router.gointernal/api/router_test.gointernal/api/config_reload_test.gocmd/wavehouse/main.gointernal/config/reload_test.gointernal/api/ingest.gointernal/api/ingest_test.go
📚 Learning: 2026-05-20T01:02:00.784Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 164
File: internal/api/router_test.go:289-350
Timestamp: 2026-05-20T01:02:00.784Z
Learning: In WaveHouse’s internal API tests (files matching internal/api/**/*_test.go), follow the existing separation-of-concerns convention for testing the RequireRole middleware: inject `ContextKeyRole` directly into the request `context.Context` instead of using `testutil.MakeJWT`/JWT-driven flows. Do not refactor role-gate tests to use JWT tokens—JWT parsing and token handling are covered separately in `middleware_test.go` (the dedicated JWT parsing tests), and mixing those concerns would expand the failure surface and reduce isolation.
Applied to files:
internal/api/router_test.gointernal/api/config_reload_test.gointernal/api/ingest_test.go
📚 Learning: 2026-05-23T01:23:59.268Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 174
File: internal/api/ingest_test.go:111-111
Timestamp: 2026-05-23T01:23:59.268Z
Learning: In WaveHouse Go tests in internal/api/**/*_test.go, use internal/testutil.AssertJSONErrorResponse(t, w) for HTTP error-path JSON assertions. Do not use (or reintroduce) package-local assertJSONErrorResponse helpers. AssertJSONErrorResponse verifies the response Content-Type is application/json, includes the X-Content-Type-Options: nosniff header, and that the JSON body contains an "error" field.
Applied to files:
internal/api/router_test.gointernal/api/config_reload_test.gointernal/api/ingest_test.go
📚 Learning: 2026-06-26T12:23:22.696Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 346
File: internal/stream/subscriber_test.go:9-28
Timestamp: 2026-06-26T12:23:22.696Z
Learning: In this Go repository, prefer table-driven tests (e.g., `[]struct{...}` with `t.Run(...)`) only for tests that cover multiple scenarios/inputs and can be cleanly enumerated. Do not artificially rewrite a clear single-scenario sequential behavioral-flow test into a table-driven form just to fit the pattern; if there’s only one meaningful scenario, keep the test as a straightforward linear flow (as in `TestSubscriber_SendDeliversThenDropsWhenFull`).
Applied to files:
internal/api/router_test.gointernal/api/config_reload_test.gointernal/config/reload_test.gointernal/api/ingest_test.go
📚 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:
CHANGELOG.mddocs/src/content/docs/api.md
🔇 Additional comments (17)
CHANGELOG.md (1)
14-14: LGTM!config.yaml (1)
71-72: LGTM!docs/src/content/docs/api.md (1)
749-765: LGTM!docs/src/content/docs/configuration.mdx (1)
155-156: LGTM!internal/config/reload.go (2)
75-94: LGTM!
24-47: LGTM!Also applies to: 96-111
internal/config/reload_test.go (2)
15-93: LGTM!
111-128: 🎯 Functional CorrectnessNo action needed
go.modalready pins Go 1.26.5, and CI reads that file viago-version-file, so therangeform is supported.> Likely an incorrect or invalid review comment.internal/api/ingest.go (1)
38-80: LGTM!Also applies to: 427-441
internal/api/ingest_test.go (2)
532-563: LGTM!
166-166: LGTM!Also applies to: 182-182, 498-498, 517-517, 815-815
cmd/wavehouse/main.go (1)
341-366: LGTM!Also applies to: 422-422
internal/api/config_reload.go (1)
1-32: LGTM!internal/api/router.go (2)
189-193: 📐 Maintainability & Code QualityConfirm docs/api.md documents the new endpoint.
The route wiring itself is correct and admin-gated, but per repo guidelines router changes should stay in sync with the API docs. Docs aren't in this review batch — please confirm
docs/src/content/docs/api.mddocumentsPOST /v1/admin/config/reload.As per coding guidelines: "Route registrations in
router.gomust stay in sync with the API docs and middleware/dependency wiring."Source: Coding guidelines
31-31: LGTM!internal/api/config_reload_test.go (1)
15-62: LGTM!internal/api/router_test.go (1)
402-441: LGTM!
github-project-automation
Bot
moved this from In progress
to In review
in WaveHouse Task Board
…upe snapshot; single apply
github-actions
Bot
added
the
dependencies
|
|
…HotFields; sync docs
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2375a584-6be3-40d8-93cc-0bf0c6e42640
📒 Files selected for processing (16)
AGENTS.mdCHANGELOG.mdCONTRIBUTING.mdcmd/wavehouse/hotfields_test.gocmd/wavehouse/main.godocs/src/content/docs/access-control.mdxdocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/development.mdinternal/api/ingest.gointernal/config/config.gointernal/config/config_test.gointernal/config/reload.gointernal/config/reload_test.go
💤 Files with no reviewable changes (1)
- cmd/wavehouse/main.go
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: Unit tests
- GitHub Check: Coverage
- GitHub Check: Docs build
- GitHub Check: E2E tests
- GitHub Check: Integration tests
- GitHub Check: Lint
- GitHub Check: Analyze (go)
🧰 Additional context used
📓 Path-based instructions (9)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Agents must create draft PRs only, use a Conventional Commits title of at most 72 characters, and validate titles with
scripts/lint-pr-title.sh.
Files:
docs/src/content/docs/deployment.mdcmd/wavehouse/hotfields_test.gointernal/config/config.gointernal/config/config_test.godocs/src/content/docs/access-control.mdxCONTRIBUTING.mddocs/src/content/docs/architecture.mdAGENTS.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/api.mdCHANGELOG.mdinternal/config/reload_test.gointernal/config/reload.godocs/src/content/docs/development.mdinternal/api/ingest.go
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Documentation prose must remain accurate, clear, complete, and runnable; documentation review covers tracked Markdown/MDX files within the documented scope.
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/access-control.mdxCONTRIBUTING.mddocs/src/content/docs/architecture.mdAGENTS.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/api.mdCHANGELOG.mddocs/src/content/docs/development.md
docs/src/content/docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Mermaid diagrams should default to top-down layouts, avoid large side-by-side diagrams, keep labels short, and use semantic classes and theme variables.
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/access-control.mdxdocs/src/content/docs/architecture.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/api.mddocs/src/content/docs/development.md
docs/src/content/docs/{api.md,configuration.mdx,architecture.md,deployment.md,development.md}
📄 CodeRabbit inference engine (AGENTS.md)
Update the corresponding documentation files when changing endpoints, configuration, architecture, ingest formats, deployment, or build/test processes, and add notable changes under
[Unreleased]inCHANGELOG.md.
Files:
docs/src/content/docs/deployment.mddocs/src/content/docs/architecture.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/api.mddocs/src/content/docs/development.md
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Use Go 1.26, format code strictly withgofumpt, return wrapped errors instead of panicking, pass dependencies explicitly, and use lowercase single-word package names.
Use structured logging throughlog/slogwith the repository's JSON-handler conventions.
Files:
cmd/wavehouse/hotfields_test.gointernal/config/config.gointernal/config/config_test.gointernal/config/reload_test.gointernal/config/reload.gointernal/api/ingest.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Use table-driven tests with
t.Run, shared mocks and test helpers frominternal/testutil, and add corresponding tests for every new function.
Files:
cmd/wavehouse/hotfields_test.gointernal/config/config_test.gointernal/config/reload_test.go
cmd/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Keep binary entry points thin: wire dependencies and boot behavior in
cmd/wavehouse, while implementation remains in internal packages.
Files:
cmd/wavehouse/hotfields_test.go
internal/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Keep internal package behavior interface-first where multiple implementations are expected, and preserve the documented security and correctness invariants for auth, policy, query, pipes, ingest, streaming, and observability packages.
Files:
internal/config/config.gointernal/config/config_test.gointernal/config/reload_test.gointernal/config/reload.gointernal/api/ingest.go
internal/api/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
All
/v1routes must use always-on bearer-token authentication; preserve fail-closed authorization, bearer-only CORS withoutAccess-Control-Allow-Credentials, and the documented health endpoints and aliases.
Files:
internal/api/ingest.go
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-26T22:57:50.929Z
Learning: Run `make ci` locally before every push, using the documented background invocation with output redirected to `tmp/ci.log`; do not use CI as the first feedback loop.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-26T22:57:50.929Z
Learning: On PR branches, run every applicable reviewer through `/prepush` and obtain a `ship_it` marker, or record an explicit skip for genuinely out-of-scope reviewers, before pushing.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-26T22:57:50.929Z
Learning: Every code change must update corresponding documentation and `CHANGELOG.md` in the same PR.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-26T22:57:50.929Z
Learning: Address every review finding with a substantive response, a fix or tracking issue, and thread resolution before merge.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-26T22:57:50.929Z
Learning: Never force-push or rebase PR branches; merge `origin/main` instead.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-26T22:57:50.929Z
Learning: Do not hand-write review or CI markers and do not use `--no-verify` to bypass gates.
📚 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/deployment.mdCONTRIBUTING.mddocs/src/content/docs/architecture.mdAGENTS.mddocs/src/content/docs/api.mdCHANGELOG.mddocs/src/content/docs/development.md
📚 Learning: 2026-06-26T12:23:22.696Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 346
File: internal/stream/subscriber_test.go:9-28
Timestamp: 2026-06-26T12:23:22.696Z
Learning: In this Go repository, prefer table-driven tests (e.g., `[]struct{...}` with `t.Run(...)`) only for tests that cover multiple scenarios/inputs and can be cleanly enumerated. Do not artificially rewrite a clear single-scenario sequential behavioral-flow test into a table-driven form just to fit the pattern; if there’s only one meaningful scenario, keep the test as a straightforward linear flow (as in `TestSubscriber_SendDeliversThenDropsWhenFull`).
Applied to files:
cmd/wavehouse/hotfields_test.gointernal/config/config_test.gointernal/config/reload_test.go
📚 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:
cmd/wavehouse/hotfields_test.gointernal/config/config.gointernal/config/config_test.gointernal/config/reload_test.gointernal/config/reload.gointernal/api/ingest.go
🪛 LanguageTool
docs/src/content/docs/development.md
[style] ~141-~141: Consider using the typographical ellipsis character here instead.
Context: ... air -c .air.toml ``` deps-up runs `docker compose ... up -d --wait clickhouse`, which blocks ...
(ELLIPSIS)
🔇 Additional comments (15)
CHANGELOG.md (1)
14-14: LGTM!docs/src/content/docs/api.md (1)
749-765: LGTM!docs/src/content/docs/architecture.md (1)
72-72: LGTM!Also applies to: 108-108, 204-204
docs/src/content/docs/configuration.mdx (1)
30-37: LGTM!Also applies to: 49-49, 159-160
docs/src/content/docs/deployment.md (1)
116-117: LGTM!docs/src/content/docs/development.md (1)
141-141: LGTM!Also applies to: 150-150
docs/src/content/docs/access-control.mdx (1)
65-65: LGTM!AGENTS.md (1)
307-307: LGTM!Also applies to: 316-316, 366-366
CONTRIBUTING.md (1)
46-46: LGTM!internal/config/config.go (1)
308-313: LGTM!internal/config/config_test.go (1)
80-90: LGTM!internal/config/reload.go (1)
4-6: LGTM!Also applies to: 59-61
internal/config/reload_test.go (1)
90-110: LGTM!Also applies to: 112-127
internal/api/ingest.go (1)
12-12: LGTM!Also applies to: 38-54, 66-82, 189-193, 214-217, 233-233, 245-245, 282-282, 311-311, 389-389, 437-462
cmd/wavehouse/hotfields_test.go (1)
1-27: LGTM!
github-project-automation
Bot
moved this from In progress
to In review
in WaveHouse Task Board
github-actions
Bot
added
area/policy
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/policy/store.go (1)
88-104: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCache swap is not ordered against the database commit in both control-plane stores. Each store commits its transaction outside the mutex and then swaps its in-memory cache under a separate lock. SQLite serializes the commits, because the pool is capped at one connection, but the cache swaps can happen in the opposite order. The cache then serves the losing document while the database holds the winning one, and the divergence lasts until the next full load at construction time.
internal/policy/store.go#L88-L104: add awriteMu sync.Mutexfield toStoreand take it at the start ofPut, sopersistand thes.cachedswap run as one critical section.internal/pipes/pipes.go#L106-L112: add the samewriteMufield and take it at the start of bothPutandDelete, so each database write and its matching cache mutation stay ordered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 779a3684-9c6e-4cc4-aded-7be5dcecb681
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (47)
AGENTS.mdCHANGELOG.mdREADME.mdcmd/wavehouse/main.goconfig.yamldeployments/Dockerfiledeployments/compose/dev-policy.yamldeployments/compose/standalone.yamldocs/src/content/docs/access-control.mdxdocs/src/content/docs/api.mddocs/src/content/docs/architecture.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/deployment.mddocs/src/content/docs/development.mddocs/src/content/docs/getting-started.mddocs/src/content/docs/index.mdxdocs/src/content/docs/pipes.mdxdocs/src/content/docs/sdk/admin.mddocs/src/content/docs/why-wavehouse.mdgo.modinternal/api/ingest.gointernal/api/ingest_test.gointernal/api/router.gointernal/api/router_test.gointernal/api/settings.gointernal/api/settings_test.gointernal/config/config.gointernal/config/config_test.gointernal/config/retired.gointernal/controldb/controldb.gointernal/controldb/controldb_test.gointernal/controldb/migrations/001_control_plane.sqlinternal/pipes/pipes.gointernal/pipes/pipes_test.gointernal/pipes/store_db_test.gointernal/pipes/store_kv_test.gointernal/policy/store.gointernal/policy/store_test.gointernal/settings/disjoint.gointernal/settings/disjoint_test.gointernal/settings/settings.gointernal/settings/settings_test.gointernal/settings/store.gointernal/settings/store_test.gointernal/testutil/natsjs.gotests/e2e/fixtures/config.yamltests/e2e/sdk/admin.test.ts
💤 Files with no reviewable changes (2)
- internal/pipes/store_kv_test.go
- internal/testutil/natsjs.go
📜 Review details
⏰ Context from checks skipped due to timeout. (5)
- GitHub Check: E2E tests
- GitHub Check: Unit tests
- GitHub Check: Coverage
- GitHub Check: Integration tests
- GitHub Check: Lint
🧰 Additional context used
📓 Path-based instructions (20)
**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Markdown files: use
make lintto check spelling (misspell), style (markdownlint); accuracy/clarity/doc-sync checked bydocs-reviewergate
Files:
README.mddocs/src/content/docs/index.mdxdocs/src/content/docs/getting-started.mddocs/src/content/docs/pipes.mdxdocs/src/content/docs/why-wavehouse.mddocs/src/content/docs/access-control.mdxCHANGELOG.mddocs/src/content/docs/configuration.mdxdocs/src/content/docs/architecture.mddocs/src/content/docs/api.mdAGENTS.mddocs/src/content/docs/sdk/admin.mddocs/src/content/docs/deployment.mddocs/src/content/docs/development.md
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (AGENTS.md)
Mermaid diagrams in docs: author vertically (flowchart TB/TD) so they fit the column width at legible size; default to top-down, reserve LR for short chains (≤3–4 nodes); never sit two large diagrams side-by-side; keep node labels short with
<br/>for second line; use semantic node classes (wh,win,pain,fail,infra,neutral,store,client) and--wh-mermaid-*vars; diagrams are click-to-zoom so illegibility is recoverable fallback not license to ship illegible inline diagram
Files:
docs/src/content/docs/index.mdxdocs/src/content/docs/getting-started.mddocs/src/content/docs/pipes.mdxdocs/src/content/docs/why-wavehouse.mddocs/src/content/docs/access-control.mdxdocs/src/content/docs/configuration.mdxdocs/src/content/docs/architecture.mddocs/src/content/docs/api.mddocs/src/content/docs/sdk/admin.mddocs/src/content/docs/deployment.mddocs/src/content/docs/development.md
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Go code: use Go 1.26 with strict formatting viagofumpt
Go code: use structured logging withlog/slog(JSON handler)
Go code: return errors, never panic; wrap withfmt.Errorf("context: %w", err)
Go code: no global state; pass dependencies explicitly via constructor injection
Go code: package naming must be lowercase, single word (or abbreviated);internal/enforces module privacy
Every new Go function must have corresponding test cases; runmake testbefore considering work complete
Files:
internal/pipes/pipes_test.gointernal/api/settings.gointernal/settings/settings.gointernal/api/router.gointernal/api/router_test.gointernal/settings/settings_test.gointernal/config/config_test.gointernal/config/config.gointernal/settings/disjoint.gointernal/settings/disjoint_test.gointernal/controldb/controldb.gointernal/api/ingest.gointernal/settings/store.gocmd/wavehouse/main.gointernal/pipes/store_db_test.gointernal/api/settings_test.gointernal/controldb/controldb_test.gointernal/pipes/pipes.gointernal/settings/store_test.gointernal/policy/store.gointernal/policy/store_test.gointernal/config/retired.gointernal/api/ingest_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Use table-driven tests withtests := []struct{ name string; ... }andt.Run(tt.name, ...)for test cases
Use shared mocks frominternal/testutil/(MockPublisher, MockCache, MockDeduplicator, MockSubscriber) instead of ad-hoc mocks
Aim for 80%+ coverage on new Go code; project-wide CI-enforced minimum is 80% (merged unit + integration + e2e); per-suite minima: unit 80%, integration 20%, e2e 60%, sdk 50%
Files:
internal/pipes/pipes_test.gointernal/api/router_test.gointernal/settings/settings_test.gointernal/config/config_test.gointernal/settings/disjoint_test.gointernal/pipes/store_db_test.gointernal/api/settings_test.gointernal/controldb/controldb_test.gointernal/settings/store_test.gointernal/policy/store_test.gointernal/api/ingest_test.go
**/pipes/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Use
pipes.NewMemoryStore(queries...)for in-memory pipes testing without the control db
Files:
internal/pipes/pipes_test.gointernal/pipes/store_db_test.go
internal/pipes/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Named query pipes: fail-closed (security); pre-defined SQL templates with param binding + caching;
GET/POST /v1/pipes/{name}sits outsideRequireAdmin, so per-pipeallowed_rolesis only execute-path gate viapolicy.RoleAllowed: exact allowlist membership (no "*"), admin always passes, empty/absent role and empty-string entries authorize nobody, noallowed_roles→ admin-only
Files:
internal/pipes/pipes_test.gointernal/pipes/store_db_test.gointernal/pipes/pipes.go
internal/{auth,policy,query,ingest,controldb,pipes}/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Prefer using existing named invariants (indexed in
AGENTS.mdwith stable numbers) when commenting critical code behavior; preserve the invariant's number and name when it's touched
Files:
internal/pipes/pipes_test.gointernal/controldb/controldb.gointernal/pipes/store_db_test.gointernal/controldb/controldb_test.gointernal/pipes/pipes.gointernal/policy/store.gointernal/policy/store_test.go
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript/JavaScript: format with Biome (enforced in
make verify)
Files:
tests/e2e/sdk/admin.test.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use
runtools/tests-module-from-lockfile(Biome + tsc) for static checks on TypeScript code
Files:
tests/e2e/sdk/admin.test.ts
tests/e2e/sdk/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
tests/e2e/sdk/**/*.test.ts: E2E tests via SDK intests/e2e/sdk/: each test file owns its own ClickHouse tables (clicks_, events_, users_); add suite name toSUITESintables.tsand get names viaconst T = suiteTables("<suite>")
E2E tests: run sequentially (maxWorkers: 1invitest.config.ts) because they share global policy state; policy-mutating tests must snapshot and restore the full policy document
Files:
tests/e2e/sdk/admin.test.ts
**/*.{js,ts,tsx,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Biome linting (
make lint/make fix) owns JS/TS/JSON; markdownlint owns Markdown style; misspell owns spelling — all undermake lint/make fix
Files:
tests/e2e/sdk/admin.test.ts
internal/api/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/api/**/*.go: Go code: use Chi v5 for HTTP routing
Handler error responses must agree with error tables indocs/src/content/docs/api.md
Files:
internal/api/settings.gointernal/api/router.gointernal/api/router_test.gointernal/api/ingest.gointernal/api/settings_test.gointernal/api/ingest_test.go
internal/api/router.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/api/router.go: Bearer-token-only CORS posture (security): Bearer JWT on every request, no cookies/sessions;corsMiddlewaredeliberately never emitsAccess-Control-Allow-Credentials(not needed,*+ credentials is spec violation browsers reject).cors_allowed_originscontrols read-response access, not cookie scope; CSRF protection is structural. Never reintroduce cookie auth orAllow-Credentialswithout design discussion.
Health endpoints: liveness/livez, readiness/readyz(k8s convention);/healthzis permanent alias of/livez;/health+/readydeprecated (removal v0.2.0, CHANGELOG#144)./v1/healthis SDK public ping (no CH check),/v1route survives reverse-proxy probe filtering. Point k8s at/livez//readyz, SDK/online-checks at/v1/health, never deprecated aliases.
Route registrations ininternal/api/router.gomust agree with endpoint list indocs/src/content/docs/api.md
Go code ininternal/api/router.go: never reintroduce cookie auth,Access-Control-Allow-Credentials, or*+ credentials CORS posture without design discussion
Files:
internal/api/router.go
**/api/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Use
testutil.AssertJSONResponse(t, rec, status, expected)andtestutil.AssertJSONContains(t, rec, status, substring)for HTTP handler assertions
Files:
internal/api/router_test.gointernal/api/settings_test.gointernal/api/ingest_test.go
**/settings/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Use
settings.NewMemoryStore(values)for in-memory runtime-settings testing without the control db
Files:
internal/settings/settings_test.gointernal/settings/disjoint_test.gointernal/settings/store_test.go
internal/config/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/config/**/*.go: Two-tier config: a knob lives in exactly one tier — boot config (internal/config: yaml +WH_*env, read once at boot, restart to change) vs. runtime settings (internal/settings: compiled defaults + control-db overrides via/v1/admin/settings, applied live). Disjointness enforced:settings/disjoint.gopanics at init on clash or settings field withyaml/envtags;config.Loadrefuses boot on retired key. Never give runtime setting env form, never re-declare knob in both tiers, promote field between tiers as explicit refactor. No SIGHUP, no config reload endpoint — edits apply on restart.
Do not expose settings config as an env var if it already exists in boot config; usesettings/disjoint.gochecks to enforce disjointness
Files:
internal/config/config_test.gointernal/config/config.gointernal/config/retired.go
internal/config/config.go
📄 CodeRabbit inference engine (AGENTS.md)
Config struct tags in
internal/config/config.gomust agree with docs indocs/src/content/docs/configuration.mdx,config.yaml, and compose env blocks
Files:
internal/config/config.go
internal/controldb/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
Control plane lives in SQLite; integrity lives in schema — policy, pipes, settings persist in
<data_dir>/control.db(internal/controldb, STRICT tables,foreign_keys=ON, WAL, single connection). Every rule SQL can express is constraint not convention: predicate operator whitelist, insert-check operator restriction (#224), dedupe require-without-field CHECK, role foreign keys (parts cascade, dependencies refuse). Admin writes single transactions; stores keep in-memory snapshots; settings writes revision-guarded (If-Match→ conditional UPDATE, 0 rows → 409 on lost race). Tests use same engine viacontroldb.MustOpenMemory— no fake storage mode. NATS remains data plane (ingest streams, DLQ, SSE).
Files:
internal/controldb/controldb.gointernal/controldb/controldb_test.go
internal/policy/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
internal/policy/**/*.go: Hasura-style access control: fail-closed (security);policy.IsAdmin(role ==admin_role, exact case-sensitive, default "admin") is the single admin check; empty/absent role matches nothing;nilpolicy (deleted) denies everyone incl. admin; bootstrap from policy file, never implicit admin grant. Operator keyauth.IsOperatorbit passes/v1/admingate undernilpolicy (break-glass restore).default_roleis sanctioned roleless exception;default_role == admin_rolepermitted but dev-only and loudly warned.
Go code ininternal/policy/: preserve thepolicy.IsAdminsingle-check invariant (role ==admin_role, exact case-sensitive, default "admin"); preservepolicy.IsColumnAllowedsingle decision for both structured and live-stream reads
Files:
internal/policy/store.gointernal/policy/store_test.go
**/policy/**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
Use
policy.NewMemoryStore(p)for in-memory policy testing without the control db
Files:
internal/policy/store_test.go
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Validate locally before every push using `make ci` as documented, not using CI as the first feedback loop
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Run `/prepush` before pushing a PR branch to ensure every pre-push reviewer listed in `scripts/pre-push-reviewers.sh` returns `ship_it` or is deliberately skipped with logging
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Every code change must update corresponding documentation and `CHANGELOG.md` in the same PR
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Address and resolve every review finding with a substantive reply, fix it or track in an issue, `@-mention` the reviewer, then resolve the thread
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Create PRs with `gh pr create --draft` (never `gh pr ready`/approve); PR title must pass Conventional Commits gate (≤72 chars, lowercase-first, no trailing period), validate with `scripts/lint-pr-title.sh`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Never force-push or rebase a PR branch; use `git merge origin/main` to absorb upstream commits
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Never hand-write review markers or use `--no-verify`; if tempted, the gate is wrong-shaped for the situation — fix the gate instead
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Comment the *why*, not the *what*; keep comments to 1–2 lines; re-read and cut any that merely restate the code
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: DRY — maintain one source of truth; before duplicating logic or rules, factor into one place every caller reads
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Leave it neater than you found it — fix small, safe things in passing (stale comment, typo, misnamed local, dead code) proportional to your change
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Interface-first design: core behaviors are Go interfaces (Cache, Deduplicator, Publisher, Subscriber); standalone vs. future-clustered swap implementations
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Schema-driven ingest: `POST /v1/ingest?table={table}` takes flat JSON, validated against discovered schema (unknown fields rejected, types/nullability enforced). No envelope.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Async ingestion: ingest returns 200 after optional dedup + MQ publish; ClickHouse writes happen later via `StartIngestWorker`. NATS full → 503 + Retry-After.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Per-table batching: the ingest worker groups events by table and bulk-INSERTs in schema column order; each table's batch is independent
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Dead Letter Queue: failed batch inserts publish to `WAVEHOUSE_DLQ` (`dlq.<table>`), gated by `dlq.enabled`. No silent data loss.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Optional dedup: opt-in via `dedupe.enabled` (boot config); dedup key field (`id_field`) and strict missing-id mode (`require_id`) are runtime settings (internal/settings)
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Singleflight: `TieredCache` coalesces concurrent misses (`x/sync/singleflight`) to prevent cache stampede
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Active Sweeper: purges NATS messages that are both ACKed (written to CH) and older than the gap window; SSE gap-fill uses `DeliverByStartTime`, no in-process ring buffer
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Non-fatal boot: schema-discovery failure on boot is non-fatal; `cmd/wavehouse` records `api.BootState`, binds `:8080`, serves 503 on `/livez`/`/readyz` with diagnostic, retries via `SchemaRegistry.RetryRefresh` (backoff 2s → 60s). Bounds supervisor restart loops.
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: When backend API/event format/query AST/access-control/pipes changes, update corresponding SDK (`wavehouse/sdk` in `clients/ts/`) — add typed client methods, update types, update SDK docs under `docs/src/content/docs/sdk/`
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: When adding a new internal package, add a matching `area/<pkg>` repo label (e.g. `area/foo` for `internal/foo/`); `triage.yml` discovers labels at runtime via `gh label list` with no workflow edit needed
Learnt from: CR
Repo: Wave-RF/WaveHouse
Timestamp: 2026-07-31T15:15:22.890Z
Learning: Run `make ci` via background Bash tool (run_in_background: true), redirect to file with NO_COLOR=1, wait for completion notification; never foreground, `| tee`, or `tail -f`
📚 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:
README.mddocs/src/content/docs/getting-started.mddocs/src/content/docs/why-wavehouse.mdCHANGELOG.mddocs/src/content/docs/architecture.mddocs/src/content/docs/api.mdAGENTS.mddocs/src/content/docs/sdk/admin.mddocs/src/content/docs/deployment.mddocs/src/content/docs/development.md
📚 Learning: 2026-06-26T12:23:22.696Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 346
File: internal/stream/subscriber_test.go:9-28
Timestamp: 2026-06-26T12:23:22.696Z
Learning: In this Go repository, prefer table-driven tests (e.g., `[]struct{...}` with `t.Run(...)`) only for tests that cover multiple scenarios/inputs and can be cleanly enumerated. Do not artificially rewrite a clear single-scenario sequential behavioral-flow test into a table-driven form just to fit the pattern; if there’s only one meaningful scenario, keep the test as a straightforward linear flow (as in `TestSubscriber_SendDeliversThenDropsWhenFull`).
Applied to files:
internal/pipes/pipes_test.gointernal/api/router_test.gointernal/settings/settings_test.gointernal/config/config_test.gointernal/settings/disjoint_test.gointernal/pipes/store_db_test.gointernal/api/settings_test.gointernal/controldb/controldb_test.gointernal/settings/store_test.gointernal/policy/store_test.gointernal/api/ingest_test.go
📚 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/pipes/pipes_test.gointernal/api/settings.gointernal/settings/settings.gointernal/api/router.gointernal/api/router_test.gointernal/settings/settings_test.gointernal/config/config_test.gointernal/config/config.gointernal/settings/disjoint.gointernal/settings/disjoint_test.gointernal/controldb/controldb.gointernal/api/ingest.gointernal/settings/store.gocmd/wavehouse/main.gointernal/pipes/store_db_test.gointernal/api/settings_test.gointernal/controldb/controldb_test.gointernal/pipes/pipes.gointernal/settings/store_test.gointernal/policy/store.gointernal/policy/store_test.gointernal/config/retired.gointernal/api/ingest_test.go
📚 Learning: 2026-05-20T01:02:00.784Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 164
File: internal/api/router_test.go:289-350
Timestamp: 2026-05-20T01:02:00.784Z
Learning: In WaveHouse’s internal API tests (files matching internal/api/**/*_test.go), follow the existing separation-of-concerns convention for testing the RequireRole middleware: inject `ContextKeyRole` directly into the request `context.Context` instead of using `testutil.MakeJWT`/JWT-driven flows. Do not refactor role-gate tests to use JWT tokens—JWT parsing and token handling are covered separately in `middleware_test.go` (the dedicated JWT parsing tests), and mixing those concerns would expand the failure surface and reduce isolation.
Applied to files:
internal/api/router_test.gointernal/api/settings_test.gointernal/api/ingest_test.go
📚 Learning: 2026-05-23T01:23:59.268Z
Learnt from: EricAndrechek
Repo: Wave-RF/WaveHouse PR: 174
File: internal/api/ingest_test.go:111-111
Timestamp: 2026-05-23T01:23:59.268Z
Learning: In WaveHouse Go tests in internal/api/**/*_test.go, use internal/testutil.AssertJSONErrorResponse(t, w) for HTTP error-path JSON assertions. Do not use (or reintroduce) package-local assertJSONErrorResponse helpers. AssertJSONErrorResponse verifies the response Content-Type is application/json, includes the X-Content-Type-Options: nosniff header, and that the JSON body contains an "error" field.
Applied to files:
internal/api/router_test.gointernal/api/settings_test.gointernal/api/ingest_test.go
🪛 LanguageTool
docs/src/content/docs/getting-started.md
[style] ~31-~31: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...otstrapping-and-the-policy-lifecycle)). It's deliberately lenient for trialing — a...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~31-~31: Since ownership is already implied, this phrasing may be redundant.
Context: ...ment should tune it: your own roles, real tables, scoped columns, and...
(PRP_OWN)
docs/src/content/docs/access-control.mdx
[typographical] ~409-~409: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ...nd parse**, or WaveHouse refuses to boot. That turns a typo or a missing mount in...
(WRB_QUESTION_MARK)
[typographical] ~410-~410: The word ‘When’ starts a question. Add a question mark (“?”) at the end of the sentence.
Context: ... you seed one via PUT /v1/admin/policy. In config.yaml (or the matching envi...
(WRB_QUESTION_MARK)
docs/src/content/docs/architecture.md
[style] ~74-~74: Since ownership is already implied, this phrasing may be redundant.
Context: ...e JWT/JWKS authentication middleware is its own package, [auth/](#auth--authenticatio...
(PRP_OWN)
docs/src/content/docs/development.md
[style] ~141-~141: Consider using the typographical ellipsis character here instead.
Context: ... air -c .air.toml ``` deps-up runs `docker compose ... up -d --wait clickhouse`, which blocks ...
(ELLIPSIS)
🔇 Additional comments (49)
tests/e2e/fixtures/config.yaml (1)
17-19: LGTM!internal/settings/settings.go (1)
1-37: LGTM!internal/settings/settings_test.go (1)
10-51: LGTM!internal/settings/store_test.go (1)
16-39: LGTM!Also applies to: 41-127, 129-199, 201-220
internal/api/ingest_test.go (2)
167-167: LGTM!Also applies to: 183-183, 499-499, 518-518, 565-565, 817-817
535-541: 🩺 Stability & AvailabilityNo change needed.
Handlereads one dedupe snapshot per request whenh.Settingsis non-nil, otherwiseHandle, batch, and single-record paths use zero-value dedupe settings.internal/api/settings_test.go (1)
101-148: LGTM!tests/e2e/sdk/admin.test.ts (1)
233-246: LGTM!Also applies to: 248-290, 292-324
go.mod (1)
50-52: 🔒 Security & PrivacyDependency versions and advisories are acceptable.
modernc.org/sqlite v1.55.0andgoogle.golang.org/grpc v1.82.1are published, andgoogle.golang.org/grpc v1.82.1is the patched version for the listed advisories.internal/controldb/controldb.go (3)
38-63: LGTM!
123-144: LGTM!Also applies to: 160-183
146-158: 📐 Maintainability & Code QualityNo change needed.
controldb.MustOpenMemoryis only reached throughNewMemoryStoreconstructors, and current wiring uses those constructors from tests.internal/controldb/migrations/001_control_plane.sql (1)
39-134: LGTM!internal/controldb/controldb_test.go (1)
13-179: LGTM!internal/policy/store.go (1)
109-153: LGTM!Also applies to: 156-208, 215-242, 244-329, 333-437
internal/policy/store_test.go (1)
18-24: LGTM!Also applies to: 39-197, 199-297, 319-343
internal/pipes/pipes.go (1)
42-74: LGTM!Also applies to: 122-141, 163-200, 365-446, 469-477, 489-504
internal/pipes/pipes_test.go (1)
221-221: LGTM!Also applies to: 234-234
internal/pipes/store_db_test.go (1)
15-168: LGTM!cmd/wavehouse/main.go (1)
22-30: LGTM!Also applies to: 223-231, 283-305, 358-358, 415-415
internal/config/config.go (2)
4-6: LGTM!Also applies to: 137-143, 190-190
301-337: 🗄️ Data Integrity & IntegrationNo change needed. The retired dedupe keys are documented only as runtime settings or retired values;
dedupe.enabledandWH_DEDUPE_ENABLEDremain the only boot-config knobs.internal/config/config_test.go (1)
29-29: LGTM!Also applies to: 79-88, 532-579
internal/config/retired.go (1)
1-63: LGTM!internal/settings/disjoint.go (1)
1-90: LGTM!internal/settings/disjoint_test.go (1)
1-64: LGTM!internal/api/ingest.go (1)
21-21: LGTM!Also applies to: 44-49, 198-201, 217-217, 229-229, 266-266, 295-295, 373-373, 421-433
internal/api/router_test.go (1)
6-15: LGTM!Also applies to: 402-449
internal/api/router.go (1)
31-31: 🗄️ Data Integrity & IntegrationSettings API docs are complete.
AGENTS.md (2)
29-34: LGTM!Also applies to: 36-45, 59-59, 71-71, 128-130, 400-409
35-35: 🗄️ Data Integrity & IntegrationNo documentation change needed.
The documented boot-only/reload-is-gone contract matches the implementation: only
SIGHUP/SIGTERM/SIGINThandling is present, and runtime settings are live-updated through/v1/admin/settings.> Likely an incorrect or invalid review comment.CHANGELOG.md (1)
16-17: LGTM!Also applies to: 36-36, 457-457
README.md (1)
62-62: LGTM!config.yaml (1)
1-4: LGTM!Also applies to: 73-75, 100-100
docs/src/content/docs/getting-started.md (1)
31-31: LGTM!Also applies to: 116-116
docs/src/content/docs/index.mdx (1)
96-96: LGTM!docs/src/content/docs/pipes.mdx (1)
151-151: LGTM!Also applies to: 189-189, 201-202
docs/src/content/docs/sdk/admin.md (1)
94-95: LGTM!docs/src/content/docs/why-wavehouse.md (1)
110-110: LGTM!Also applies to: 196-196
deployments/Dockerfile (1)
28-28: LGTM!deployments/compose/dev-policy.yaml (1)
21-21: LGTM!deployments/compose/standalone.yaml (1)
21-24: LGTM!Also applies to: 43-45, 49-51
docs/src/content/docs/access-control.mdx (1)
65-65: LGTM!Also applies to: 391-391, 406-418
docs/src/content/docs/api.md (1)
238-238: LGTM!Also applies to: 355-355, 675-675, 749-791
docs/src/content/docs/architecture.md (1)
57-64: LGTM!Also applies to: 73-76, 146-163, 186-187, 219-219
docs/src/content/docs/configuration.mdx (2)
68-68: LGTM!Also applies to: 168-168, 207-213, 255-255, 283-284, 353-353
28-29: 🎯 Functional CorrectnessNo documentation change needed. The documented startup-only boot config contract matches the implementation:
config.Loadruns once at boot, comments describe cold config as immutable for the process lifetime, and there are no in-process config reload paths in the codebase.> Likely an incorrect or invalid review comment.docs/src/content/docs/deployment.md (1)
157-160: LGTM!Also applies to: 173-179, 243-243, 255-255
docs/src/content/docs/development.md (1)
233-237: LGTM!Also applies to: 428-429
| require.Equal(t, http.StatusOK, rec.Code) | ||
| assert.JSONEq(t, `{ | ||
| "values": {"dedupe": {"id_field": "event_id", "require_id": false}}, | ||
| "sources": [{"section": "dedupe", "source": "default"}] | ||
| }`, rec.Body.String()) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use the shared HTTP assertion helpers.
This file asserts handler responses with require.Equal on rec.Code plus assert.JSONEq/assert.Contains. The package convention is testutil.AssertJSONResponse and testutil.AssertJSONContains, and testutil.AssertJSONErrorResponse on error paths. The shared helpers also check the Content-Type and X-Content-Type-Options: nosniff headers, which no assertion in this file covers.
Apply the same change to the success assertions at Lines 37-38 and Lines 145-146, and to the error assertions at Lines 56, 70-71, 84-85, and 98.
♻️ Proposed refactor for the success path
- require.Equal(t, http.StatusOK, rec.Code)
- assert.JSONEq(t, `{
- "values": {"dedupe": {"id_field": "event_id", "require_id": false}},
- "sources": [{"section": "dedupe", "source": "default"}]
- }`, rec.Body.String())
+ testutil.AssertJSONResponse(t, rec, http.StatusOK, `{
+ "values": {"dedupe": {"id_field": "event_id", "require_id": false}},
+ "sources": [{"section": "dedupe", "source": "default"}]
+ }`)As per coding guidelines: "Use testutil.AssertJSONResponse(t, rec, status, expected) and testutil.AssertJSONContains(t, rec, status, substring) for HTTP handler assertions", and based on learnings that testutil.AssertJSONErrorResponse is the required helper for error-path JSON assertions in internal/api/**/*_test.go.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| require.Equal(t, http.StatusOK, rec.Code) | |
| assert.JSONEq(t, `{ | |
| "values": {"dedupe": {"id_field": "event_id", "require_id": false}}, | |
| "sources": [{"section": "dedupe", "source": "default"}] | |
| }`, rec.Body.String()) | |
| testutil.AssertJSONResponse(t, rec, http.StatusOK, `{ | |
| "values": {"dedupe": {"id_field": "event_id", "require_id": false}}, | |
| "sources": [{"section": "dedupe", "source": "default"}] | |
| }`) |
Sources: Coding guidelines, Learnings
| afterAll(async () => { | ||
| // Revert to compiled defaults even if an assertion failed mid-flow. | ||
| await fetch(`${settingsURL}/dedupe`, { method: "DELETE", headers: adminHeaders() }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether Vitest globals are enabled for the e2e suite, and how this file imports hooks.
fd -t f 'vitest.config.*' tests --exec cat -n {}
fd -t f 'admin.test.ts' tests --exec sed -n '1,12p' {}Repository: Wave-RF/WaveHouse
Length of output: 5003
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== admin.test.ts location and relevant outline =="
fd -t f 'admin.test.ts' tests --exec sh -c '
file="$1"
echo "--- $file ($(wc -l < "$file") lines) ---"
ast-grep outline "$file" 2>/dev/null | sed -n "1,120p"
echo "--- imports/head ---"
sed -n "1,80p" "$file"
echo "--- dedupe section ---"
sed -n "200,250p" "$file"
' sh {}
echo "== settings API implementation references =="
rg -n "dedupe|source.*default|values.*dedupe|DELETE.*/dedupe|PUT.*/dedupe" tests clients packages . 2>/dev/null | head -200Repository: Wave-RF/WaveHouse
Length of output: 50373
Snapshot and restore the global dedupe setting instead of resetting it.
This global settings document persists across e2e runs. The describe("Settings") block should capture the dedupe section in beforeAll, start from compiled defaults with DELETE /dedupe, and restore the captured sources[] entry in afterAll. Resetting to defaults makes later failures leave the server in an unintended state.
Source: Coding guidelines
2 participants
Summary
Adds config hot reload:
SIGHUPor the new admin-gatedPOST /v1/admin/config/reload(both serialize on one code path) re-runs the full config load — file,WH_*environment overrides, validation — and applies an explicit whitelist to the running process, currentlydedupe.id_fieldanddedupe.require_id. The ingest handler reads them through an atomically swapped snapshot: one lock-free load per record, so a reload lands at a record boundary and never mixes old and new settings within one record.Every other changed section is reported as
restart_required(reload log line + endpoint response) instead of silently ignored. A file that fails to parse or validate changes nothing — the previous config stays active.dedupe.enabledstays restart-only (it owns the Pebble store lifecycle), env-pinned keys honestly report no change, and a drift-guard test forces every future config field to be classified hot or restart-only.Related Issues
Closes #48 — delivers the reload mechanism (SIGHUP + admin endpoint) with an explicit hot-field whitelist; log-level hot reload split out to #415. Groundwork for the per-table dedupe overrides of #222.