Skip to content

feat(analytics): add dashboard model filter#861

Merged
wesm merged 2 commits into
kenn-io:mainfrom
rodboev:pr/633-analytics-model-filter
Jun 28, 2026
Merged

feat(analytics): add dashboard model filter#861
wesm merged 2 commits into
kenn-io:mainfrom
rodboev:pr/633-analytics-model-filter

Conversation

@rodboev

@rodboev rodboev commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

The analytics dashboard can already narrow by project, machine, agent, and time,
but still rolls every chart and summary together across models. This adds a
model-name filter to the main dashboard. A comma-separated model value is
threaded through the analytics route layer, the shared AnalyticsFilter, the
analytics/trends/filtered-model lookup helpers, and the dashboard request state.

When a model filter is active, the dashboard first scopes to sessions that contain
at least one matching model message, then derives message, token, tool-call,
trend-term, session-shape, and velocity metrics from the matching model rows within
that session set. Summary totals, activity, heatmap, projects, tools, skills, top
sessions, signals, trend terms, session shape, and velocity stay aligned across the
SQLite, PostgreSQL, and DuckDB backends, including mixed-model sessions and hour/day
slices.

model is the first message-grain analytics filter — a session spans many models —
so it can't ride the session-grain WHERE builders the other filters use. Each panel
instead needs a user->assistant pairing pass over candidate message rows. Rather
than copy that pairing into every panel, the model membership, user-turn pairing,
and day/hour match now live in one shared streaming reducer (the internal/db
message-scope helpers); each backend keeps its own candidate-row SQL (dialect,
placeholders, driver) and feeds rows through the shared reducer and its stats/timing
projections. Analytics and trends across all three backends share one implementation
instead of six near-duplicate pairing loops. The reducer requires only that rows
arrive grouped by session with ascending per-session ordinal — what
ORDER BY session_id, ordinal yields under any collation — so PostgreSQL's
collation-dependent ordering stays correct alongside SQLite and DuckDB.

On the frontend, the analytics store gains model-filter state, request params,
clear/toggle helpers, and active-filter chips, and the toolbar gains a model
dropdown that keeps known models stable across refreshes and filtered reloads.

One known limitation: the summary aggregation (median, p90, concentration) is still
computed per backend rather than shared; folding it into the same shared path is a
pre-existing cleanup left out here to keep this change scoped to model filtering.

Reviewers: the shared reducer (internal/db/messagescope.go,
messagescope_reducer.go) and the per-backend candidate-row resolvers
(internal/{db,postgres,duckdb}/analytics_scope.go) are the core; the rest is
route, filter, and frontend wiring. New analytics tests cover the filter across all
three backends.

Fixes #633

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (41a49ab)

Analytics model filtering has two medium-severity correctness/scalability issues; no high or critical findings.

Medium

  • internal/db/analytics.go:267 - The model filter is implemented as a session-level EXISTS, but message-aggregating analytics queries still aggregate all message rows from matching sessions. In a mixed-model session matching gpt-4o, claude-* messages can still contribute to hour-of-week, trends, activity, and message counts. Apply the model predicate to message-row aggregations, or explicitly separate session-scoped vs message-scoped model filtering and add mixed-model coverage for SQLite and PostgreSQL.

  • internal/db/analytics.go:406 - getAnalyticsModelsForSessionIDs builds a single IN (...) list for every filtered session. On the Go summary path, common for DST timezones, archives exceeding SQLite’s bind-variable limit can make the summary endpoint fail while collecting models. The PostgreSQL mirror has the same unchunked pattern at internal/postgres/analytics.go:452. Chunk session IDs with the existing helpers and merge/dedupe/sort returned model names.


Panel: ci_default_security | Synthesis: codex, 12s | Members: codex_default (codex/default, done, 4m24s), codex_security (codex/security, done, 13s) | Total: 4m49s

@rodboev

rodboev commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Applied the chunking fix in d5134519 for both SQLite and PostgreSQL model-list lookups, so the summary path no longer builds one oversized IN (...) clause.

