Add Coach-inspired session quality scoring and insights

[mariusvniekerk]

This PR adds session-quality measurement to agentsview. It turns archived sessions into inspectable quality signals: deterministic scoring for behavior that can be measured locally, plus opt-in generated recommendations that summarize those facts without becoming canonical score data.

The core goal is explainable review. Quality labels should be backed by stored signal counts, filtered evidence, score explanations, and session drilldowns so reviewers can decide whether the measurements are useful before treating them as calibrated defaults.

Source Inspiration

This work was shaped by Microsoft's AI Engineering Coach, especially its rule catalog, scoring detector, insights analyzer, and metric definitions.

The agentsview implementation adapts those ideas to a local, multi-agent session archive. Coach-inspired rules such as lazy prompting, missing file context, repeated prompts, missing spec structure, runaway loops, prompt maturity, premium-model waste, and sustainable-work-pattern signals were reviewed and then sorted into deterministic scoring, aggregate analytics, generated-insight inputs, already-covered behavior, or blocked future work depending on the data available in agentsview.

That distinction is intentional: this PR borrows the framing and rule inventory, but it does not make Copilot/VS Code-specific assumptions or use an LLM as the source of truth for score computation.

What This Adds

• A documented mapping from Coach rules to agentsview signal families, including what is scoreable now, what belongs in analytics or generated insights, and what is blocked by missing metadata.
• Deterministic prompt/context/workflow heuristics for local session scoring, including short task starts, missing criteria, missing verification, repeated prompts, missing code context, and repeated failing tool cycles.
• Persisted quality signal counts across SQLite, PostgreSQL, DuckDB, service, sync, and generated frontend API surfaces without recreating the archive database.
• Analytics and Insights surfaces for quality pattern summaries, grade/calibration metadata, signal totals, filtered evidence drilldowns, and deep links back to affected sessions.
• An opt-in canned LLM insight path using fixed templates, deterministic aggregate payloads, validation, provenance, cache keys, and template versions. Generated prose stays separate from health scores and signal rows.
• Rollout documentation, golden score assertions, resync preservation checks, frontend coverage, and screenshots so reviewers can evaluate the model before final calibration.

Important Boundaries

• LLMs do not perform canonical scoring. Session health remains deterministic and locally computable.
• Generated recommendations are opt-in, cached, provenance-bearing, and kept out of score/signal writes.
• Coach rules that require missing metadata, such as agent mode, plan mode, slash commands, approval mode, AI-generated LOC, review timing, or request cancellation, are documented but not scored.
• Timing and work-pattern ideas such as weekend or late-night activity remain analytics material, not session-quality penalties.
• Archive migrations remain additive. Existing session, orphaned, trashed, and saved insight data must be preserved.

Screenshots

Review Focus

• Whether the score-affecting heuristics are conservative enough and have acceptable false-positive behavior.
• Whether the scoring caps, grade deltas, and explanation fields are understandable enough for calibration.
• Whether the signal persistence/API contract is complete across SQLite, PostgreSQL, DuckDB, service, sync, and frontend clients.
• Whether the Insights page makes deterministic evidence easy to inspect before reading generated recommendation text.
• Whether generated insights are clearly bounded as optional summaries rather than a second source of scoring truth.
• Whether the rollout checklist is sufficient before enabling these semantics broadly on real archives.

Remaining Rollout Work

Before treating this as final calibration, run a representative local archive pass and record score deltas, threshold crossings, false-positive themes, runtime impact, and source diversity. The added rollout document captures the expected report shape and the contract checklist for future signal/API changes.

[roborev-ci]

roborev: Combined Review (1c68fc1)

Medium confidence: one medium issue needs attention before merge.

Medium

• internal/server/insights.go:719 — Canned insight generation rebuilds aggregates with only date/project/automated scope and hard-coded UTC, ignoring the active analytics filters used by the visible dashboard such as session agent, machine, termination, min-user-message, one-shot inclusion, and browser timezone. This can generate and cache recommendations for a broader or different cohort than the user is viewing.

  Fix: Add a separate session-filter payload to generation requests, send the current analytics filter params, apply them to AnalyticsFilter, UsageFilter, and SessionFilter, and include them in provenance/cache keys.

---

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 17m30s), codex_security (codex/security, done, 4m51s) | Total: 22m28s

[cpcloud]

I think my only feedback right now is that I would probably like to see the all the recommendations first and then be presented with the detailed data.

[roborev-ci]

roborev: Combined Review (b10b1d1)

Medium issue found; no Critical or High findings.

Medium

• frontend/src/lib/stores/usage.svelte.ts:226 — The shared termination session filter is not forwarded to usage summary/top-session requests, so the Usage page can show costs for sessions outside the selected termination scope even though the backend and generated client support the filter. Add termination: sessionFilters.termination || undefined to baseParams() and include termination in the Usage page URL/session filter key handling.

