-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add tests that confirm that resuming a step preceded by map step works fine #11511
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
WalkthroughAdds three regression tests (one per test suite) that verify input propagation when resuming a suspended step that was preceded by a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
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 |
🚨 Redirect Validation FailedThe redirect validation found issues in Action Required: Review and fix the redirect configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
workflows/inngest/src/index.test.ts (1)
5672-5957: Suspend/resume after.mapflow is covered wellThe test wiring and expectations look solid: you correctly exercise the suspend at
promptAgent, the.mapstep (evaluateToneConsistency), and the subsequent suspend/resume atimproveResponse, and you assert both payloads and averaged scores all the way throughevaluateImprovedResponse. The storage/server setup is consistent with the surrounding suspend/resume tests, and the use of both string and step references inrun.resumematches existing patterns.Two minor, non‑blocking nits you might consider if you touch this again:
- The test name has a typo: “preceeded” → “preceded”.
- The property
overralScoreis spelled inconsistently with “overall”; since it’s only local to this test, you could rename it (fields and expectations) for clarity.- The test description mentions
inputValuebut the workflow input field isinput; aligning those names could reduce confusion.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/core/src/workflows/evented/evented-workflow.test.tspackages/core/src/workflows/workflow.test.tsworkflows/inngest/src/index.test.ts
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
pnpm typecheckto validate TypeScript types across all packages
Files:
packages/core/src/workflows/evented/evented-workflow.test.tsworkflows/inngest/src/index.test.tspackages/core/src/workflows/workflow.test.ts
**/*.{ts,tsx,js,jsx,json,md}
📄 CodeRabbit inference engine (CLAUDE.md)
Run
pnpm prettier:formatto format code andpnpm formatto run linting with auto-fix across all packages
Files:
packages/core/src/workflows/evented/evented-workflow.test.tsworkflows/inngest/src/index.test.tspackages/core/src/workflows/workflow.test.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
packages/**/*.{ts,tsx}: All packages must use TypeScript with strict type checking enabled
Use telemetry decorators for observability across components
Files:
packages/core/src/workflows/evented/evented-workflow.test.tspackages/core/src/workflows/workflow.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{test,spec}.{ts,tsx}: Use Vitest for testing framework in test files
Co-locate test files with source code using naming patterns like *.test.ts or *.spec.ts
Files:
packages/core/src/workflows/evented/evented-workflow.test.tsworkflows/inngest/src/index.test.tspackages/core/src/workflows/workflow.test.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: taofeeq-deru
Repo: mastra-ai/mastra PR: 11276
File: packages/core/src/workflows/evented/evented-workflow.test.ts:940-1055
Timestamp: 2025-12-18T21:52:52.667Z
Learning: In packages/core Workflows tests, Run.stream returns a synchronous object (with fullStream and result), so awaiting run.stream() is not required in tests.
Learnt from: taofeeq-deru
Repo: mastra-ai/mastra PR: 11486
File: packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts:643-678
Timestamp: 2025-12-30T15:02:58.132Z
Learning: In `packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts`, the system guarantees that `metadata.suspendedTools` and `metadata.pendingToolApprovals` never contain resumed tools, so no filtering is needed when using metadata directly.
Learnt from: CR
Repo: mastra-ai/mastra PR: 0
File: packages/codemod/AGENTS.md:0-0
Timestamp: 2025-11-24T16:42:04.244Z
Learning: Applies to packages/codemod/src/test/__fixtures__/**/*.ts : In output fixtures, ensure all NEGATIVE test cases remain EXACTLY IDENTICAL to their input fixture counterparts to verify the codemod only transforms intended patterns
📚 Learning: 2025-12-30T15:02:58.132Z
Learnt from: taofeeq-deru
Repo: mastra-ai/mastra PR: 11486
File: packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts:643-678
Timestamp: 2025-12-30T15:02:58.132Z
Learning: In `packages/core/src/loop/workflows/agentic-execution/llm-execution-step.ts`, the system guarantees that `metadata.suspendedTools` and `metadata.pendingToolApprovals` never contain resumed tools, so no filtering is needed when using metadata directly.
Applied to files:
packages/core/src/workflows/evented/evented-workflow.test.tsworkflows/inngest/src/index.test.tspackages/core/src/workflows/workflow.test.ts
📚 Learning: 2025-11-24T16:42:04.244Z
Learnt from: CR
Repo: mastra-ai/mastra PR: 0
File: packages/codemod/AGENTS.md:0-0
Timestamp: 2025-11-24T16:42:04.244Z
Learning: Applies to packages/codemod/src/test/__fixtures__/**/*.ts : In output fixtures, ensure all NEGATIVE test cases remain EXACTLY IDENTICAL to their input fixture counterparts to verify the codemod only transforms intended patterns
Applied to files:
packages/core/src/workflows/evented/evented-workflow.test.tsworkflows/inngest/src/index.test.tspackages/core/src/workflows/workflow.test.ts
📚 Learning: 2025-12-18T21:52:52.667Z
Learnt from: taofeeq-deru
Repo: mastra-ai/mastra PR: 11276
File: packages/core/src/workflows/evented/evented-workflow.test.ts:940-1055
Timestamp: 2025-12-18T21:52:52.667Z
Learning: In packages/core Workflows tests, Run.stream returns a synchronous object (with fullStream and result), so awaiting run.stream() is not required in tests.
Applied to files:
packages/core/src/workflows/evented/evented-workflow.test.tsworkflows/inngest/src/index.test.tspackages/core/src/workflows/workflow.test.ts
📚 Learning: 2025-11-24T16:42:04.244Z
Learnt from: CR
Repo: mastra-ai/mastra PR: 0
File: packages/codemod/AGENTS.md:0-0
Timestamp: 2025-11-24T16:42:04.244Z
Learning: Applies to packages/codemod/src/test/**/*.test.ts : Include test cases for multiple occurrences of the transformation pattern, aliased imports, type imports, and mixed imports to verify the codemod works consistently
Applied to files:
packages/core/src/workflows/workflow.test.ts
🧬 Code graph analysis (2)
packages/core/src/workflows/evented/evented-workflow.test.ts (2)
packages/core/src/workflows/evented/workflow.ts (2)
createStep(119-275)createWorkflow(277-308)packages/core/src/events/event-emitter.ts (1)
EventEmitterPubSub(5-41)
packages/core/src/workflows/workflow.test.ts (3)
packages/core/src/workflows/workflow.ts (1)
createStep(184-831)packages/core/src/workflows/evented/workflow.ts (1)
createStep(119-275)packages/core/src/mastra/index.ts (1)
Mastra(262-3111)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: test
- GitHub Check: test (hono)
- GitHub Check: test (express)
- GitHub Check: test
- GitHub Check: Prebuild
- GitHub Check: Lint
- GitHub Check: test
- GitHub Check: test
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
packages/core/src/workflows/workflow.test.ts (1)
12303-12550: Runpnpm typecheckto validate TypeScript types across all packages.This is a required step per coding guidelines for TypeScript files.
packages/core/src/workflows/evented/evented-workflow.test.ts (1)
6548-6815: Suspend/resume +.mapregression test looks correct and well‑wiredThe new test’s step graph, schemas, and expected
stepssnapshots are internally consistent and exercise exactly the “resume after a.mapstep” path you’re targeting (both forpromptAgentandimproveResponse). The computedmodelOutputand averagedoverralScorevalues line up with the step implementations, so this should give solid regression coverage for the bug.Only very minor nits, purely optional:
- Test name string: “preceeded” → “preceded”.
- Field name
overralScoreis misspelled but consistent across the step and assertions; change only if you want to normalize naming.
…s fine (mastra-ai#11511) ## Description <!-- Provide a brief description of the changes in this PR --> ## Related Issue(s) <!-- Link to the issue(s) this PR addresses, using hashtag notation: mastra-ai#123 --> mastra-ai#11505 ## Type of Change - [ ] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Code refactoring - [ ] Performance improvement - [x] Test update ## Checklist - [ ] I have made corresponding changes to the documentation (if applicable) - [ ] I have added tests that prove my fix is effective or that my feature works <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Tests** * Added coverage across multiple test suites validating that resuming a step after a preceding map operation correctly propagates and merges input values through the resume chain; includes duplicated test cases to ensure regression protection for map-assisted resume flows. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Related Issue(s)
#11505
Type of Change
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.