I left the mixed-model behavior session-scoped on purpose. The issue asks for filtering the main dashboard by model, and specifically calls out seeing model usage in sessions. This PR keeps the dashboard scoped to sessions that contain at least one matching model, which is the contract already described in the PR body. That keeps session-level cards, projects, tools, skills, and top sessions on one consistent population instead of partially rewriting some of them into message-level metrics.

A message-scoped model-usage dashboard still seems useful, I just see that as a separate slice from this session dashboard filter.

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (d513451)

Summary verdict: one medium issue remains; no high or critical findings reported.

Medium

  • internal/db/trends.go:71; internal/postgres/trends.go:35
    The trends model filter is applied through the shared session predicate, which only checks that the session has at least one matching model. The outer query still counts every eligible m row in that session, so mixed-model sessions leak non-selected model messages into message_count and term totals.
    Fix: For message-row aggregations, clear sessionFilter.Model before building the session WHERE, then apply the CSV model predicate directly to outer m.model in both SQLite and PostgreSQL paths. Add mixed-model tests.

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 5m8s), codex_security (codex/security, done, 15s) | Total: 5m29s

@rodboev

rodboev commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

The trends paths now clear sessionFilter.Model before building the shared session predicate, then apply the selected model directly to outer m.model in both SQLite and PostgreSQL. That keeps mixed-model sessions from leaking non-selected messages into trend message_count and term totals.

I also added mixed-model coverage for both backends, plus a PostgreSQL query-shape test that keeps the model predicate on the outer message rows.

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (4e09c6c)

Medium issue found; security review found no additional issues.

Medium

  • frontend/src/lib/api/generated/services/AnalyticsService.ts:628
    getApiV1AnalyticsSignalSessions does not accept or include the new model query parameter, so signal evidence requests drop the active model filter even though analytics.signalEvidenceParams() includes it. This can show unfiltered signal examples for model-filtered signal totals.
    Fix: Add model to this method’s destructuring, parameter type, and query object, and cover the signal evidence request path in a frontend test.

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 7m21s), codex_security (codex/security, done, 52s) | Total: 8m19s

@rodboev

rodboev commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

AnalyticsService.getApiV1AnalyticsSignalSessions() now accepts model and includes it in the query object, so signal evidence requests stay aligned with the active analytics model filter.

I also added a focused frontend test for that request path, and npm test -- --run AnalyticsService.test.ts passed locally.

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (be14b6f)

Summary verdict: Medium-risk correctness issues remain in model-filtered analytics for mixed-model sessions.

Medium

  • Location: internal/db/analytics.go:1085, internal/db/analytics.go:1637
    Problem: Model-filtered message aggregations still count every message in sessions that contain the selected model. In mixed-model sessions, Activity and Hour-of-Week can include off-model messages because the model predicate is only a session-level EXISTS. PostgreSQL counterparts reportedly use the same query shape.
    Fix: Apply the model CSV predicate to the joined messages m rows for message-level aggregations in both SQLite and PostgreSQL, and add mixed-model session test coverage.

  • Location: internal/db/analytics.go:548, internal/db/analytics.go:584
    Problem: Model and hour/day filters are evaluated against separate message existence checks. A session with gpt-4o at 09:00 and another model at 10:00 can match model=gpt-4o&hour=10, so time-filtered panels can include sessions where the selected model was not active in the selected time bucket. PostgreSQL filteredSessionIDs has the same issue.
    Fix: When time filters and model filters are both active, apply the model predicate inside the same message timestamp check used for the time filter in both backends.


Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 6m41s), codex_security (codex/security, done, 1m7s) | Total: 7m58s

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (f6342d2)

Medium issue found: tool-call analytics can overcount across mixed-model sessions.

Medium

  • internal/db/analytics.go:1249, internal/db/analytics.go:2210, internal/postgres/analytics.go:823, internal/postgres/analytics.go:1627
    • The new model filter is applied when selecting sessions, but tool-call aggregations still count every tool_calls row in those sessions.
    • In mixed-model sessions, ActivityEntry.ToolCalls, the Tools panel, and Skills panel can include tool calls from unselected models even though message counts are filtered by m.model.
    • Fix by joining tool_calls back to messages in the activity/tools/skills tool-call queries and applying the same model CSV predicate in both SQLite and PostgreSQL. Add mixed-model tests with tool calls on selected and unselected model messages.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 5m22s), codex_security (codex/security, done, 14s) | Total: 5m43s

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (25f8649)

