Merged
Conversation
|
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:
📝 WalkthroughWalkthroughAdds inline execution for queries and mutations via Changes
Sequence Diagram(s)sequenceDiagram
participant WF as WorkflowHandler
participant Step as StepCoordinator
participant Func as FunctionHandler
participant Pool as Workpool
WF->>Step: step.runQuery(..., inline: true)
alt inline path
Step->>Func: invoke query/mutation handler (inline)
Func-->>Step: return result / throw error
Step-->>WF: provide runResult (completed)
else workpool path
WF->>Pool: enqueue step (query/mutation/action)
Pool->>Func: execute handler asynchronously
Func-->>Pool: result / error
Pool-->>WF: deliver completion notification
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
commit: |
ianmacartney
commented
Mar 10, 2026
ianmacartney
force-pushed
the
ian/inline-query-mutation
branch
from
57b0d94 to
ffa4bec
Compare
Member
Author
|
@BugBot run |
|
Skipping Bugbot: Bugbot is disabled for this repository. Visit the Bugbot dashboard to update your settings. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
example/convex/inlineTest.ts (2)
59-65: Misleading function name:slowActionis not actually slow.The function is named
slowActionand the comment at line 175 says "action goes through workpool", but the implementation returns immediately without any delay. This could confuse future maintainers trying to understand the test behavior.Consider either:
- Adding an actual delay (e.g.,
await new Promise(r => setTimeout(r, 100)))- Renaming to something more accurate like
testActionorsimpleAction🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example/convex/inlineTest.ts` around lines 59 - 65, The function slowAction defined via internalAction (handler async (_ctx, { label }) => ...) is misleading because it returns immediately; either add an explicit small delay inside the handler (e.g., await a short timeout before returning) to match the "slow" name or rename the exported symbol slowAction to a truthful name like testAction or simpleAction and update any references; locate the export slowAction and modify the handler to include the delay or perform a safe rename across the codebase where slowAction is used.
49-53: Inserting empty string asworkflowIdbypasses type safety and may cause issues.The
workflowId: "" as anycoercion inserts an invalid workflow ID into theflowstable. While this works for test purposes, it:
- Bypasses TypeScript's type safety with
as any- Could cause issues if code elsewhere queries by
workflowIdindex expecting valid IDs- May cause confusion when debugging test data
Consider passing a valid workflow ID through the args if one is available, or using a dedicated test table without the
workflowIdrequirement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example/convex/inlineTest.ts` around lines 49 - 53, The insert into "flows" uses a type-bypassing workaround (workflowId: "" as any) which should be replaced with a real test value; modify the code around ctx.db.insert in inlineTest.ts so workflowId is provided from test args or generated deterministically (e.g., args.workflowId or a local testId/UUID) and pass that value into ctx.db.insert("flows", { in: key, workflowId: <validId>, out: newVal }); alternatively switch to a dedicated test table that doesn't require workflowId if no valid id is available.example/convex/inlineTest.test.ts (2)
10-16: Duplicatevi.useFakeTimers()calls.
vi.useFakeTimers()is called at module level (line 10) and again inbeforeEach(line 15). The module-level call is redundant sincebeforeEachalready sets up fake timers before each test, andafterEachrestores real timers.Consider removing the module-level call to avoid confusion:
Suggested fix
-vi.useFakeTimers(); - // TODO: When we have tests running without messing with globals, enable these describe.skip("inline queries and mutations", () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example/convex/inlineTest.test.ts` around lines 10 - 16, Remove the redundant module-level call to vi.useFakeTimers() and rely on the existing beforeEach setup inside the describe.skip("inline queries and mutations") block; specifically delete the top-level vi.useFakeTimers() so the test suite only calls vi.useFakeTimers() in beforeEach (with afterEach restoring real timers) to avoid global/test setup confusion.
6-6: Remove empty import statement.The import
import {} from "@convex-dev/workflow";imports nothing and appears to be dead code.Suggested fix
-import {} from "@convex-dev/workflow";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@example/convex/inlineTest.test.ts` at line 6, Remove the dead empty import statement `import {} from "@convex-dev/workflow";` in the test file; locate the import line in inlineTest.test.ts (the `import {}` declaration) and delete it so there are no unused/empty imports left in the module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 332-334: Fix the grammar in the README sentence referencing
step.runQuery() and step.runMutation(): change "independently transaction" to
"independent transaction" so the sentence reads that calls are dispatched
through the workpool which run the function in an independent transaction; keep
the mention of the inline opt-in ({ inline: true }) unchanged.
In `@src/component/journal.ts`:
- Around line 157-165: The inline-completed branch currently logs stepCompleted
before the unconditional started event later, causing reversed ordering; fix by
emitting the started event before completed when step.runResult is present (call
console.event("stepStarted", { workflowId: entry.workflowId, workflowName:
workflow.name, stepName: step.name, stepNumber }) immediately before the
existing console.event("stepCompleted", ...)), and also guard the later
unconditional console.event("stepStarted", ...) so it only runs when
step.runResult is falsy (e.g., wrap it in if (!step.runResult) {...}) to avoid
duplicate events.
---
Nitpick comments:
In `@example/convex/inlineTest.test.ts`:
- Around line 10-16: Remove the redundant module-level call to
vi.useFakeTimers() and rely on the existing beforeEach setup inside the
describe.skip("inline queries and mutations") block; specifically delete the
top-level vi.useFakeTimers() so the test suite only calls vi.useFakeTimers() in
beforeEach (with afterEach restoring real timers) to avoid global/test setup
confusion.
- Line 6: Remove the dead empty import statement `import {} from
"@convex-dev/workflow";` in the test file; locate the import line in
inlineTest.test.ts (the `import {}` declaration) and delete it so there are no
unused/empty imports left in the module.
In `@example/convex/inlineTest.ts`:
- Around line 59-65: The function slowAction defined via internalAction (handler
async (_ctx, { label }) => ...) is misleading because it returns immediately;
either add an explicit small delay inside the handler (e.g., await a short
timeout before returning) to match the "slow" name or rename the exported symbol
slowAction to a truthful name like testAction or simpleAction and update any
references; locate the export slowAction and modify the handler to include the
delay or perform a safe rename across the codebase where slowAction is used.
- Around line 49-53: The insert into "flows" uses a type-bypassing workaround
(workflowId: "" as any) which should be replaced with a real test value; modify
the code around ctx.db.insert in inlineTest.ts so workflowId is provided from
test args or generated deterministically (e.g., args.workflowId or a local
testId/UUID) and pass that value into ctx.db.insert("flows", { in: key,
workflowId: <validId>, out: newVal }); alternatively switch to a dedicated test
table that doesn't require workflowId if no valid id is available.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c39ac27d-d6e8-41e0-9ce2-66bd30d7e113
⛔ Files ignored due to path filters (2)
example/convex/_generated/api.d.tsis excluded by!**/_generated/**package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
README.mdexample/convex/inlineTest.test.tsexample/convex/inlineTest.tsexample/convex/nestedWorkflow.tsexample/convex/passingSignals.tsexample/convex/userConfirmation.tspackage.jsonsrc/client/step.tssrc/client/workflowContext.tssrc/component/journal.tssrc/component/workflow.tssrc/types.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
20-20: Guardconvex initwith an initialization check.Line 20 unconditionally runs
convex initbefore everypredev. Convex's documentation recommends checking for initialization state (specifically.env.localwithCONVEX_DEPLOYMENTset and theconvex/directory) before running initialization commands. Consider gatingconvex initsimilar to hownpm run buildis guarded—for example,(path-exists .env.local && path-exists convex) || convex init, to avoid redundant initialization on subsequent runs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 20, The predev npm script currently runs "convex init" unconditionally; change it to first check for initialization artifacts and only call convex init if missing. Modify the "predev" script (the npm script named predev) to guard the convex init call by checking for .env.local containing CONVEX_DEPLOYMENT and the convex/ directory (e.g., use path-exists checks similar to the existing build guard) so that convex init only runs when those initialization indicators are absent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@package.json`:
- Line 20: The predev npm script currently runs "convex init" unconditionally;
change it to first check for initialization artifacts and only call convex init
if missing. Modify the "predev" script (the npm script named predev) to guard
the convex init call by checking for .env.local containing CONVEX_DEPLOYMENT and
the convex/ directory (e.g., use path-exists checks similar to the existing
build guard) so that convex init only runs when those initialization indicators
are absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b4d956eb-c5c5-45d1-991b-5d8cf6ef8ce4
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
CHANGELOG.mdpackage.json
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 26: The phrase "to recover failed workflows after third party outages or
fixing code bugs." should be rewritten for correct parallelism and hyphenation;
change it to something like "to recover failed workflows after third-party
outages or to fix code bugs." Apply the same cleanup to the identical/related
occurrences (the similar sentences around the other mentioned spots) so they use
"third-party" and consistent parallel construction ("after third-party outages
or to fix...") and correct punctuation.
- Line 197: The example workflow invocation uses the wrong argument key: replace
the object literal containing exampleArg (e.g., "{ exampleArg: \"James\" }")
with the correct key "name" to match the workflow's args definition (args: {
name: v.string() }) so the call becomes "{ name: \"James\" }"; update any
related examples or labels referencing exampleArg to use name for consistency.
- Around line 64-69: The example calling step.runMutation is invalid because it
places the options object ({ inline: true }) inside the args object; update the
call to pass the mutation identifier (internal.emails.sendWelcomeEmail), then
the args object ({ userId, content }) as the second argument, and the options
object ({ inline: true }) as the third argument to step.runMutation so the
syntax matches other examples and is valid JavaScript/TypeScript.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9fbd620-df91-40c5-b3e7-a334cb286ef9
📒 Files selected for processing (4)
README.mdexample/convex/inlineTest.test.tsexample/convex/inlineTest.tssrc/component/journal.ts
✅ Files skipped from review due to trivial changes (1)
- example/convex/inlineTest.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/component/journal.ts
- example/convex/inlineTest.ts
Merged
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
436-436:⚠️ Potential issue | 🟡 MinorTypo: "immmediately" has three m's.
📝 Proposed fix
-execution immmediately. In-progress calls to `step.runAction()`, however, will +execution immediately. In-progress calls to `step.runAction()`, however, will🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 436, Fix the typo in the README where the phrase "execution immmediately." appears (near the mention of `step.runAction()`); change "immmediately" to "immediately" so the sentence reads "execution immediately."
♻️ Duplicate comments (1)
README.md (1)
757-757:⚠️ Potential issue | 🟡 MinorDuplicate word: "the the".
📝 Proposed fix
- functions and passing IDs around within the the workflow. + functions and passing IDs around within the workflow.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 757, Replace the duplicated word in the README sentence "functions and passing IDs around within the the workflow." by removing the extra "the" so it reads "functions and passing IDs around within the workflow." Locate that exact phrase in the README and update it accordingly.
🧹 Nitpick comments (1)
README.md (1)
754-754: Consider consistent size unit terminology.The documentation uses both "1 MB" (line 754) and "8MiB" (line 761). For technical precision, consider using MiB consistently, as these are likely binary units (1024-based). However, if "1 MB" reflects the actual limit as defined in the codebase, this may be intentional.
Also applies to: 761-761
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 754, Replace the inconsistent size unit wording by standardizing on MiB (or on MB if code enforces decimal MB) across the README: update the string "1 MB" to "1 MiB" (or change "8MiB" to "8 MB" if you confirm the code uses decimal megabytes), and ensure any other occurrences of "MB" vs "MiB" are made consistent (search for the literals "1 MB" and "8MiB" and update to the chosen unit), optionally adding a short parenthetical note clarifying that MiB = 1024^2 bytes if you choose MiB for precision.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 26: Update the README text to hyphenate the compound adjective
"third-party" in the sentence containing "to recover failed workflows after
third party outages or fixing code bugs" and add the missing verb "is" in the
sentence that should read "the overall workflow is guaranteed to run to
completion" so both phrases read correctly.
---
Outside diff comments:
In `@README.md`:
- Line 436: Fix the typo in the README where the phrase "execution
immmediately." appears (near the mention of `step.runAction()`); change
"immmediately" to "immediately" so the sentence reads "execution immediately."
---
Duplicate comments:
In `@README.md`:
- Line 757: Replace the duplicated word in the README sentence "functions and
passing IDs around within the the workflow." by removing the extra "the" so it
reads "functions and passing IDs around within the workflow." Locate that exact
phrase in the README and update it accordingly.
---
Nitpick comments:
In `@README.md`:
- Line 754: Replace the inconsistent size unit wording by standardizing on MiB
(or on MB if code enforces decimal MB) across the README: update the string "1
MB" to "1 MiB" (or change "8MiB" to "8 MB" if you confirm the code uses decimal
megabytes), and ensure any other occurrences of "MB" vs "MiB" are made
consistent (search for the literals "1 MB" and "8MiB" and update to the chosen
unit), optionally adding a short parenthetical note clarifying that MiB = 1024^2
bytes if you choose MiB for precision.
reeceyang
approved these changes
Mar 20, 2026
| ): Promise<unknown> { | ||
| const { name, retry, ...schedulerOptions } = opts ?? {}; | ||
| const { name, retry, inline, ...schedulerOptions } = opts ?? {}; | ||
| if (inline && ("runAt" in schedulerOptions || "runAfter" in schedulerOptions)) { |
There was a problem hiding this comment.
should we also console.warn if inline is true but the function is an action?
Member
Author
There was a problem hiding this comment.
I'm going to add a check in step.ts where it will throw if it's not an action, instead of doing the current behavior of ignoring "inline" if it's not a query/mutation
example/convex/inlineTest.ts
Outdated
| }); | ||
|
|
||
| // ── Test 6: Mixed inline + action ───────────── | ||
| // The query runs inline (shareTransaction), while the action goes through |
There was a problem hiding this comment.
Suggested change
| // The query runs inline (shareTransaction), while the action goes through | |
| // The query runs inline, while the action goes through |
README.md
Outdated
Comment on lines
90
to
91
| queries, mutations, and actions into long-lived workflows, and the system will | ||
| always fully execute a workflow to completion. |
There was a problem hiding this comment.
This was referenced Mar 21, 2026
This was referenced Apr 4, 2026
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.
TL;DR
Added inline execution support for workflow queries and mutations, allowing them to run within the same transaction as the workflow instead of being dispatched through the work pool.
What changed?
Added shareTransaction option to workflow definitions that enables inline execution by default for queries and mutationsinlineoption to override the default behavior for individual function callsstartStepsthat runs queries and mutations directly within the workflow transactionworkIdoptional for inline stepsHow to test?
Run the functions in
example/convex/inlineTest.ts(see inlineTest.test.ts for example on how to test it - but unfortunately the vitest tests are skipped until we can test workflows without blowing up globals) which covers:Why make this change?
This change eliminates the round-trip overhead of dispatching queries and mutations through the work pool when they can safely execute within the workflow's transaction. This improves performance for workflows that primarily use queries and mutations, while maintaining the existing behavior for actions and providing flexibility through per-call overrides.