Skip to content

feat: surface model information in the UI#182

Draft
MCBoarder289 wants to merge 4 commits intowesm:mainfrom
MCBoarder289:copilot-parsing-updates
Draft

feat: surface model information in the UI#182
MCBoarder289 wants to merge 4 commits intowesm:mainfrom
MCBoarder289:copilot-parsing-updates

Conversation

@MCBoarder289
Copy link
Contributor

@MCBoarder289 MCBoarder289 commented Mar 17, 2026

This is an initial draft to address #123 , and surfaces model information that is available from the agent sessions.

Copilot parsing is the first that I built in here. It establishes a common pattern for a "main" model at the session level near the breadcrumb, and then surfacing specific models on messages when it differs from the main model. Took a minimal approach that we could iterate on.

My main concern would be the scalability of computing the main model on an active session (currently would grow with the number of messages). Could refactor that but didn't want to prematurely optimize just yet.

Let me know what you think, and happy to iterate on the approach.

@MCBoarder289 MCBoarder289 mentioned this pull request Mar 17, 2026
Open
@roborev-ci
Copy link

roborev-ci bot commented Mar 17, 2026

roborev: Combined Review (ecb9705)

Summary: The PR successfully surfaces AI model
information from Copilot sessions to the UI, but requires a fix in the parser to avoid dropping valid model data.

Findings

Medium

  • internal/parser/copilot.go:104-123,354-366: The parser drops valid
    model information when a Copilot session only reports session.shutdown.currentModel. handleSessionShutdown() stores that value in b.currentModel, but ParseCopilotSession() never uses it for MainModel unless there are later assistant messages or modelMetrics entries. The added test at
    internal/parser/copilot_test.go:427-447 documents this broken case instead of asserting the expected model. As a result, some sessions that do contain model info will still show an empty session model in the UI.
    • Suggested fix: After ComputeMain Model(b.messages), fall back to b.currentModel when no per-message/main-model winner is available; optionally backfill the last assistant message too if that better matches Copilot semantics.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@roborev-ci
Copy link

roborev-ci bot commented Mar 17, 2026

roborev: Combined Review (e1c50c3)

Verdict: The new main_model feature is securely implemented, but requires a UI fix for resolving child-session models in subagent conversations.