Summary verdict: One medium backend parity issue remains; no high or critical findings.

Medium

  • internal/duckdb/analytics_usage.go:127: AnalyticsFilter.Model is wired into the API and SQLite/PostgreSQL paths, but DuckDB analytics still ignores it. duckBuildAnalyticsWhere has no model predicate, and DuckDB activity/trends/tool/skill queries do not constrain joined messages by m.model, so agentsview duckdb serve returns unfiltered analytics for ?model=... and the summary does not populate model options.
    • Fix: Add DuckDB parity for the model filter across analytics/trends queries, including message-level predicates where SQLite/PostgreSQL now apply them, and add DuckDB tests for model-filtered analytics.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 6m31s), codex_security (codex/security, done, 1m2s) | Total: 7m40s

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (42f2026)

Summary verdict: One medium correctness issue remains; no security findings were reported.

Medium

  • Location: internal/db/analytics.go:837
    • Problem: The model filter only narrows sessions, but the summary still sums session-level message_count. Mixed-model sessions selected by model=gpt-4o can include messages from other models, while activity, tools, and trends count only matching message rows. The same session-level pattern remains in heatmap/projects and in PostgreSQL/DuckDB summaries.
    • Fix: For model-filtered message metrics, aggregate from messages with the same m.model predicate, or keep model filtering consistently session-scoped across all analytics endpoints and tests.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 6m5s), codex_security (codex/security, done, 48s) | Total: 7m0s

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (7e798f2)

Medium-risk issue remains: AnalyticsSummary.Models can include models outside the active message-level filter scope.

Medium

  • internal/db/analytics.go:1075, internal/postgres/analytics.go:679, internal/duckdb/analytics_usage.go:606
    • AnalyticsSummary.Models is populated from every model in the filtered sessions, not from the same message-level model/time scope used for the summary counts. A mixed-model session filtered with model=gpt-4o can still return unrelated models like claude-3-5-sonnet in models, making the response and dropdown options inconsistent with the active filter.
    • Suggested fix: pass the active AnalyticsFilter into the model-list helper and apply the same model/hour/day message predicates, or derive the list from the same filtered message rows used for counts. Add mixed-model regression coverage for SQLite, PostgreSQL, and DuckDB.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 6m8s), codex_security (codex/security, done, 25s) | Total: 6m40s

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (8378a7a)

Medium issues remain in analytics model filtering; no High or Critical findings were reported.

Medium

  • Location: internal/db/analytics.go:1150
    Problem: summary.models only uses the message-level filtered lookup when Model is set. With only hour/dow active, mixed-model sessions can contribute models from messages outside the selected time window, so the model dropdown can show models that are not actually present in the filtered message scope. The same pattern appears in the PostgreSQL and DuckDB summary paths.
    Fix: Use the filtered model lookup whenever f.HasTimeFilter() or a real model filter is active, and make the SQLite fast path constrain the joined message rows by the same time predicates.

  • Location: internal/db/analytics.go:4835
    Problem: Top sessions now accepts the model filter, but the messages metric still orders and returns the full session message_count. A session with one selected-model message and many other-model messages can rank above a session with many selected-model messages, making the model-filtered dashboard misleading. PostgreSQL and DuckDB have the same session-total behavior.
    Fix: When metric == "messages" and a model filter is active, rank and return per-session counts of matching model messages across all backends.


Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 6m12s), codex_security (codex/security, done, 1m19s) | Total: 7m40s

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (0a096a9)

Summary verdict: The PR has medium-severity analytics correctness issues in model-filtered aggregations; no security issues were found.

