ci: type-check examples/** and testing/** to catch call-site regressions (#820)#857
Open
tombeckenham wants to merge 2 commits into
Conversation
3 tasks
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR broadens typecheck coverage into example and testing surfaces, updates example routes to normalize tool-result content and media prompts, and revises testing-panel/e2e routes and trace recording to use AG-UI-style stream chunks and direct adapters. ChangesTypecheck coverage
Example content normalization
Panel recording and adapters
E2E adapter wiring
Sequence Diagram(s)sequenceDiagram
participant apiChat as api.chat route
participant Recording as createEventRecording
participant ChunkFile as ChunkRecording file
participant Debugger as stream-debugger
participant Processor as StreamProcessor
apiChat->>Recording: wrapAdapterForRecording(traceId)
Recording->>ChunkFile: write AG-UI StreamChunk events
Debugger->>ChunkFile: load trace data
Debugger->>Processor: createProcessor() or reuse existing processor
Processor-->>Debugger: stepForward updates result state
Estimated code review effort: 5 (Critical) | ~90+ minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🚀 Changeset Version PreviewNo changeset entries found. Merging this PR will not cause a version bump for any packages. |
|
View your CI Pipeline Execution ↗ for commit ce8eb7a
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-acp
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-bedrock
@tanstack/ai-claude-code
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-codex
@tanstack/ai-devtools-core
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-grok-build
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-mcp
@tanstack/ai-mistral
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-opencode
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-sandbox
@tanstack/ai-sandbox-cloudflare
@tanstack/ai-sandbox-daytona
@tanstack/ai-sandbox-docker
@tanstack/ai-sandbox-local-process
@tanstack/ai-sandbox-sprites
@tanstack/ai-sandbox-vercel
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
testing/e2e/src/routes/$provider/index.tsx (1)
27-31: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve the active
persistencequery instead of clearing it.Line 31 always resets
persistencetoundefined, so navigation from/$provider/?persistence=localStoragedrops the mode before/$provider/$featurecan consume it. The downstream route already readssearch.persistence, so this should be forwarded from the current page instead of hard-coded away.Proposed fix
function ProviderPage() { const { provider } = Route.useParams() as { provider: Provider } + const { persistence } = Route.useSearch() const features = getSupportedFeatures(provider) @@ search={{ testId: undefined, aimockPort: undefined, mode: undefined, - persistence: undefined, + persistence, }}🤖 Prompt for 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. In `@testing/e2e/src/routes/`$provider/index.tsx around lines 27 - 31, The navigation in the $provider route is clearing the active persistence query, which prevents downstream routes from reading it. Update the search object in the index route component to forward the current search.persistence value instead of hard-coding persistence to undefined, so the $feature route can consume it correctly. Use the existing route search handling in the $provider/index.tsx component as the place to preserve this query.
🧹 Nitpick comments (4)
testing/panel/src/routes/api.simulator-chat.ts (1)
130-132: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an interface for the
chatStreamoptions shape.The new inline object type should be promoted to an interface to match the repository TypeScript guideline.
Proposed refactor
+interface SimulatorChatStreamOptions { + messages: Array<ModelMessage> +} + function createSimulatorAdapter() { @@ - async *chatStream(options: { - messages: Array<ModelMessage> - }): AsyncIterable<StreamChunk> { + async *chatStream( + options: SimulatorChatStreamOptions, + ): AsyncIterable<StreamChunk> {As per coding guidelines,
**/*.{ts,tsx}should useinterfacefor defining object shapes.🤖 Prompt for 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. In `@testing/panel/src/routes/api.simulator-chat.ts` around lines 130 - 132, The chatStream method currently defines its options shape inline, which violates the TypeScript guideline to use interfaces for object types. Move the { messages: Array<ModelMessage> } parameter shape into a named interface near chatStream, then update the chatStream signature to use that interface so the options type is reusable and consistent with the repository style.Source: Coding guidelines
examples/ts-react-native-chat/package.json (1)
20-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlias
test:typestotypecheckinstead of duplicating the command.These two scripts are now the same command. Pointing
test:typesattypecheckkeeps CI and local typecheck behavior from drifting the next time flags change.🤖 Prompt for 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. In `@examples/ts-react-native-chat/package.json` around lines 20 - 21, The package scripts duplicate the same TypeScript command for type checking, so update the package.json scripts to make test:types reference typecheck instead of repeating tsc --noEmit; keep the change within the scripts section and use the existing typecheck entry as the single source of truth.testing/react-native-smoke/package.json (1)
16-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the existing
typecheckscript here too.
test:typesandtypecheckcurrently duplicate the sametsc --noEmitinvocation. Making one call the other avoids silent drift between the smoke flow and the CI gate.🤖 Prompt for 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. In `@testing/react-native-smoke/package.json` around lines 16 - 17, The package scripts for type checking are duplicated, so update the react-native-smoke package.json scripts to reuse the existing typecheck command from test:types instead of repeating the same tsc --noEmit invocation. Keep typecheck as the single source of truth and make test:types delegate to it so both paths stay aligned if the command ever changes.examples/ts-angular-chat/src/app.component.ts (1)
189-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract this inline message shape into an interface.
The
unknownbroadening makes sense, but the changed code still defines the object shape inline. Pulling it into aninterfacekeeps this aligned with the repo’s TS conventions and makes the render contract easier to reuse.As per coding guidelines,
**/*.{ts,tsx}: Useinterfacefor defining object shapes in TypeScript.🤖 Prompt for 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. In `@examples/ts-angular-chat/src/app.component.ts` around lines 189 - 190, The message shape in isRenderable is still defined inline and should be extracted into a reusable TypeScript interface to match repo conventions. Create an interface for the message/parts contract used by AppComponent and update isRenderable to reference it instead of an inline object type, keeping the existing unknown broadened content field.Source: Coding guidelines
🤖 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 `@CLAUDE.md`:
- Around line 233-242: The “second pass” description in CLAUDE.md does not match
the actual script wiring: the examples/testing typecheck is run via pnpm
test:types:examples (backed by nx run-many), not nx affected. Update the
paragraph near the documented CI command sequence to reference the real command
path, using either pnpm test:types:examples or the exact nx run-many
--targets=test:types --projects=examples/**,testing/** invocation, so
contributors can reproduce CI accurately.
In `@testing/e2e/src/routes/api.summarize.ts`:
- Around line 33-46: The current OpenRouter setup uses a non-existent
`createOpenRouterSummarize`/`httpClient` pattern and an `HTTPClient.addHook` API
that the SDK does not support. Replace the `openRouterHttpClient` helper and the
`httpClient` option with the SDK’s supported `hooks.beforeRequest` configuration
on the OpenRouter client or adapter factory (for example, the same
request-header mutation logic applied directly in the client config). Make sure
the updated implementation uses the existing OpenRouter client symbols in this
route module so the request headers are injected through the supported hook path
and the code compiles against `@openrouter/sdk`.
In `@testing/e2e/src/routes/api.tool-call-lifecycle-wire.ts`:
- Line 124: The structuredOutput implementation does not match the
AnyTextAdapter contract: update the adapter method to accept the required
StructuredOutputOptions argument and return the expected shape used by
adapter.structuredOutput, including the raw field instead of rawText. Locate the
fix in structuredOutput and align both the signature and returned object with
the contract defined in the adapter interface.
In `@testing/e2e/src/routes/tools-test.tsx`:
- Line 4: The import order in tools-test.tsx violates ESLint import/order
because the UIMessage type import is placed before the zod import. Reorder the
imports so the `@tanstack/ai-react` type import comes after the zod import,
keeping the existing import grouping consistent and satisfying the lint rule.
In `@testing/panel/README.md`:
- Around line 39-45: The Trace Format example is stale and still shows the old
chunk shape, while the current captured-data flow uses AG-UI EventType values.
Update the README example near the Trace Format section to reflect the new
recorded chunk structure used by the panel, replacing the pre-AG-UI `"chunk": {
"type": "content", ... }` shape with examples that match the actual
EventType-based records such as TEXT_MESSAGE_CONTENT, and keep the surrounding
description consistent with the updated flow.
In `@testing/panel/src/lib/recording.ts`:
- Around line 362-382: The tool-call reconstruction in recording.ts is emitting
TOOL_CALL_ARGS incorrectly by using the full argument snapshot as delta and
never closing the call with TOOL_CALL_END. Update the logic around the
TOOL_CALL_START / toolCallArgs emission so toolCallArgs gets only the
incremental arguments change (or an empty delta when rebuilding from a
snapshot), then emit a TOOL_CALL_END chunk immediately after the arguments for
each tool call. Use the existing stream.startedToolCalls tracking and the
toolCallStart, toolCallArgs, and pushChunk helpers to keep the recorded trace
aligned with StreamProcessor and adapter traces.
---
Outside diff comments:
In `@testing/e2e/src/routes/`$provider/index.tsx:
- Around line 27-31: The navigation in the $provider route is clearing the
active persistence query, which prevents downstream routes from reading it.
Update the search object in the index route component to forward the current
search.persistence value instead of hard-coding persistence to undefined, so the
$feature route can consume it correctly. Use the existing route search handling
in the $provider/index.tsx component as the place to preserve this query.
---
Nitpick comments:
In `@examples/ts-angular-chat/src/app.component.ts`:
- Around line 189-190: The message shape in isRenderable is still defined inline
and should be extracted into a reusable TypeScript interface to match repo
conventions. Create an interface for the message/parts contract used by
AppComponent and update isRenderable to reference it instead of an inline object
type, keeping the existing unknown broadened content field.
In `@examples/ts-react-native-chat/package.json`:
- Around line 20-21: The package scripts duplicate the same TypeScript command
for type checking, so update the package.json scripts to make test:types
reference typecheck instead of repeating tsc --noEmit; keep the change within
the scripts section and use the existing typecheck entry as the single source of
truth.
In `@testing/panel/src/routes/api.simulator-chat.ts`:
- Around line 130-132: The chatStream method currently defines its options shape
inline, which violates the TypeScript guideline to use interfaces for object
types. Move the { messages: Array<ModelMessage> } parameter shape into a named
interface near chatStream, then update the chatStream signature to use that
interface so the options type is reusable and consistent with the repository
style.
In `@testing/react-native-smoke/package.json`:
- Around line 16-17: The package scripts for type checking are duplicated, so
update the react-native-smoke package.json scripts to reuse the existing
typecheck command from test:types instead of repeating the same tsc --noEmit
invocation. Keep typecheck as the single source of truth and make test:types
delegate to it so both paths stay aligned if the command ever changes.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9f09057-1092-4975-b927-dd018d073df9
📒 Files selected for processing (47)
CLAUDE.mdexamples/ts-angular-chat/package.jsonexamples/ts-angular-chat/scripts/typecheck.mjsexamples/ts-angular-chat/src/app.component.tsexamples/ts-code-mode-web/src/lib/tool-result-content.tsexamples/ts-code-mode-web/src/routes/_banking-demo/banking-demo.tsxexamples/ts-code-mode-web/src/routes/_database-demo/api.database-demo.tsexamples/ts-code-mode-web/src/routes/_database-demo/database-demo.tsxexamples/ts-code-mode-web/src/routes/_home/api.product-regular.tsexamples/ts-code-mode-web/src/routes/_home/index.tsxexamples/ts-code-mode-web/src/routes/_npm-github-chat/api.codemode.tsexamples/ts-code-mode-web/src/routes/_npm-github-chat/npm-github-chat.tsxexamples/ts-code-mode-web/src/routes/_reporting/api.reports.tsexamples/ts-code-mode-web/src/routes/_reporting/reporting-agent.tsxexamples/ts-react-chat/src/routes/generations.image.tsxexamples/ts-react-chat/src/routes/generations.structured-output.tsxexamples/ts-react-chat/src/routes/generations.video.tsxexamples/ts-react-native-chat/package.jsonexamples/ts-react-native-chat/tsconfig.jsonexamples/ts-solid-chat/src/routes/index.tsxpackage.jsonpackages/ai-grok/tests/summarize-callsite-type-safety.test.tstesting/e2e/package.jsontesting/e2e/src/routes/$provider/index.tsxtesting/e2e/src/routes/api.arktype-tool-wire.tstesting/e2e/src/routes/api.openai-shell-skills-wire.tstesting/e2e/src/routes/api.openrouter-cost.tstesting/e2e/src/routes/api.openrouter-web-tools-wire.tstesting/e2e/src/routes/api.otel-usage.tstesting/e2e/src/routes/api.summarize.tstesting/e2e/src/routes/api.tool-call-lifecycle-wire.tstesting/e2e/src/routes/tools-test.tsxtesting/panel/README.mdtesting/panel/app.config.tstesting/panel/package.jsontesting/panel/src/components/Header.tsxtesting/panel/src/lib/recording.tstesting/panel/src/routes/api.image.tstesting/panel/src/routes/api.simulator-chat.tstesting/panel/src/routes/api.structured.tstesting/panel/src/routes/api.summarize.tstesting/panel/src/routes/api.transcription.tstesting/panel/src/routes/api.video.tstesting/panel/src/routes/stream-debugger.tsxtesting/panel/tests/basic-inference.spec.tstesting/panel/tests/helpers.tstesting/react-native-smoke/package.json
💤 Files with no reviewable changes (2)
- testing/panel/app.config.ts
- examples/ts-react-chat/src/routes/generations.structured-output.tsx
tombeckenham
force-pushed
the
820-ci-type-check-is-excluded-from-examples-and-testing-call-site-type-regressions-slip-through-testpr
branch
from
8c173ca to
f0bde42
Compare
…ons (#820) `test:pr`/`test:ci` excluded examples/** and testing/** from every target, so `test:types` never checked the apps where the library is actually consumed — call-site type regressions (e.g. a provider summarize adapter not assignable to `summarize()`) slipped through CI. Because those surfaces were never type-checked, they had also accumulated ~80 latent type errors. CI wiring: - test:pr / test:ci now run a second pass — `test:types` over examples/** and testing/** — after the existing packages-only run. Heavy targets (build/lib/...) stay excluded; only the cheap, high-value type check is added. - Add `test:types:examples` convenience script and document the gate in CLAUDE.md. Coverage: - Add a `test:types` target to every example/testing project that lacked one (e2e, panel, react-native-chat, react-native-smoke). ts-angular-chat type-checks templates via `ngc`, resolved from an existing @angular/build peer (no new dep, no lockfile change). Fixes (examples/testing drift from the current API; nothing under packages/**): - ts-react-chat, ts-solid-chat, ts-code-mode-web: MediaPrompt, maxTokens→modelOptions, ContentPart[] narrowing, transport XOR, onResult result-type inference (#848). - testing/e2e: drop poisoning `as never` model casts, fix adapter/tool typings, OpenRouter summarize httpClient header injection. - testing/panel: AnyAdapter factory maps, AG-UI EventType migration, remove dead app.config.ts (Vinxi-era), rewrite createEventRecording to emit AG-UI StreamChunks. Guard: - packages/ai-grok/tests/summarize-callsite-type-safety.test.ts asserts the grokSummarize → summarize() contract in an included package — a positive assertion now that the options-shape fix (#854) has landed, so a regression surfaces as a real type error instead of silently. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
tombeckenham
force-pushed
the
820-ci-type-check-is-excluded-from-examples-and-testing-call-site-type-regressions-slip-through-testpr
branch
from
f0bde42 to
f7780e5
Compare
AlemTuzlak
reviewed
Jul 3, 2026
Per review feedback, drop the --exclude=examples/**,testing/** carve-out and the separate test:types:examples pass; include the example/testing projects directly in the main affected/run-many invocation. Nx runs whatever of the target set each project defines (in practice build + test:types), so call-site type regressions in examples/testing are caught without a bolted-on command. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #820.
test:pr/test:ciexcludedexamples/**andtesting/**from every target, sotest:typesnever type-checked the apps where the library is actually consumed. Call-site type regressions — like a provider summarize adapter not being assignable tosummarize()'sadapterparam — slipped through CI. Because those surfaces were never type-checked, they had also accumulated ~80 latent type errors, all fixed here.CI wiring
test:pr/test:cinow run a second pass —nx … --targets=test:types --projects=examples/**,testing/**— after the existing packages-only run. Heavy targets (build/test:lib/etc.) stay excluded for these surfaces; only the cheap, high-value type check is added (the gap ci: type-check is excluded from examples/** and testing/** — call-site type regressions slip through test:pr #820 is about).test:types:examplesconvenience script and updated the documented canonical command inCLAUDE.md.pr.ymlalready runspnpm test:pr.Coverage
test:typestarget to every example/testing project that lacked one (testing/e2e,testing/panel,ts-react-native-chat,react-native-smoke).ts-angular-chattype-checks templates viangc, resolved from the existing@angular/buildpeer — no new dependency, no lockfile change (anode scripts/typecheck.mjswrapper).Fixes (examples/testing drift from the current API — nothing under
packages/**)MediaPrompt,maxTokens→modelOptions,ContentPart[]narrowing, the chat transport XOR.as nevermodel casts, fixed adapter/tool typings, OpenRouter summarize header injection viahttpClient.AnyAdapterfactory maps, AG-UIEventTypemigration, removed deadapp.config.ts(Vinxi-era config; the app runs onvite.config.ts), and rewrotecreateEventRecordingto reconstruct AG-UIStreamChunks (with START/END boundaries) instead of the obsolete flat chunk shape.In-package guard (acceptance criterion 2)
packages/ai-grok/tests/summarize-callsite-type-safety.test.tsasserts thegrokSummarize → summarize()contract in an included package, so the per-provider adapter→activity contract is exercised by CI without depending on the (excluded) example apps. It uses@ts-expect-errorto track the known Grok options-shape bug (#821); delete the directives to flip it to a positive regression guard once #835 lands.Test plan
pnpm test:pr— green end-to-end (RN smoke + affected package gate for 36 projects +test:typesfor all 14 example/testing projects).test:types/ngc.Notes / follow-ups (out of scope, pre-existing)
pnpm-lock.yamlpins the now-yankedrolldown@1.1.2(viats-react-search→nitro). It breaks any install that re-resolves the workspace; frozen CI install still works. Worth a separate fix (bumpnitroor apnpm.overridespin).gpt-5.5/gpt-5.5-proare missing fromOpenAIChatModelToolCapabilitiesByName, so provider tools can't be typed with those models.createEventRecordingtype-checks and emits a structurally-correct AG-UI sequence, but hasn't been replayed through the stream-debugger live.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
test:pr/test:cicoverage and refreshed recording instructions.