Repository navigation
docs(hermes): catalog pins ctx_checkpoint 0.6.2 — #147170 merged upstream - #67
Merged
Merged
Conversation
…ream NousResearch/hermes-agent#147170 was merged on 2026-11-02T22:57:57Z (merge commit d1d2be3ad0). plugin-catalog/ctx_checkpoint.yaml on hermes-agent main now reads `version: "0.6.2"` at `sha: 9f98ff6` (release v5.29.6). Bring the sentences that still described the catalog as 0.5.0 or the re-pin as open up to that state: - integrations/hermes/README.md, install section: current pin `9f98ff6` (release v5.29.6), plugin 0.6.2, re-pinned in #147170. - integrations/hermes/README.md, Versions: catalog pin 0.6.2 at `9f98ff6`; history 0.4.4 -> 0.5.0 (#145302) -> 0.6.2 (#147170); the "re-pin to 0.6.0 is open upstream" clause is dropped. - plugin/ctx_checkpoint/README.md: the checkout installer is for code newer than the catalog pin, no longer the only way to the memory features. - plugin/ctx_checkpoint/memory/README.md: a catalog install includes the memory engine; the "until the re-pin lands" paragraph is replaced. - README.md: both layers are listed in the catalog, not only the checkpoint side (that wording dates from 0.6.0 while the catalog was still at 0.5.0). Upstream merge: NousResearch/hermes-agent#147170 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Catalog pin and installation guidance README.md, integrations/hermes/README.md, integrations/hermes/plugin/ctx_checkpoint/README.md, integrations/hermes/plugin/ctx_checkpoint/memory/README.md |
The documentation records catalog version 0.6.2 and its commit, includes the memory engine in the catalog description, and directs users to the installation script or a ctx checkout for code newer than the pin. |
Estimated code review effort: 1 (Trivial) | ~5 minutes
Change: Other
Possibly related PRs
- GottZ/ctx#51: Updates
ctx_checkpointto version 0.6.1 and documents its Hermes catalog installation state. - GottZ/ctx#53: Ships
ctx_checkpoint0.6.2 and updates its plugin and memory documentation.
Merge Risk: 
77991
-
The catalog and installation guidance is consistent across the updated pages. No issue identified here prevents merging.
Architecture risk: 
aea62
-
The change affects 2 systems.
Changed systems:
integrations,README.mdArchitecture concerns
No architecture-level concerns identified.Review details
-
Systems and components
- observed — integrations (service) was modified; 3 changed files map to changed impact.
- observed — README.md (service) was modified; 1 changed file maps to changed impact.
Before / after behavior
- observed — Modified behavior in README.md: The Hermes integration entry now says both sides are listed in the plugin catalog; previously it referred only to the checkpoint side.
- observed — Modified behavior in integrations/hermes/README.md: The documented catalog commit and plugin version change from
ee8cb2e/ 0.5.0 to9f98ff6/ 0.6.2, with the re-pin reference changing from PR#145302to#147170. - observed — Modified behavior in integrations/hermes/README.md: The catalog-pin entry changes from 0.5.0 at
ee8cb2eto 0.6.2 at9f98ff6, dates the#147170re-pin to 2026-11-02, and removes the note that the re-pin was open upstream. - observed — Modified behavior in integrations/hermes/plugin/ctx_checkpoint/README.md: The install-script note now applies to code newer than the catalog’s 0.6.2 pin and records the associated Hermes pull request and merge date; it replaces the previous note that the script was needed while the catalog pinned 0.5.0.
-
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the docs change, the Hermes scope, and the new catalog pin of ctx_checkpoint 0.6.2. The changeset moves the catalog pin from 0.5.0 to 0.6.2 at commit 9f98ff6 and updates the Hermes int… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title names the docs change, the Hermes scope, and the new catalog pin of ctx_checkpoint 0.6.2. The changeset moves the catalog pin from 0.5.0 to 0.6.2 at commit 9f98ff6 and updates the Hermes integration and memory READMEs to match. The title covers the main change. It adds a reference to upstream PR
#147170, which the diff does not show directly, but the objectives tie that merge to the re-pin. The title is conventional and specific enough for a teammate scanning history.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)
-
Finishing Touches
-
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
CodeRabbit Verify
- Run CodeRabbit Verify to test the changes in this pull request.
There was a problem hiding this comment.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🟢 Approval recommended
The change is limited to updating Hermes integration documentation (version pins, wording, and references) to match the already-merged upstream catalog state, with no review evidence of unresolved problems.
No open issues 🎉
What changed in this PR
This pull request updates Hermes integration documentation to reflect that the upstream Hermes plugin catalog is now pinned to ctx_checkpoint 0.6.2 (commit 9f98ff6, release v5.29.6) after NousResearch/hermes-agent #147170 merged, removing outdated “still pinned to 0.5.0 / re-pin pending” wording.
Changes:
- Refresh catalog pin references from 0.5.0/
ee8cb2eto 0.6.2/9f98ff6, and update upstream references to merged PR #147170. - Clarify that catalog installs now include the memory engine; position
install.shguidance for code newer than the catalog pin. - Update the root README Hermes row to state both checkpoint + memory sides are listed in the Hermes plugin catalog.
| File | Summary |
|---|---|
README.md |
Updates the Hermes integration row to reflect both checkpoint and memory sides are listed in the Hermes plugin catalog. |
integrations/hermes/README.md |
Updates install and version history text to the merged catalog pin (0.6.2 at 9f98ff6, PR #147170). |
integrations/hermes/plugin/ctx_checkpoint/README.md |
Rewords install.sh guidance to target code newer than the catalog pin (now 0.6.2) and references #147170 as merged. |
integrations/hermes/plugin/ctx_checkpoint/memory/README.md |
Updates catalog-pin wording to 0.6.2 and notes catalog installs include the memory engine as of that pin. |
Review effort: Balanced
DamieMoon
requested changes
Nov 3, 2026
DamieMoon
left a comment
DamieMoon
left a comment
Collaborator
There was a problem hiding this comment.
Review at aea6275. Every fact in the diff checks out against the primary sources: hermes-agent#147170 merged 2026-11-02T22:57:57Z, and plugin-catalog/ctx_checkpoint.yaml at the merge commit d1d2be3 pins sha: 9f98ff65…, version: "0.6.2". v5.29.6 is 9f98ff6, whose VERSION reads 0.6.2. The history line matches #144212 (2026-10-25, 0.4.4 @ 98749f3) and #145302 (2026-10-29, 0.5.0 @ ee8cb2e). "A catalog install includes this engine" holds too: the catalog install clones the repo and sparse-checks out the plugin subdir at entry.sha (hermes_cli/plugins_cmd_git.py:207-248, plugins_cmd_catalog.py:332), and 9f98ff6 ships memory/, the ctx_* tool schemas behind memory: true and the "Memory features" setup field (__init__.py:535-538, 613-614). I found no stale pin sentence elsewhere and no conflict with #55/#57/#63.
integrations/hermes/plugin/ctx_checkpoint/README.md:41-43: don't name the current pin in this file. The catalog page renders this README from the pinned commit (website/scripts/extract-plugins.py:85-94, and:264"README renders by default from the pinned commit";plugin-catalog/README.md:229-233"the build fetches the README … at the pinned sha"). A commit can't know the re-pin that will later pin it, so after the next re-pin to newer code the page shows "0.6.2 since hermes-agent #147170" under a newer version. That is what the live page shows today: "…while the catalog still pins 0.5.0." under "Pinned commit 9f98ff6 · Version 0.6.2" (https://hermes-agent.nousresearch.com/docs/plugins/ctx_checkpoint). Suggest something version-neutral, e.g. "— for code newer than the catalog pin (see Versions)." The integration README is reached viadocs_urlonroot, so it's the right home for the pin.
Nit: memory/README.md:84-87 has the same pattern ("pins plugin 0.6.2 (9f98ff6) since hermes-agent #147170"). It isn't rendered on the page, but the page links it at the pinned sha and it ships inside the install. Could say "the catalog version includes this engine (since 0.6.2)" and leave the current pin to the Versions section.
FYI, not for this PR:
- The page keeps today's stale sentence until the next re-pin; this PR can't change that.
- The upstream entry declares
capabilities.provides_tools: []while its own description names thectx_query/ctx_store/ctx_search/ctx_gettools of the memory layer. Upstream metadata, maybe worth fixing at the next re-pin.
…t from the pinned commit The hermes-agent catalog page fetches and renders the plugin README at the pinned sha (website/scripts/extract-plugins.py:85-94 and :264, plugin-catalog/README.md:229-233). A commit cannot know the re-pin that will later pin it, so a pin named in that README goes stale on the page after the next re-pin. - plugin/ctx_checkpoint/README.md: drop "0.6.2 since hermes-agent #147170"; link the Versions section of the integration README on root instead. - plugin/ctx_checkpoint/memory/README.md: "the catalog version includes this engine (since 0.6.2)" and the same Versions link instead of naming the current pin and sha. The current pin stays in integrations/hermes/README.md (Versions), reached through docs_url on root. Review 5745219772, point 1 + nit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Owner
Author
|
Thanks — I had missed that the catalog page renders this README from the pinned commit. Fixed in 77991da. What was wrong What I did
Checks
FYI |
This was referenced Nov 3, 2026
GottZ
added a commit
that referenced
this pull request
Nov 3, 2026
…top_grace_period (#71) Wave W06-1 of the OpenAI-compatible API plan (RC-A, telemetry foundation; design/06 §7 block A). No `/v1` surface, no migration. ## What this changes The `context_llm_log` write path started one goroutine and one pool connection per row, logged failures at DEBUG only, and ctxd closed its pool on shutdown without waiting for pending inserts; the `ctx` compose service had no `stop_grace_period`, so Docker's 10 s default could SIGKILL the daemon inside its 15 s HTTP drain. - `internal/llmlog`: per-pool bounded writer queue — at most 4 inserts run at once, at most 4 096 rows wait; beyond that `Record` drops and counts the row. Failed inserts are counted; one WARN per minute at most, carrying the totals since start (the per-failure DEBUG line stays). New readers `InsertFailures()` / `InsertDropped()`. `Record` stays non-blocking; `Settle` now also waits for queued rows that have not started. - `cmd/ctxd`: the teardown is `shutdownSequence`: `srv.Shutdown` (15 s) → `llmlog.Settle` (5 s) → `scheduler.Wait` → `llmlog.Settle` (5 s) → pool close. Settling twice is deliberate: a running dream cycle does not stop on SIGTERM (own context, `dream.cycle_timeout`, 700 s by default), so a settle only after `Wait` would hand the HTTP rows to the SIGKILL that ends an over-long wait. - `internal/handler`: server-admin-only `/api/status` section `"llmlog": {"insert_failures", "insert_dropped"}`, present at 0/0, absent on the tenant path and on the SSE frame. - `docker-compose.yml`: `stop_grace_period: 30s` on `ctx`, pinned ≥ 25 s by `internal/config` `TestComposeCtxStopGracePeriod` (scanner follows the top-level section, so only `services.ctx` counts). - `docs/operations.md`: shutdown / write-path section and an upgrading note for operators with their own compose file; `docs/api.md`: the new status section. ## Behaviour changes (intended) - A burst of LLM calls no longer takes the whole 20-connection pool for its telemetry rows (before: 20 of 20 under a 200-row burst; now 4). - Rows beyond 4 096 queued are dropped and counted instead of queued without bound. - Shutdown waits up to 2 × 5 s longer; rows still pending after that are lost and logged as such. - Operators running ctx from their own compose file should set `stop_grace_period: 30s` (upgrading note). ## Consumers of the changed contracts | Changed symbol | Callers / readers | Covered by | |---|---|---| | `llmlog.Record` (same signature, queued) | every pipeline that writes telemetry (`chat/engine.go` `recordEntry`, `llm/*`, `dream/*`, `digest`, `events/audit.go`, `events/scheduler.go` backfill, handlers) | `TestRecordInsertConcurrencyBounded`, `TestRecordQueueBoundDropsAndCounts`, `TestRecordInsertSlotsBounded`, `TestRecordAfterSettleAndClose`, existing `TestChatQuota`, `TestDistillExtract` | | `llmlog.Settle` (now waits for queued rows too) | `cmd/ctxd` shutdown (new), `chat_quota_integration_test.go`, `distill_extract_integration_test.go` | `TestSettle`, `TestSettleLLMLogRespectsTimeout` | | `llmlog.InsertFailures/InsertDropped` (new) | `handler/status.go`, `cmd/ctxd/main.go` log line | `TestInsertFailureCountedAndWarnThrottled`, `TestStatusLLMLogSection`, `TestStatusCollectorWiresLLMLogCounters` | | `statusResponse.LLMLog` (new key) | `/api/status` admin path; golden key set | `TestStatusGoldenKeys`, `TestStatusPerTenantView`, `TestStatusEventCarriesEveryDualPathField` | | `shutdownSequence` (extracted) | `main.go` | `TestShutdownWithoutSettleLosesRows`, `TestShutdownSequenceSettlesLLMLog`, `TestShutdownSettlesHTTPRowsBeforeSchedulerWait` | ## Overlap search Searched open PRs #55/#57/#63/#67 for shared paths on 2026-11-03: none (`docs/operations.md` is touched by #63 in a different section — whichever lands second rebases trivially). #69 (merged `16fe6436`) touched ingest only; this branch is rebased on it. ## Cost Per LLM call: one mutex lock on enqueue instead of one goroutine spawn; inserts run on ≤ 4 connections instead of ≤ 20. Measured by the reviewer's throughput probe: 4 096 real rows in 327 ms (0.32 ms per row and slot), so a 5 s settle covers a full queue on a healthy database; the break-even is 4.9 ms per insert. Status: the two counters are process memory, read per call, no I/O. ## Verification Gates, each red on `7f17de21` first: - (a) failure counter + throttled WARN + status field: red = compile errors (`InsertFailures`, `LLMLog` undefined; failures were DEBUG only) → `DROP TABLE context_llm_log` makes `InsertFailures()` +1; 100 failures → exactly 1 WARN, the next after the interval with total 101; `llmlog.insert_failures` 1 on the admin path, absent for tenants. - (b) 200 simultaneous `Record` against a 50 ms insert delay (pgx tracer, MaxConns 20): peak concurrent inserts 20 → 4, rows 200/200. - (c) 50 `Record` + slow inserts, then the shutdown sequence: old order lands 0/50; `shutdownSequence` lands 50/50 plus 10/10 written during `scheduler.Wait`. Hardening: HTTP rows landed at `Wait` entry 0/50 → 50/50. - (d) compose pin: red "declares no stop_grace_period" → 30s; scanner pin 3/6 → 6/6 on the shapes that fooled the first version. `go/deadcode-testonly-allow.txt`: the `llmlog.Settle` line is removed — it recorded Settle as reachable from tests only, and this PR gives it its production caller; `deadcode.sh testonly` was red on the first push for exactly that line and is green without it (third commit). Suites on the rebased head: `go build ./...`, `go vet -tags=integration ./...`, `golangci-lint run` with and without `--build-tags=integration` (0), `go test -short -race ./...` (65 ok), integration `-p 1` for `cmd/ctxd`, `internal/config`, `internal/llmlog`, `internal/handler` (full handler suite on the first commit). Independent review by a fresh agent: stress test with `-race` at GOMAXPROCS 1/4/16 (50 × 1 000 `Record`, 10 % injected failures, 3 concurrent `Settle` callers): landed + failed + dropped == sent in all three runs, `pending` 0 after `Settle`, `running` ≤ 4, queue ≤ 4 096; lost-wakeup probe 160 000/160 000 rows, 0 settle timeouts. ## Not verified - Frequency of shutdowns that coincide with a dream cycle longer than the grace period on the live host (the second settle is lost in that case; the first still lands the HTTP rows). - `scheduler.Wait` covers the dream workers only; `runSensitivityAudit` (`events/audit.go:124`) and the dream-loop backfill (`scheduler.go:1832`) can still `Record` after `Wait` returns — those rows are covered by the second settle only if they are queued before it runs. Candidate for the W06-13 force-settle wave. - `ctxServiceEnvValues` in `compose_surface_test.go` scans the same way the first version of this gate did (no `services:` check); left as is, noted for a later sweep. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **New Features** * Server-admin status now reports LLM log insert failures and dropped entries, including when both counts are zero. Tenant status does not include these process-wide counters. * **Improvements** * LLM log writes are queued with bounded capacity and concurrency; failures and dropped entries are counted. * Shutdown allows pending LLM log entries additional time to finish saving. The Docker shutdown grace period is set to 30 seconds. * **Documentation** * Added guidance on LLM log handling during shutdown and upgrades, including the new status counters. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
There was a problem hiding this comment.
Note
Copilot was unable to complete this review at the selected tier due to model policy or plan restrictions.
🟡 Changes recommended
Two documentation nits remain unresolved because README links hard-code blob/root, which can misdirect readers to a moving branch instead of the exact revision being viewed, so the change needs a small fix before approval.
2 open issues
Review effort: Balanced
GottZ
added a commit
that referenced
this pull request
Nov 3, 2026
…K (Migration 158) (#72) Wave W06-2 of the OpenAI-compatible API plan (RC-A, telemetry foundation; design/06 §3.1 M-06-1). Migration **158**, no new table, no `/v1` surface, no reader. ## What this changes - `go/migrations/158_llm_log_consumer.sql`: `context_llm_log.consumer_id UUID` (nullable, no FK — like `api_key_id`, the log outlives the credential), `CHECK ck_llm_log_one_identity (num_nonnulls(api_key_id, consumer_id) <= 1)` on the hypertable and every chunk, partial index `idx_llm_log_consumer (consumer_id, created_at DESC) WHERE consumer_id IS NOT NULL`. Idempotent (`IF NOT EXISTS`, `duplicate_object` swallowed), `lock_timeout 3s`, no backfill (every existing row is internal). - `internal/llmlog`: `Entry.ConsumerID` and the 24th insert parameter (`nullUUID`). No production code sets it yet — the gateway stamp (W06-9) will be the only setter; every row keeps `consumer_id` NULL until then. - Schema-contract manifest regenerated (column + index; minimum extension versions kept at 2.30.1 / 0.8.6 as 156/157 did). `test.sh` T07 unchanged (0 new tables; the column counter covers `context_blocks` only). - `docs/operations.md` "Migration 158" (cost, downgrade, the export now carries `"consumer_id": null` per JSONL line), `docs/architecture.md` LLM-log paragraph. ## Behaviour changes (intended) - A row that names both a knowledge key and a consumer is refused by the database (`23514`); through `llmlog.Record` such a row is lost and counted in `llmlog.insert_failures`. No current writer can produce one. - `ctx-llmlog-export` JSONL lines gain the key `consumer_id` (additive). ## Consumers of the changed contracts | Changed symbol / object | Callers and readers | Covered by | |---|---|---| | `context_llm_log` INSERT column list (24 columns) | the one production INSERT in `llmlog.go`; 33 INSERT sites in 18 test files, all with explicit column lists; no `SELECT *` reader | `TestRecordConsumerAttribution`, existing llmlog/handler/backends suites | | `Entry.ConsumerID` (new, unset) | none in production | `TestOneIdentityCheck` (Record path and raw SQL) | | `ck_llm_log_one_identity`, `idx_llm_log_consumer` | future `/v1` readers (W06-3 rollup trigger, W06-4 window reader) | `TestMigration158OnPopulatedHypertable` (chunk inheritance, idempotence, plan uses the index) | | schema-contract manifest | `/api/contract`, `TestCheck_FreshDB_NoDrift` | red without the 158 entries, green with them | ## Overlap search Open PRs #55/#57/#63/#67 on 2026-11-03: no shared files (`docs/operations.md` is touched by #63 in another section). Rebased on #71 (W06-1); `llmlog.go` hunks sit below W06-1's writer queue and do not overlap. ## Cost Boot-time migration, one transaction, `context_llm_log` locked (ACCESS EXCLUSIVE) until commit. Measured on a synthetic copy of production shape (1 000 000 rows, 29 chunks, 976 MB heap, TimescaleDB 2.31.0; reproducible with `CTX_M158_PROBE_ROWS=1000000 go test -tags=integration -run TestMigration158CostProbe ./internal/llmlog/`): `ADD COLUMN` 3 ms, `CHECK` validation 0.38–0.45 s, index build 0.43–0.74 s, whole file through the boot runner 0.73–0.75 s warm. Cold (buffers evicted, independent review): 1 M / 976 MB ≈ 0.9–1.25 s, 3 M / 2.9 GB ≈ 2.85 s, i.e. ≈ 0.7–1 s per GB and scan; a 30 GB heap would extrapolate to roughly 40–60 s of lock at boot (not measured). Live table today: 352 565 rows, 29 chunks, 318 MB heap. Under a foreign lock the migration aborts after 3 s with `55P03`, leaves no partial state (no column, no constraint, no index, no `_migrations` row) and the next boot retries. ## Verification Gates, each red on `83461171` first: - (a) `Record` with `ConsumerID` → column set; red: `unknown field ConsumerID` / raw SQL `42703`. - (b) a row with both identities → `23514` on a new and on a pre-existing chunk, and via `Record` +1 insert failure and no row; red: without the CHECK block the insert succeeds, 0 of 9 chunks carry the constraint. - (c) existing writers keep `consumer_id` NULL; `EXPLAIN` of the per-consumer window uses `idx_llm_log_consumer` on 5/5 chunks without `enable_seqscan = off`; red: created_at index + seq scans. - (d) second apply leaves the catalog byte-identical (incl. `convalidated`, `conislocal`, `coninhcount`, `indisvalid`); red: without the `duplicate_object` handler `42710`. Suites on the head: `go build ./...`, `go vet -tags=integration ./...`, `golangci-lint run` with and without `--build-tags=integration` (0), `go test -short -race ./...` (65 ok), `deadcode.sh` + `deadcode.sh testonly` (ok), integration `-p 1` for `internal/llmlog`, `internal/store` (migration/schema/manifest suites incl. the T07 count gate), `internal/schemacontract`, `cmd/ctxd` (137 passed, 2 expected skips). Independent review by a fresh agent re-ran idempotence, chunk inheritance (`\d+` on an old and a post-158 chunk), the `55P03` abort under a foreign AccessShare lock and under a chunk-only lock, 8 concurrent writers during the migration (28 001/28 001 rows landed, no deadlock), the rollback (`DROP INDEX/CONSTRAINT/COLUMN`, 5 ms, old binary unaffected, new binary gets `42703` as documented) and the cold scans above. ## Not verified - The schema contract does not model CHECK constraints (pre-existing): a `ck_llm_log_one_identity` dropped by hand would not show up on `/api/contract`. Candidate for a later wave. - Behaviour on a heap far beyond 3 GB is extrapolated, not measured. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **New Features** * LLM log records can now store an optional consumer identity, kept separate from API key identity. Existing records are unchanged, and current production requests do not yet populate this field. * **Documentation** * Updated architecture and operations guides to cover the new identity field, exported JSONL format, database migration, and upgrade and downgrade considerations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
GottZ
added a commit
that referenced
this pull request
Nov 3, 2026
… and backfill (Migration 159) (#73) Wave W06-3 of the OpenAI-compatible API plan (RC-A, telemetry foundation; design/06 §3.2 M-06-2). Migration **159**, one new table, no reader yet (W06-4/W06-5 follow), no `/v1` surface. ## What this changes - `go/migrations/159_llm_usage_rollup.sql`: `context_llm_usage_hourly` — one row per UTC hour and dimension (`api_key_id` | `consumer_id`, `pipeline`, `backend_name`, `model`, `model_alias`, `dispatch_class`, `backend_locality`, `hop`) with counters (`calls`, `errors`, `unattributed_ok`, `estimated_calls`, tokens, `duration_ms_sum/_n`, `queue_wait_ms_sum/_n`, `cost_usd`) and three 16-bucket log2 histograms (latency, queue wait, TTFT; design/05 §3.5 contract). Unique index with `NULLS NOT DISTINCT`, three secondary indexes, `fillfactor 50`, per-5 000-dead-tuple autovacuum. SQL helpers `llm_usage_hist_idx` (via `width_bucket`), `llm_usage_hist_one`, `hist_add` (one inlined `ARRAY[…]` expression). `AFTER INSERT` row trigger `trg_llm_usage_rollup` on `context_llm_log` that never raises (type-guarded `metadata.gw` reads, clamps, every text dimension cut to 100 characters so no unique-index entry can reach the 2 704-byte B-tree limit). Backfill of every existing row in the same transaction after `CREATE TRIGGER` (overwriting, `IS DISTINCT FROM` guard ⇒ a second run is a no-op). - Manifest regenerated (table, four indexes, four functions, one trigger; minimum extension versions unchanged); `test.sh` T07 58 → 59 tables. - `docs/operations.md` "Migration 159" (why a trigger, cost, delete semantics, isolation note, rollback) + two follow-ups from the #72 review (NOT VALID switch point corrected to ≈ 15–20 s at 10 GB; helper comment grammar); `docs/architecture.md`. ## Behaviour changes (intended) - Every `context_llm_log` insert now also upserts its hour's rollup row (cost below). - Deleting or truncating `context_llm_log` rows does not decrement the rollup (ledger semantics; ctx deletes no log rows; `EvictBodies` is an `UPDATE`). - Text dimensions longer than 100 characters share a rollup row with their 100-character prefix (today's longest is 25). ## Deviations from design/06 §3.2, each measured - `llm_usage_hist_idx` uses `width_bucket` over the 15 bounds instead of `ceil(log2(…))` on numeric: ≈ 0.15 µs vs ≈ 11 µs per call; equal for every ms in [0, 1 000 000] (full comparison, 0 differences) and at the bigint edges. - `hist_add` is one `ARRAY[…]` expression the planner inlines; the `unnest`/`array_agg` form pushed p99 Δ to 1.9–2.2 ms under eight writers on one row (threshold 2 ms). - `cost_usd NUMERIC` without typmod: `NUMERIC(18,8)` overflows (22003) after ≈ 10 001 rows at the llmlog column maximum. - Alias cut to 100 instead of 128 (04 R-06-2): one cut for all six text dimensions keeps the worst-case index entry provably under 2 510 bytes (six 4-byte-per-char texts of 128 already measured at 3 096 bytes ⇒ 54000). ## Consumers of the changed contracts | Changed object | Callers / readers | Covered by | |---|---|---| | `context_llm_log` INSERT path (trigger now fires) | the one production INSERT (`llmlog.go`), 33 test INSERT sites, psql | `TestUsageRollupParity` (1 000 random seeds, `Record` and raw SQL, trigger and backfill path, Go oracle), `TestUsageRollupTotality` (64 + 2 oversize forms), `TestUsageRollupLoad` | | `context_llm_usage_hourly` (new) | none yet (W06-4 `internal/llmusage`, W06-5 status) | `TestUsageRollupHistogram`, `TestUsageRollupNullConflict`, `TestMigration159Idempotent`, `TestUsageRollupSurvivesEvictBodies` | | schema-contract manifest, T07 | `/api/contract`, `TestCheck_FreshDB_NoDrift`, `test.sh` | red without the 159 entries (10 drifts, 59 ≠ 58), green with them | ## Overlap search Open PRs #55/#57/#63/#67 on 2026-11-03: no shared files. Rebased on #72 (W06-2, `dc9923d8`); no further root movement since. ## Cost Boot: the backfill is one `GROUP BY` over the log inside the migration's transaction. Synthetic copy of production shape (1 000 000 rows, 29 chunks, 976 MB heap, TimescaleDB 2.31.0; reproducible with `CTX_M159_PROBE_ROWS=1000000 go test -tags=integration -run TestMigration159BackfillProbe ./internal/llmlog/`): backfill 8.0–8.3 s, whole file through the boot runner 7.5–8.3 s, 77 239 rollup rows (one per 12.9 log rows), 108 MB with indexes. Live table (352 720 rows) ≈ 3 s extrapolated. `CREATE TRIGGER` holds SHARE ROW EXCLUSIVE on the hypertable until commit: concurrent inserts wait, reads do not (probed with two sessions: an insert before `CREATE TRIGGER` is counted once by the backfill, one after waits and is counted once by the trigger). Per insert (`CTX_M159_LOAD_INSERTS=50000 go test -tags=integration -run TestUsageRollupLoad ./internal/llmlog/`, 8 writers, 50 000 inserts per side, baseline = same table without the trigger): one dimension (all writers on one rollup row) p50 258 → 634 µs, p99 626 → 1 583 µs (Δ 0.96 ms, threshold 2 ms), HOT updates 0.918 (0.894 at fillfactor 100 — red); 64 dimensions p99 Δ 0.35 ms, HOT 0.998. The gate refuses runs below 10 000 inserts. ## Verification Gates, each red on `dc9923d8` first (`42P01` without the migration): (a) parity over 1 000 seeds / 17 655 rows, red without `NULLS NOT DISTINCT` (37 deviations); (b) NULL conflict: 2 rows → 1 row with `calls = 2`; (c) histogram: p95 from buckets within one bucket width of `percentile_cont(0.95)`, all 30 bucket bounds exact, red with `x + EXCLUDED.x` (42883); (d) totality over 64 malformed `metadata.gw` forms, red with a naive cast (58/64 fail, 22P02), plus a 3 000-byte `model` and six 128-char four-byte texts (54000 before the cut, landed and counted after); (e) second apply leaves catalog and rollup rows (incl. `xmin`) byte-identical; (f) `EvictBodies` leaves the rollup unchanged; (g) load gate above; (h) manifest, T07 59, count gate; both deadcode ratchets ok. Suites on the head: `go build ./...`, `go vet -tags=integration ./...`, `golangci-lint run` with and without `--build-tags=integration` (0), `go test -short -race ./...` (65 ok), `deadcode.sh` + `deadcode.sh testonly`, integration `-p 1` for `internal/llmlog` (37 passed, 3 env-gated probes run separately), `internal/store` + `internal/schemacontract` + `cmd/ctxd` migration/schema/manifest suites (320 passed in store), `internal/handler`/`backends`/`chat`/`llm` status and quota suites. Independent review by a fresh agent: 37 totality forms (`-infinity`, year 294276, `1e400` hop, `NaN` cost, NUL, surrogates — none raises in the trigger that the table itself accepts), full bucket comparison over [0, 1 000 000] (0 differences), exactly-once with two sessions around `CREATE TRIGGER`, lock inventory (`pg_locks`), load gate re-run (p99 Δ 1.04 / 0.49 ms, HOT 0.918 / 0.998), red-proofs for (b)/(d) and a no-trigger overlay (6 tests red). ## Not verified - Hour-boundary crossing inside the load gate (expected update count is now derived from the rollup rows actually created; a clock-controlled red-proof was not built). - Under `default_transaction_isolation = repeatable read` the upsert raises 40001 for concurrent writers (reviewer: 1 449 of 2 000 inserts lost); ctx runs `read committed` and sets no isolation — documented, not changed. - Heaps far beyond 1 M rows are extrapolated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **New Features** * Added hourly summaries of LLM usage, including call and error counts, token totals, latency, queue wait, and cost. Existing log data is included, and new log entries update the summaries automatically. * **Documentation** * Updated operational guidance with migration details, estimated locking considerations, and downgrade steps. Clarified that deleting source log entries does not reduce existing hourly summaries. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
GottZ
added a commit
that referenced
this pull request
Nov 3, 2026
…ur (W06-4) (#74) Wave W06-4 of the OpenAI-compatible API plan (RC-A, telemetry foundation; design/06 §4.7 / §4.5, masterplan K17). No migration, no `/v1` surface. ## What this changes - New leaf package `internal/llmusage` (imports only the `pgxdb` handle atoms; depguard rule `llmusage-leaf` pins that): `Window(ctx, q, Spec{Now, Span, APIKeyIDs, Locality})` reads one rolling window as **hourly rollup from the first full UTC hour after `now − span`** plus **the raw `context_llm_log` rows of that first, partial hour** — the exact semantics of the raw `created_at > now() - interval` queries it replaces, with at most one hour of raw rows per window. It also returns the earliest relief of each figure (oldest counted row + span), microsecond-exact. - `backends.QuotaAccountant.spend` reads its three figures (24 h calls, 24 h external cost, 30-day rolling external cost) through `llmusage.Window` instead of a 30-day scan over the hypertable. The filter stays the tenant's knowledge keys (`api_key_id`): **a `/v1` consumer row never counts here** (K17, B-06-18) — consumer traffic cannot exhaust `daily_calls` or the cost budget and lock a tenant's people out of `/api/query` and the web chat. Relief timestamps are stored on the tenant state for W06-14 (no reader yet). - Docs: `docs/multi-tenancy.md` (spend source, asymmetry), `docs/operations.md` (Migration 159 rollback now coupled to the reader), `docs/architecture.md`. ## Behaviour changes (intended) - Spend figures are byte-equal to the old query for the same clock (parity property below); the windows now end at the accountant's clock instead of the database's `now()` (same host, same clock in production). - Three index probes per tenant refresh (one per window) instead of one 30-day scan. ## Deviations from design/06 §4.7, with reasons - The raw edge always ends at the **next** full hour, also when `since` lies exactly on one: the bucket starting at `since` holds a row stamped exactly at `since`, which the strict lower bound excludes; the design formula counted such a row twice (90 parity deviations with it, 0 with the fix). - Keys are probed one by one (`LATERAL … OFFSET 0`) instead of `api_key_id = ANY($1)`: with `= ANY` the planner preferred the time-leading indexes (`uq_llm_usage_hourly`, the log's `created_at` index) and read every tenant's rows in the range — under the generic plan the whole instance's edge hour. The locality condition sits in the aggregates' `FILTER` so both renderings share one access path. Pinned by `TestQuotaSpendPlan` under the custom and the forced-generic plan. - Relief is exact (oldest raw row of the oldest counted rollup hour, one bounded lookup) rather than `bucket_start`: the bucket variant reports relief **too early** (11:00 for a row at 11:05:00.000007); a safe conservative variant would have been `bucket_start + 1 h`. ## Consumers of the changed contracts | Changed symbol | Callers / readers | Covered by | |---|---|---| | `QuotaAccountant.spend` (signature, now fills `tenantState` incl. relief) | `refresh` (`quota.go`) | `TestQuotaAccountant_*` (backends 80 → 83), `TestChatQuota`, `TestSynthesizeQuota`, handler status/quota suites | | `NewQuotaAccountant(q quotaDB, …)` (needs `Querier` + `Rower`) | `cmd/ctxd`, tests | compile; existing callers pass `*pgxpool.Pool` | | `context_llm_usage_hourly` (first reader) | `llmusage.Window` | `TestWindowParity` (500 seeds vs the old raw SQL), `TestWindowBounds` (unit, hour-exact edges), `TestQuotaSpendPlan` | | `.golangci.yml` depguard | CI lint | `golangci-lint run` 0 | ## Overlap search Open PRs #55/#57/#63/#67 on 2026-11-03: no shared files. Based on #73 (`43041709`); root unchanged since. ## Cost Per policy tenant every 30 s: three statements, each ≤ 1 h of raw rows through `idx_llm_log_apikey` plus the rollup through `idx_llm_usage_key`; measured in `TestQuotaSpendPlan`: raw rows touched 3–5 (base: 1 441, with no upper bound on `created_at` and no rollup). Planner edge: on a rollup of a few dozen tenants the custom plan may still pick the time-leading `uq_llm_usage_hourly` for the 30-day window (≈ 1.5× the probe's cost); from ~100 tenants on it moves to `idx_llm_usage_key`. Candidate for later: `INCLUDE` columns on `idx_llm_usage_key`, one batched statement over all tenants. ## Verification Gates, each red on `43041709` first: (a) existing quota suites green before and after (backends 80/83, chat 1, llm 4, handler 52); (b) parity property `llmusage.Window` vs the old `spend` SQL over 500 random seeds with rows around both window edges — 425 deviations without the edge rows, 90 with the design's `ceilHour`, 0 final; (c) `EXPLAIN` under custom and generic plan: raw rows 1 441 → 3–5, rollup via `idx_llm_usage_key`; (d) asymmetry pin: a spend-union variant (`OR consumer_id = ANY`) counts 5 consumer calls and the gate refuses on `daily_calls`, the shipped code counts 0 and lets the tenant through, the control with 5 knowledge-key calls refuses; (e) relief with a fixed clock: compile error on base, `bucket_start` variant 11:00 vs exact 11:05:00.000007. Suites on the head: `go build ./...`, `go vet -tags=integration ./...`, `golangci-lint run` with and without `--build-tags=integration` (0), `go test -short -race ./...` (65 ok), `deadcode.sh` + `deadcode.sh testonly` (ok; the new package is reached from `spend`), integration `-p 1 -race` for `internal/llmusage` + `internal/backends`, and `-run 'Quota|Status|LLMLog'` for `internal/chat`, `internal/llm`, `internal/handler`. Independent review by a fresh agent during the build: 4 minor + 5 nit, all fixed or documented in the wave report. ## Not verified - Clock skew between the daemon and the database shifts the windows by that skew (the old query used the database clock); in production both run on one host. - The planner edge above is observed at ~30 synthetic tenants, not measured on the live rollup. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Quota Improvements** * Tenant quota calculations now use rolling 24-hour and 30-day windows for attributed calls and provider-reported costs. Consumer-only activity is excluded from tenant quota totals. * Quota status now includes the earliest time counted calls or costs are expected to leave each window. * Usage totals combine hourly summaries with raw records from the current partial hour, preserving accurate rolling-window figures. * **Documentation** * Updated quota and operations guidance to explain usage windows, deleted-log behavior, and migration rollback considerations. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
GottZ
added a commit
that referenced
this pull request
Nov 3, 2026
…-5) (#75) Wave W06-5 of the OpenAI-compatible API plan (RC-A, telemetry foundation; design/06 §4.7). No migration, no `/v1` surface. ## What this changes - `/api/status` `llm_24h` (server view, `queryLLM24h`) and the per-tenant `llm_24h` generation (`buildTenantRollupInto`) read the hourly rollup `context_llm_usage_hourly` from the first full UTC hour after `now − 24 h` plus the raw `context_llm_log` rows of that first, partial hour, through two new readers in `internal/llmusage` (`Groups`, `TenantGroups`), instead of scanning 24 h of raw rows per tick. The tenant reader joins both parts through `context_api_keys` → `context_tenant_scopes` exactly as the raw query did; a consumer's row (`api_key_id` NULL) counts for no tenant. - `llm_24h_complete` comes from the rollup's `unattributed_ok` (successful rows without a backend) plus the same count over the edge rows: a failed row without a backend keeps `complete` true, as before. - Output is byte-identical to the raw aggregate (golden below); the one deliberate difference is a deterministic tie order. - Docs: `docs/operations.md` (rollup readers now quota + status; rollback coupling), `docs/architecture.md`. ## Behaviour changes (intended) - Rows with the same `calls` are now ordered by `backend`, then `pipeline` (the raw query ordered by `calls` alone and left ties to the plan; live has 16/16 ties). - Backend and pipeline names longer than 100 characters show cut to 100 — the rollup's dimension width since migration 159 (the longest live name has 24 characters). Pinned by a test. - The window ends at the daemon's clock, not the database's `now()` (same host in production; tests pin the anchor on the boundaries). - On an error in the middle of the result the server view returns an empty list and `complete=false` instead of the rows read so far plus `false`. ## Consumers of the changed contracts | Changed symbol | Callers / readers | Covered by | |---|---|---| | `queryLLM24h`, `buildTenantRollupInto` (same signatures, new source) | status tick (`assemble`), `SnapshotForTenant`, SSE `/api/events` | `TestStatusLLM24hGolden` (byte-equal to the base), `TestStatusLLM24hParity` (300 random seeds vs the raw SQL), `TestStatusLLM24hPlan`, `TestStatusPerTenantView`, `TestStatusGoldenKeys`, `TestStatusEventCarriesEveryDualPathField`, 58 handler top-level tests | | `llmusage.Groups`, `TenantGroups` (new) | the two readers above | `internal/llmusage` suite (9/9), the handler suites | | `promptguard` call-site census | `internal/handler:g.Pipeline` passthrough (a copied string, no prompt) | `promptguard_callsite_test.go` | ## Overlap search Open PRs #55/#57/#63/#67 on 2026-11-03: no shared files. Based on #74 (`14b99639`); root unchanged since. ## Cost Per status tick: the edge is at most one hour of raw rows bounded on both sides (23 rows in the plan test vs 1 079 for the raw 24 h scan), the rest through `idx_llm_usage_bucket`; pinned under the custom and the forced-generic plan. Tenant view: the same two parts joined to the key and scope tables. ## Verification Gates, each red on `14b99639` first: (a) existing suites green before and after (handler 52 → 58, llmusage 9/9); (b) golden: a 16 027-byte JSON of both views generated on the base tree from the base SQL (only `now()` → `$1` and the tie-break added; the base functions match it up to tie order), new code byte-equal (same sha256, empty diff); red on base by compile error, red against overlays: no edge hour 253/300 seeds, truncated mean 262/300, no tie-break 298/300; (c) `complete` probe: red when the rollup read omits `unattributed_ok` and when the edge does not count it; an error row without a backend keeps `complete` true; (d) `EXPLAIN` custom + generic: 23 raw rows (exactly the edge) vs 1 079, rollup via `idx_llm_usage_bucket`; the base SQL fails the same check; (e) consumer rows absent from the tenant view — pinned, red against an overlay that credits them to a tenant. Suites on the head: `go build ./...`, `go vet -tags=integration ./...`, `golangci-lint run` with and without `--build-tags=integration` (0), `go test -short -race ./...`, `deadcode.sh` + `deadcode.sh testonly`, integration `-p 1` for `internal/llmusage` and `-run 'Status|Events|LLMLog|Quota'` for `internal/handler` + `internal/backends`. Independent review by a fresh agent during the build: 1 minor (the 100-character cut — documented and pinned), 4 nits (3 fixed, the error-path change documented above). ## Not verified - Clock skew between daemon and database shifts the window by that skew (same host in production). - The 100-character cut is only observable with names ctx does not produce today. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **Updates** * Server-wide and tenant-level 24-hour LLM usage summaries now use hourly usage data alongside records from the current partial hour. * Summaries indicate when usage data is incomplete because successful calls could not be attributed. * Backend and pipeline names are limited to 100 characters in summaries; names that match after truncation are grouped together. * **Documentation** * Updated architecture and operations guidance for usage summaries, attribution, retries, and the effects of removing or disabling rollup data collection. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
GottZ
added a commit
that referenced
this pull request
Nov 3, 2026
…r and channel (Migration 160) (#76) Wave W06-6 of the OpenAI-compatible API plan (RC-A, telemetry foundation; design/06 §3.3 M-06-3, masterplan K16). Migration **160**, no new table, no `/v1` surface. The read surface `GET /api/audit` follows in W06-7. ## What this changes - `go/migrations/160_audit_keys_quota.sql`: audit rows in `context_settings_audit` for `context_api_keys` (`create`, `update`, `revoke`, `reactivate`, `delete`) and `context_tenant_quota` (`create`, `update`, `delete`). State is projected through **whitelist** functions `audit_state_api_key` / `audit_state_tenant_quota` (never `to_jsonb(NEW) - …`): `key_hash`, `last_used_at`, `created_at`, `updated_at`, `id` never land in `old_value`/`new_value`. The key UPDATE trigger fires only on the ten audited columns and only when one of them changed, so the per-request `last_used_at` write of authentication never audits (B-06-1). `audit_actor_meta()` reads the transaction-local stamps (`ctx.api_key_id`, `ctx.request_id`, `ctx.audit_via`) and resolves the actor's label, principal, tenant and server-admin flag into `metadata` — the inputs of the W06-7 redaction. Backfill: one `snapshot` row per existing key and quota row with `metadata.tenant_id` (12 rows on the live install), idempotent. Index `idx_settings_audit_entity (entity_type, created_at DESC)`. - `internal/store/audit_via.go`: the `via` vocabulary as Go constants (`api`, `bootstrap`, `provision`, `offboarding`, `migration`; `sql` = no stamp), `AuditStamp`, `stampAudit` (one `set_config(…, true)` statement inside the write transaction). Every API-driven writer of the two tables now stamps actor + request id + via: `MintKeyWithQuota`, `MintOwnerKey` (tenant create, project provision), `DeleteApiKey`, `UpdateApiKey`, `UpsertTenantQuota`, the bootstrap admin key (now in a transaction, `via='bootstrap'`, no actor), tenant offboarding (`via='offboarding'`). A row written by psql reads `via='sql'` with no actor — the signal of a direct intervention. - Classification pin: `audit_classification_test.go` compares the live column list of each audited table against audited ∪ explicitly excluded (as data); a new unclassified column turns it red (B-06-2), per table and per column. - Docs: `docs/security.md` "Audit trail" (what is audited, what never is, `via` values, actor fields), `docs/operations.md` "Migration 160". - Hardening after review (`258e0953`): `stampAudit` refuses a non-empty stamp on a handle that is not a `pgx.Tx` (`store: audit stamp outside a transaction`) before anything is written — on a pool the `set_config(…, true)` would be its own transaction and the write would silently read `via='sql'` without an actor (red-proof: `MintOwnerKey(pool, APIStamp)` created the key with a `via='sql'` row on `7d4adf3d`; now an error and 0 rows). Every production caller passes a transaction. - The private plan submodule pointer is bumped in a separate commit (plan corpus of this build). ## Behaviour changes (intended) - Key and quota writes now leave one audit row each; the audit table grows with the lifecycle of keys and quotas (rare), never with requests. - The bootstrap admin key is inserted inside a transaction (was a single statement). - `UpsertTenantQuota` is stamped as well (otherwise the quota row would carry no `request_id`). ## Deviations from design/06 §3.3, with reasons - `via` falls back to `api` only when an actor was stamped, else `sql` (the design's `COALESCE` would read a psql quota update as `api`). - Quota trigger: the `updated_by` fallback actor applies on INSERT or when `updated_by` changed; an UPDATE that changes no audited state writes no row (no empty rows from `ON DELETE SET NULL`). - Quota `entity_key` is the `scope` (the row's natural key). - The index is created before the triggers (shorter lock window, reviewer finding). - Two existing tenant-create tests used `"non-admin-key-id"` as an actor id; the stamp now casts it to `uuid`, so the fixtures use a UUID (production actor ids are always UUIDs). ## Consumers of the changed contracts | Changed symbol / object | Callers / readers | Covered by | |---|---|---| | `MintKeyWithQuota`, `MintOwnerKey`, `DeleteApiKey`, `UpdateApiKey`, `UpsertTenantQuota` (new `AuditStamp` parameter) | `handler/context_manage.go`, `tenant_manage.go`, `quota_manage.go`, `project_provision.go`, `store/provision.go`, `store/tenant.go`, `bootstrap_admin_key.go` | `api_keys_audit_integration_test.go` (store, 8 gates; handler, 4 gates), `TestProvisionAuditVia`, bootstrap/prune suites | | `context_settings_audit` rows for `api_key` / `tenant_quota` | W06-7 reader (next wave), `/api/settings/{key}` audit (unchanged: filters `entity_type='setting'`) | settings suites unchanged | | schema-contract manifest (+ functions, triggers, index) | `/api/contract`, `TestCheck_FreshDB_NoDrift` | red without the 160 entries | ## Overlap search Open PRs #55/#57/#63/#67 on 2026-11-03: no shared files. Based on #75 (`b4d430cb`); root unchanged since. ## Cost Boot: catalog work plus the backfill (12 rows live); `CREATE TRIGGER` takes SHARE ROW EXCLUSIVE on `context_api_keys` briefly (lock_timeout 3 s). Per key/quota write: one `audit_actor_meta` lookup (one indexed `SELECT` on `context_api_keys`) and one audit INSERT. Per authentication: nothing — the `last_used_at` UPDATE is outside the trigger's column list (gate: 6 authentications + 2 no-op updates ⇒ 0 rows). ## Verification Gates, each red on `b4d430cb` first: store gates 8 FAIL → 8 PASS, handler gates 4 FAIL → 4 PASS, `via` pin compile error → PASS. Negative 1 (trigger without the column list): 8 audit rows for 6 authentications + 2 no-op updates → 0. Negative 2 (blacklist `to_jsonb(k) - 'key_hash'`): `last_used_at`, `created_at`, `updated_at`, `id` leak into old/new → only the ten whitelisted columns. Negative 3 (probe column `secret_probe`): classification pin red → green. Negative 4 (`MintOwnerKey` without stamp): `via='sql'`, no actor → `via='api'` with actor and `request_id`; with all stamps disabled every writer gate was red, bootstrap, provision and offboarding included. Backfill: N keys ⇒ N `snapshot` rows with `tenant_id`, second run 0 rows and byte-identical catalog. Suites on the head: `go build ./...`, `go vet -tags=integration ./...`, `golangci-lint run` with and without `--build-tags=integration` (0), `go test -short -race ./...` (66 ok), `deadcode.sh` + `deadcode.sh testonly`, integration `-p 1`: `internal/store` (330), `internal/handler` (138 in the manage/tenant/key/audit/settings/provision selection), `internal/auth` (14), `cmd/ctxd` (51), `internal/schemacontract` (44). Independent reviews: one during the build (0 blocking, 2 should-fix applied — docs downgrade sentence, per-column pin — 6 notes) and one briefed from the main context after it (findings folded in below where they changed the diff). ## Not verified - The settings trigger writes shadow rows with `via='api'` during offboarding and `PruneTenant` leaves quota rows in place — both pre-existing, not touched here. - `metadata.tenant_id` has no index yet; the W06-7 tenant filter runs on `idx_settings_audit_entity` plus a JSON predicate (fine at today's row counts, to be measured at 1 M+ rows). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> <!-- coderabbit-ai-generated:v1 --> ## Summary by CodeRabbit * **New Features** * Added audit trails for API key and tenant quota creation, updates, and deletion, with relevant actor and request details. * Provisioning and offboarding actions are also attributed in audit records. Unchanged updates and authentication-only activity are excluded. * Existing keys and quotas without audit history receive an initial snapshot. * **Documentation** * Added guidance on audit coverage, recorded details, attribution, and inspecting audit records. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
W4-NERF
approved these changes
Nov 3, 2026
W4-NERF
left a comment
W4-NERF
left a comment
Collaborator
There was a problem hiding this comment.
Review at 77991da. Verified independently against the primary sources, not just against the PR text:
plugin-catalog/ctx_checkpoint.yamlonhermes-agentmaintoday:sha: 9f98ff65a51831ca72739c095b134f543e59bc32,version: "0.6.2".- #147170 merged 2026-11-02T22:57:57Z, merge commit
d1d2be3ad0. - In this repository: tag
v5.29.6→9f98ff65,VERSIONat that commit reads0.6.2, andintegrations/hermes/plugin/ctx_checkpoint/memory/exists there. - History line: the same catalog file at the #144212 merge
611d1e4pins98749f3/0.4.4(2026-10-25); at the #145302 merge3c6fe03it pinsee8cb2e/0.5.0(tagv5.27.0, 2026-10-29). The new Versions sentence matches all three pins. - The stale-pin grep returns nothing on the head. The remaining
0.5.0/ee8cb2ehits are the historical entries the PR lists as unchanged (Setup (plugin 0.5.0 and later), Versions 0.4.3–0.5.1,test_setup.py,install.sh). - Bidi/zero-width scan over the four changed files at head (U+202A–U+202E, U+2066–U+2069, U+200B–U+200F, U+FEFF): zero hits. The "hidden or bidirectional Unicode" banner on the PR page does not correspond to anything in this diff.
- CI on 77991da is green: Build, Lint, Unit Tests, Unit Tests (windows), Integration ×6, Web, cross-compile ×2. Docker, attestation and the live tiers are correctly skipped on
pull_request.
DamieMoon's change request is addressed at 77991da:
plugin/ctx_checkpoint/README.mdno longer names the current pin. It reads "for code newer than the catalog pin (see Versions)" and points at the integration README onroot— where the pin belongs, because that file is reached viadocs_urland is not rendered from the pinned sha.- The
memory/README.mdnit is handled the same way: "the catalog version includes this engine (since 0.6.2)", with the current pin deferred to Versions.
The rendering path confirms that shape: https://hermes-agent.nousresearch.com/docs/plugins/ctx_checkpoint renders the README at the pinned sha (website/scripts/extract-plugins.py, readme_url(repo, sha, subdir); plugin-catalog/README.md: "the build fetches the README … at the pinned sha"). Right now that page shows "Version 0.6.2 at 9f98ff6…" and, below it, the old sentence "the way to get the memory features while the catalog still pins 0.5.0" — it disappears only with the next re-pin, which is the part DamieMoon flagged as out of scope here.
Copilot's two blob/root nits are correctly declined in the inline threads: a relative link would resolve against the pinned tree (plugin-catalog/README.md: "links and images resolved against the pinned tree") and would therefore show the Versions section as of the pinned commit, i.e. the pin before it — the same staleness this PR removes. For the reader's actual question ("is my code newer than the catalog?") blob/root is the right target.
FYI, not blocking and not for this PR: the upstream entry still declares capabilities.provides_tools: [] while its own description names ctx_query/ctx_store/ctx_search/ctx_get. Worth fixing at the next re-pin; DamieMoon raised it as FYI too.
Approving. The PR stays blocked by DamieMoon's CHANGES_REQUESTED until he re-reviews or the maintainer dismisses that review — a reviewer cannot lift it.
DamieMoon
approved these changes
Nov 3, 2026
DamieMoon
left a comment
DamieMoon
left a comment
Collaborator
There was a problem hiding this comment.
Re-review at 77991da. Both points from my change request on aea6275 are addressed.
integrations/hermes/plugin/ctx_checkpoint/README.md:41-42no longer names the current pin. It reads "for code newer than the catalog pin (see Versions)", which is the version-neutral shape I suggested. The## Versionsanchor exists atintegrations/hermes/README.md:371, on this head and onroot.- Nit:
memory/README.md:84-87now reads "The catalog version includes this engine (since 0.6.2)" and leaves the current pin to Versions. "Since 0.6.2" is a fixed fact in this repo:memory/exists under the plugin dir atv5.29.6=9f98ff65(VERSION 0.6.2), and not atv5.27.0=ee8cb2e(0.5.0) orv5.26.1=98749f3(0.4.4).
I grepped both files that the catalog renders or ships from the pinned sha for 0.6.2|147170|9f98ff6|0.5.0. At aea6275 the grep hits README.md:42 and memory/README.md:84-85. At 77991da the only hit is the version-neutral "since 0.6.2". The current pin now lives only in integrations/hermes/README.md:153-155 and :393-398, which the catalog reaches on root via docs_url (checked upstream in my first review, not re-checked here).
On Copilot's two blob/root threads I agree with keeping the absolute link. A relative link would resolve against the pinned tree, and that tree's Versions section can only show an older pin. At 9f98ff65 it still says "Catalog pin: 0.5.0 at ee8cb2e".
Also checked: the stale-pin grep from the PR body returns nothing on head. There are no bidi or zero-width characters in the four files. go build ./..., go vet ./... and golangci-lint v2.15.0 are clean, and CI is green on 77991da. I could not re-check the upstream side (the #147170 merge, the catalog yaml, the rendered page) from my environment for this re-review; my first review covered it at aea6275.
One thing before merge. The PR description still describes aea6275, and per GOVERNANCE.md the body becomes the merge commit body:
- "16 lines in / 16 out": the head diff is 15 in / 17 out.
- The table rows for
plugin/ctx_checkpoint/README.mdandmemory/README.mdquote the aea6275 wording ("0.6.2 since hermes-agent #147170, merged 2026-11-02" and "catalog pins 0.6.2 (9f98ff6) since … so a catalog install includes this engine"). Neither is in the head diff. - "Every remaining
147170reference … (integrations/hermes/README.md:155,:396,plugin/ctx_checkpoint/README.md:42,memory/README.md:85)": at head onlyintegrations/hermes/README.md:155and:396remain. - "The only link this PR adds points to #147170.": the head diff also adds two links to
blob/root/integrations/hermes/README.md#versions(plugin/ctx_checkpoint/README.md:42,memory/README.md:85). - The "Summary by CodeRabbit" block at the end of the body would end up in the merge commit body too.
Please update the body, or rewrite the merge commit body when merging. No new commit or re-review is needed for that.
Approving.
Inline (optional):
- integrations/hermes/README.md:395 — Nit:
#145302is plain text here while#147170on the next line is a link.[#145302](https://github.com/NousResearch/hermes-agent/pull/145302)would match. Not blocking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Docs only, 4 files, 16 lines in / 16 out. Every sentence that still described the Hermes plugin catalog as pinned to 0.5.0, or the re-pin as open upstream, now states the merged pin:
integrations/hermes/README.md(install section)ee8cb2e(release v5.27.0), plugin 0.5.0, re-pinned in #1453029f98ff6(release v5.29.6), plugin 0.6.2, re-pinned in #147170integrations/hermes/README.md(Versions)ee8cb2e… the re-pin to 0.6.0 is open upstream as #147170 (2026-10-31)9f98ff6; history 0.4.4 → 0.5.0 (#145302) → 0.6.2 (#147170, 2026-11-02)integrations/hermes/plugin/ctx_checkpoint/README.mdintegrations/hermes/plugin/ctx_checkpoint/memory/README.mdctx_*tools. Until the re-pin lands upstream …9f98ff6) since hermes-agent #147170 (merged 2026-11-02), so a catalog install includes this engine. For code newer than the catalog pin, install from a ctx checkout …README.md(Hermes row)The
README.mdwording came in with e9ae601 (0.6.0 import, 2026-10-31) while the catalog was still at 0.5.0 and therefore carried only the checkpoint layer.Historical version notes (
Setup (plugin 0.5.0 and later), the 0.4.3–0.5.1 entries under Versions,test_setup.py,install.sh) are unchanged.Why
NousResearch/hermes-agent#147170 ("chore(plugin-catalog): re-pin ctx_checkpoint to 0.6.2") was merged by teknium1 at 2026-11-02T22:57:57Z, merge commit
d1d2be3ad0: NousResearch/hermes-agent#147170plugin-catalog/ctx_checkpoint.yamlon hermes-agentmain(blob4761c990), lines 3 and 5:9f98ff65is the commit tagv5.29.6points at, andintegrations/hermes/plugin/ctx_checkpoint/VERSIONthere reads0.6.2.Verification
On the head of this branch:
Every remaining
147170reference describes it as the merged re-pin (integrations/hermes/README.md:155,:396,plugin/ctx_checkpoint/README.md:42,memory/README.md:85).Not verified
9f98ff65, the page keeps the old "while the catalog still pins 0.5.0" sentence until the next re-pin.hermes plugins install ctx_checkpoint) against 0.6.2. "A catalog install includes this engine" rests onmemory/existing under the pinned subdir at9f98ff65.🤖 Generated with Claude Code
Summary by CodeRabbit