Medium

  • Location: internal/db/analytics.go:1375
    Problem: Model + hour/day filters only use timeIDs to admit sessions; the activity query then counts every message for that model in the admitted session. A session with gpt-4o messages at 09:00 and 10:00 will report both messages for model=gpt-4o&hour=10. The same pattern affects tool-call counts and the PostgreSQL/DuckDB paths.
    Fix: Apply the hour/day timestamp predicate to the message and tool-call aggregation queries when a time filter is active, or reuse a shared helper that filters rows by both model and message time.

  • Location: internal/db/analytics.go:1651
    Problem: Model-filtered requests recompute message counts, but output-token metrics still use full session totals. Mixed-model sessions will leak off-model tokens into output_tokens heatmaps, top-session output-token ranking, and summary token totals; PostgreSQL and DuckDB keep the same session-total behavior.
    Fix: Aggregate messages.output_tokens and token coverage from rows matching the selected model and active time filter, and use those model-scoped totals across SQLite, PostgreSQL, and DuckDB.


Panel: ci_default_security | Synthesis: codex, 12s | Members: codex_default (codex/default, done, 12m26s), codex_security (codex/security, done, 44s) | Total: 13m22s

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (f049114)

Medium confidence: one substantive issue remains in model-filtered velocity analytics; no security issues were reported.

Medium

  • internal/db/analytics.go:3494 - The model filter reaches the velocity endpoint, but velocity still loads all messages and tool calls for any session containing the selected model. In mixed-model sessions, off-model timings, message rates, character rates, and tool rates can leak into the filtered velocity panel. The same issue exists in the PostgreSQL and DuckDB velocity paths.

    Fix: Make velocity model-aware across SQLite/PostgreSQL/DuckDB, or stop sending model to the velocity endpoint if velocity is intentionally session-scoped.


Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 10m54s), codex_security (codex/security, done, 1m36s) | Total: 12m37s

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (3a5aa0d)

Medium findings:

  • internal/postgres/analytics.go:618; internal/duckdb/analytics_usage.go:976
    Model-filtered tool-call counting requires a parseable message timestamp even when no hour/day filter is active. GetAnalyticsVelocity uses this helper for any model filter, so PostgreSQL and DuckDB undercount selected-model tool calls for sessions with NULL/missing tool-call message timestamps, while SQLite counts them unless a time filter is set.
    Fix: only parse/apply the timestamp check when f.HasTimeFilter() is true; otherwise count every tool call whose joined message matches the selected model.

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 9m15s), codex_security (codex/security, done, 2m37s) | Total: 11m58s

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (228c6d8)

Summary verdict: One medium issue remains; no high or critical findings were reported.

Medium

  • internal/db/analytics.go:2383; internal/postgres/analytics.go:1930; internal/duckdb/analytics_usage.go:1592
    The new model filter reaches the session-shape endpoint, but length distribution still buckets by full-session message_count, and autonomy still queries all messages in the session. Mixed-model sessions can therefore show off-model message volume and tool/user ratios for the selected model. The same unfiltered count is also used for velocity complexity buckets.

    Fix: When f.Model is set, derive session-shape length/autonomy and velocity complexity from the same filtered message/tool stats used by the other analytics views, and add mixed-model tests across SQLite, PostgreSQL, and DuckDB.


Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 9m27s), codex_security (codex/security, done, 3m10s) | Total: 12m45s

@rodboev

rodboev commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Fixed on 8a23705.

Session-shape now derives length buckets and autonomy from the same filtered per-session message stats that the model-scoped analytics views already use in internal/db/analytics.go, internal/postgres/analytics.go, and internal/duckdb/analytics_usage.go, so mixed-model sessions stop leaking off-model message volume and off-model user turns into the selected model's shape view.

Velocity complexity now uses those filtered message counts as well, which keeps model-filtered sessions in the correct 1-15 or 16-60 bucket instead of classifying them by the full session totals.

I also added mixed-model regressions for SQLite and DuckDB, plus matching PostgreSQL pgtests for the same shape and complexity cases.

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (8a23705)

Medium issue found: model-filtered analytics can still return off-model signal evidence.

Medium

  • internal/db/analytics.go:4026: GetAnalyticsSignalSessions receives the model filter, but signalMessages is called without it and loads every message for each candidate session. Mixed-model sessions can return off-model evidence excerpts even when filtered to one model. The same issue exists in PostgreSQL and DuckDB stores.

    Fix: Pass the model filter into the signal-message loaders and apply the same CSV model predicate there, with backend coverage for mixed-model sessions.


Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 7m32s), codex_security (codex/security, done, 3m1s) | Total: 10m39s

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (f571938)

