feat(agent-platform): parse kagent session tasks into a timeline - #2003
Conversation
marians
left a comment
There was a problem hiding this comment.
Careful pass over the whole diff (parsing layer, timeline builder, client methods, fixtures). The parsing is genuinely defensive and the reasoning in the comments is easy to follow — the findings below are all in the "wire shape we haven't seen yet" category the PR description already flags, plus one observability regression.
Highest-value ones: the adk_request_confirmation response leaking through the orphan branch as a raw tool call, and the drift-dedup key now being shared by three endpoints (which makes the new listSessionTasks warning effectively unreachable — the test has to dodge it with a different installation).
Nothing here blocks the groundwork landing; most are cheap to close before the UI PR exercises these paths against a real payload.
One thing I could not verify statically and would check against the real gazelle capture: whether kagent repeats the same usage_metadata bag on several a2a messages derived from one LLM event. If it does, the message-level accumulation in buildTimeline over-counts the session total, and the messageId dedup only saves you when the ids actually repeat.
ef20612 to
232a271
Compare
Validated against a real payload — three correctionsYour curl let me run the parsers over a live gazelle session (4 tasks, 32 messages, 12 tool calls) before this merges. It produced 29 items, no drift, no skipped rows, and all 12 tool calls paired with their responses. But it also disproved something I'd built on, so the diff has grown: 1. Per-message timestamps are not obtainable — that code is deleted. kagent's Go type says No This also had a knock-on effect worth its own commit on #2002: with events unread, 2. kagent repeats each user message verbatim under the same So the session-wide dedupe is load-bearing on real data, not defensive as I'd written it. Without it every turn opens by saying the same thing twice. Now has its own test and the fixture reproduces it. 3. One message can carry prose and several tool calls, always text first — observed What the real data does not exerciseWorth knowing when reviewing the fixtures: that session had no On the fixturesThe real payload is not committed. A
One thing to flag for the UI PRToken totals are large in a way that will need careful labelling: that 4-turn session sums to 1.5M prompt tokens across 14 model calls (min 3,854, max 144,379). That's not a bug — each call re-sends the whole context and muster's tool schemas are big, so it's genuine cumulative billed usage, and kagent's own UI sums it the same way. But "1.5M input tokens" on a short chat will read as alarming or broken unless the label says what it's counting. |
Code reviewFound 4 issues:
backstage/plugins/agent-platform/src/lib/kagentTaskSchema.ts Lines 40 to 47 in 232a271
backstage/plugins/agent-platform/src/apis/types.ts Lines 37 to 58 in 232a271
backstage/plugins/agent-platform/src/lib/kagentTimeline.ts Lines 64 to 67 in 232a271
backstage/.changeset/agent-platform-session-timeline.md Lines 1 to 20 in 232a271 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
b811452 to
3dab6df
Compare
…e parts
Found by opening a real gazelle session: the stats strip read
"Total 0" beside "Input tokens 1.4M".
Every message on that session carries exactly
`adk_usage_metadata: {promptTokenCount, candidatesTokenCount}` — no
`totalTokenCount` at all — so summing the reported totals stayed at
zero while the parts ran into millions. kagent's own UI has the same
hole (`total: usage.totalTokenCount ?? 0`), so this faithfully
reproduced a kagent bug.
The total is now derived from prompt + completion when kagent doesn't
report one. A reported total still wins when present, because a model
that bills thinking tokens separately counts them in the total but in
neither part, so it is not always the sum.
Also corrects two comments. That session's metadata uses the **adk_**
prefix, while the session probed a day earlier used kagent_ — so both
prefixes really do occur on one installation, and the dual-prefix reader
is load-bearing rather than defensive: reading only one silently zeroes a
session's tokens. I had told the reviewer on #2003 that real data showed
"no adk_ in sight" and that the fallback was not wire-verified. Both
wrong.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C2M37V7ubYwimkkS7YZ9iz
Correction: real data does use
|
Groundwork for the session detail page. No visible change — nothing renders these yet. A session is a list of A2A tasks (turns), each holding a history of messages, each holding parts. kagent discriminates a part's meaning via prefixed keys in its metadata rather than by wire type, so lib/kagentParts.ts holds those predicates and lib/kagentTimeline.ts flattens the nesting into user/agent messages, reasoning, tool calls, delegations and approval requests. Decisions worth knowing: - Metadata is read adk_<key> first, then kagent_<key>. That is kagent's own interop mechanism (getMetadataValue in its UI), not a guess: upstream ADK writes one prefix, kagent the other, and one session can contain both. Exactly one helper spells the prefixes out. - A tool call and its result are one item. The function_response is folded into the function_call it answers, matched on call id, with open calls scoped per task so a repeated tool cannot get a result attached to the wrong call. An orphan response still renders. - Delegations are their own kind. They look like tool calls on the wire (the tell is __NS__ in the name), but a subagent runs in its own session, so the response is the only place its token usage appears. That usage is counted once, keyed on the call rather than the response, since responses do not always repeat the name. - An unrecognised approval verdict leaves the verdict unset instead of defaulting to approved. The message is still recognised as a decision, but guessing would claim consent to an action the user may have refused. - Session state comes from the last task (kagent returns them ORDER BY created_at ASC): an earlier turn having completed says nothing about whether the session is working now. An unknown A2A state renders as itself and counts as inactive, so it cannot produce a spinner that never resolves. No tasks at all is its own condition. - Per-message timestamps are recovered from the stored events, whose data is a doubly-encoded A2A message, joined on messageId. A2A messages carry no time of their own, and the join is treated as decoration: a miss falls back to the task timestamp, then to no time. Nothing in this layer throws; malformed rows are skipped and counted. Fixtures are hand-authored from kagent's source and its own test payloads. A captured production payload follows with the UI, where the page can actually exercise the endpoints. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C2M37V7ubYwimkkS7YZ9iz
…gainst real data Ran the parsers against a live gazelle session (4 tasks, 32 messages, 12 tool calls) before letting this land. It produced 29 items with no drift and no skipped rows, and corrected three things. Per-message timestamps are not obtainable, so that code is gone. kagent's Go type says `Data string // JSON-serialized protocol.Message`, which implied the session's stored events could supply them. They cannot: the decoded value is an ADK event (author, content, invocation_id, partial, timestamp, ...) with no messageId anywhere, so there is nothing to join task history against — 36 events, zero usable ids. invocation_id does correlate, but only per turn, which the task's own timestamp already gives. Every item now takes its task's timestamp, so items in a turn share one by design and the UI should present it per turn rather than imply per-message precision. kagentEventTimestamps.ts is deleted. kagent repeats each user message verbatim under the same messageId, on every turn. The session-wide dedupe turns out to be load-bearing on real data rather than defensive: without it every turn would open by saying the same thing twice. Now covered by its own test. One message can carry prose and several tool calls, always text first (observed `text -> data(function_call) -> data(function_call)`). Walking parts in order and merging only runs of same-kind text is what keeps the narration ahead of the calls it introduces. Also covered. Fixtures are updated to be structurally faithful to all three. The real payload is deliberately not committed — a /tasks body is the full conversation including tool arguments and results, which is not ours to put in this repo. tasks.adk-prefixed.json is now generated from tasks.v0-9-9.json by swapping only the metadata KEY prefix, so the two cannot drift in any other dimension, which is what the prefix test asserts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C2M37V7ubYwimkkS7YZ9iz
All seven were real. Verified each against kagent's source before
changing anything.
- The approval's own function_response leaked into the timeline as a
tool call literally named adk_request_confirmation, carrying ADK's
internal payload, right after the approval card. The approval is never
registered as an open call, so that response always reaches the orphan
branch. kagent's own UI filters the same name
(ui/src/lib/toolCallExtraction.ts:63). Fixture extended to include the
resume response, which it previously stopped short of.
- Approvals are now discriminated on the tool name alone. Requiring the
long-running flag failed open in the ugliest direction: a missing flag
— or one arriving as the string "true", which the strict boolean check
rejects by design — downgraded an approval into a raw tool call
exposing the originalFunctionCall wrapper as its arguments.
- A decision recorded in a later task than its approval was swallowed
and the approval left permanently unanswered: the user approved and
the UI said they never replied. openApprovalIndex is now session
scoped, matching the "only one open at a time" assumption the code
already made.
- Task timestamps now go through normalizeTimestamp like every other
kagent timestamp. These are non-pointer Go time.Time, so an unset one
arrives as 0001-01-01T00:00:00Z and renders as "Dec 31, 0000" — and
since this is now the only timestamp source, it would have done so for
every item in the task.
- A call message repeated across tasks with its response only in the
second rendered twice: once stuck pending, once as an orphan result.
Dedupe is session-wide but open calls are per task, so the deduped
copy never registered. A deduped message now re-arms its calls in the
current task.
- skippedMessages no longer counts well-formed non-message history
entries (artifact and status updates). It documents itself as data
loss and will feed a "N messages could not be read" warning, so
counting healthy entries would warn about sound sessions.
parseMessage became parseHistoryEntry returning a discriminated
result.
- The drift dedupe key now includes the endpoint. Three reads share the
module-level set and all three emit skipped-rows, so whichever fired
first permanently silenced the others for that installation — one
dropped session row would mute a task list that later dropped thirty.
The warning prefix was also wrong ("kagent sessions response drift"
for a task read). "No readable session" gets its own missing-payload
kind rather than borrowing skipped-rows, whose message counts rows.
- describeSessionState guards the lookup with hasOwnProperty. Only
`constructor` and `__proto__` survive toLowerCase() unchanged (the
reviewer's toString/valueOf/hasOwnProperty examples lower-case to
non-members), but either would have spread a function or the prototype
into a badge with an undefined label and tone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C2M37V7ubYwimkkS7YZ9iz
3dab6df to
0eb9b15
Compare
…e parts
Found by opening a real gazelle session: the stats strip read
"Total 0" beside "Input tokens 1.4M".
Every message on that session carries exactly
`adk_usage_metadata: {promptTokenCount, candidatesTokenCount}` — no
`totalTokenCount` at all — so summing the reported totals stayed at
zero while the parts ran into millions. kagent's own UI has the same
hole (`total: usage.totalTokenCount ?? 0`), so this faithfully
reproduced a kagent bug.
The total is now derived from prompt + completion when kagent doesn't
report one. A reported total still wins when present, because a model
that bills thinking tokens separately counts them in the total but in
neither part, so it is not always the sum.
Also corrects two comments. That session's metadata uses the **adk_**
prefix, while the session probed a day earlier used kagent_ — so both
prefixes really do occur on one installation, and the dual-prefix reader
is load-bearing rather than defensive: reading only one silently zeroes a
session's tokens. I had told the reviewer on #2003 that real data showed
"no adk_ in sight" and that the fallback was not wire-verified. Both
wrong.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C2M37V7ubYwimkkS7YZ9iz
…e parts
Found by opening a real gazelle session: the stats strip read
"Total 0" beside "Input tokens 1.4M".
Every message on that session carries exactly
`adk_usage_metadata: {promptTokenCount, candidatesTokenCount}` — no
`totalTokenCount` at all — so summing the reported totals stayed at
zero while the parts ran into millions. kagent's own UI has the same
hole (`total: usage.totalTokenCount ?? 0`), so this faithfully
reproduced a kagent bug.
The total is now derived from prompt + completion when kagent doesn't
report one. A reported total still wins when present, because a model
that bills thinking tokens separately counts them in the total but in
neither part, so it is not always the sum.
Also corrects two comments. That session's metadata uses the **adk_**
prefix, while the session probed a day earlier used kagent_ — so both
prefixes really do occur on one installation, and the dual-prefix reader
is load-bearing rather than defensive: reading only one silently zeroes a
session's tokens. I had told the reviewer on #2003 that real data showed
"no adk_ in sight" and that the fallback was not wire-verified. Both
wrong.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01C2M37V7ubYwimkkS7YZ9iz
What does this PR do?
Turns kagent's A2A task payloads into a renderable timeline. Groundwork for the session detail page — nothing renders these yet, so there is no visible change.
A session is a list of A2A tasks (turns) →
historyof messages → parts. kagent discriminates a part's meaning via prefixed keys inmetadatarather than by wire type, so the new files split along that seam:lib/kagentMetadata.tsadk_/kagent_prefix readerlib/kagentParts.tsfunction_call,function_response, thought, agent-tool)lib/kagentTimeline.tsbuildTimeline(tasks, timestamps)→ flatTimelineItem[]+ token totalslib/kagentSessionState.tsTaskState→ label/tonelib/kagentEventTimestamps.tslib/kagentTaskSchema.ts,lib/kagentSessionDetail.tsAny background context you can provide?
Decisions I'd most like a second opinion on:
Metadata is read
adk_<key>first, thenkagent_<key>. Not a guess — it's kagent's own interop mechanism (getMetadataValue,ui/src/lib/messageHandlers.ts:479): upstream ADK writes one prefix, kagent writes the other, and a single session can contain both. There are two fixtures with identical content under different prefixes, and a test asserting they normalize identically.A tool call and its result are one item, matched on the function-call id, with open calls scoped per task so a repeatedly-used tool can't get a result attached to the wrong call. An orphan response (call in a task we don't have, or no id) still renders — hiding a result seemed worse than showing one without its request.
Delegations to other agents are their own kind. They look like tool calls on the wire (the tell is
__NS__in the name, same check kagent's UI makes), but a subagent runs in its own session, so its messages never appear here and the response is the only place its token usage surfaces. That usage is counted once toward the session total, keyed on the call rather than the response — responses don't always repeat the name, and keying on them would silently under-count.An unrecognised approval verdict leaves the verdict unset rather than defaulting to "approved". The message is still recognised as a decision (so it doesn't render as user prose), but guessing would claim the user consented to an action they may have refused.
Session state comes from the last task. kagent returns tasks
ORDER BY created_at ASC(verified intasks.sql.go), so the array is chronological; an earlier turn having completed says nothing about whether the session is working now. An unknown A2A state renders as itself and counts as inactive, so it can't produce a spinner that never resolves. "No tasks at all" stays its own condition rather than being flattened into a state.Per-message timestamps come from joining the session's stored events on
messageId, because A2A messages carry no time of their own. I've treated that join as decoration everywhere: whether the two storage paths agree on ids isn't guaranteed anywhere, so a miss falls back to the task's timestamp and then to showing no time. Tests cover all three levels.How does it look like?
Nothing visual yet. The shape it produces:
Do the docs need to be updated?
Not in this PR — the user-facing write-up in
docs/agent-platform.mdlands with the page, where there's something to describe.One thing to flag: the fixtures here are hand-authored from kagent's Go/Python source and its own test payloads, not captured from production. PR 2 adds no UI, so nothing in the app would exercise the new endpoints yet. I'll capture a real gazelle payload and add it as the primary fixture in the UI PR, where the page actually calls them — same approach that caught the
z.unknown().transform()bug in the sessions list. Until then, treat the parsing as verified against the source but not against the wire.Should this change be mentioned in the release notes?