feat(session): add session keeping with sticky load balancing and threaded audit logs - #602
Merged
Conversation
…eaded audit logs Detect which client session each request belongs to and use it in two places: load-balanced virtual models route a session to the target that served it first (session_affinity, default on, disable per redirect), and the Audit Logs page groups requests into session threads (Group by session, default on, with expandable children and tree connectors). Session identity: explicit headers from a built-in registry of known tools (Claude Code, Codex CLI, OpenCode, Kilo Code, Goose, LiteLLM/Helicone conventions), then body signals (Anthropic metadata.user_id in both Claude Code formats, session_id, litellm_session_id, prompt_cache_key, responses conversation), then content auto-detection hashing the conversation opening. Configurable via SESSION_KEEPING_ENABLED / SESSION_AUTO_DETECT / SESSION_BUILTIN_RULES / SESSION_HEADER_<N> or the session: YAML block; zero config works. Non-UUID ids are scoped by user path. Audit entries persist an indexed session_id across SQLite/PostgreSQL/MongoDB (and the live preview events); GET /admin/audit/sessions lists threads (latest entry + count + span, sessionless entries as singletons) and GET /admin/audit/log?session_id=... fetches one thread. Sticky pins are per-instance with a 6h idle TTL, are never taken on the all-saturated 429 fallback, and re-pin when a pinned target becomes unavailable. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughSession keeping spans configuration, request detection, virtual-model affinity, audit persistence and APIs, and dashboard session-thread grouping. Requests receive detected session IDs, routing can pin them to targets, and audit logs can be retrieved as grouped threads. ChangesSession keeping
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SessionCapture
participant Detector
participant VirtualModelService
participant AuditStore
participant Dashboard
Client->>SessionCapture: model request
SessionCapture->>Detector: detect session id
Detector-->>SessionCapture: session id
SessionCapture->>VirtualModelService: resolve with session id
VirtualModelService-->>Client: selected target
SessionCapture->>AuditStore: persist session_id
Dashboard->>AuditStore: request grouped sessions
AuditStore-->>Dashboard: session heads and latest entries
Dashboard->>AuditStore: request session children
AuditStore-->>Dashboard: grouped audit entries
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; authentication establishes the effective user path before session detection, and normal and streaming audit persistence recover the resulting session identity from the final request context.
 T-Rex LogsWhat T-Rex did
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/auditlog/reader_sessions_sql.go`:
- Around line 20-66: The GetSessions session-grouping query needs index support
for its partitioning and per-session ordering. Inspect the audit_logs schema and
migrations for the existing session_id index, then add or update a composite
index covering auditThreadKey (session_id), timestamp, and id in the appropriate
schema/migration location; preserve the current filtering, ranking, and
pagination behavior.
In `@internal/auditlog/session_id_test.go`:
- Around line 120-128: Add grouped-read SessionID filtering assertions in
internal/auditlog/session_id_test.go:120-128 and
internal/auditlog/reader_sessions_mongodb_test.go:61-68. In each GetSessions
test, query SessionID "sess-a" and verify exactly one thread, the matching total
count, and its latest entry; preserve the existing status-filter assertions and
use the backend-specific test setup.
In `@internal/session/detect.go`:
- Around line 99-109: Replace the ambiguous string concatenation in
scopeSessionID with collision-resistant hash-based scoping that incorporates
userPath and id as distinct values, reusing the existing sha256 and hex imports.
Preserve the current behavior for empty userPath and UUID ids, which must
continue returning id unchanged.
In `@web/dashboard/src/pages/audit-logs/auditList.svelte.js`:
- Around line 218-243: Update the stale-result branch in the audit session fetch
flow to remove liveLogs.auditThreadChildren[sessionId] before returning. Reuse
the same placeholder cleanup behavior as the catch path, while keeping stale
handling silent and allowing toggleThread to retry the fetch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 92f94991-4e5a-4482-9b07-b83c32b7fedb
⛔ Files ignored due to path filters (4)
internal/admin/dashboard/static/dist/assets/index-CJ1Lm7IE.cssis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-D19L8xXa.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-DU1ycplF.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (66)
.env.templateCLAUDE.mdcmd/gomodel/docs/docs.goconfig/config.example.yamlconfig/config.goconfig/session.goconfig/session_test.goconfig/virtualmodels.godocs/docs.jsondocs/features/session-keeping.mdxdocs/features/virtual-models.mdxdocs/openapi.jsoninternal/admin/handler.gointernal/admin/handler_audit.gointernal/admin/handler_audit_sessions_test.gointernal/admin/handler_test.gointernal/admin/handler_virtualmodels.gointernal/admin/routes.gointernal/admin/routes_test.gointernal/app/app.gointernal/auditlog/auditlog.gointernal/auditlog/middleware.gointernal/auditlog/reader.gointernal/auditlog/reader_mongodb.gointernal/auditlog/reader_sessions_mongodb.gointernal/auditlog/reader_sessions_mongodb_test.gointernal/auditlog/reader_sessions_sql.gointernal/auditlog/reader_sql.gointernal/auditlog/session_id_test.gointernal/auditlog/store_mongodb.gointernal/auditlog/store_sql.gointernal/auditlog/stream_wrapper.gointernal/core/context.gointernal/live/broker.gointernal/server/http.gointernal/server/internal_chat_completion_executor.gointernal/server/session.gointernal/server/session_test.gointernal/session/detect.gointernal/session/detect_test.gointernal/session/factory.gointernal/session/factory_test.gointernal/session/session.gointernal/virtualmodels/balancer.gointernal/virtualmodels/config.gointernal/virtualmodels/resolve.gointernal/virtualmodels/service.gointernal/virtualmodels/snapshot.gointernal/virtualmodels/sticky.gointernal/virtualmodels/sticky_test.gointernal/virtualmodels/store_sql.gointernal/virtualmodels/types.goweb/dashboard/src/pages/audit-logs/AuditEntryRow.svelteweb/dashboard/src/pages/audit-logs/AuditEntrySummary.svelteweb/dashboard/src/pages/audit-logs/AuditFilters.svelteweb/dashboard/src/pages/audit-logs/AuditLogsPage.svelteweb/dashboard/src/pages/audit-logs/AuditThreadGroup.svelteweb/dashboard/src/pages/audit-logs/audit-logic.jsweb/dashboard/src/pages/audit-logs/auditList.svelte.jsweb/dashboard/src/pages/audit-logs/live-logs-logic.jsweb/dashboard/src/pages/audit-logs/liveLogs.svelte.jsweb/dashboard/src/pages/models/VirtualModelEditor.svelteweb/dashboard/src/pages/models/virtualModels.svelte.jsweb/dashboard/src/pages/models/virtualModelsLogic.jsweb/dashboard/tests/audit-list.test.jsweb/dashboard/tests/live-logs.test.js
…rage - Move SessionCapture after authentication so weak/auto session ids are scoped by the effective user path (a managed key's bound path), closing a cross-tenant thread/pin collision; audit entries pick the id up in the post-handler re-read, live previews carry it from the terminal events. - Scope weak ids by hashing user path and id as distinct values instead of ambiguous "path|id" concatenation. - Drop the thread-children loading placeholder on stale fetches so the spinner cannot persist forever. - Make the session_id index composite with timestamp (SQL and MongoDB) to back thread-detail ordering and the grouping query. - Cover the SessionID filter on grouped reads in both backends. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/auditlog/reader_sessions_mongodb_test.go`:
- Around line 69-79: The filter scenarios in the GetSessions test should be
table-driven rather than hard-coded separately. Refactor the existing
status-code and SessionID cases into a test-case table with subtests, reusing
the shared setup and assertion flow while preserving each case’s query
parameters and expected results.
In `@internal/auditlog/store_sql.go`:
- Around line 113-115: Rename the composite index declaration in the audit log
schema to a new name, such as idx_audit_session_timestamp, instead of reusing
idx_audit_session_id. Keep the indexed columns ordered as (session_id,
timestamp) so existing deployments create the required composite index.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9e232c87-0275-44db-bee8-657fefe2dbc4
⛔ Files ignored due to path filters (2)
internal/admin/dashboard/static/dist/assets/index-CSnAP_4I.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (10)
internal/auditlog/middleware.gointernal/auditlog/reader_sessions_mongodb_test.gointernal/auditlog/session_id_test.gointernal/auditlog/store_mongodb.gointernal/auditlog/store_sql.gointernal/server/http.gointernal/server/session.gointernal/session/detect.gointernal/session/detect_test.goweb/dashboard/src/pages/audit-logs/auditList.svelte.js
…rive filter tests - Retain 128 bits of the scoping/auto hash (sum[:16]) for practical collision resistance. - Recreate the session index as idx_audit_session_timestamp and drop the single-column predecessor, so databases that created it get the composite (verified against an existing SQLite file). - Fold the GetSessions filter scenarios into shared table-driven subtests run by both the SQL and MongoDB suites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hole-session expansion, outage-safe pinning Four fixes for session-keeping behavior gaps introduced or exposed by moving session detection after authentication: - CreateStreamEntry now takes the request context and stamps the session id onto the persisted stream copy — the copy is made mid-handler, before the audit middleware's post-handler enrichment, so every streamed request was losing its session id. - The live dashboard re-folds a row into its on-screen thread when a later event delivers the session id (audit.started always precedes detection now, so rows arrive sessionless and previously stayed separate heads until a refetch). - A session_id audit query without explicit date parameters is now unbounded instead of silently applying the default trailing window, so expanding a thread shows the whole session even when browsing a historical range. - Sticky affinity is keyed to the redirect's configured targets rather than the currently supported ones: with a single target momentarily available (provider outage, startup) the session still pins it, so a recovering target cannot move an active conversation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Follow-up review round addressed in 76171be:
🤖 Generated with Claude Code |
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/dashboard/src/pages/audit-logs/live-logs-logic.js (1)
127-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared id/request_id matcher.
The fold/regroup logic (lines 138-336) is correct and well-tested. Separately, the
id/request_idequality check is duplicated across four sites in this file (mergeLiveAuditEntry,mergeLiveAuditChild,removeLiveAuditThreadChild,removeLiveAuditEntry). A shared helper would keep matching semantics from silently diverging as more thread-grouping call sites are added.♻️ Proposed helper extraction
+function matchesLiveKey(entry, id, requestID) { + return (!!id && String(entry.id || '').trim() === id) || + (!!requestID && String(entry.request_id || '').trim() === requestID); +}Then replace each inline check, e.g.:
- const index = currentEntries.findIndex((entry) => { - return String(entry.id || '').trim() === key || - (incoming.request_id && String(entry.request_id || '').trim() === String(incoming.request_id).trim()); - }); + const index = currentEntries.findIndex((entry) => matchesLiveKey(entry, key, String(incoming.request_id || '').trim()));Also applies to: 218-221, 313-317, 428-431
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/dashboard/src/pages/audit-logs/live-logs-logic.js` around lines 127 - 130, Extract the duplicated id/request_id comparison into a shared matcher helper in live-logs-logic.js, preserving the current trimming, string conversion, and incoming request_id guard semantics. Replace the inline checks in mergeLiveAuditEntry, mergeLiveAuditChild, removeLiveAuditThreadChild, and removeLiveAuditEntry with calls to that helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@web/dashboard/src/pages/audit-logs/live-logs-logic.js`:
- Around line 127-130: Extract the duplicated id/request_id comparison into a
shared matcher helper in live-logs-logic.js, preserving the current trimming,
string conversion, and incoming request_id guard semantics. Replace the inline
checks in mergeLiveAuditEntry, mergeLiveAuditChild, removeLiveAuditThreadChild,
and removeLiveAuditEntry with calls to that helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: daaa594f-cbd4-49b8-876b-9d17b308eecc
⛔ Files ignored due to path filters (2)
internal/admin/dashboard/static/dist/assets/index-DbIJkqkC.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (12)
internal/admin/handler_audit.gointernal/admin/handler_audit_sessions_test.gointernal/auditlog/auditlog_test.gointernal/auditlog/session_id_test.gointernal/auditlog/stream_entry_request_fields_test.gointernal/auditlog/stream_wrapper.gointernal/server/passthrough_support.gointernal/server/translated_inference_service.gointernal/virtualmodels/balancer.gointernal/virtualmodels/sticky_test.goweb/dashboard/src/pages/audit-logs/live-logs-logic.jsweb/dashboard/tests/live-logs.test.js
…ity-complete stream copies, recency-aware re-fold - Sticky assignment (lookup, strategy choice, pin) now happens in one critical section via stickySessions.resolve, so concurrent first requests of a session — common for agents firing parallel calls — agree on a single target instead of racing to overwrite each other's pins (race test added). - Session detection materializes chat/responses bodies through the shared requestBodyBytes stage when ingress capture skipped them (over 64 KiB or chunked), so body signals and content detection work for exactly the large agent conversations where affinity matters most; bodies beyond the 1 MiB audit bound fall back to header signals. - CreateStreamEntry finalizes the full context-derived identity through EnrichLogEntryWithRequestContext instead of hand-picking the session id, which also fixes managed-key labels being absent from streamed audit records. - The live re-fold keeps the newest request as thread head when completions arrive out of order, matching what a refresh would show. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
|
Third review round addressed in fe605c0:
On the architecture suggestions: the atomic-resolve seam matches the proposed 🤖 Generated with Claude Code |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/features/session-keeping.mdx`:
- Around line 50-51: Update the body-based signals documentation in
session-keeping to describe the server body_size_limit configuration, including
its default of 10M, rather than stating a fixed 1 MiB limit. Clarify that bodies
exceeding this configured limit are excluded from the captured snapshot used for
body and automatic session detection, while preserving the note about
header-signal fallback.
In `@internal/virtualmodels/balancer.go`:
- Around line 84-91: Restructure the balancing flow around
stickySessions.resolve so strategy selection and choose/pick execution occur
before acquiring the sticky-session lock, keeping catalog reads from
targetCost/LookupModel outside the critical section. Keep only the existing
sticky pin lookup and assignment atomic under the lock, while preserving
candidate validation through poolTarget and the current fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b40cff92-3442-4be2-83b0-c43b13d3d2c6
⛔ Files ignored due to path filters (2)
internal/admin/dashboard/static/dist/assets/index-Z0MNC1F2.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (10)
docs/features/session-keeping.mdxinternal/auditlog/session_id_test.gointernal/auditlog/stream_wrapper.gointernal/server/session.gointernal/server/session_test.gointernal/virtualmodels/balancer.gointernal/virtualmodels/sticky.gointernal/virtualmodels/sticky_test.goweb/dashboard/src/pages/audit-logs/live-logs-logic.jsweb/dashboard/tests/live-logs.test.js
Contributor
Author
|
Final review-comment cleanup in 47e3d71: extracted the shared audit id/request_id matcher noted in the outside-diff review, and moved sticky strategy/catalog selection outside the session mutex while preserving atomic first-pin behavior. Full Go tests, targeted race tests, all dashboard tests, Svelte checks, and commit hooks pass. |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/virtualmodels/balancer.go`:
- Around line 79-105: The saturated-fallback path in
internal/virtualmodels/balancer.go lines 79-105 must return pool[0].selector
directly when saturatedFallback is true, avoiding s.sticky.resolve and
preserving session pins; retain normal strategy selection for non-saturated
routing. In internal/virtualmodels/sticky.go lines 43-61, update lookup so only
expired entries are deleted; treat a live pin that is not currently viable as a
miss without removing it, leaving repinning to the caller.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6327f626-1dcc-4cb9-8030-cc267373b213
⛔ Files ignored due to path filters (3)
internal/admin/dashboard/static/dist/assets/index-BDVc7dxb.cssis excluded by!**/dist/**internal/admin/dashboard/static/dist/assets/index-oXFoUAuu.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (12)
docs/features/session-keeping.mdxinternal/auditlog/session_id_test.gointernal/auditlog/stream_wrapper.gointernal/server/session.gointernal/server/session_test.gointernal/session/detect.gointernal/session/detect_test.gointernal/virtualmodels/balancer.gointernal/virtualmodels/sticky.gointernal/virtualmodels/sticky_test.goweb/dashboard/src/pages/audit-logs/live-logs-logic.jsweb/dashboard/tests/live-logs.test.js
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/dashboard/src/pages/audit-logs/live-logs-logic.js (2)
187-191: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPropagate late usage updates to thread children.
A demoted audit entry can now be merged as a child here, but
applyLiveUsageToAuditonly searchesauditLog.entries. If its usage event arrives afterward, the child never receives usage enrichment. Search and updateauditThreadChildrenas well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/dashboard/src/pages/audit-logs/live-logs-logic.js` around lines 187 - 191, Update the live usage handling around applyLiveUsageToAudit so usage events also search auditThreadChildren, not only auditLog.entries. Ensure matching child entries receive the same usage enrichment, including children created through mergeLiveAuditChild, while preserving the existing top-level entry behavior.
437-443: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve a session thread when its head is removed.
At Line 437, removing a grouped head with
session_count > 1drops the only visible thread row while its remaining children stay cached but unreachable. Promote a loaded child or reload the grouped list; decrement the thread count instead of deleting the thread outright.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/dashboard/src/pages/audit-logs/live-logs-logic.js` around lines 437 - 443, Update the removal logic around matchesLiveAuditKey and removeLiveAuditThreadChild so deleting a grouped session head with session_count > 1 preserves the visible thread: promote an already-loaded child or reload the grouped list, decrementing the thread count rather than removing the session entirely. Keep the current full-removal behavior when no child remains.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@web/dashboard/src/pages/audit-logs/live-logs-logic.js`:
- Around line 187-191: Update the live usage handling around
applyLiveUsageToAudit so usage events also search auditThreadChildren, not only
auditLog.entries. Ensure matching child entries receive the same usage
enrichment, including children created through mergeLiveAuditChild, while
preserving the existing top-level entry behavior.
- Around line 437-443: Update the removal logic around matchesLiveAuditKey and
removeLiveAuditThreadChild so deleting a grouped session head with session_count
> 1 preserves the visible thread: promote an already-loaded child or reload the
grouped list, decrementing the thread count rather than removing the session
entirely. Keep the current full-removal behavior when no child remains.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 21ac7bf4-762f-4e46-8195-a30c09177cfd
⛔ Files ignored due to path filters (2)
internal/admin/dashboard/static/dist/assets/index-UYgiC4ZC.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (16)
cmd/gomodel/docs/docs.goconfig/config.example.yamlconfig/config.godocs/openapi.jsoninternal/admin/handler.gointernal/admin/handler_audit.gointernal/admin/handler_audit_sessions_test.gointernal/admin/handler_test.gointernal/auditlog/auditlog.gointernal/auditlog/reader.gointernal/auditlog/store_sql.gointernal/server/http.gointernal/virtualmodels/balancer.gointernal/virtualmodels/sticky.gointernal/virtualmodels/sticky_test.goweb/dashboard/src/pages/audit-logs/live-logs-logic.js
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/dashboard/src/pages/audit-logs/live-logs-logic.js (1)
143-157: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
audit.detailfallback insert skips session folding, unlike the general branch.The general
audit.*path explicitly triesfoldLiveAuditIntoThreadbefore falling back to a plain insert whenauditGroupSessionsis on (lines 193-201), but theaudit.detailbranch's fallback (154-157) does not. If a detail event arrives for a request that's no longer inauditLog.entries(e.g. scrolled past theauditLog.limitwindow) nor in any loadedauditThreadChildrenlist, while another head for the samesession_idis still on-screen, this path inserts a second head for that session and double-incrementsauditLog.total, breaking the single-head-per-session invariant the rest of this file maintains.🐛 Proposed fix: mirror the general branch's fold-before-insert step
const child = this.mergeLiveAuditChild(incoming, patch); if (child) { this.notifyLiveConversation(child); return child; } if (!this.auditLiveInsertAllowed()) return; + if (this.auditGroupSessions) { + const folded = this.foldLiveAuditIntoThread(patch); + if (folded) { + this.notifyLiveConversation(folded); + return folded; + } + } this.auditLog.entries = [this.mergeLiveAuditUsagePatch(patch), ...currentEntries].slice(0, this.auditLog.limit || 25); this.auditLog.total = Number(this.auditLog.total || 0) + 1; return this.auditLog.entries[0];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/dashboard/src/pages/audit-logs/live-logs-logic.js` around lines 143 - 157, Update the audit.detail fallback in the live audit merge flow to attempt session folding via foldLiveAuditIntoThread before inserting a new entry, matching the general audit.* branch when auditGroupSessions is enabled. Use the folded result when available; otherwise preserve the existing insert and total-increment behavior, maintaining one head per session.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/dashboard/tests/live-logs.test.js`:
- Around line 967-978: Add a grouped-mode regression test for an audit.detail
event whose request is absent from auditLog.entries and auditThreadChildren
while another head with the same session_id is visible. Assert the
fallback-inserted entry folds into the existing thread rather than creating a
second head, preserving the existing head’s session_id and grouped structure.
---
Outside diff comments:
In `@web/dashboard/src/pages/audit-logs/live-logs-logic.js`:
- Around line 143-157: Update the audit.detail fallback in the live audit merge
flow to attempt session folding via foldLiveAuditIntoThread before inserting a
new entry, matching the general audit.* branch when auditGroupSessions is
enabled. Use the folded result when available; otherwise preserve the existing
insert and total-increment behavior, maintaining one head per session.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eb9219aa-c88d-480f-bcc7-c9880d0852f0
⛔ Files ignored due to path filters (2)
internal/admin/dashboard/static/dist/assets/index-Gd_0GZP5.jsis excluded by!**/dist/**internal/admin/dashboard/static/dist/index.htmlis excluded by!**/dist/**
📒 Files selected for processing (2)
web/dashboard/src/pages/audit-logs/live-logs-logic.jsweb/dashboard/tests/live-logs.test.js
Comment on lines
+967
to
+978
| test("audit.removed reloads an unexpanded grouped thread when its head disappears", () => { | ||
| const app = createLiveLogsApp({ auditGroupSessions: true }); | ||
| app.auditLog.entries = [{ id: "head", session_id: "s-a", session_count: 3 }]; | ||
| app.auditLog.total = 1; | ||
|
|
||
| app.removeLiveAuditEntry({ id: "head" }); | ||
|
|
||
| assert.deepEqual(app.auditLog.entries, []); | ||
| assert.equal(app.auditLog.total, 1); | ||
| assert.equal(app.fetchAuditCalls, 1); | ||
| }); | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Add regression coverage for audit.detail fallback-insert folding.
None of the grouped-mode tests cover an audit.detail event arriving for a request absent from both auditLog.entries and auditThreadChildren while another head for the same session_id is on-screen — the scenario affected by the fold-skip gap flagged in live-logs-logic.js (lines 143-157). Consider adding a case similar to this test that asserts the entry folds into the existing thread instead of creating a second head with session_id: "s-a".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/dashboard/tests/live-logs.test.js` around lines 967 - 978, Add a
grouped-mode regression test for an audit.detail event whose request is absent
from auditLog.entries and auditThreadChildren while another head with the same
session_id is visible. Assert the fallback-inserted entry folds into the
existing thread rather than creating a second head, preserving the existing
head’s session_id and grouped structure.
SantiagoDePolonia
added a commit
that referenced
this pull request
Jul 29, 2026
…615) * fix(auditlog): cast audit thread key to text for legacy uuid schemas PostgreSQL databases created before the unified schema (#586) keep their original uuid audit_logs.id column - CREATE TABLE IF NOT EXISTS never retypes it. The session thread key added in #602 coalesces id with the text session_id column, so every GET /admin/audit/sessions call on such a database failed with SQLSTATE 42804 (COALESCE types text and uuid cannot be matched), leaving the Audit Logs page empty in its default group-by-session view. Cast the id to text inside the thread key; the cast is valid on both backends and a no-op on SQLite. Covered by a regression test that recreates the exact pre-unification PostgreSQL DDL and runs GetSessions against it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(auditlog): pin per-thread latest entry in legacy-schema test Table-drive the session summaries and assert Latest.ID for the grouped thread too, so a regression in the per-thread recency ranking on the uuid id column cannot slip past the count check. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 31, 2026
This was referenced Aug 11, 2026
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
Requests from one client session (one conversation / one agent task) are now recognized and used in two places:
session_affinity: false(YAML/env/admin API/editor checkbox).How a session is identified
First match wins, zero client configuration needed:
X-Session-Id(OpenCode, Kilo Code),X-Claude-Code-Session-Id(Claude Code, documented for gateways),Session-Id/Session_id(Codex CLI, Roo Code),X-Litellm-Session-Id,Helicone-Session-Id,Agent-Session-Id(Goose).metadata.user_id(both Claude Code formats parsed),session_id,litellm_session_id,prompt_cache_key(Zed),/v1/responsesconversation.Config:
SESSION_KEEPING_ENABLED/SESSION_AUTO_DETECT/SESSION_BUILTIN_RULES/ numberedSESSION_HEADER_<N>(+_TRANSFORM), or thesession:YAML block. Credential headers are rejected as sources. Non-UUID ids are scoped by user path so weak client ids cannot collide across tenants.Provider-specific / operational notes
session_idis a new indexed column onaudit_logs(idempotent ALTER migration on SQLite/PostgreSQL, index on MongoDB) and a new tri-statesession_affinitycolumn onvirtual_models.GET /admin/audit/sessions(threads: latest entry + count + span; filters apply to entries before grouping; sessionless entries are singleton threads);GET /admin/audit/loggains asession_idfilter that works without date params./v1/responsesprevious_response_idchain-following, failover-sweep re-pinning, do-not-pass stripping for session headers.Docs
New
docs/features/session-keeping.mdx, updates to virtual-models docs,.env.template,config.example.yaml, CLAUDE.md config reference; swagger regenerated.Testing
metadata.user_idformats + auto-detect stability; sticky semantics (both strategies, re-pin, no-pin-on-saturation, TTL, prune, eviction); audit column roundtrip + sessions listing verified live against SQLite, PostgreSQL, and MongoDB; stream-entry whitelist guard; admin handler + routes; config env/yaml merge. Dashboard: 376 node tests (grouped merge, live folding, gating, flat-mode regression), svelte-check clean, dist rebuilt.🤖 Generated with Claude Code
Summary by CodeRabbit
session_affinitysupport for virtual models in the dashboard and admin/API (configurable to return to stateless balancing).session_id, allow exact filtering, and support a threaded “group by session” view via a new/admin/audit/sessionsendpoint.