Summary verdict: one medium-severity analytics correctness issue should be addressed before merge.

Medium

  • internal/db/analytics.go:304
    Model-filtered message stats only keep rows whose own messages.model matches. Several parsers store the model only on assistant messages and leave user turns empty, so filtered analytics can drop all user messages and skew user counts, autonomy, velocity, and trends for normal sessions.
    Fix: Attribute user turns to the selected-model exchange, or define model filtering around assistant/model turns and add parser-realistic tests where user messages have an empty model.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 10m14s), codex_security (codex/security, done, 2m19s) | Total: 12m40s

@rodboev

rodboev commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

I traced the last roborev report back to the model-scoped message loaders rather than any single panel. The shared filtered-message helpers, the velocity loaders, and the trend scans were all treating messages.model as a row-local gate, which drops parser-realistic user turns when only the assistant row carries the selected model.

This rework keeps the model filter anchored on the selected assistant turn, but it now buffers empty-model user rows and attributes them to that assistant exchange when the reply matches the active model. I’m applying that rule in SQLite, PostgreSQL, and DuckDB, and I switched the existing mixed-model regressions over to empty-model user rows so the tests exercise the real parser shape that roborev flagged.

@roborev-ci

roborev-ci Bot commented Jun 25, 2026

Copy link
Copy Markdown

roborev: Combined Review (b074d05)

Summary verdict: model-filter analytics mostly look safe, but two Medium correctness issues remain.

Medium

  • Location: internal/db/analytics.go:1504, internal/postgres/analytics.go:724
    Problem: Model-filtered activity always calls filteredSessionIDs, even when no hour/day filter is active. That helper requires a non-empty/non-NULL parseable message timestamp, so sessions whose selected-model messages lack timestamps are dropped from SQLite/PostgreSQL activity, while other model-filtered panels still count them.
    Fix: Only call and apply filteredSessionIDs when f.HasTimeFilter() is true; otherwise rely on the model EXISTS predicate already in where.

  • Location: internal/db/analytics.go:4270, internal/postgres/analytics.go:3361, internal/duckdb/analytics_usage.go:2664
    Problem: Model-filtered signal evidence filters messages strictly by model, which drops parser-style user turns that usually have an empty model. User-prompt signals such as short prompts, duplicate prompts, and frustration markers can then lose their actual evidence and fall back to unfiltered FirstMessage with no ordinal.
    Fix: Reuse the model-scoped message selection logic that preserves pending model-less user turns before selected-model assistant messages, or otherwise include those scoped user evidence rows for signal examples.


Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 8m35s), codex_security (codex/security, done, 2m48s) | Total: 11m33s

@rodboev

rodboev commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up pushed in

@roborev-ci

roborev-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (043dcc7)

Summary verdict: one medium correctness issue remains; no high or critical findings were reported.

Medium

  • internal/db/analytics.go:2931, internal/postgres/analytics.go:2226, internal/duckdb/analytics_usage.go:1878
    Model-filtered tool analytics ignore the active day/hour filter when counting tool calls. The session prefilter can require a selected-model message in the active time window, but the aggregation query then counts every tool call for that model in those sessions, including calls outside the selected hour/day.

    Fix: Apply the same message timestamp filter used by getAnalyticsFilteredToolCallCounts when aggregating tool categories, and add SQLite/PostgreSQL/DuckDB tests for Model + Hour or Model + DayOfWeek tool counts.


Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 10m12s), codex_security (codex/security, done, 2m44s) | Total: 13m2s

@rodboev

rodboev commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Applied the tool analytics fix. Model-scoped tool counts now resolve each call against its own message timestamp, with session fallback only when the tool row has no message timestamp, so Model + Hour and Model + DayOfWeek stop pulling in off-window tool calls from the same session. I added regression coverage for SQLite, PostgreSQL, and DuckDB, and reran CGO_ENABLED=1 go test -tags "fts5,kit_posthog_disabled" ./internal/db ./internal/duckdb ./internal/postgres -count=1.

@roborev-ci

roborev-ci Bot commented Jun 26, 2026

Copy link
Copy Markdown

