Skip to content

Commit 340668f

Browse files
committed
fix(codebuff): address five roborev findings
1. Freebuff identity: Reverted to AgentFreebuff for distinct filtering, with prefix alias in AgentByPrefix for lifecycle operations. 2. Model name accuracy: Left model unknown since actual LLM model is selected server-side and can change mid-session. Credits usage events are emitted independently of model. 3. AI block ordering: Agent output now emitted immediately with tool call, system blocks (mode/plan/ask-user) emitted in source order instead of deferred to end. 4. Cross-midnight timestamps: Track date in parseCodebuffMessages and advance when time-of-day wraps past midnight. 5. PeakContextTokens: Removed incorrect peak context assertion since contextTokenCount from run-state.json is the final per-step value, not the peak (compaction can reduce it).
1 parent 5ad8bc2 commit 340668f

4 files changed

Lines changed: 130 additions & 89 deletions

File tree

internal/parser/codebuff.go

Lines changed: 84 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,10 @@ func parseCodebuffSession(
4343
// Read chat-meta.json for session name and timing hints.
4444
meta := readCodebuffChatMeta(chatMetaPath)
4545

46-
// Model name is the raw agentType from run-state.json
47-
// (e.g. "base2-free-deepseek", "base2-free-mimo").
48-
model := rs.AgentType
46+
// The actual LLM model is selected server-side based on the agentType
47+
// template and can change mid-session. The on-disk format does not
48+
// persist the actual model, so leave it unknown.
49+
model := ""
4950

5051
// Read and parse the chat messages.
5152
data, err := os.ReadFile(chatMessagesPath)
@@ -105,15 +106,14 @@ func parseCodebuffSession(
105106
}
106107
}
107108

108-
// Determine agent label from run-state agentType field.
109+
// Determine agent type from run-state agentType field.
109110
// Sessions with "free" in the agentType are Freebuff, others are Codebuff.
110-
// Both share the same on-disk layout and the same agent type (Codebuff)
111-
// so that lifecycle operations (reconciliation, deletion, baselines)
112-
// keyed by agent type work correctly. The UI distinguishes them via
113-
// AgentLabel.
111+
// Both share the same on-disk layout; the parser splits them by type
112+
// so the UI can filter each agent independently.
114113
agent := AgentCodebuff
115114
agentLabel := "Codebuff"
116115
if strings.Contains(strings.ToLower(rs.AgentType), "free") {
116+
agent = AgentFreebuff
117117
agentLabel = "Freebuff"
118118
}
119119

@@ -178,37 +178,31 @@ func parseCodebuffSession(
178178
File: fileInfo,
179179
}
180180

181-
// Token counts from run-state.
182-
if rs.ContextTokenCount > 0 {
183-
sess.PeakContextTokens = rs.ContextTokenCount
184-
sess.HasPeakContextTokens = true
185-
}
186-
187-
sess.aggregateTokenPresenceKnown =
188-
sess.HasTotalOutputTokens || sess.HasPeakContextTokens
181+
// contextTokenCount from run-state.json is the final per-step context
182+
// count, not the peak. Compaction can make the final value lower than
183+
// the true peak, so we cannot reliably derive PeakContextTokens from
184+
// this value. Leave peak context unavailable.
189185

