Skip to content

fix(chat,templates): keep reasoning in stream order and pin starter parser imports - #3438

Merged
kwakayama merged 2 commits into
mainfrom
fix/chat-reasoning-order-and-pinned-template-imports
Aug 7, 2026
Merged

kwakayama merged 2 commits into
mainfrom
fix/chat-reasoning-order-and-pinned-template-imports

Conversation

@kwakayama

@kwakayama kwakayama commented Aug 6, 2026 •

Copy link
Copy Markdown
Contributor

Two problems a new project hits on its first npm run dev after npm create veryfront@latest.

Reasoning rendered below the tool calls that came after it

The stream order was reasoning → tool → tool → reasoning → text. The UI rendered tool → tool → reasoning → text, and step 1's reasoning was gone entirely.

Providers reuse per-step reasoning part ids, so reasoning-0 arrives again in step 2 carrying the same AG-UI messageId. handleReasoningStart overwrote the map entry, which assigned a fresh partOrderCounter value and reset text to "". Step 2's reasoning jumped below the step 1 tool cards; step 1's reasoning was dropped.

A start now distinguishes the two cases:

  • Still open — a replayed start (resumed stream). Keep the accumulated text and the original order. Re-creating the block would move it below everything that streamed in the meantime.
  • Already closed — a genuinely new span. Retire the closed block to closedReasoningBlocks so it keeps its own text and position instead of being overwritten.

Test replays the exact wire trace from the reproduction:

before: [tool-calculator, reasoning:second thought, text]
after:  [reasoning:first thought, tool-calculator, reasoning:second thought, text]

Unversioned import warning in every chat starter

! Unversioned import may cause reproducibility issues
    import=react-markdown suggestion="Pin version: import 'react-markdown@x.y.z'"

The starters already pin react-markdown to 9.0.3 in package.json. The module pipeline resolves browser imports from the specifier, not package.json, so the pin does nothing and every request warns. Reading package.json to fill the version is gated behind VERYFRONT_DEPENDENCY_PINNING=1 plus a rollout cohort, so it never fires for a new project.

The five chat starters now carry the version inline, with a tsconfig wildcard alias so consumer tsc still resolves it (react-markdown@9.0.3 is not a module TypeScript can find on its own). Two tests assert the imported version matches the installed one and that the aliases exist, so they cannot drift.

SSR is unaffected — the bare strategy strips the version and resolves from node_modules.

Verification

Fresh scaffold from these templates (init ai-agent, npm install, npm run dev):

  • Zero Unversioned import warnings.
  • Module resolves to https://esm.sh/react-markdown@9.0.3?external=react,react-dom&target=es2022; both pinned CDN URLs return 200.
  • Chat UI mounts, no console errors.
  • tsc --noEmit clean.

Suites: cli 278 passed / 0 failed, chat+react+agent 205 passed, deno task typecheck exit 0, deno fmt --check and deno lint clean.

Summary by CodeRabbit

  • Bug Fixes

    • Improved streaming reasoning display so content remains in the correct order across tool calls and repeated reasoning events.
    • Preserved open reasoning text and position when a reasoning segment restarts.
    • Standardized Markdown rendering behavior across generated starter templates with consistent package versions and configuration.
  • Tests

    • Added coverage for reasoning order, repeated identifiers, and Markdown renderer configuration.

…arser imports

Two problems a new project hits on its first `npm run dev`.

Reasoning rendered below the tool calls that came after it. Providers reuse
per-step reasoning part ids, so `reasoning-0` arrives again in step 2 with the
same AG-UI messageId. `handleReasoningStart` overwrote the map entry, which
assigned a fresh order and reset the text: step 2's reasoning jumped below the
step 1 tool cards and step 1's reasoning was dropped entirely.

A start now distinguishes the two cases. Still open means a replayed start, so
the text and the original order stay. Already closed means a genuinely new span,
so the closed one is retired with its own text and position instead of being
overwritten.

The chat starters also imported `react-markdown` and `remark-gfm` bare. The
module pipeline resolves browser imports from the specifier, not package.json,
so an exact pin there does nothing and the dev server warns about an unversioned
import on every request. The renderers carry the version inline, and tsconfig
aliases it back to node_modules so consumer tsc still resolves it.
@kwakayama
kwakayama requested a review from kojiwakayama as a code owner August 6, 2026 19:23
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d0a65409-62ed-4112-a1d9-845b6d0cb99f