roborev: Combined Review (9f4b1ab)

Summary verdict: one medium issue remains; no security findings were reported.

Medium

  • internal/db/analytics.go:612 - SQLite analytics summary models can include models from sessions outside the exact requested local date range when an hour/day filter is active. filteredSessionIDs uses the padded UTC session window but not the same local date predicate as the summary SQL, so SQLite can return off-scope model options while PostgreSQL/DuckDB do not.
    • Fix: Build the model session ID set from the same exact summary scope, or apply the local From/To predicate before calling getAnalyticsModelsForSessionIDsFiltered; add a regression with a matching-hour session just outside the date window.

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 11m24s), codex_security (codex/security, done, 2m21s) | Total: 13m51s

@wesm

wesm commented Jun 26, 2026

Copy link
Copy Markdown
Member

Looking at this. Trying to figure out why this PR is so big so will see if there is some refactoring that can be done to make the change simpler

@rodboev

rodboev commented Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Tests are more than half the diff. Most of the rest comes from three backends (SQLite/PostgreSQL/DuckDB) that don't share a query abstraction, so the model filter had to be threaded through each one independently. The rest grew from iterative edge-case fixes during review.

I can split future work like this into stacked PRs if that's easier to review. A shared analytics query layer would also cut this kind of cross-backend feature down significantly; that felt like a bigger refactor to take on inside this PR.

@wesm

wesm commented Jun 26, 2026

Copy link
Copy Markdown
Member

No worries, I get it! I have agents looking to see what refactoring could be done now to help with this

@wesm

wesm commented Jun 26, 2026

Copy link
Copy Markdown
Member

I'm working on this, will update the PR in place

@rodboev rodboev force-pushed the pr/633-analytics-model-filter branch from 9f4b1ab to 7ec8ae5 Compare June 26, 2026 23:59
@rodboev

rodboev commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Sorry, just saw the comment. I'll leave it for you to work on.

The force push was just to replace the old head with the rebased one. I resolved conflicts in internal/db/analytics_test.go, internal/db/analytics.go, and internal/duckdb/analytics_usage.go, then reran the focused backend tests before pushing.

@roborev-ci

roborev-ci Bot commented Jun 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (7ec8ae5)

Summary verdict: One medium correctness issue remains; no security issues were found.

Medium

  • internal/db/analytics.go:5421
    Model-filtered top-sessions requests now use the Go path, but duration requests still read only the first 200 rows before exact date/hour filtering. If longer selected-model sessions outside the active hour/local day fill that window, valid in-scope duration results are never considered, and SQLite can disagree with PostgreSQL.
    Fix: Drop the pre-filter LIMIT for duration and time-filtered Go paths, or push the exact filters into SQL before limiting.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 12m10s), codex_security (codex/security, done, 3m14s) | Total: 15m31s

@wesm

wesm commented Jun 27, 2026

Copy link
Copy Markdown
Member

Okay, I'll force push over what is here once my agents are done working, they are in the middle of a large superpowers plan

@wesm wesm force-pushed the pr/633-analytics-model-filter branch from 7ec8ae5 to 367846b Compare June 27, 2026 12:13
@roborev-ci

roborev-ci Bot commented Jun 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (367846b)

Analytics model filtering has two medium correctness issues; no security issues were found.

Medium

  • internal/db/analytics.go:799
    Model + hour/day filters preselect sessions using only messages whose model matches. The reducer treats empty-model user turns paired with a selected assistant as model-scoped, so a prompt at the selected hour followed by the selected-model assistant in a different hour is dropped before the reducer can count it. The same predicate shape is mirrored in PostgreSQL and DuckDB.
    Fix: For model-scoped time filters, derive matching sessions from resolveAnalyticsMessageScope/shared reducer semantics, or avoid the direct m.model time prefilter and filter out zero matched stats afterward across SQLite, PostgreSQL, and DuckDB.

  • internal/postgres/analytics_scope.go:76
    PostgreSQL model-scoped message resolution formats timestamps with whole-second precision, unlike SQLite and DuckDB. Model-filtered velocity calculations lose subsecond timing and can turn valid short response times into zero-second deltas that are discarded.
    Fix: Preserve fractional precision by scanning timestamp as *time.Time and formatting with FormatISO8601, or use a fractional TO_CHAR format compatible with time.RFC3339Nano.


Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 7m40s), codex_security (codex/security, done, 3m24s) | Total: 11m13s