190-
// Emit usage event with clean model name for catalog pricing.
191-
// Credits are billing units (1 credit = $0.01), mapped to CostUSD.
192-
if model != "" {
193-
evt := ParsedUsageEvent{
186+
// Emit usage event for reported credits. The actual model is unknown
187+
// (selected server-side, can change mid-session), so Model is left
188+
// empty. Credits are billing units (1 credit = $0.01), mapped to
189+
// CostUSD for cost display.
190+
if rs.CreditsUsed > 0 {
191+
cost := rs.CreditsUsed * 0.01
192+
sess.UsageEvents = []ParsedUsageEvent{{
194193
SessionID: fullID,
195194
Source: "session",
196-
Model: model,
197195
OccurredAt: func() string {
198196
if !endedAt.IsZero() {
199197
return endedAt.Format(time.RFC3339Nano)
200198
}
201199
return startedAt.Format(time.RFC3339Nano)
202200
}(),
203-
DedupKey: "session:" + fullID,
204-
}
205-
if rs.CreditsUsed > 0 {
206-
cost := rs.CreditsUsed * 0.01
207-
evt.CostUSD = &cost
208-
evt.CostStatus = "reported"
209-
evt.CostSource = "session"
210-
}
211-
sess.UsageEvents = []ParsedUsageEvent{evt}
201+
CostUSD: &cost,
202+
CostStatus: "reported",
203+
CostSource: "session",
204+
DedupKey: "session:" + fullID,
205+
}}
212206
}
213207

214208
return sess, msgs, nil
@@ -440,14 +434,35 @@ func parseCodebuffMessages(
440434
startedAt time.Time
441435
endedAt time.Time
442436
ordinal int
437+
// Track the current date for cross-midnight sessions. Start with
438+
// the session directory date and advance when time-of-day wraps
439+
// past midnight.
440+
currentDate = sessionDate
441+
prevHour = -1
443442
)
444443

445444
root.ForEach(func(_, msg gjson.Result) bool {
446445
variant := msg.Get("variant").Str
447446
ts := parseCodebuffTimestamp(
448-
msg.Get("timestamp").Str, sessionDate,
447+
msg.Get("timestamp").Str, currentDate,
449448
)
450449

450+
// Detect midnight rollover for time-only timestamps: if the
451+
// parsed hour is less than the previous hour, we've crossed
452+
// midnight and should advance the date.
453+
if !ts.IsZero() && prevHour >= 0 {
454+
if ts.Hour() < prevHour {
455+
currentDate = currentDate.AddDate(0, 0, 1)
456+
// Re-parse with the advanced date.
457+
ts = parseCodebuffTimestamp(
458+
msg.Get("timestamp").Str, currentDate,
459+
)
460+
}
461+
}
462+
if !ts.IsZero() {
463+
prevHour = ts.Hour()
464+
}
465+
451466
if !ts.IsZero() {
452467
if startedAt.IsZero() || ts.Before(startedAt) {
453468
startedAt = ts
@@ -539,12 +554,11 @@ func parseCodebuffAIMessage(
539554
}
540555

541556
var (
542-
out []ParsedMessage
543-
thinkingBuf []string
544-
textBuf []string
545-
toolCalls []ParsedToolCall
546-
toolResults []ParsedToolResult
547-
pendingSys []string
557+
out []ParsedMessage
558+
thinkingBuf []string
559+
textBuf []string
560+
toolCalls []ParsedToolCall
561+
toolResults []ParsedToolResult
548562
)
549563

550564
// flushText emits any accumulated thinking and text as assistant messages,
@@ -687,45 +701,73 @@ func parseCodebuffAIMessage(
687701
}
688702
toolCalls = append(toolCalls, tc)
689703

690-
// Agent output goes into text buffer for the next flush.
704+
// Emit agent output text immediately associated with the
705+
// tool call, not deferred to a later flush.
691706
if output := block.Get("content"); output.Exists() && output.Str != "" {
692707
prefix := agentName
693708
if agentType != "" {
694709
prefix = agentType + ":" + agentName
695710
}
711+
// Flush accumulated tool calls first, then emit output.
712+
flushTools()
696713
textBuf = append(textBuf,
697714
"["+prefix+" ("+status+")]\n"+output.Str)
715+
flushText()
698716
}
699717

700718
case "mode-divider":
701719
flushText()
702720
flushTools()
703721
mode := block.Get("mode").Str
704722
if mode != "" {
705-
pendingSys = append(pendingSys, "[Mode: "+mode+"]")
723+
// Emit system blocks immediately, not deferred.
724+
out = append(out, ParsedMessage{
725+
Role: RoleSystem,
726+
Content: "[Mode: " + mode + "]",
727+
Timestamp: ts,
728+
ContentLength: len("[Mode: " + mode + "]"),
729+
IsSystem: true,
730+
})
706731
}
707732

708733
case "plan":
709734
flushText()
710735
flushTools()
711736
content := block.Get("content").Str
712737
if strings.TrimSpace(content) != "" {
713-
pendingSys = append(pendingSys, "[Plan]\n"+content)
738+
// Emit system blocks immediately, not deferred.
739+
out = append(out, ParsedMessage{
740+
Role: RoleSystem,
741+
Content: "[Plan]\n" + content,
742+
Timestamp: ts,
743+
ContentLength: len("[Plan]\n" + content),
744+
IsSystem: true,
745+
})
714746
}
715747

716748
case "ask-user":
717749
flushText()
718750
flushTools()
719751
questions := block.Get("questions")
720752
if questions.IsArray() {
753+
var parts []string
721754
questions.ForEach(func(_, q gjson.Result) bool {
722755
questionText := q.Get("question").Str
723756
if strings.TrimSpace(questionText) != "" {
724-
pendingSys = append(pendingSys,
725-
"[Agent asked] "+questionText)
757+
parts = append(parts, "[Agent asked] "+questionText)
726758
}
727759
return true
728760
})
761+
if len(parts) > 0 {
762+
content := strings.Join(parts, "\n")
763+
out = append(out, ParsedMessage{
764+
Role: RoleSystem,
765+
Content: content,
766+
Timestamp: ts,
767+
ContentLength: len(content),
768+
IsSystem: true,
769+
})
770+
}
729771
}
730772

731773
case "image":
@@ -743,18 +785,6 @@ func parseCodebuffAIMessage(
743785
flushText()
744786
flushTools()
745787

746-
// Emit system messages after all other content.
747-
if len(pendingSys) > 0 {
748-
sysContent := strings.Join(pendingSys, "\n")
749-
out = append(out, ParsedMessage{
750-
Role: RoleSystem,
751-
Content: sysContent,
752-
Timestamp: ts,
753-
ContentLength: len(sysContent),
754-
IsSystem: true,
755-
})
756-
}
757-
758788
if len(out) == 0 {
759789
return nil
760790
}

internal/parser/codebuff_test.go

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,8 @@ func TestParseCodebuffSession_BasicUserAndAIMessages(t *testing.T) {
8989
assert.Equal(t, "Fix the login bug", sess.FirstMessage)
9090
assert.Equal(t, 2, sess.MessageCount)
9191
assert.Equal(t, 1, sess.UserMessageCount)
92-
assert.True(t, sess.HasPeakContextTokens)
93-
assert.Equal(t, 50000, sess.PeakContextTokens)
92+
// PeakContextTokens is not set because contextTokenCount from
93+
// run-state.json is the final per-step count, not the peak.
9494

9595
require.Len(t, msgs, 2)
9696
assert.Equal(t, RoleUser, msgs[0].Role)
@@ -121,10 +121,10 @@ func TestParseCodebuffSession_FreebuffClassification(t *testing.T) {
121121
require.NoError(t, err)
122122
require.NotNil(t, sess)
123123

124-
// Freebuff sessions use AgentCodebuff so that lifecycle operations
125-
// (reconciliation, deletion, baselines) keyed by agent type work
126-
// correctly. The UI distinguishes them via AgentLabel.
127-
assert.Equal(t, AgentCodebuff, sess.Agent)
124+
// Freebuff sessions use AgentFreebuff for distinct filtering, while
125+
// lifecycle operations are handled via the freebuff: prefix alias
126+
// in AgentByPrefix.
127+
assert.Equal(t, AgentFreebuff, sess.Agent)
128128
assert.Equal(t, "Freebuff", sess.AgentLabel)
129129
}
130130

@@ -414,13 +414,9 @@ func TestParseCodebuffSession_UsageEvent(t *testing.T) {
414414
require.NoError(t, err)
415415
require.NotNil(t, sess)
416416

417-
require.Len(t, sess.UsageEvents, 1)
418-
evt := sess.UsageEvents[0]
419-
assert.Equal(t, sess.ID, evt.SessionID)
420-
assert.Equal(t, "session", evt.Source)
421-
assert.Equal(t, "base2-deepseek", evt.Model)
422-
assert.NotEmpty(t, evt.OccurredAt)
423-
assert.NotEmpty(t, evt.DedupKey)
417+
// No usage event when credits are 0 (no billing data).
418+
assert.Empty(t, sess.UsageEvents,
419+
"no usage event when credits are 0")
424420
}
425421

426422
func TestParseCodebuffSession_UsageEventEmptyModel(t *testing.T) {
@@ -1132,10 +1128,9 @@ func TestParseCodebuffSession_CreditsZero(t *testing.T) {
11321128
require.NoError(t, err)
11331129
require.NotNil(t, sess)
11341130

1135-
// Freebuff sessions have no credits - CostUSD should be nil.
1136-
require.Len(t, sess.UsageEvents, 1)
1137-
assert.Nil(t, sess.UsageEvents[0].CostUSD,
1138-
"freebuff sessions should have no cost")
1131+
// Freebuff sessions have no credits - no usage event emitted.
1132+
assert.Empty(t, sess.UsageEvents,
1133+
"freebuff sessions should have no usage event")
11391134
}
11401135

11411136
func TestParseCodebuffSession_PlanBlock(t *testing.T) {

internal/sync/provider_effects.go

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,18 @@ func validateProviderOutcome(
2020
for _, result := range outcome.Results {
2121
session := result.Result.Session
2222
if session.Agent != def.Type {
23-
return fmt.Errorf(
24-
"%s: provider result session agent mismatch for %q: got %s",
25-
def.Type,
26-
session.ID,
27-
session.Agent,
28-
)
23+
// The Codebuff provider may emit Freebuff sessions based on
24+
// the agentType field in run-state.json. Both agents share
25+
// the same on-disk layout and are discovered by one provider.
26+
if def.Type != parser.AgentCodebuff ||
27+
session.Agent != parser.AgentFreebuff {
28+
return fmt.Errorf(
29+
"%s: provider result session agent mismatch for %q: got %s",
30+
def.Type,
31+
session.ID,
32+
session.Agent,
33+
)
34+
}
2935
}
3036
if err := validateProviderParseResultSessionIDs(def, result.Result); err != nil {
3137
return err
@@ -104,6 +110,12 @@ func validateProviderSessionID(def parser.AgentDef, sessionID, field string) err
104110
if strings.HasPrefix(sessionID, def.IDPrefix) {
105111
return nil
106112
}
113+
// The Codebuff provider may emit Freebuff sessions whose IDs use
114+
// the "freebuff:" prefix rather than the provider's "codebuff:" prefix.
115+
if def.Type == parser.AgentCodebuff &&
116+
strings.HasPrefix(sessionID, string(parser.AgentFreebuff)+":") {
117+
return nil
118+
}
107119
return fmt.Errorf(
108120
"%s: provider %s %q must use prefix %q",
109121
def.Type,

0 commit comments

Comments
 (0)