---

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

[roborev-ci]

roborev: Combined Review (f7b4297)

Medium findings remain; no Critical or High issues reported.

Medium

• internal/postgres/analytics.go:133 - PostgreSQL analytics treats AnalyticsFilter.Machine as one literal value, while SQLite and the UI support comma-separated machine filters. In PG serve, machine=a,b matches a literal machine name instead of either selected machine. Split and trim f.Machine and build an IN (...) predicate with placeholders to match SQLite behavior.

• internal/duckdb/analytics_usage.go:175 - DuckDB analytics ignores AutomatedScope and still relies on ExcludeAutomated. The frontend sends automated_scope=all|automated without include_automated, so DuckDB serve can keep filtering to human sessions when the user selects “Both” or “Only automated”. Normalize f.AutomatedScope like SQLite/PostgreSQL and drive automated-session predicates from that scope.

---

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

[roborev-ci]

roborev: Combined Review (c64db84)

Medium issues remain in DuckDB analytics parity and timestamp handling.

Medium

• internal/duckdb/analytics_usage.go:1866
duckSignalMessages coalesces a DuckDB TIMESTAMP column with '' and scans it into a string, unlike the rest of the DuckDB store which scans timestamp values as any and normalizes with formatDBTime. Signal evidence can fail or return non-RFC3339 timestamps for rows with null timestamps.
  Fix: Select timestamp directly, scan into any, and set m.Timestamp = formatDBTime(ts).

• internal/duckdb/analytics_usage.go:2122
  DuckDB usage queries ignore UsageFilter.Termination, while SQLite/Postgres now apply it and the frontend sends it. In duckdb serve, usage summaries, comparisons, and top-cost sessions will not respect active/stale/unclean/clean filters.
  Fix: Add DuckDB usage termination predicates in appendDuckUsageSessionFilterClauses, matching the SQLite/Postgres semantics.

---

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 12m49s), codex_security (codex/security, done, 5m24s) | Total: 18m20s

[roborev-ci]

roborev: Combined Review (a21c0cd)

Medium-risk issue found; no Critical or High findings.

Medium

• internal/server/insights.go:529 - The Coach portion of canned insight payloads is scoped with db.SessionFilter.DateFrom/DateTo, which ignores filters.Timezone. Signals and usage in the same payload use timezone-aware filters, so non-UTC users can receive generated insight prompts where the Coach summary covers a different session set than the signal and usage aggregates.

  Fix: Apply the same local-date semantics before building the Coach summary, either by using a timezone-aware store query or by overfetching and filtering session activity dates in filters.Timezone.

---

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 14m21s), codex_security (codex/security, done, 6m9s) | Total: 20m36s

[roborev-ci]

roborev: Combined Review (ffc4df9)

Review verdict: one medium issue should be fixed before merge; no high or critical findings reported.

Medium

• internal/server/insights.go:60 - Canned insight filter timezones are trimmed/defaulted but not validated. Invalid timezones can flow into analytics, usage, and Coach session aggregation paths with inconsistent fallback behavior instead of matching the 400 response used by other analytics APIs.
  • Fix: validate filters.Timezone with time.LoadLocation after defaulting and reject invalid values before building the canned insight payload.

---

Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 13m14s), codex_security (codex/security, done, 6m38s) | Total: 20m2s

[roborev-ci]

roborev: Combined Review (cc37881)

Medium issue found; no Critical or High findings.

Medium

• frontend/src/lib/components/insights/qualityPatterns.ts:367
  The tool reliability pattern bases severity, affected session count, and retry/edit-churn driver session counts on sessions_with_failures. Backend aggregation only increments that field for ToolFailureSignalCount > 0, while retry and edit-churn signals are computed independently. Sessions with only retries or edit churn can therefore be hidden as 0 sessions / Clear, and retry/churn rows can get the wrong session count.

  Fix: Track sessions with retries and edit churn separately in the tool health aggregate, or derive driver counts from signals.calibration["tool_retries"].affected_sessions and signals.calibration["edit_churn"].affected_sessions.

---

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 11m41s), codex_security (codex/security, done, 4m5s) | Total: 15m52s

[roborev-ci]

roborev: Combined Review (c00b82d)

Medium issue found; no high or critical findings.

Medium

• internal/duckdb/analytics_usage.go:2140 - DuckDB usage queries ignore UsageFilter.AutomatedScope, so dashboard-scoped canned insight payloads can include the wrong session set compared with SQLite/Postgres. human still includes automated usage, automated does not restrict to automated sessions, and one-shot exclusion does not preserve automated one-shots for non-human scopes.
  • Fix: Mirror the SQLite/Postgres usage filter logic in appendDuckUsageSessionFilterClauses, including normalized automated scope, automated-scope predicates, and the automated one-shot exemption. Add DuckDB usage coverage for human, all, and automated scopes.