@roborev-ci

roborev-ci Bot commented Jun 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (6644408)

Model filtering is mostly consistent, but one medium-severity analytics parity issue remains.

Medium

  • internal/db/analytics.go:2126, internal/postgres/analytics.go:1625, internal/duckdb/analytics_usage.go:1455
    The hour-of-week model filter directly filters m.model, bypassing the shared model-scope reducer used by the other model-filtered analytics paths. User turns with empty model that are paired with a selected-model assistant are counted in summary/activity/velocity/trends, but dropped from the hour-of-week grid.
    Fix: For model-filtered hour-of-week requests, bucket messages from resolveAnalyticsMessageScope / ScopeReducer so paired user turns are included consistently across backends.

Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 10m41s), codex_security (codex/security, done, 2m43s) | Total: 13m32s

@rodboev

rodboev commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for this. The single-owner approach with thin adapters eliminates the scope leaks at the source. I'll dive deeper into this to make sure future work is aligned along these principles so extraction opportunities are caught before the code gets written. Appreciate you taking the time to do it right.

@wesm

wesm commented Jun 27, 2026

Copy link
Copy Markdown
Member

All good! I was hoping it might be possible to make the change smaller but at least architecture wise this appears to be a tidier way to go about it. I'll work on getting this merged here today

@wesm wesm force-pushed the pr/633-analytics-model-filter branch from 6644408 to 3c41559 Compare June 27, 2026 21:00
@roborev-ci

roborev-ci Bot commented Jun 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (3c41559)

Summary verdict: One medium correctness issue remains; no security issues were found.

Medium

  • Location: internal/db/analytics.go:798
    Problem: Model plus day/hour filtering requires the message that satisfies the time filter to have m.model set to the selected model, but the new scope reducer intentionally pairs empty-model user turns with the following selected-model assistant. A session with an empty-model user message at 09:00 followed by a selected-model assistant at 10:00 appears in the model-filtered hour-of-week grid for 09:00, but clicking that hour drops it from summary/activity/velocity/signals because filteredSessionIDs rejects it. The same direct-model time prefilter exists in the PostgreSQL and DuckDB paths.
    Fix: For model-filtered time filters, derive matching session IDs from the scoped message reducer or otherwise apply day/hour after model scoping, including paired empty-model user turns, across SQLite, PostgreSQL, and DuckDB.

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 14m37s), codex_security (codex/security, done, 3m6s) | Total: 17m52s

@roborev-ci

roborev-ci Bot commented Jun 27, 2026

Copy link
Copy Markdown

roborev: Combined Review (634d0db)

Medium: DuckDB still has model/time filter parity gaps.

  • Medium: internal/duckdb/analytics_usage.go:1253, internal/duckdb/analytics_usage.go:2373
    DuckDB only routes model-filtered messages/output_tokens through the scoped reducer. heatmap?metric=sessions and top sessions with metric=duration fall through to duckBuildAnalyticsWhere, whose day/hour predicate requires the same message to both have m.model and match the time filter. That drops sessions where the matching row is a paired empty-model user turn, while SQLite/PostgreSQL include them.
    Fix: Route all model-filtered session-level metrics that apply day/hour filters through analyticsSessions/the scope reducer before bucketing or sorting.

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 9m12s), codex_security (codex/security, done, 3m46s) | Total: 13m7s

@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (6d37d21)

Summary verdict: one medium issue should be addressed before merge.

Medium

  • frontend/src/lib/stores/analytics.svelte.ts:386: The new model parameter is added to shared analytics params that are also used by Insights page signal fetches and signal drilldowns. A model selected on Analytics can silently carry over to Insights, where there is no model control or visible active filter, and generated insight filters do not include the same model scope.
    • Fix: Clear or omit model for fetchSignalsForInsights() and insight signal evidence, or add an Insights model control and include the model consistently in insight generation filters.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 12m58s), codex_security (codex/security, done, 3m22s) | Total: 16m27s

