fix(agents,prompts): upgrade oxlint/oxfmt and resolve all lint errors#78
Open
zrosenbauer wants to merge 5 commits intomainfrom
Open
fix(agents,prompts): upgrade oxlint/oxfmt and resolve all lint errors#78zrosenbauer wants to merge 5 commits intomainfrom
zrosenbauer wants to merge 5 commits intomainfrom
Conversation
- Lowercase `describe` title for StepFinishEvent to satisfy prefer-lowercase-title - Append `()` to function-named describe titles to avoid prefer-describe-function-title Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: f994b47 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughMany test files had Vitest mocks tightened with explicit TypeScript generics; three type-test suite labels were renamed; Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
- Upgrade oxlint 1.57.0 → 1.59.0, oxfmt 0.42.0 → 0.44.0 - Add type parameters to all vi.fn() calls (require-mock-type-parameters) - Replace dirname(fileURLToPath()) with import.meta.dirname (prefer-import-meta-properties) - Combine toHaveBeenCalledOnce + toHaveBeenCalledWith into toHaveBeenCalledExactlyOnceWith - Expand single-line JSDoc to multiline (multiline-blocks) - Fix describe title casing and function name matching Co-Authored-By: Claude <noreply@anthropic.com>
zrosenbauer
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agents/src/core/agents/base/agent.test.ts`:
- Around line 11-14: Replace the explicit any types and lint suppressions on the
top-level mocks: change the vi.fn generic signatures for mockGenerateText and
mockStreamText from vi.fn<(...args: any[]) => any>() to vi.fn<(...args:
unknown[]) => unknown>() and remove the corresponding eslint-disable comments;
this matches the pattern used in evolve.test.ts and preserves compatibility with
mockResolvedValue, mockImplementation and mockReturnValue.
In `@packages/agents/src/core/agents/flow/steps/all.test.ts`:
- Line 202: The mocks onError and onDone declared as vi.fn<() => void>() do not
match the callbacks' actual signatures (they receive event objects); update
those mocks in all.test.ts to include the event parameter in the generic, e.g.
change vi.fn<() => void>() for onError to vi.fn<(event:
<appropriateErrorEventType>) => void>() and for onDone to vi.fn<(event:
<appropriateDoneEventType>) => void>(), importing the correct event types from
the module that defines the flow event interfaces so the test enforces the real
callback contracts.
In `@packages/agents/src/core/agents/flow/steps/map.test.ts`:
- Line 190: The mocks onError and onFinish are typed as vi.fn<() => void>() but
the tests call them with event objects; update their typings so the mock
signatures match the hook contracts: change onError to vi.fn<(event: { id:
string; error: Error }) => void>() and onFinish to vi.fn<(event: { id: string;
result: unknown[]; duration: number }) => void>() (or use the concrete R type if
available) so TypeScript will enforce the correct payload shape in the tests
referencing onError and onFinish.
In `@packages/agents/src/core/agents/flow/steps/race.test.ts`:
- Line 170: The mocks for onError and onFinish are currently typed as vi.fn<()
=> void>() which loses the event payload typings; update the mocks to use the
actual event types used by the race step tests (for example, change onError to
vi.fn<(e: RaceStepErrorEvent) => void>() and onFinish to vi.fn<(e:
RaceStepFinishEvent) => void>()), importing those event type symbols from the
module that defines the race step hooks so assertions in the test are
type-checked against the real event shapes.
In `@packages/agents/src/core/agents/flow/steps/step.test.ts`:
- Line 169: The tests currently declare mock callbacks with no-arg signatures
(e.g., parentFinish, onFinish, onError) which strips the event payload types;
update each vi.fn declaration to accept the correct payload shape: type
parentFinish as (evt: StepFinishEvent) => void where StepFinishEvent includes
stepId, output, duration, etc.; type onFinish as (payload: { id: string; result:
unknown; duration: number }) => void; and type onError as (payload: { id:
string; error: Error }) => void; replace the nullary vi.fn<() => void>() calls
with vi.fn<typeof callbackSignature>() or vi.fn<(payload: PayloadType) =>
void>() so the compiler preserves the payload contracts used by the assertions.
In `@packages/agents/src/core/agents/flow/steps/while.test.ts`:
- Line 45: Mocks use nullary signatures but must match the actual typed
callbacks; update the Vitest mock generics and their implementations for
executeSpy, onError, and onFinish to the correct parameterized signatures: set
executeSpy as vi.fn< (params: { index: number; $: StepBuilder }) =>
Promise<string> >(async ({ index, $ }) => "value"), set onError as vi.fn<
(event: { id: string; error: Error }) => void | Promise<void> >(event =>
{/*...*/}) and set onFinish as vi.fn< (event: { id: string; result: string |
undefined; duration: number }) => void >(event => {/*...*/}); ensure each mock
implementation accepts and (if used) references the incoming parameter names so
the test types line up with while.execute, onError, and onFinish signatures.
In `@packages/agents/src/lib/middleware.test.ts`:
- Around line 13-14: The mocks doGenerate and doStream are typed as () =>
Promise<never>, which strips required parameters and true return types; update
these mocks to use the actual middleware/model hook signatures instead (import
and use the real hook types used in production tests—e.g., the Generate/Stream
hook types or the exported types for the middleware functions) and apply them to
vi.fn (for example vi.fn<GenerateHook>() / vi.fn<StreamHook>() or vi.fn<(args:
Parameters<GenerateHook>[0]) => ReturnType<GenerateHook>>), and make the same
change for the other occurrences at lines noted so the mocks preserve parameters
and return contracts for doGenerate and doStream.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c9091344-d344-4366-be3c-0cfc15d66d68
📒 Files selected for processing (20)
packages/agents/src/core/agents/base/agent.test.tspackages/agents/src/core/agents/base/utils.test.tspackages/agents/src/core/agents/evolve.test.tspackages/agents/src/core/agents/flow/engine.test.tspackages/agents/src/core/agents/flow/flow-agent.test.tspackages/agents/src/core/agents/flow/steps/agent.test.tspackages/agents/src/core/agents/flow/steps/all.test.tspackages/agents/src/core/agents/flow/steps/each.test.tspackages/agents/src/core/agents/flow/steps/factory.test.tspackages/agents/src/core/agents/flow/steps/map.test.tspackages/agents/src/core/agents/flow/steps/race.test.tspackages/agents/src/core/agents/flow/steps/reduce.test.tspackages/agents/src/core/agents/flow/steps/step.test.tspackages/agents/src/core/agents/flow/steps/while.test.tspackages/agents/src/core/types.tspackages/agents/src/integration/lifecycle.test.tspackages/agents/src/lib/hooks.test.tspackages/agents/src/lib/middleware.test.tspackages/agents/src/testing/logger.tspackages/prompts/src/partials-dir.ts
- Use event parameter types on onError/onFinish/parentFinish mocks - Use NonNullable<LanguageModelMiddleware["wrapGenerate"]> for middleware mocks - Keep any-typed top-level mocks with oxlint-disable comments where unknown breaks mockImplementation Co-Authored-By: Claude <noreply@anthropic.com>
zrosenbauer
commented
Apr 10, 2026
zrosenbauer
commented
Apr 10, 2026
There was a problem hiding this comment.
♻️ Duplicate comments (3)
packages/agents/src/lib/middleware.test.ts (1)
54-54:⚠️ Potential issue | 🟠 MajorFix OXFmt violations blocking CI
Line 54 and Lines 70-71 are the formatting offenders causing
oxfmt --checkto fail in this file.Proposed fix
- const middleware = createStubMiddleware({ wrapGenerate: vi.fn<NonNullable<LanguageModelMiddleware["wrapGenerate"]>>() }); + const middleware = createStubMiddleware({ + wrapGenerate: vi.fn<NonNullable<LanguageModelMiddleware["wrapGenerate"]>>(), + }); @@ - const mw1 = createStubMiddleware({ wrapGenerate: vi.fn<NonNullable<LanguageModelMiddleware["wrapGenerate"]>>() }); - const mw2 = createStubMiddleware({ wrapGenerate: vi.fn<NonNullable<LanguageModelMiddleware["wrapGenerate"]>>() }); + const mw1 = createStubMiddleware({ + wrapGenerate: vi.fn<NonNullable<LanguageModelMiddleware["wrapGenerate"]>>(), + }); + const mw2 = createStubMiddleware({ + wrapGenerate: vi.fn<NonNullable<LanguageModelMiddleware["wrapGenerate"]>>(), + });As per coding guidelines: "Use OXFmt for code formatting across the project."
Also applies to: 70-71
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agents/src/lib/middleware.test.ts` at line 54, Replace the OXFmt-breaking generic vi.fn usage with a casted call: instead of vi.fn<NonNullable<LanguageModelMiddleware["wrapGenerate"]>>(), call vi.fn() as NonNullable<LanguageModelMiddleware["wrapGenerate"]> in the createStubMiddleware invocation (and the other occurrences on lines 70-71); this keeps the same typed value but uses formatting that satisfies OXFmt and preserves the reference to LanguageModelMiddleware["wrapGenerate"] and createStubMiddleware.packages/agents/src/core/agents/base/agent.test.ts (1)
11-14:⚠️ Potential issue | 🟠 MajorRemove explicit
anyfrom top-level mock signatures.
vi.fn<(...args: any[]) => any>()violates the repo’s no-anyrule and weakens type guarantees in the rest of the test file.Proposed fix
-// oxlint-disable-next-line `@typescript-eslint/no-explicit-any` -- top-level mock needs flexible signature for mockImplementation compatibility -const mockGenerateText = vi.fn<(...args: any[]) => any>(); -// oxlint-disable-next-line `@typescript-eslint/no-explicit-any` -- top-level mock needs flexible signature for mockImplementation compatibility -const mockStreamText = vi.fn<(...args: any[]) => any>(); +const mockGenerateText = vi.fn<(...args: unknown[]) => unknown>(); +const mockStreamText = vi.fn<(...args: unknown[]) => unknown>();#!/bin/bash rg -nP 'vi\.fn<\(\.\.\.args:\s*any\[\]\)\s*=>\s*any>\(\)' packages/agents/src/core/agents/base/agent.test.tsAs per coding guidelines,
**/*.{ts,tsx}: “Do not use theanytype — useunknownand narrow appropriately.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agents/src/core/agents/base/agent.test.ts` around lines 11 - 14, The top-level mocks mockGenerateText and mockStreamText use vi.fn with explicit any types; replace those generics with unknown (e.g., vi.fn<(...args: unknown[]) => unknown>()) to satisfy the no-any rule and then narrow types where the mock is used (provide typed mockImplementation or type assertions at call sites) so callers get correct argument/return types; update usages of mockGenerateText and mockStreamText in the test to cast or implement with the specific expected parameter and return shapes rather than relying on any.packages/agents/src/core/agents/flow/steps/step.test.ts (1)
169-169:⚠️ Potential issue | 🟡 MinorType
parentFinishasStepFinishEventinstead ofunknown.
Usingunknownhere drops the callback payload contract this PR is trying to enforce.Proposed fix
import { describe, expect, it, vi } from "vitest"; import { createStepBuilder } from "@/core/agents/flow/steps/factory.js"; +import type { StepFinishEvent } from "@/core/types.js"; import { createMockCtx } from "@/testing/index.js"; @@ - const parentFinish = vi.fn<(event: unknown) => void>(); + const parentFinish = vi.fn<(event: StepFinishEvent) => void>();#!/bin/bash rg -nP 'const parentFinish = vi\.fn<\(event:\s*unknown\)\s*=>\s*void>\(\);' packages/agents/src/core/agents/flow/steps/step.test.ts -C2 rg -nP 'onStepFinish\?:' packages/agents/src/core/agents/flow/steps/step.ts -C3 rg -nP 'export type StepFinishEvent' packages/agents/src/core/types.ts -C3As per coding guidelines,
**/*.ts: “Follow type-driven design … Make illegal states unrepresentable.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agents/src/core/agents/flow/steps/step.test.ts` at line 169, The test's mock callback is currently typed as vi.fn<(event: unknown) => void>() which loses the StepFinishEvent payload contract; update the mock to vi.fn<(event: StepFinishEvent) => void>() and (if missing) import the StepFinishEvent type from the module that exports it so the test enforces the correct payload shape for parentFinish (which corresponds to the onStepFinish? handler used by Step/step.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/agents/src/core/agents/base/agent.test.ts`:
- Around line 11-14: The top-level mocks mockGenerateText and mockStreamText use
vi.fn with explicit any types; replace those generics with unknown (e.g.,
vi.fn<(...args: unknown[]) => unknown>()) to satisfy the no-any rule and then
narrow types where the mock is used (provide typed mockImplementation or type
assertions at call sites) so callers get correct argument/return types; update
usages of mockGenerateText and mockStreamText in the test to cast or implement
with the specific expected parameter and return shapes rather than relying on
any.
In `@packages/agents/src/core/agents/flow/steps/step.test.ts`:
- Line 169: The test's mock callback is currently typed as vi.fn<(event:
unknown) => void>() which loses the StepFinishEvent payload contract; update the
mock to vi.fn<(event: StepFinishEvent) => void>() and (if missing) import the
StepFinishEvent type from the module that exports it so the test enforces the
correct payload shape for parentFinish (which corresponds to the onStepFinish?
handler used by Step/step.ts).
In `@packages/agents/src/lib/middleware.test.ts`:
- Line 54: Replace the OXFmt-breaking generic vi.fn usage with a casted call:
instead of vi.fn<NonNullable<LanguageModelMiddleware["wrapGenerate"]>>(), call
vi.fn() as NonNullable<LanguageModelMiddleware["wrapGenerate"]> in the
createStubMiddleware invocation (and the other occurrences on lines 70-71); this
keeps the same typed value but uses formatting that satisfies OXFmt and
preserves the reference to LanguageModelMiddleware["wrapGenerate"] and
createStubMiddleware.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 933ef4dc-ca31-4078-9a76-715c1d88f7b5
📒 Files selected for processing (7)
packages/agents/src/core/agents/base/agent.test.tspackages/agents/src/core/agents/flow/steps/all.test.tspackages/agents/src/core/agents/flow/steps/map.test.tspackages/agents/src/core/agents/flow/steps/race.test.tspackages/agents/src/core/agents/flow/steps/step.test.tspackages/agents/src/core/agents/flow/steps/while.test.tspackages/agents/src/lib/middleware.test.ts
- Pin oxlint ≥1.59.0, oxfmt ≥0.44.0 in package.json - Update lockfile so CI installs correct versions - Add empty changeset (no publishable package changes) Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <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
1 participant
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
1.57.0→1.59.0and oxfmt0.42.0→0.44.0require-mock-type-parameterserrors — added type params to everyvi.fn()callprefer-import-meta-properties— useimport.meta.dirnameinstead ofdirname(fileURLToPath(...))prefer-called-exactly-once-with— combine assertion pairsmultiline-blocks— expand single-line JSDocprefer-lowercase-titleandprefer-describe-function-title(pre-existing warnings)Changes
@funkai/agentsand@funkai/promptspackage.json+ lockfile updated for version bumpsTest plan
pnpm exec oxlint— 0 warnings, 0 errorspnpm typecheck— passespnpm test --filter=@funkai/agents— 665 tests passpnpm test --filter=@funkai/prompts— 35 tests passpnpm format:check— all files formatted