Skip to content

fix(workflows): transcript defaults to path-only, inline on tail/limit - #1316

Merged
lavaman131 merged 2 commits into
mainfrom
fix/1314-transcript-lazy-read
Jun 8, 2026
Merged

fix(workflows): transcript defaults to path-only, inline on tail/limit#1316
lavaman131 merged 2 commits into
mainfrom
fix/1314-transcript-lazy-read

Conversation

@lavaman131

@lavaman131 lavaman131 commented Jun 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

Transcript introspection now returns path metadata and a lazy-read prompt by default when a `sessionFile`/`transcriptPath` exists, keeping entry bodies and tool output out of model context. Inline previews are an explicit opt-in via `tail`/`limit`; stages with no transcript path fall back to a bounded preview with a `fallbackNote`.

Closes #1314

Key changes

  • Path-only default: when `sessionFile`/`transcriptPath` is present and neither `tail` nor `limit` is set, the transcript result has `entries: []`, `entryLimit: 0`, and a `lazyReadPrompt` guiding the model to read the file lazily — no entry text or tool output is injected into context.
  • Explicit opt-in preview: passing `tail` or `limit` still returns a bounded inline preview (`inlineMode: "preview"`).
  • `includeToolOutput` scoped to previews: only applies when an explicit preview or no-path fallback is active; it no longer bypasses the path-only default.
  • No-path fallback: when no transcript path exists, falls back to up to 5 recent snapshot entries with a `fallbackNote` explaining the situation (`inlineMode: "fallback_preview"`).
  • New result fields: `inlineMode` (`"path_only" | "preview" | "fallback_preview" | "notice"`), `lazyReadPrompt`, and `fallbackNote` added to `TranscriptResult` and `WorkflowTranscriptResult`.
  • `shapeTranscriptResult()` helper: centralises path-only vs. preview vs. fallback branching, replacing duplicated inline assembly at call sites. Uses a lazy `buildEntries` thunk so entry bodies are never materialised for the path-only hot path.
  • Render updates: `renderTranscriptToolContent` emits `lazyReadPrompt`/`fallbackNote` lines and prints `entries: not inlined` in path-only mode; `transcriptNoticeText` is refactored to surface path, entry count, and fallback note in the UI banner.
  • Schema / tool description docs: `action`, `limit`, `tail`, and `includeToolOutput` parameter descriptions updated across `WorkflowParametersSchema`, `WORKFLOW_TOOL_DESCRIPTION`, and `README.md` to reflect the new default behaviour.
  • Test coverage: existing test updated to assert path-only default behaviour; two new test scenarios added — explicit `limit` with `includeToolOutput`, and the no-transcript-path fallback preview path.

Breaking changes / migration notes

The default transcript response shape changes: callers that previously expected `entries` to contain inline content when no `tail`/`limit` was passed will now receive `entries: []` with `entryLimit: 0` and a `lazyReadPrompt`. Pass `tail` or `limit` explicitly to restore inline preview behaviour.

Acceptance criteria

  • Default transcript action does not inject entry text or tool output when a transcript path exists.
  • Results include `sessionFile`/`transcriptPath`, entry metadata (`availableEntries`, `entryLimit: 0`), and a concrete `lazyReadPrompt`.
  • Explicit `tail`/`limit` still returns bounded inline previews.
  • Missing transcript paths fall back to a bounded preview with a clear `fallbackNote`.
  • Tests cover default path-only output, explicit preview opt-in, and missing-session-file fallback.

Validation

  • `bun run typecheck`
  • `bun run lint`
  • `bun run test:unit`
  • `bun test test/integration/mock-extension-api.test.ts`

@lavaman131

Copy link
Copy Markdown
Collaborator Author

Implementation Notes

Task: Implement GitHub issue #1314 in the @bastani/workflows package: "Workflow transcript tools should return a transcript file path + lazy-read prompt instead of dumping full transcript text."

Context

The repo is the atomic-monorepo Bun workspace. Read AGENTS.md before starting. CRITICAL CONSTRAINTS: use Bun only (never node/npm/npx/yarn/pnpm); @bastani/workflows ships raw .ts with NO build step (never add dist/, tsconfig.build.json, outDir, or bundling). Source files use .js import extensions (TypeScript ESM convention) even though files are .ts.

Problem

The workflow transcript introspection tool currently injects large amounts of transcript text directly into the parent model/session context. For large runs this bloats context and can push a session over budget, surfacing an Auto-compaction failed warning. This issue is NARROWLY SCOPED: only change the transcript tool OUTPUT behavior so it does not dump full transcript text by default. Do NOT change the broader workflow-completion UX or the TUI on-screen notice budget (out of scope).

