Skip to content

fix: always write model field to checkpoint metadata.json#882

Open
dipree wants to merge 6 commits intomainfrom
fix/always-write-model-to-metadata
Open

fix: always write model field to checkpoint metadata.json#882
dipree wants to merge 6 commits intomainfrom
fix/always-write-model-to-metadata

Conversation

@dipree
Copy link
Copy Markdown
Contributor

@dipree dipree commented Apr 8, 2026

Summary

  • Remove omitempty from CommittedMetadata.Model JSON tag so the field is always present in metadata.json (empty string when unknown, never missing)
  • Wire up model reporting on all hook events across agents that had gaps:
    • Claude Code: parseTurnEnd and parseSessionEnd were ignoring the model field already present in sessionInfoRaw
    • Codex: parseTurnStart and parseTurnEnd had model available in raw types but unused
    • Cursor: parseSessionStart was ignoring raw.Model despite the field being parsed
    • Vogon: Add model to all hook payloads and event parsing for canary E2E coverage
  • Add Model field to E2E SessionMetadata test struct

All agents now report model info where feasible:

Agent Model Source
Claude Code SessionStart, TurnEnd, SessionEnd
Codex SessionStart, TurnStart, TurnEnd
Gemini CLI BeforeModel (ModelUpdate event)
OpenCode TurnStart, TurnEnd
Cursor SessionStart, TurnStart, TurnEnd
Factory AI Droid TurnStart, TurnEnd + transcript fallback
Copilot CLI TurnEnd (transcript extraction)
Vogon All hooks

Test plan

  • mise run fmt — no formatting issues
  • mise run lint — 0 issues
  • mise run test — all unit tests pass
  • mise run test:e2e:canary — all 4 canary tests pass

🤖 Generated with Claude Code

The model field was tagged omitempty, causing it to be silently absent
when the agent didn't provide model info. Consumers couldn't distinguish
"unknown model" from "field not supported yet". Now the field is always
present (empty string when unknown).

Also wires up model reporting for Cursor SessionStart (was available but
ignored) and Vogon (test agent), so all agents now report model info.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 8, 2026 23:40
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ensures the LLM model identifier is consistently captured and persisted in checkpoint session metadata, so downstream consumers can rely on the model field always being present.

Changes:

  • Always emit model in checkpoint session metadata.json by removing omitempty from CommittedMetadata.Model.
  • Plumb Cursor hook model into the normalized SessionStart lifecycle event.
  • Add model to Vogon E2E hook payloads and extend E2E metadata parsing to include the field.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
e2e/vogon/main.go Includes a deterministic model value in Vogon’s emitted hook payloads so canary E2E exercises model propagation.
e2e/testutil/metadata.go Adds Model to the E2E SessionMetadata struct to read/validate the new persisted field.
cmd/entire/cli/checkpoint/checkpoint.go Makes CommittedMetadata.Model always serialize to JSON (even when empty).
cmd/entire/cli/agent/vogon/hooks.go Wires parsed hook model into Vogon lifecycle events (SessionStart/TurnStart/TurnEnd/SessionEnd).
cmd/entire/cli/agent/cursor/lifecycle.go Wires Cursor SessionStart hook model into the normalized event.

Comment on lines 88 to 92
Type: agent.SessionStart,
SessionID: raw.ConversationID,
SessionRef: raw.TranscriptPath,
Model: raw.Model,
Timestamp: time.Now(),
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parseSessionStart now populates Event.Model, but there is no unit test asserting that SessionStart hooks propagate the model (Cursor tests currently only cover model on TurnStart). Adding a focused test would help prevent regressions, especially since headless Cursor runs rely on SessionStart/SessionEnd hooks.

Copilot uses AI. Check for mistakes.
Comment on lines 89 to 92
SessionID: raw.ConversationID,
SessionRef: raw.TranscriptPath,
Model: raw.Model,
Timestamp: time.Now(),
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor CLI hooks commonly send transcript_path: null (unmarshals to empty string). parseSessionStart currently forwards raw.TranscriptPath directly, unlike TurnStart/TurnEnd/SessionEnd which resolve the transcript path dynamically. Consider passing ctx into parseSessionStart and using resolveTranscriptRef(ctx, raw.ConversationID, raw.TranscriptPath) so SessionStart has a consistent, non-empty SessionRef in CLI mode.

Copilot uses AI. Check for mistakes.
dipree and others added 2 commits April 8, 2026 20:54
Claude Code's parseTurnEnd and parseSessionEnd were ignoring the model
field already available in sessionInfoRaw. Codex's parseTurnStart and
parseTurnEnd similarly had model available but unused. Wire them up so
model info propagates on every hook event, not just SessionStart.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@dipree dipree marked this pull request as ready for review April 9, 2026 02:30
@dipree dipree requested a review from a team as a code owner April 9, 2026 02:30
dipree and others added 3 commits April 8, 2026 21:35
Address Copilot review comment: add unit tests asserting that model
fields are propagated through hook event parsing for all affected agents.

- Claude Code: TurnEnd and SessionEnd model propagation
- Cursor: SessionStart model propagation (with and without model)
- Codex: TurnStart and TurnEnd model assertions on existing tests

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sessionEndRaw includes model but parseSessionEnd wasn't forwarding it,
leaving the final model name unpersisted when a Cursor session ends
without a useful Stop event.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

4 participants