Medium

  • [MessageContent.svelte:40](/home/roborev/.roborev/clones/wesm
    /agentsview/frontend/src/lib/components/content/MessageContent.svelte#L40) and [SubagentInline.svelte:100](/home/roborev/.roborev/clones/wesm/agentsview/frontend/src/lib/components/content/
    SubagentInline.svelte#L100):     MessageContent resolves owningSession from sessions.sessions and otherwise falls back to sessions.activeSession. When it is rendered inside SubagentInline, child-session metadata usually lives in sessions.childSessions, not sessions .sessions, so the new off-main-model badge compares a child message against the parent session’s main_model. That will show incorrect model badges for subagent conversations whenever parent and child use different models. Suggested fix: resolve message.session_id against sessions.childSessions before
    falling back, or pass the owning session explicitly from SubagentInline.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@MCBoarder289
Copy link
Contributor Author

MCBoarder289 commented Mar 17, 2026

That is a good review and caught a case I don't see often locally (subagent sessions). I went ahead and added that scenario to the frontend and added some tests for that.

@roborev-ci
Copy link

roborev-ci bot commented Mar 17, 2026

roborev: Combined Review (93470d4)

Summary Verdict: The
commits successfully implement model tracking and UI rendering, but there is one medium-severity issue regarding model tracking on tool completions.

Medium

  • [internal/parser/copilot.go](/home/roborev/.roborev/clones/wesm/agentsview/internal/parser/cop
    ilot.go):238
    handleToolComplete() only backfills the preceding assistant message when b.messages[i].Model == "". That misses the case where the assistant tool-call message was parsed while b.currentModel still held the previous model, so the message gets
    stamped with a stale model and is never corrected even though tool.execution_complete.data.model is the newer, more specific value. That would surface the wrong per-message badge in the UI and can skew ComputeMainModel() after a model switch that is first observed on tool completion. Suggested fix
    : when a tool completion includes model, overwrite the matching preceding assistant message’s model instead of only filling empties, ideally keyed by the related toolCallId. Add a test for “model switches on a tool-backed turn without a prior session.model_change”.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

MCBoarder289 and others added 4 commits March 18, 2026 09:06
@MCBoarder289
feat: extract and display model info for Copilot sessions
aa83b1d 
   - Parse model from three sources in priority order:

    1. session.shutdown modelMetrics: accumulate request counts across
    all shutdown events (Copilot appends one per reconnect) and pick
    the majority — stored as main_model on the session

    2. tool.execution_complete model field: backfills the preceding
    assistant message and keeps currentModel current for subsequent
    messages

    3. session.model_change: sets currentModel for future messages
   - Add ComputeMainModel helper as a fallback when no shutdown metrics
   are available (counts model assignments across assistant messages)
   - Add main_model column to sessions table with idempotent migration;
   bump dataVersion to 5 to trigger full resync
   - Add MainModel to db.Session, ParsedSession, and plumb through
   UpsertSession, all column scan sites, and toDBSession in sync

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@MCBoarder289
feat: surface model information in the UI (wesm#123)
a6ba917 
- Add main_model field to frontend Session interface (core.ts)
- Add shortModelName() utility to format.ts: strips claude-/gpt-/gemini-
  vendor prefixes for compact display; full name preserved as tooltip title
- SessionBreadcrumb: show model-badge next to agent/token badges when
  session.main_model is set (e.g. "sonnet-4.6" for claude-sonnet-4.6)
- SubagentInline: show toggle-model label alongside token info in the
  subagent header row
- MessageContent: derive offMainModel — when an assistant message's model
  differs from the owning session's main_model, show a small muted label
  next to the timestamp (non-invasive; hidden when all messages use the
  same model)
- Tests: add shortModelName unit tests; add model-badge show/hide tests
  to SessionBreadcrumb.test.ts; update all session mock objects with
  main_model field (736 tests pass)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@MCBoarder289
fix: use b.currentModel as final MainModel fallback
07782d9 
When session.shutdown has currentModel but empty modelMetrics (and no
session.model_change or tool.execution_complete set the model before
any assistant message), shutdownModelCounts is empty and
ComputeMainModel returns ''. b.currentModel holds the correct value
from the shutdown event but was never used for MainModel.

Add it as a final fallback: shutdownModelCounts > ComputeMainModel >
b.currentModel. Update TestParseCopilotSession_ModelFromShutdownCurrentModel
to assert the expected model instead of documenting the broken behavior.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@MCBoarder289
fix: pass owning session explicitly to MessageContent in subagent con…
86d4eaf 
…text

MessageContent resolved owningSession from sessions.sessions, which only
holds top-level sessions. When rendered inside SubagentInline, the child
session lives in sessions.childSessions, so the model comparison fell back
to the parent's main_model and showed incorrect off-main-model badges for
subagent messages.

Fix: add optional owningSession prop to MessageContent. SubagentInline
already has the child session in subagentSession (from sessions.childSessions)
and now passes it explicitly. The fallback lookup in sessions.sessions is
preserved for all other call sites.

Adds MessageContent.test.ts with 7 cases covering:
- badge hidden when message.model matches session main_model
- badge shown when model differs from session main_model
- no badge when message has no model
- no badge for user-role messages
- explicit owningSession prop used for child session comparison (subagent case)
- badge shown when subagent message differs from child session main_model
- fallback to sessions.sessions lookup when no prop provided

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@MCBoarder289 MCBoarder289 force-pushed the copilot-parsing-updates branch from 93470d4 to 86d4eaf Compare March 18, 2026 14:06
@roborev-ci
Copy link

roborev-ci bot commented Mar 18, 2026

roborev: Combined Review (86d4eaf)

Summary: The changes add extraction, persistence, and UI display of session main-model metadata, but a High severity SQL syntax error needs to be addressed to prevent session syncing failures.

High

  • Location: internal/db/sessions.go:519
  • Problem: UpsertSession now includes main_model in the inserted column list, but the VALUES clause still has one fewer placeholder than columns/arguments. SQLite will reject the
    insert/update at runtime with a values-for-columns mismatch, which breaks session syncing.
  • Fix: Add the missing ? in the VALUES list so the placeholder count matches the 19 inserted columns and bound arguments.

Synthesized from 3 reviews (agents: codex, gemini | types: default, security)

@MCBoarder289
Copy link
Contributor Author

MCBoarder289 commented Mar 18, 2026 •
edited
Loading

^ I think that latest roborev review is a false positive... I added the ? in the VALUES list for the new column I added. There were 17 args previously, now there are 18. Not sure where it got 19?

This was after I rebased and cleaned up some super small merge conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@MCBoarder289