Exact scope of changes

  • packages/workflows/src/extension/index.ts — the transcript action handler (around the case "transcript": near line 1407) and renderTranscriptToolContent (around line 630), plus the helper selectTranscriptEntries/requestedTranscriptEntryLimit/DEFAULT_TRANSCRIPT_LIMIT (around line 848) and the WORKFLOW_TOOL_DESCRIPTION constant text (around line 125-132) which currently says transcript defaults to 5 entries — update wording to reflect the new path-only default.
  • packages/workflows/src/extension/render-result.ts — the TranscriptResult type (around line 118) and the transcript notice rendering transcriptNoticeText / case "transcript" (around line 216 and 351). The on-screen TUI notice is already bounded (~240 chars); keep it consistent and add rendering of the new lazy-read prompt to the MODEL-FACING tool content (the renderTranscriptToolContent in index.ts), not necessarily the TUI notice.

Required behavior

  1. By DEFAULT (no explicit tail/limit arg), the transcript action returns ONLY metadata plus the transcript file path and NO inline entry bodies. The model-facing content must include: runId, stageId, source, transcriptPath/sessionFile, and entryCount (the number of available entries). It must NOT include entry text or tool output bodies.
  2. Include a short, explicit prompt/instruction in the result telling the agent to read the transcript lazily, e.g.: "Transcript not inlined to protect context. Read it lazily from with your file read tools (read small ranges; rg/grep for targeted lookups)." The concrete transcriptPath must be substituted into the message.
  3. Keep an explicit OPT-IN for inlining a BOUNDED preview via the existing tail/limit args (for quick recent-context checks). When tail or limit is explicitly provided, inline that many most-recent entries as before — but never dump the full transcript by default.
  4. When a transcript file path is UNAVAILABLE (e.g. a live run with no session file yet, or an error/notice path), fall back to a bounded preview (the existing DEFAULT_TRANSCRIPT_LIMIT bound) and clearly say so in the result (a note explaining the fallback because no session file path is available). The existing error/notice transcript paths (ambiguous/not_found/missing-stage) should keep returning their notice entries.

Acceptance criteria (all must hold)

  • Calling the transcript action does NOT inject full transcript text into the parent model context by default (path-only metadata + lazy-read prompt).
  • The result includes the transcript file path AND a prompt instructing the agent to read it lazily with file tools.
  • A bounded preview remains available via an explicit tail/limit opt-in.
  • The missing-session-file case falls back to a bounded preview with a clear note.
  • Unit tests cover: (a) default path-only output, (b) explicit preview opt-in via tail/limit, and (c) the missing-session-file fallback. Tests go in test/unit/ (repo root) using bun:test + node:assert/strict, matching the existing test style. If the transcript helpers (renderTranscriptToolContent, selectTranscriptEntries, the transcript result shaping) are not currently exported, export what is minimally needed for testing (or test through makeExecuteWorkflowTool), following existing repo conventions — prefer testing the actual tool output shape and the rendered model-facing content.

Validation (must all pass before opening PR)

  • bun run typecheck (from repo root of the worktree) passes with no errors.
  • bun run lint passes.
  • bun run test:unit passes, including the new tests. You may run the focused new test file directly first (e.g. bun test test/unit/<your-new-test>.test.ts) but the full test:unit suite must stay green.
  • Add a CHANGELOG entry under ## [Unreleased] in packages/workflows/CHANGELOG.md in the appropriate subsection (likely ### Changed and/or ### Fixed), describing the new path-only default + lazy-read prompt and the bounded-preview opt-in. Read the existing [Unreleased] section first and append to existing subsections rather than duplicating. Do NOT modify already-released version sections and do NOT bump the package version.

PR

Open a pull request from branch fix/1314-transcript-lazy-read against origin/main. The PR description should summarize the change, reference Closes #1314, list the acceptance criteria satisfied, and note the validation commands run. Keep the diff minimal and focused on the transcript tool output behavior.

Running Notes

  • Record implementation decisions, deviations from the spec, tradeoffs, blockers, validation notes, and anything else the user should know.

Iteration 1 notes (2026-06-08)

Delegation and setup

  • Preflight was delegated to codebase-locator. It found the checkout initialized: root package.json declares bun@1.3.14, bun.lock and node_modules/ are present, repo scripts/CI use Bun, and packages/workflows exports raw TypeScript with no build/dist setup. No bun install setup step was needed.
  • Code analysis was delegated to codebase-analyzer. The key decision was to enforce the safe default in structured TranscriptResult shaping, not only in text rendering, so format: "json" cannot dump entries by default.
  • Implementation was delegated to typescript-expert. Validation was delegated and then confirmed from the parent session with Bun commands.