📥 Commits

Reviewing files that changed from the base of the PR and between 176b843 and 25e1042.

📒 Files selected for processing (1)
  • cli/templates/index.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • cli/templates/index.test.ts

📝 Walkthrough

Walkthrough

The change pins Markdown parser imports across five CLI templates and adds matching TypeScript aliases and tests. It also preserves streaming reasoning order when providers reuse reasoning IDs.

Changes

Template Markdown dependency pinning

Layer / File(s) Summary
Pin template Markdown dependencies
cli/templates/files/*/app/markdown-renderer.tsx, cli/templates/files/*/tsconfig.json, cli/templates/manifest.json
Renderers use pinned react-markdown and remark-gfm versions. TypeScript resolves the pinned packages through local aliases.
Validate pinned template imports
cli/templates/index.test.ts
Tests require exact parser versions and matching TypeScript path aliases in each chat starter.

Streaming reasoning order

Layer / File(s) Summary
Track reused reasoning spans
src/agent/react/use-chat/streaming/handler.ts
Streaming state stores closed reasoning spans, clears them at message start, and retires completed spans when IDs are reused.
Build and test ordered reasoning transcripts
src/agent/react/use-chat/streaming/parts-builder.ts, src/agent/react/use-chat/streaming/handler.test.ts
Part construction includes closed reasoning spans in transcript order. Tests cover reused IDs and restarted open spans.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant StreamProtocol
  participant handler
  participant parts-builder
  participant Transcript
  StreamProtocol->>handler: Emit reasoning start
  handler->>handler: Preserve or close the existing reasoning span
  handler->>parts-builder: Pass active and closed reasoning blocks
  parts-builder->>Transcript: Build ordered reasoning parts
Loading

Possibly related PRs

Suggested reviewers: kojiwakayama

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes both primary changes: preserving reasoning stream order and pinning starter parser imports.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/chat-reasoning-order-and-pinned-template-imports

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cli/templates/index.test.ts`:
- Around line 972-997: Replace the substring and bare-import checks in the test
around chatTemplates with structural validation: parse the renderer import
specifiers and assert react-markdown and remark-gfm use exactly the installed
version, then read compilerOptions.paths and assert the relevant aliases exist
with non-empty targets. Ensure the test validates consumer TypeScript resolution
by compiling a minimal consumer or reusing the existing TypeScript verification.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2a048ec7-dbc1-48d2-869f-453e4033db40

📥 Commits

Reviewing files that changed from the base of the PR and between 36c073f and 176b843.

📒 Files selected for processing (15)
  • cli/templates/files/ai-agent/app/markdown-renderer.tsx
  • cli/templates/files/ai-agent/tsconfig.json
  • cli/templates/files/coding-agent/app/markdown-renderer.tsx
  • cli/templates/files/coding-agent/tsconfig.json
  • cli/templates/files/docs-agent/app/markdown-renderer.tsx
  • cli/templates/files/docs-agent/tsconfig.json
  • cli/templates/files/multi-agent-system/app/markdown-renderer.tsx
  • cli/templates/files/multi-agent-system/tsconfig.json
  • cli/templates/files/saas-starter/app/markdown-renderer.tsx
  • cli/templates/files/saas-starter/tsconfig.json
  • cli/templates/index.test.ts
  • cli/templates/manifest.json
  • src/agent/react/use-chat/streaming/handler.test.ts
  • src/agent/react/use-chat/streaming/handler.ts
  • src/agent/react/use-chat/streaming/parts-builder.ts

Comment thread cli/templates/index.test.ts
…rally

The substring checks could pass on an import that does not match what the
starter installs: `9.0.3` is a prefix of `9.0.31`, and scanning the tsconfig
text accepted an alias parked outside `compilerOptions.paths` or pointing at
nothing.

Compare the parsed import specifier against the exact installed version, and
read the alias through `compilerOptions.paths` so its target has to name the
installed package. The specifier regex is static now, which also drops the
ReDoS lint on a pattern built from a variable.
@kwakayama
kwakayama added this pull request to the merge queue Aug 7, 2026
Merged via the queue into main with commit 8f223ed Aug 7, 2026
31 checks passed
@kwakayama
kwakayama deleted the fix/chat-reasoning-order-and-pinned-template-imports branch August 7, 2026 02:27
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.

1 participant