feat(workflows): show model + thinking level on graph node cards - #1859
feat(workflows): show model + thinking level on graph node cards#1859sina85 wants to merge 14 commits into
Conversation
There was a problem hiding this comment.
Hey @sina85, thanks for the contribution. This looks like a very cool idea! For the background widget change can you make sure that parallel workflow stages with different models are intuitive to see? I think the graph node cards are cleaner since they can each show the model, but the background widget gets messy. Also, can you make sure you properly handle model fallbacks and change the model in the node card when that happens? Another edge case is to make sure you display (fast) for the model if using the fast tier for OpenAI providers. Finally, you need to make sure that when you run /workflow resume that this data is saved. If you give evidence for each of these will review again and we can look into getting this checked in for the next release.
|
Hi @lavaman131, thanks for the feedback, I will double check the edge cases and get back to you. |
e94b547 to
b732e18
Compare
|
@lavaman131 thanks for the detailed review. Update (edited): after the first pass I reconsidered your parallel point and removed the model/thinking display from the 1. Parallel stages → handled by the graph node cards, not the widgetPer your observation, the widget was the wrong surface. Each stage gets its own node card, so parallel stages on different models are each shown cleanly, no cramming: 2. Model fallbacks update the node cardThe card renders from the live
3. Fast tier shown with the modelMirrors the footer's Tests: node‑card "shows the fast tier on the model row, not the deps row", "…keeps both the thinking level and the fast marker when they fit", "…drops the thinking level to preserve the fast marker when the card would overflow". 4.
|
| * {@link model} so background-run surfaces can show the same model + | ||
| * thinking identity the main session footer shows. Optional: absent for | ||
| * restored runs and stages whose model has no reasoning control. | ||
| */ | ||
| thinkingLevel?: string; |
There was a problem hiding this comment.
Widget path missing
StageSnapshot.thinkingLevel is documented for background-run surfaces here, but packages/workflows/src/tui/widget.ts still builds its meta line only from mode, progress, and elapsed time (metaLine/themedRunLines), so the BACKGROUND widget never displays the model or thinking level promised by this PR. This leaves direct background runs opaque even when stageSnapshot.model and thinkingLevel are populated.
Context Used: AGENTS.md (source)
Artifacts
- Contains supporting evidence from the run (text/typescript; charset=utf-8).
Repro: execution output showing the widget meta line omits model and thinking level
- Keeps the command output available without making the summary code-heavy.
Ran code and verified through T-Rex
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/workflows/src/shared/store-types.ts
Line: 215-219
Comment:
**Widget path missing**
`StageSnapshot.thinkingLevel` is documented for background-run surfaces here, but `packages/workflows/src/tui/widget.ts` still builds its meta line only from `mode`, progress, and elapsed time (`metaLine`/`themedRunLines`), so the `BACKGROUND` widget never displays the model or thinking level promised by this PR. This leaves direct background runs opaque even when `stageSnapshot.model` and `thinkingLevel` are populated.
**Context Used:** AGENTS.md ([source](https://app.greptile.com/bastani-inc/github/bastani-inc/atomic/-/custom-context?memory=6b6bc6e3-dafb-4fa7-9d98-538aac70844a))
How can I resolve this? If you propose a fix, please make it concise.a256d30 to
a6b8afb
Compare
lavaman131
left a comment
There was a problem hiding this comment.
Hey @sina85, can you address the review bot (Greptile) feedback and rebase with the latest origin/main. Otherwise, once you do that, looks great and we can check it in pending CI is green.
25400a3 to
8930fb1
Compare
8930fb1 to
168d858
Compare
The background-workflow (BACKGROUND) widget listed each running direct task/chain with mode, progress, and duration, but not which model or thinking level the run was using — information the main-session footer already shows. Background runs were opaque about their model identity. Surface the running stage's model and thinking level on each run card, mirroring the footer's `<model> <thinking>` format (thinking omitted when off). Chains show the currently running stage's model; the segment is skipped until a stage records an effective model. Plumbing (live snapshot path): - StageModelFallbackMeta gains `thinkingLevel`, populated by the stage controller from the live session (or a pending level). - The executor applies it to `StageSnapshot.thinkingLevel`, next to the existing `model` field. - The widget's meta line renders `<model> <thinking>` for the active stage. Scope: live in-memory snapshot only (the "currently running" case). Durable persistence parity for thinkingLevel is a follow-up; `model` already persists. Tests: widget rendering for single/chain/off/no-model cases, plus a controller assertion that the session's thinking level reaches the meta. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extend the model/thinking transparency to the `/workflow connect` graph overlay. The orchestrator node cards showed title, duration, status, and dependency (root/deps) metadata but deliberately hid the model — so an attached run gave no per-stage model visibility. Add a dedicated compact model row to each node card, beneath the status line: `<short-model> · <thinking>`. The provider prefix is dropped and the thinking level is omitted when off so it fits the ~22-cell card; `—` shows when no model is resolved yet. All existing fields are preserved (duration, status, deps), so the card grows from 5 to 6 rows (NODE_H). The graph layout and edge routing derive from NODE_H and adapt automatically. Reverses the earlier "cards hide model metadata" contract; the three tests that pinned it are updated to assert the model row is shown, and the dependency-row assertions move one row down. Tests: node-card model row (compact model, thinking append/omit, em-dash when absent, fast marker stays on the deps row); full overlay-graph suite passes unchanged against the new height. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…he BACKGROUND widget Addresses review feedback that the widget "gets messy" for parallel stages and did not show the Codex fast tier. - Parallel fan-outs: when >1 stage runs concurrently the run is labelled `parallel`, and if those stages use different models the segment lists the distinct, provider-stripped models deduped and capped with `+N` (e.g. `gpt-5, opus +1`). Single-model runs are unchanged (full provider id). - Fast tier: the active-stage model now appends the Codex `fast` marker, mirroring the main-session footer (`<model> <thinking> fast`). Extracts the model-segment formatting into `widget-model-label.ts` and the fast-tier suffix into `codex-fast-label.ts` (a local mirror of `formatCodexFastModeModelLabel` — importing the `@bastani/atomic` barrel into the TUI graph breaks the pi-tui-mocked overlay test subprocesses). Keeps widget.ts under the 500-line gate; the model+thinking tests move to a dedicated `widget-model-thinking.test.ts` for the same reason. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lect model fallbacks Addresses review feedback to display `(fast)` for the fast tier and to update the node card's model when a fallback changes it. - The fast tier now rides the model row (footer parity) instead of the dependency row; the deps row returns to plain `root`/`N deps`. When the card would overflow (~22 cells) the thinking level is dropped so the fast marker is never truncated away. - The model row renders from the live `StageSnapshot.model`, which `applyModelFallbackMeta` rewrites on every fallback — so a fallback visibly swaps the displayed model. Added a test rendering the same snapshot before and after a fallback to lock this in. The three tests that pinned the old "fast on the deps row" behaviour are updated; new tests cover the width-guard and the fallback swap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extends the reasoning-suffix retry test to assert that after a 429 fallback the meta surfaced to the background/graph UIs (`meta.model` → `StageSnapshot.model` → node card) is the fallback model, not the failed primary — the controller→meta→snapshot half of the fallback-display chain. Also condenses two `StageSessionController` helpers to keep the file within the 500-line gate after the earlier thinking-level addition. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Addresses review feedback that `/workflow resume` must save this data. The PR previously scoped durable persistence of `thinkingLevel` as a follow-up; this promotes it into the PR so restored runs show the same model + thinking identity, not just the model. `thinkingLevel` now rides alongside `model` at every serialize/restore site of the mandatory DBOS durable path: the `DurableStageCheckpoint` type, the stage/task checkpoint metadata, the DBOS envelope encode/validate/decode, the completed-run catalog merge, the checkpoint→snapshot hydration, and the replay `__modelFallbackMeta`. `WorkflowTaskResult` carries it too. Tests: the DBOS envelope round-trip and the cached-replay hydration test now assert `thinkingLevel` survives encode→decode and checkpoint→snapshot. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…urfaces Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dget Per review, the model + thinking + fast display does not belong on the above-editor BACKGROUND widget: it summarises each run on a single line, so a parallel fan-out on several models has no clean way to show them (that was the "gets messy" concern). Model/thinking/fast identity stays on the `/workflow connect` graph node cards, which have one box per stage and room to show each model. Restores widget.ts and widget-rendering.test.ts to their upstream state (zero net widget diff on this PR) and removes the widget-only helper and test (`widget-model-label.ts`, `widget-model-thinking.test.ts`). The shared `codex-fast-label.ts` stays — the node card still uses it. Node-card model row, live fallback reflection, and durable `thinkingLevel` persistence are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Review feedback: `/workflow resume` persisted the model + thinking identity but never rendered it. `shouldReplay` stages build their graph node card from the replay-source snapshot, and that initialization copied only the result and session fields -- `model`, `thinkingLevel`, and `fastMode` were left absent, so a replayed card rendered the em-dash placeholder instead of the restored model row. The durable layer was already storing them; only the read-back was missing. `replaySource` is a full `StageSnapshot`, so the persisted values are copied straight across next to the existing session fields, guarded the same way. Also retires the stale `StageSnapshot.thinkingLevel` doc wording: it pointed at "background-run surfaces" (from when the BACKGROUND widget was in scope, since reverted) and claimed the level is "absent for restored runs", which this change makes untrue. It now names the graph node card and the resume path. Tests: a new continuation-replay suite asserts a resumed run restores model, thinking level, and fast tier onto the replayed stage, and that a source without model identity does not gain one. This covers a different path from the existing `recordCachedStageIntoStore` hydration test -- the continuation snapshot path is the one that regressed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
168d858 to
644ae34
Compare
Bring the node-card model/thinking/fast + durable thinkingLevel persistence up to date with upstream's tabs reformat, the ModelRuntime refactor (bastani-inc#2064), and the reshaped graph node cards (bastani-inc#2140). Re-applied thinkingLevel across the durable serialize/restore path, integrated the model row into the reshaped card, and preserved the replay model-identity fix. NODE_H stays 6 for the model row; updated the bastani-inc#2100 graph-perf composed-row expectation to match the taller card.
Bring node-card model/thinking display + durable thinkingLevel persistence up to date with upstream 0.9.12. Only CHANGELOG conflicted (our [Unreleased] bullet vs the cut 0.9.12 release section); the thinkingLevel type additions auto-merged.
Bring node-card model/thinking display + durable thinkingLevel persistence up to date with upstream 0.9.13-alpha.1. Only CHANGELOG conflicted; the stage-runner thinkingLevel additions auto-merged cleanly against upstream's model-fallback convergence (bastani-inc#2201).
|
Sorry for the delay in responding. I just had a chance to check this after the latest commits. I really appreciate the contribution. I verified the exact PR head (
The core behavior otherwise worked in manual testing: model and thinking rows appeared in Please rerun the focused tests and |
|
By the way, you can also ask Atomic to verify your PR directly. For example:
You can ask to attach evidence to this PR or even change the prompt to automatically apply the fixes. |
Thanks for replying, I will run those changes and update the PR. |
Resolves @flora131's three blockers on bastani-inc#1859: 1. The new replay-identity test imported `describe` from `bun:test`, so it failed to load under the repo's Vitest runner ("Cannot find package 'bun:test'"). Switched to the `vitest` import; the suite now runs and passes under `vitest --run --project unit`. 2. A long configured Codex model name truncated the required `fast` marker (`gpt-5.3-codex-spark f…`). `modelText` now drops the thinking level first and then truncates the model name, reserving the whole ` fast` marker so it is never cut. Added a long-name regression test. 3. Formatted the branch with Biome (tabs in codex-fast-label.ts, import order in node-card.ts and the test) so `npm run check` passes. Verified: `npm run check` clean (Biome + tsc + shrinkwrap); the six focused suites pass under `npx vitest --run --project unit` (68 tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@flora131 thank you — all three were spot on, and the 1. Replay test now loads under Vitest
2. Long model name truncates first,
|
|
Recorded a hands-on run of this PR on a local build of the head commit (15ac7c1), driving the Atomic TUI in a tmux session and inspecting the /workflow connect graph node cards. Attaching the screen recording of that session for reference. pr1859-review.mp4 |
|
Thanks for the fix here. While testing this PR's exact head ( Thinking levels can be truncated or omitted for normal model IDs. Repro:
Expected: the effective model and full thinking level remain identifiable. Actual:
It looks like the 22-cell card interior applies the model-row width budget only in fast mode; otherwise the generic card truncation can consume the thinking level. The attached test video shows this around 1:24–1:40. Are you able to reproduce this as well? If so, could the row reserve space for the thinking level before truncating the model name? |
Follow-up to @flora131's review: the model-row width budget (reserve the suffix, truncate the model name first) was applied only in fast mode, so for ordinary models the generic card truncation consumed the thinking level: claude-haiku-4-5 · high → claude-haiku-4-5 · hi… claude-sonnet-4-5-20250929 · medium → claude-sonnet-4-5-202… (medium gone) modelText now treats the thinking level and the ` fast` marker as one load-bearing suffix and always truncates the model name first, in every mode: claude-haiku-4-5 · high → claude-haiku-4… · high claude-sonnet-4-5-20250929 · medium → claude-sonne… · medium gpt-5.1-codex · high (fast) → gpt-5.1-c… · high fast Short names are unchanged. Added regression tests for the non-fast long-name case and the combined level+fast overflow. Verified: npm run check clean; the six focused suites pass under `npx vitest --run --project unit` (70 tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@flora131 reproduced it exactly — thanks, and good catch. My earlier fix only reserved space in fast mode, so for ordinary models the generic 22-cell truncation ate the thinking level. Fixed in
And the combined + short cases stay correct: Added two regression tests: the non-fast long-name case ( Verified with the real gates: Ready for another look. (Also — appreciate you recording the TUI run; that's a much better signal than my earlier headless checks, and it caught exactly what I missed.) |
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
What this does
Shows the effective model + thinking level (and Codex fast tier) for each workflow stage on the
/workflow connectgraph node cards, and persists that identity across/workflow resume. The main-session footer has always shown<model> <thinking>, but background/orchestrated runs were opaque about which model (and reasoning level) each stage used. This closes that gap in the place you inspect a running workflow — the graph overlay.1. Graph node cards (
/workflow connect)Node cards showed title / duration / status / deps but deliberately hid the model. They now carry a dedicated compact model row beneath the status line:
offto fit the ~22-cell card;—shows when no model is resolved yet.formatCodexFastModeModelLabel(<model> <thinking> fast). When the card would overflow, the thinking level is dropped so thefastmarker is never truncated away.StageSnapshot.model, whichapplyModelFallbackMetarewrites on each fallback).NODE_H); graph layout and edge routing derive fromNODE_Hand adapt automatically.2. Persistence across
/workflow resumethinkingLevelnow rides alongsidemodelat every serialize/restore site of the mandatory DBOS durable path — theDurableStageCheckpointtype, stage/task checkpoint metadata, the DBOS envelope encode/validate/decode, the completed-run catalog merge, checkpoint→snapshot hydration, and the replay__modelFallbackMeta(plusWorkflowTaskResult). A resumed run therefore restores the same model + thinking identity, not just the model.Plumbing (shared live-snapshot path)
StageModelFallbackMetagainsthinkingLevel, populated byStageSessionController.currentModelFallbackMeta()from the live session (or a pending level set before the session exists).executor-stage-factory'sapplyModelFallbackMetawrites it toStageSnapshot.thinkingLevel, next to the existingmodelfield.<short-model> · <thinking> [fast]row via a small localcodex-fast-label.ts— a mirror of the footer helper, kept local because importing the@bastani/atomicbarrel into the TUI module graph breaks thepi-tui-mocked overlay test subprocesses.Contract change
The node cards previously hid the model by design; this reverses that. The three tests pinning the old behavior are updated to assert the model row is shown, and the dependency-row assertions move down one row.
Tests
node-card.test.ts: compact model row; thinking append/omit; fast marker on the model row (not the deps row); width-guard (drop the level to preservefast);—when absent; mid-run fallback swap; geometry (NODE_H= 6).durable-dbos-topology.test.ts: model + thinking-level round-trip through the DBOS envelope (encode → decode).durable-stage-frontier-fixes.test.ts: cached-replay hydration restoresthinkingLevel(checkpoint → snapshot).stage-runner-reasoning-suffix.test.ts: a 429 fallback surfaces the fallback model and thinking level onmeta.bun run typecheck/lint/check:file-lengthclean; theoverlay-graph-*anddurable-*suites pass.Notes
main.test:unitfailures on this branch (MCP lazy-startup ×3 and thePi v0.81.1 declarations and publish artifactssync check) reproduce identically with these changes stashed on a clean tree — pre-existing and unrelated.Greptile Summary
This PR shows workflow stage model identity on graph node cards and preserves it across resume. The main changes are:
thinkingLevelthrough live fallback metadata, task results, replay, and DBOS checkpoint encode/decode.Confidence Score: 5/5
This PR is safe to merge with minimal risk.
The changes consistently extend existing model metadata paths with
thinkingLeveland include tests for DBOS round-trip, cached replay, executor replay, fallback metadata, and node-card rendering. No verified functional or security issues were found.Files Needing Attention: No files require special attention.
What T-Rex did
Important Files Changed
thinkingLevelupdates.thinkingLevel.thinkingLevelmetadata through checkpoint hydration.Sequence Diagram
Reviews (13): Last reviewed commit: "fix(workflows): reserve the thinking lev..." | Re-trigger Greptile