Decisions / tradeoffs

  • Added additive structured result fields: lazyReadPrompt, fallbackNote, and inlineMode (path_only, preview, fallback_preview, notice) to make renderer/test behavior explicit.
  • Only explicit tail or limit is treated as a transcript preview opt-in. includeToolOutput: true by itself does not inline output bodies when a transcript path exists.
  • Path-only default sets entries: [], entryLimit: 0, entryCount to the tool-known entry count, and truncated: entryCount > 0 to signal omitted inline content.
  • Missing transcript path keeps the existing bounded preview behavior using DEFAULT_TRANSCRIPT_LIMIT and adds a clear fallbackNote.
  • The detailed lazy-read instruction is rendered in model-facing tool content; the TUI notice remains compact and bounded.
  • README/schema wording was updated alongside WORKFLOW_TOOL_DESCRIPTION to preserve description sync tests.

Files changed

  • packages/workflows/src/extension/index.ts
  • packages/workflows/src/extension/render-result.ts
  • packages/workflows/src/extension/workflow-schema.ts
  • packages/workflows/README.md
  • packages/workflows/CHANGELOG.md
  • test/unit/slash-dispatch.test.ts
  • test/unit/workflow-schema.test.ts
  • test/integration/mock-extension-api.test.ts

Validation outcomes

  • bun run typecheck passed.
  • bun run lint passed.
  • bun run test:unit passed: 2164 pass / 0 fail across 121 files.
  • Additional focused README/tool-description sync validation: bun test test/integration/mock-extension-api.test.ts passed: 99 pass / 0 fail.

Deviations / blockers

  • No PR was opened per orchestrator instruction to ignore PR submission requests in this stage.
  • No package version bump was made.
  • No blockers remain for this iteration.

@claude claude Bot changed the title fix(workflows): return transcript paths by default fix(workflows): default transcript to path-only, inline only on explicit tail/limit Jun 8, 2026
@claude

claude Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review — fix(workflows): return transcript paths by default

Reviewed against the acceptance criteria in #1314. This is a clean, well-scoped change: the default transcript action now returns path + metadata + a lazyReadPrompt instead of inlining bodies, explicit tail/limit still opt into bounded previews, and the missing-path case falls back with a clear fallbackNote. Docs, schema, README, CHANGELOG, and tests are all updated consistently. Nice work.

Verified the logic against the source — the three behavioral paths in shapeTranscriptResult and the includeSnapshotOutput gating in the snapshot branch line up with the intent, and the new tests cover the required cases (path-only default, explicit preview opt-in, no-path fallback) plus the includeToolOutput-doesn't-bypass-default case.

A few minor, non-blocking observations:

1. Redundant fallback in shapeTranscriptResult (index.ts:910-911)

const transcriptPath = input.transcriptPath ?? input.sessionFile;
const pathForLazyRead = transcriptPath ?? input.sessionFile;

transcriptPath already falls back to input.sessionFile, so the second ?? input.sessionFile can never fire — pathForLazyRead is always identical to transcriptPath. The two can collapse into one, making the path-resolution intent clearer.

2. Wasted entry construction in path-only mode

In the default (path-exists) case the caller still computes snapshotTranscriptEntries(...) / maps liveHandle.messages, and shapeTranscriptResult then discards them (entries: []). For the large-transcript runs this PR is specifically protecting against, that's an O(n) build-and-throw-away. Functionally harmless (it never reaches model context), but you could short-circuit before materializing entries if you want the hot path cheap as well as safe — entryCount only needs the length, not the bodies.

3. truncated semantics in path-only mode (index.ts:919)

truncated: entryCount > 0 makes a path-only result with entries-on-disk render as (truncated) even though nothing was requested-then-cut. Defensible ("more exists on disk"), but worth confirming no downstream consumer reads truncated as "your explicit limit clipped results" and reacts by re-fetching.

4. Unused "notice" inline mode

TranscriptInlineMode includes "notice" (render-result.ts:118) but no producer emits it. Either wire it up or drop the dead variant.

Nit

  • Consider whether availableEntries in path-only snapshot mode (built with includeToolOutput=false) could ever differ in count from what an agent sees reading the raw file — likely fine, just flagging since the number is advisory.

None of these block merge. The default-no-longer-inlines behavior change is intentional and clearly documented in schema/README/CHANGELOG, so any agent relying on default-populated entries is correctly steered to the path + lazyReadPrompt.