@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (58181c8)

PostgreSQL model-scoped top sessions can return too many rows; no security findings.

Medium

  • internal/postgres/analytics.go:3073: The PostgreSQL model-scoped messages / output_tokens top-sessions path disables the SQL LIMIT so it can recompute and sort by model-filtered counts, but it never reapplies the top-10 cap before returning. With more than 10 matching sessions, /api/v1/analytics/top-sessions?model=... can return every matching session, unlike SQLite and DuckDB.
    • Fix: Truncate sessions to 10 after rankTopSessions, and add a PostgreSQL integration test with more than 10 model-matching sessions for messages and/or output_tokens.

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 8m8s), codex_security (codex/security, done, 3m50s) | Total: 12m5s

@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (bf47fcd)

Medium-severity issue found; no high or critical findings.

Medium

  • internal/db/messagescope_reducer.go:74 - Selected non-assistant rows can be emitted while earlier empty-model user rows remain buffered. When a later selected assistant flushes the buffer, ScopeTiming() can return rows out of ordinal order. Velocity processing assumes ordinal order, so model-scoped velocity can pair the wrong prompt/response when user messages carry model values.
    • Fix: Preserve ordinal order for emitted scoped rows, or sort timing rows by ordinal before velocity calculations. Add a regression test with an empty-model user followed by a model-bearing user before the selected assistant.

Panel: ci_default_security | Synthesis: codex, 20s | Members: codex_default (codex/default, done, 23m6s), codex_security (codex/security, done, 3m7s) | Total: 26m33s

@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (e52dbe6)

No Medium, High, or Critical findings were reported.

The only finding was Low severity and is omitted per instructions.


Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 5m9s), codex_security (codex/security, done, 3m40s) | Total: 8m57s

Add a model filter to the analytics dashboard so the stats, velocity,
signals, activity, trends, and hour-of-week panels can be scoped to a
selected model's user/assistant exchanges. Session Health stays
session-scoped, counting whole sessions that used the model rather than
recomputing its rollups from only that model's messages.

Model-scoped message pairing is centralized in internal/db message-scope
helpers (a streaming reducer, a day/hour predicate, and stats and timing
projections) consumed by the SQLite, PostgreSQL, and DuckDB backends, so
observable output is identical across all three. The reducer's grouping
contract is collation-independent, so PostgreSQL ordering matches SQLite
and DuckDB, and the timing projection restores ordinal order so velocity
pairs each prompt with its response. The usage guide documents the control
and its dashboard-only scope.

Co-Authored-By: Wes McKinney <wesmckinn+git@gmail.com>
@wesm wesm force-pushed the pr/633-analytics-model-filter branch from e52dbe6 to f20b071 Compare June 28, 2026 17:57
@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (f20b071)

Medium issue found: SQLite top sessions can return too many model-filtered results.

Medium

  • internal/db/analytics.go:5410 - The SQLite top-sessions path removes the SQL limit for model-filtered messages / output_tokens so it can re-sort by model-scoped counts, but rankTopSessions only rounds fields and never truncates. A model-filtered dashboard can therefore return every matching session instead of the expected top 10.

    Suggested fix: After the model-scoped re-sort, cap sessions to 10, or make the SQLite rankTopSessions helper truncate like the PostgreSQL implementation.


Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 9m56s), codex_security (codex/security, done, 3m46s) | Total: 13m50s

Add a regression test asserting the SQLite model-filtered top-sessions
path caps at ten for the messages and output_tokens metrics. The
model-scoped re-sort drops the SQL LIMIT and ranks every matching
session, so the caller's len > 10 truncation is what keeps the result at
the top ten. Verified to fail (returns twelve) when that cap is removed.
@roborev-ci

roborev-ci Bot commented Jun 28, 2026

Copy link
Copy Markdown

roborev: Combined Review (46382da)

No issues found.


Panel: ci_default_security | Synthesis: codex | Members: codex_default (codex/default, done, 14m38s), codex_security (codex/security, done, 3m3s) | Total: 17m41s

@wesm wesm merged commit 7081861 into kenn-io:main Jun 28, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Model filter on the main dashboard

2 participants