---

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 14m10s), codex_security (codex/security, done, 3m41s) | Total: 17m57s

[wesm]

rebasing

[roborev-ci]

roborev: Combined Review (215791f)

Summary verdict: Changes need fixes before merge due to two medium behavioral regressions.

Medium

• Location: frontend/src/lib/components/insights/InsightsPage.svelte:520
  Problem: The Insights page now drives its date controls and generation payload from analytics.from/to, but the Activity panel link still pre-fills insights.dateFrom/dateTo. Opening “Open in Insights page” loses the panel’s range, so generation runs for the analytics default/current range instead.
  Fix: Initialize/sync analytics.from/to from the prefilled insights dates when entering the Insights route, or update the Activity link to set the analytics date range before navigating.

• Location: internal/server/huma_routes_insights.go:390
  Problem: activityRangeSummary always sets ExcludeAutomated: true, even after humaGenerateInsight normalizes and passes automated_scope to BuildPrompt. Requests for automated_scope=automated or all get a session list for that scope but an activity summary for human-only sessions, producing contradictory prompt context.
  Fix: Pass AutomatedScope: req.AutomatedScope into the analytics filter and remove the hard-coded human-only filter/comment.

---

Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 22m59s), codex_security (codex/security, done, 7m1s) | Total: 30m8s

[roborev-ci]

roborev: Combined Review (ffb1e65)

Medium issue found: DuckDB usage filtering is inconsistent with SQLite/PostgreSQL for automated scope handling.

Medium

• internal/duckdb/analytics_usage.go:2143
  DuckDB usage filters still apply legacy ExcludeOneShot / ExcludeAutomated booleans directly and ignore UsageFilter.AutomatedScope. A query with AutomatedScope: "automated" can include human sessions, and ExcludeOneShot still drops one-shot automated sessions, unlike the SQLite and PostgreSQL paths.
  Fix: Mirror the SQLite/PG logic: normalize with duckNormalizeAutomatedScope, exempt automated sessions from the one-shot predicate when scope is not human, append duckAutomatedScopePredicate(scope, "COALESCE(s.is_automated, FALSE)"), and add a DuckDB usage-scope regression test.

---

Panel: ci_default_security | Synthesis: codex, 34s | Members: codex_default (codex/default, done, 16m51s), codex_security (codex/security, done, 4m38s) | Total: 22m3s

[roborev-ci]

roborev: Combined Review (5baa768)

Code needs changes before merge: one medium backend parity issue remains.

Medium

• internal/duckdb/analytics_usage.go:2143 - DuckDB usage filtering ignores UsageFilter.AutomatedScope. SQLite and PostgreSQL honor human/all/automated scopes and exempt automated sessions from the one-shot filter when scope is not human, but DuckDB still only checks ExcludeOneShot/ExcludeAutomated. This can make DuckDB usage totals, top-cost sessions, session counts, and canned insight usage payloads return a different session set for automated or all scopes.

  Fix: Mirror the SQLite/PostgreSQL logic in appendDuckUsageSessionFilterClauses: normalize f.AutomatedScope, apply the one-shot automated exemption for non-human scopes, and add duckAutomatedScopePredicate(scope, "COALESCE(s.is_automated, FALSE)").

---

Panel: ci_default_security | Synthesis: codex, 14s | Members: codex_default (codex/default, done, 14m18s), codex_security (codex/security, done, 3m21s) | Total: 17m53s

[roborev-ci]

roborev: Combined Review (c3cb4b1)

No issues found.

---

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

[roborev-ci]

roborev: Combined Review (8cbccbc)

Summary verdict: one medium issue needs attention; no critical or high findings were reported.

Medium

• frontend/src/lib/api/generated/models/DbSession.ts:5, frontend/src/lib/api/generated/models/ServiceSessionDetail.ts:5, frontend/src/lib/api/generated/models/DbSignalSessionsResponse.ts:6
  • Problem: The generated API types are out of sync with the new quality/evidence responses. Session models omit the runtime quality_signals object, and signal evidence sessions are typed as any[] instead of the generated DbSignalSessionExample[].
  • Fix: Expose these fields through schema-visible response DTOs or Huma schema overrides, then regenerate the client.

---

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 25m48s), codex_security (codex/security, done, 5m13s) | Total: 31m7s

[roborev-ci]

roborev: Combined Review (e38d1cd)

Clean overall: no Medium, High, or Critical findings reported.

The only findings were Low severity test-style/heuristic/UTF-8 truncation nits, so they are omitted per the review-combination rules.

---

Panel: ci_default_security | Synthesis: codex, 4s | Members: codex_default (claude-code/default, done, 10m38s), codex_security (claude-code/security, done, 3m49s) | Total: 14m31s