Note: I couldn't run bun run typecheck / bun test in this environment (commands gated), so this is based on static analysis of the diff and surrounding source; the author reports all four validation commands pass.

Address PR #1316 review feedback:
- Collapse redundant `pathForLazyRead` fallback; `transcriptPath` already
  resolves to `sessionFile`, so the second `?? sessionFile` was dead.
- Make the path-only default the cheap hot path: pass entries via a lazy
  `buildEntries` thunk plus an explicit cheap `entryCount` so large
  transcripts are no longer mapped/built just to be discarded.
- Document `truncated` semantics in path-only mode (advisory "more on disk",
  cosmetic suffix only; no consumer re-fetches on it).
- Drop the unused `"notice"` TranscriptInlineMode variant.

No behavior change; full unit suite, typecheck, and lint pass.
@claude claude Bot changed the title fix(workflows): default transcript to path-only, inline only on explicit tail/limit fix(workflows): transcript defaults to path-only, inline on tail/limit Jun 8, 2026
@claude

claude Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review: default transcript to path-only

Solid, well-scoped change. The core idea — keeping large transcript bodies/tool output out of model context by default and surfacing a path + lazy-read prompt — is implemented cleanly, and the docs (README, tool description, schema, changelog) are all updated consistently with no stale "5-entry default" references left behind. Test coverage is genuinely good: path-only default, explicit tail, explicit limit + includeToolOutput, no-path fallback, and the empty-live-handle case are all exercised, including the rendered notice text.

Things I liked

  • The thunk-based buildEntries optimization is the right call. The default hot path never materializes entry bodies just to discard them, and the cheap snapshotEntryCount (toolEvents.length + result? + error?) provably mirrors snapshotTranscriptEntries(...).length since sorting/includeOutput never change the entry count. Same for the live path (messages.length is 1:1 with transcriptEntryFromMessage). availableEntries stays consistent across all modes.
  • includeToolOutput correctly scoped to explicit-preview / no-path branches so it cannot silently bypass the path-only default — and there is a test asserting exactly that.
  • Renamed transcriptNoticeText to transcriptEntriesNoticeText plus the new wrapper is clean; verified it is the only caller, no dangling references.

Minor points (non-blocking)

  1. tail: 0 / limit: 0 with a path falls into preview mode, not path-only. isTranscriptPreviewExplicit is true whenever tail/limit is defined, so a non-positive explicit count returns entries: [] with inlineMode: preview and no lazyReadPrompt. The caller gets neither inline entries nor the lazy-read guidance — arguably the worst of both. Consider treating non-positive explicit counts as still emitting the lazyReadPrompt, or at least documenting the edge. No test covers this case.

  2. truncated: input.entryCount > 0 in path-only mode renders a (truncated) suffix even though nothing was clipped by a limit. The inline comment acknowledges it is cosmetic, but a truncated reading on a default call that simply did not inline anything could mislead. Worth considering truncated: false for path-only, since lazyReadPrompt / entries: not inlined already communicate the state.

  3. PR description lists an inlineMode value notice that does not exist in TranscriptInlineMode (path_only | preview | fallback_preview). Harmless doc drift, but worth correcting so it does not read as an unimplemented branch.

  4. Nit (docstring grammar): shapeTranscriptResult JSDoc — "keeping the context-safe path-only default the cheap hot path" reads as missing an "as".

Nothing here blocks merge — (1) and (2) are the only behavioral edges worth a second look. Could not run bun run typecheck/tests locally (sandbox denied), so relying on the stated validation for those.

@lavaman131
lavaman131 merged commit db2057e into main Jun 8, 2026
9 checks passed
@lavaman131
lavaman131 deleted the fix/1314-transcript-lazy-read branch June 8, 2026 23:28
lavaman131 added a commit that referenced this pull request Jun 29, 2026
#1316)

* fix(workflows): return transcript paths by default

* refactor(workflows): tidy transcript path-only result shaping

Address PR #1316 review feedback:
- Collapse redundant `pathForLazyRead` fallback; `transcriptPath` already
  resolves to `sessionFile`, so the second `?? sessionFile` was dead.
- Make the path-only default the cheap hot path: pass entries via a lazy
  `buildEntries` thunk plus an explicit cheap `entryCount` so large
  transcripts are no longer mapped/built just to be discarded.
- Document `truncated` semantics in path-only mode (advisory "more on disk",
  cosmetic suffix only; no consumer re-fetches on it).
- Drop the unused `"notice"` TranscriptInlineMode variant.

No behavior change; full unit suite, typecheck, and lint pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Workflow transcript tools should return a transcript file path + lazy-read prompt instead of dumping full transcript text

1 participant