Migrate Vision and AI-room-renovate for new motia version#140
Open
guibeira wants to merge 11 commits into
Open
Migrate Vision and AI-room-renovate for new motia version#140guibeira wants to merge 11 commits into
guibeira wants to merge 11 commits into
Conversation
guibeira
changed the title
📝 WalkthroughWalkthroughThis pull request upgrades two advanced examples (ai-room-renovate and vision-example) from beta Motia framework versions to release candidates (1.0.0-rc.14/rc.22). Changes include new YAML runtime configurations, Python dependency management via pyproject.toml, migrating all steps from event-based (subscribes/emits) to trigger-based (queue/api triggers with enqueues) architecture, updating handler signatures to the new Handlers pattern, and removing deprecated UI component overrides and old configuration files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
examples/advanced-use-cases/ai-room-renovate/steps/renovation/visual_assessor.step.ts (2)
99-102:⚠️ Potential issue | 🔴 CriticalBug:
'luxury'scope is unreachable — condition order is wrong.
avgCost > 600is a strict subset ofavgCost > 300, so theelse ifon Line 102 can never execute. Every value above 600 is already captured by the> 300branch on Line 101.Reverse the checks so the highest threshold is evaluated first:
🐛 Proposed fix
let scope = 'moderate'; - if (avgCost < 100) scope = 'cosmetic'; - else if (avgCost > 300) scope = 'full'; - else if (avgCost > 600) scope = 'luxury'; + if (avgCost > 600) scope = 'luxury'; + else if (avgCost > 300) scope = 'full'; + else if (avgCost < 100) scope = 'cosmetic';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/visual_assessor.step.ts` around lines 99 - 102, The conditional ordering in the block that sets scope based on avgCost makes the 'luxury' branch unreachable; reorder the comparisons in the conditional that assigns scope (the code using variable scope and avgCost) so the largest threshold is checked first (e.g., if (avgCost > 600) scope = 'luxury'; else if (avgCost > 300) scope = 'full'; else if (avgCost < 100) scope = 'cosmetic'; else scope = 'moderate';) to ensure each branch can be reached.
97-98:⚠️ Potential issue | 🟡 Minor
budgetmay not be a number — consider adding a numeric guard.
storedBudgetis typedany(from state) androomDetails.budgetcomes from string parsing. If either yields a string like"5000", the division on Line 98 still works in JS due to implicit coercion, but a non-numeric string (e.g."$5,000") would produceNaN, silently breaking the cost estimate.A simple
Number(assessment.budget)coercion (with aNaNcheck) would make this more robust:🛡️ Suggested guard
if (assessment.budget) { - const avgCost = assessment.budget / assessment.squareFootage; + const budgetNum = Number(assessment.budget); + if (isNaN(budgetNum)) return; + const avgCost = budgetNum / assessment.squareFootage;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/visual_assessor.step.ts` around lines 97 - 98, The calculation using assessment.budget can produce NaN if budget is non-numeric; in the visual_assessor.step.ts code locate the block that computes avgCost from assessment.budget and assessment.squareFootage and coerce assessment.budget to a numeric value (e.g., via Number(...) or a sanitized parse) then check isFinite/!isNaN before performing the division, and if the value is invalid handle it (skip avgCost, use a fallback, or log/throw) so avgCost = assessment.budget / assessment.squareFootage cannot silently become NaN.examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit_rendering_api.step.ts (1)
71-77:⚠️ Potential issue | 🟡 MinorOver-broad
catchconflates validation errors with infrastructure failures.The try block covers
state.get()andenqueue()in addition tobodySchema.parse(). Any infrastructure error (state storage unavailable, queue timeout) will surface to the caller as400 "Invalid request data", which is semantically incorrect and prevents clients from distinguishing retriable failures from bad requests. The server-side log at Line 72 preserves observability, but the HTTP contract is broken for non-validation errors.Per Motia docs,
bodySchemais not validated automatically — manual.parse()is necessary — so the ZodError path is intentional, but it should be isolated.🐛 Proposed fix — narrow the catch to ZodErrors
- } catch (error) { - logger.error('Failed to process edit request', { error: String(error) }); - return { - status: 400, - body: { error: 'Invalid request data' }, - }; - } + } catch (error) { + if (error instanceof Error && error.name === 'ZodError') { + logger.error('Invalid request body', { error: String(error) }); + return { + status: 400, + body: { error: 'Invalid request data' }, + }; + } + logger.error('Failed to process edit request', { error: String(error) }); + throw error; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit_rendering_api.step.ts` around lines 71 - 77, The current catch around bodySchema.parse(), state.get(), and enqueue() treats all errors as validation failures; isolate validation by calling bodySchema.parse() inside its own try/catch that only catches ZodError (imported from "zod") and returns the 400 + "Invalid request data" response, while letting other errors from state.get() or enqueue() bubble up or be handled as server errors (log with logger.error in the outer catch and return 500). Update the handler so bodySchema.parse() is validated first (or in a small try that catches ZodError), and ensure logger.error calls (the existing logger.error('Failed to process edit request', { error: String(error) })) remain for non-validation failures.examples/advanced-use-cases/ai-room-renovate/steps/renovation/design_planner.step.ts (1)
38-44:⚠️ Potential issue | 🔴 CriticalBug:
'luxury'scope is unreachable.The
avgCost > 600check on Line 43 will never be reached becauseavgCost > 300on Line 42 is evaluated first and catches all values above 300, including those above 600.🐛 Proposed fix — reorder conditions from highest to lowest
let scope = 'moderate'; if (costEstimate) { const avgCost = costEstimate.totalLow / assessment.squareFootage; if (avgCost < 100) scope = 'cosmetic'; - else if (avgCost > 300) scope = 'full'; - else if (avgCost > 600) scope = 'luxury'; + else if (avgCost > 600) scope = 'luxury'; + else if (avgCost > 300) scope = 'full'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/design_planner.step.ts` around lines 38 - 44, The scope assignment logic makes 'luxury' unreachable because the avgCost > 300 branch runs before avgCost > 600; update the conditional order in the block that computes avgCost (which uses costEstimate and assessment.squareFootage) to check the highest threshold first (avgCost > 600), then avgCost > 300, then avgCost < 100, or refactor to explicit range checks so 'scope' can be set to 'luxury', 'full', 'moderate' or 'cosmetic' correctly.examples/advanced-use-cases/vision-example/steps/enhance_image_prompt_step.py (1)
32-33:⚠️ Potential issue | 🔴 Critical
claude-3-sonnet-20240229is retired — API calls to this model will fail at runtime.Anthropic notified developers on January 21, 2025, that Claude Sonnet 3 models would be retired, and they were retired on November 6, 2024. Retired models are no longer available for use, and requests to them will fail. The companion step
evaluate_result_step.pyalready usesclaude-sonnet-4-20250514.🐛 Proposed fix
- model="claude-3-sonnet-20240229", + model="claude-sonnet-4-5-20251001",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/vision-example/steps/enhance_image_prompt_step.py` around lines 32 - 33, The model string passed to client.messages.create ("claude-3-sonnet-20240229") is retired and will cause runtime failures; update the model parameter in the client.messages.create call to a supported model (e.g., the same "claude-sonnet-4-20250514" used in evaluate_result_step.py) so requests succeed, ensuring any related code or tests that assume the old model name are adjusted to the new model identifier.examples/advanced-use-cases/vision-example/steps/generate-image.step.ts (1)
28-40:⚠️ Potential issue | 🟠 MajorUnbounded recursion in
getRequestStatusrisks a stack overflow.If the FAL API never returns
COMPLETED(e.g., it errors, staysIN_PROGRESS, or returns an unexpected status), this function recurses indefinitely with only a 1-second delay between calls. Consider adding a max-retry limit or converting to an iterative loop with a timeout.🛡️ Proposed iterative approach with a retry cap
-const getRequestStatus = async (requestId: string) => { - const status = await fal.queue.status(FAL_MODEL, { - requestId: requestId, - logs: true, - }); - - if (status.status !== 'COMPLETED') { - await new Promise(resolve => setTimeout(resolve, 1000)); - return getRequestStatus(requestId); - } - - return status.status; -} +const getRequestStatus = async (requestId: string, maxRetries = 120): Promise<string> => { + for (let attempt = 0; attempt < maxRetries; attempt++) { + const status = await fal.queue.status(FAL_MODEL, { + requestId: requestId, + logs: true, + }); + + if (status.status === 'COMPLETED') { + return status.status; + } + + await new Promise(resolve => setTimeout(resolve, 1000)); + } + + throw new Error(`Request ${requestId} did not complete after ${maxRetries} retries`); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/vision-example/steps/generate-image.step.ts` around lines 28 - 40, The getRequestStatus function currently recurses indefinitely when fal.queue.status never returns 'COMPLETED' (risking stack overflow); replace the recursive logic in getRequestStatus with an iterative loop that polls fal.queue.status(FAL_MODEL, { requestId, logs:true }) with a delay and enforces a max retry count or overall timeout, and if the limit is reached either throw a descriptive error or return the last status object; update any callers to handle the thrown error or timeout return.examples/advanced-use-cases/vision-example/steps/eval-agent/eval-agent.step.ts (2)
133-139:⚠️ Potential issue | 🟠 MajorDivision by zero when
datasetReportis empty.If no files in
tmpmatch the_report.txtpattern or none pass thereportDatavalidation on Line 46,datasetReport.lengthwill be0, causingconfidencePercentageto beInfinity. Guard against this before the enqueue call.🛡️ Proposed fix
await enqueue({ topic: 'eval-image-generation-dataset-score', data: { reportPath, - confidencePercentage: (finalScore.success * 100) / datasetReport.length, + confidencePercentage: datasetReport.length > 0 + ? (finalScore.success * 100) / datasetReport.length + : 0, }, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/vision-example/steps/eval-agent/eval-agent.step.ts` around lines 133 - 139, Guard against division by zero when computing confidencePercentage before calling enqueue: check datasetReport.length (or compute a safe denominator variable) and if it is 0 set confidencePercentage to 0 (or an appropriate default) instead of doing (finalScore.success * 100) / datasetReport.length; then call enqueue exactly as before with reportPath and the safe confidencePercentage. Update the code around the enqueue call and any helpers that compute finalScore to use the safe denominator to avoid Infinity.
96-96:⚠️ Potential issue | 🟡 MinorMisleading comment: actual sleep is 1 second, not 30.
The comment says "Sleep for 30 seconds" but
sleep(1000)sleeps for 1 second.📝 Proposed fix
- await sleep(1000) // Sleep for 30 seconds + await sleep(1000) // Sleep for 1 second between API calls🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/vision-example/steps/eval-agent/eval-agent.step.ts` at line 96, The inline comment next to the sleep call is incorrect; in the eval-agent.step.ts code the statement await sleep(1000) sleeps for 1 second (1000 ms) but the comment says 30 seconds—update the comment to accurately reflect the actual delay (e.g., "Sleep for 1 second (1000 ms)") or, if the intent was 30 seconds, change the argument to sleep(30000) instead; locate the await sleep(...) call to apply the fix.
🧹 Nitpick comments (12)
examples/advanced-use-cases/vision-example/steps/eval-agent/eval-agent-results.api.step.ts (1)
22-31: Use asyncfs/promisesinstead of synchronousreaddirSync, and handle missingtmpdirectory.Two concerns here:
fs.readdirSyncblocks the event loop in anasynchandler. The sibling fileeval-agent.step.tsusesfs/promiseswithawait fs.readdir('tmp')— this file should be consistent.- If the
tmpdirectory doesn't exist,readdirSyncthrowsENOENTwith no try-catch, resulting in an unhandled 500 instead of a meaningful error response.Proposed fix
Update the import on line 3:
-import fs from 'fs' +import fs from 'fs/promises'Then wrap the directory read with error handling:
- // Check for minimum number of report files - const reportFiles = fs.readdirSync('tmp') - .filter(file => file.endsWith('_report.txt')) + // Check for minimum number of report files + let reportFiles: string[] = [] + try { + const files = await fs.readdir('tmp') + reportFiles = files.filter(file => file.endsWith('_report.txt')) + } catch { + return { + status: 400, + body: { message: 'No report files found. Please run the generate-image flow first.' }, + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/vision-example/steps/eval-agent/eval-agent-results.api.step.ts` around lines 22 - 31, Replace the blocking fs.readdirSync call with the async fs/promises readdir and add error handling for a missing 'tmp' directory: import/require the promise-based fs, await fs.readdir('tmp'), filter for files ending with '_report.txt' (same reportFiles variable), and catch errors—if err.code === 'ENOENT' return a 400 response with a clear message about the missing 'tmp' directory; for other errors return a 500 or appropriate error response. Ensure the logic still checks reportFiles.length < 10 and returns the existing insufficient reports message.examples/advanced-use-cases/ai-room-renovate/steps/renovation/visual_assessor.step.ts (1)
34-47: Consider extracting the state-unwrapping into a small helper.The unwrap-if-wrapped pattern is duplicated three times. A tiny generic helper would reduce noise and make it easier to add more state reads later.
♻️ Suggested helper
+function unwrapState<T>(value: unknown): T | undefined { + if (value && typeof value === 'object' && 'data' in value) { + return (value as { data: T }).data; + } + return value as T | undefined; +} +Then usage becomes:
const storedBudget = unwrapState<number>(await state.get<any>(sessionId, 'budget')); const storedRoomType = unwrapState<string>(await state.get<any>(sessionId, 'roomType')); const storedStyle = unwrapState<string>(await state.get<any>(sessionId, 'style'));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/visual_assessor.step.ts` around lines 34 - 47, Extract the repeated "if object has 'data' unwrap it" logic into a small generic helper function, e.g. unwrapState<T>(value: any): T | undefined, that returns value.data when value is an object containing 'data' and otherwise returns value; then replace the three manual blocks with calls like const storedBudget = unwrapState<number>(await state.get<any>(sessionId, 'budget')), const storedRoomType = unwrapState<string>(await state.get<any>(sessionId, 'roomType')), and const storedStyle = unwrapState<string>(await state.get<any>(sessionId, 'style')) so the unwrapping logic is centralized (refer to symbols unwrapState, storedBudget, storedRoomType, storedStyle, and the state.get(...) calls).examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit_rendering_api.step.ts (1)
42-42:state.getis missing a type parameter.Sibling steps (e.g.
get_rendering.step.tsLine 47:state.get<any>(sessionId, 'rendering')) consistently pass a type argument. Without it,renderingis implicitly typed asunknown, which can cause unexpected narrowing downstream.♻️ Proposed fix
- const rendering = await state.get(sessionId, 'rendering'); + const rendering = await state.get<Record<string, unknown>>(sessionId, 'rendering');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit_rendering_api.step.ts` at line 42, state.get is called without a type argument causing rendering to be inferred as unknown; update the call to state.get to include the appropriate generic (e.g., state.get<any>(sessionId, 'rendering') or a specific interface) so rendering is typed correctly; modify the invocation in edit_rendering_api.step.ts (the state.get call that assigns rendering) to pass the chosen type parameter.examples/advanced-use-cases/ai-room-renovate/steps/renovation/project_coordinator.step.ts (1)
77-82: Note the naming convention mismatch between TS and Python steps.The enqueue payload uses
session_id(snake_case) while this step's own input schema usessessionId(camelCase). This is intentional since the downstream consumer isgenerate-rendering_step.pywhich expectssession_id, but it's worth being aware of this TS↔Python naming boundary. A shared schema or mapping convention would help prevent drift over time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/project_coordinator.step.ts` around lines 77 - 82, The enqueue call is sending session_id (snake_case) while this step's input schema uses sessionId (camelCase); explicitly map the step input sessionId to session_id in the enqueue payload (i.e., set data.session_id = input.sessionId) and add a short comment noting the TS↔Python naming boundary, or alternatively centralize the mapping in a shared helper to prevent drift; update the call to enqueue (topic 'renovation.render') and ensure any references to sessionId/session_id across this step and generate-rendering_step.py are consistently mapped.examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit-rendering_step.py (1)
239-242: Same broadexcept Exceptionconcern as in generate-rendering_step.py.Consider adding the error type for better diagnostics, as suggested for the sibling file.
♻️ Suggested improvement
except Exception as e: error_msg = str(e) - ctx.logger.error("Error editing rendering", {"session_id": session_id, "error": error_msg}) + ctx.logger.error("Error editing rendering", {"session_id": session_id, "error": error_msg, "error_type": type(e).__name__}) await ctx.state.set(session_id, "editError", error_msg)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit-rendering_step.py` around lines 239 - 242, Replace the broad "except Exception as e" in the error handling for edit-rendering_step with more specific exception types where possible (e.g., ValueError, RuntimeError) or, if you cannot enumerate them, augment the existing handler to include the exception type and stack trace in logs/state; update the ctx.logger.error call to include type(e).__name__ (and optionally traceback.format_exc()) and store a richer error value in await ctx.state.set(session_id, "editError", ...) so both ctx.logger.error and ctx.state.set include the error type (and stack) alongside the message.examples/advanced-use-cases/ai-room-renovate/steps/renovation/generate-rendering_step.py (2)
230-236: Broadexcept Exception— consider preserving the traceback.Per Ruff BLE001/TRY400, the bare
Exceptioncatch loses traceback context. Whilectx.loggermay not support.exception(), you can still capture and log the traceback explicitly.♻️ Suggested improvement
- except Exception as e: - error_msg = str(e) - ctx.logger.error("Error generating rendering", { - "session_id": session_id, - "error": error_msg - }) + except Exception as e: + error_msg = str(e) + ctx.logger.error("Error generating rendering", { + "session_id": session_id, + "error": error_msg, + "error_type": type(e).__name__ + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/generate-rendering_step.py` around lines 230 - 236, The except block currently swallows the traceback by using "except Exception as e" and only logging str(e); import the traceback module and capture the full traceback with traceback.format_exc(), then include that traceback string (in addition to the error message) when calling ctx.logger.error and when calling ctx.state.set to preserve diagnostic context; update the error handling in the except block that surrounds ctx.logger.error / await ctx.state.set (the block catching Exception e in generate-rendering_step.py) to record both the error message and the formatted traceback.
164-174: Synchronous streaming iteration inside anasynchandler blocks the event loop.
client.models.generate_content_stream(...)returns a synchronous iterator. Using a plainforloop in theasync def handlerwill block the event loop for the entire duration of image generation. The same pattern exists inedit-rendering_step.py.The google-genai SDK provides an async variant via
Client().aiowithasync for, or you can wrap the sync call inasyncio.to_thread. This is pre-existing behavior, so deferring is fine, but worth tracking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/generate-rendering_step.py` around lines 164 - 174, The async handler is performing synchronous streaming iteration (client.models.generate_content_stream) with a blocking for loop, which will block the event loop; fix by using the SDK's async client variant (client.aio.models.generate_content_stream) and iterate with "async for" inside the async def handler, or alternatively call the sync generator inside asyncio.to_thread to avoid blocking; update the loop in generate-rendering_step.py (the async def handler that uses client.models.generate_content_stream) and make the analogous change in edit-rendering_step.py where the same pattern occurs.examples/advanced-use-cases/ai-room-renovate/steps/renovation/start_renovation.step.ts (1)
46-46:substris deprecated — usesubstringinstead.
String.prototype.substris deprecated in modern ECMAScript. Usesubstringfor the same behavior.♻️ Suggested fix
- const sessionId = `session_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; + const sessionId = `session_${Date.now()}_${Math.random().toString(36).substring(2, 11)}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/steps/renovation/start_renovation.step.ts` at line 46, Replace the deprecated String.prototype.substr call when building sessionId: in the expression that generates the random suffix (Math.random().toString(36).substr(2, 9)), switch to substring and use the equivalent end index (Math.random().toString(36).substring(2, 11)) so the produced sessionId in the const sessionId = `session_${Date.now()}_${...}` retains the same length and behavior.examples/advanced-use-cases/vision-example/steps/generate-image.step.ts (1)
63-109: Uselogger.errorinstead ofconsole.errorfor consistency.The handler destructures
logger(Line 63) and uses it for info logging (Line 64, 96), but error paths on Lines 76, 87, and 107 useconsole.error. This bypasses structured logging and any log routing the framework provides.♻️ Proposed fix
if (!requestId) { - console.error('failed to generate image') + logger.error('failed to generate image') return; }if (!result.data.images.length) { - console.error('no image generated') + logger.error('no image generated') return; }} catch (error) { - console.error('failed to generate image', error) + logger.error('failed to generate image', error) return; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/vision-example/steps/generate-image.step.ts` around lines 63 - 109, The error paths in the exported handler use console.error (three places) instead of the destructured logger; replace all console.error calls inside handler with logger.error so errors go through the app's structured logger. Specifically, update the error branches after fal.queue.submit (check for missing requestId), after checking result.data.images.length (no image generated), and in the catch block to call logger.error and include the error/context (e.g., requestId, traceId, prompt) while keeping the existing returns; locate these in the handler that calls fal.queue.submit, getRequestStatus, fal.queue.result, and saveBase64Image.examples/advanced-use-cases/vision-example/steps/eval-agent/eval-agent.step.ts (1)
20-20: Comment still referencesemit— update to reflectenqueue.Line 131 says "we only emit the report path" but the code now uses
enqueue. Minor terminology drift from the migration.📝 Proposed fix
- // For now we only emit the report path and the confidence percentage, for a future use case you can trigger + // For now we only enqueue the report path and the confidence percentage, for a future use case you can triggerAlso applies to: 131-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/vision-example/steps/eval-agent/eval-agent.step.ts` at line 20, The review notes outdated terminology: update any inline comment or log that says "emit" to use the new API name "enqueue" inside the handler function (the exported handler: Handlers<typeof config>) where you describe sending the report path (currently referenced as "we only emit the report path"); locate the comment or string near the code that calls enqueue and replace "emit" with "enqueue" (and adjust wording if it implies the old emit behavior) so comments/logs match the actual enqueue(...) call.examples/advanced-use-cases/ai-room-renovate/pyproject.toml (1)
10-11: Consider tighteninggoogle-adkandgoogle-genaiversion constraints.The
google-adkpackage is currently at version1.25.1, meaning>=0.1.0will resolve to a version that has undergone many breaking changes from the originally tested0.1.0baseline. Using~=(compatible release) or an explicit upper bound would make the example reproducible.📌 Proposed fix using compatible-release operators
- "google-adk>=0.1.0", - "google-genai>=0.2.0", + "google-adk~=1.0", + "google-genai~=1.0",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/pyproject.toml` around lines 10 - 11, The dependency constraints for "google-adk" and "google-genai" are too loose (using >=0.1.0 and >=0.2.0); tighten them to ensure reproducible installs by switching to compatible-release or adding an upper bound—for example use the ~= operator (e.g., "google-adk~=0.1" and "google-genai~=0.2") or explicit upper bounds (e.g., "google-adk>=0.1.0,<1.0.0" and "google-genai>=0.2.0,<1.0.0"); update the entries for the "google-adk" and "google-genai" dependency lines in pyproject.toml accordingly.examples/advanced-use-cases/ai-room-renovate/config.yaml (1)
26-34: Wildcard CORS origin in a dev example is acceptable but worth noting for portability.Since
hostis127.0.0.1, external access is blocked at the network layer, soallowed_origins: ['*']carries minimal risk here. If the host is ever changed to0.0.0.0, the wildcard becomes a permissive CORS policy. Consider scoping it tohttp://localhost:*for a dev config to make the intent explicit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/advanced-use-cases/ai-room-renovate/config.yaml` around lines 26 - 34, The CORS config uses a wildcard origin (cors.allowed_origins: ['*']), which is too permissive if host is changed; update cors.allowed_origins in the YAML (the cors section and allowed_origins key) to a scoped development value such as explicit localhost patterns (e.g., http://localhost:3000 or http://localhost:*), or list specific origins you expect, so the intent is explicit while keeping the current host 127.0.0.1 behavior safe.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/advanced-use-cases/ai-room-renovate/config.yaml`:
- Around line 39-41: The default SERVICE_NAMESPACE in config.yaml is set to
"production" which is misleading for an example; change the default placeholder
for service_namespace (the SERVICE_NAMESPACE env var) to a non-production value
like "example" or leave it empty/null so developers don't accidentally tag
traces as production; update the service_namespace:
${SERVICE_NAMESPACE:production} entry to use ${SERVICE_NAMESPACE:example} (or
remove the default) so service_namespace and SERVICE_NAME/service_version
defaults reflect an example/dev context.
- Around line 71-78: The ExecModule currently lists two exec commands under the
same module (exec: - npx motia dev - bun run --enable-source-maps
dist/index-dev.js) which run sequentially so the long-running `npx motia dev`
blocks the `bun run` command; fix it by splitting into two ExecModule entries:
one ExecModule with watch: ["steps/**/*.ts"] and exec: ["npx motia dev"] (for
the dev server), and a separate ExecModule that watches the built output (or
appropriate pattern) and runs exec: ["bun run --enable-source-maps
dist/index-dev.js"] so both processes can run concurrently (or alternatively run
both in a single shell command using `&` if you intend a single module).
In `@examples/advanced-use-cases/ai-room-renovate/package.json`:
- Line 9: The "build" script currently runs "npx motia dev" which starts a dev
server rather than producing build artifacts; change the "build" script to
perform a one-shot build (for example "tsc -p tsconfig.json" or your project's
actual build command) and move "npx motia dev" to a clearly named dev script
such as "dev:motia" or "motia:dev" so "build" is safe for CI/deploy; update the
"build" and new dev script names in package.json accordingly, ensuring any CI
references use the new "build" script.
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit-rendering_step.py`:
- Around line 199-207: The edit history unwrap logic is in the wrong order: when
retrieving edit_history via ctx.state.get(session_id, "editHistory"), check for
a wrapped dict first (if isinstance(edit_history, dict) and "data" in
edit_history) and unwrap edit_history = edit_history["data"] before validating
type; then if not isinstance(edit_history, list) set edit_history = []; update
the code around the edit_history handling (the block using ctx.state.get, the
dict "data" check, and the list validation) so wrapped {"data": [...]} is
unwrapped prior to the list type check.
In `@examples/advanced-use-cases/vision-example/package.json`:
- Line 9: The package.json "build" script currently runs "npx motia dev" which
starts a long‑running watcher; change the script either by renaming it (e.g.,
"dev:ts" or "watch") to reflect dev mode or replace it with a one‑shot
build/compile command (e.g., a TypeScript compiler or bundler invocation) so
pnpm build is non-blocking; then update all references to this script in
config.yaml's TS ExecModule exec array to point to the new script name or
command (look for the "build" script entry in package.json and the TS ExecModule
"exec" entries in config.yaml to make the edits).
In `@examples/advanced-use-cases/vision-example/README.md`:
- Around line 5-9: The README's Requirements list omits the bun runtime while
the project's start script ("bun run --enable-source-maps dist/index-dev.js")
depends on it; update the Requirements section to include bun as a required
runtime (mention minimum version if applicable) and add a brief note on how to
install or verify bun is installed to ensure the start script and development
workflow work as documented.
In `@examples/advanced-use-cases/vision-example/steps/evaluate_result_step.py`:
- Around line 28-29: The current try block in evaluate_result_step.py (around
the image reading, Anthropic API call, and response parsing inside the
function/method handling evaluation) only catches ValueError which leaves
FileNotFoundError, OSError and Anthropic API exceptions unhandled and can leave
raw_response unbound; update the error handling by broadening the excepts:
explicitly catch FileNotFoundError/OSError around the open(image, ...) call (or
add a small try/except there), catch the Anthropic client exceptions (e.g.,
APIError/AuthenticationError/rate limit errors from the Anthropic client) around
the API call, and add a final except Exception that logs the full exception
(including the exception object) and returns or raises a controlled error so
raw_response is never referenced if not set; also adjust the existing except
ValueError branch to only handle parsing/value errors after raw_response is
guaranteed assigned.
- Line 17: The step declares enqueues: ["eval-report"] but the step handler
never calls ctx.enqueue, so downstream step won't receive the report; either add
a ctx.enqueue("eval-report", payload) call in the handler after constructing the
evaluation result (the same object you write to file in the handler, around the
code that writes the local file and logs results), or remove "eval-report" from
the step config to avoid confusion—look for the handler function in
evaluate_result_step.py and insert ctx.enqueue("eval-report", { /* evaluation
payload */ }) immediately after the file write/logging (or remove the enqueues
entry if enqueueing is not intended).
- Around line 57-62: The payload hardcodes "media_type": "image/png" which
breaks for non-PNG inputs; determine the MIME type from the incoming image
path/filename in input_data (e.g., inspect input_data["image_path"] or the
source filename) and map extensions like .jpg/.jpeg -> "image/jpeg", .webp ->
"image/webp", .png -> "image/png" (fallback to "application/octet-stream" or use
Python's mimetypes.guess_type), assign it to a media_type variable and replace
the hardcoded string in the image source dict so the code uses media_type when
building the payload that includes image_data.
- Line 1: The async handler function (async def handler) is using the blocking
synchronous Anthropic client; replace the import and client with AsyncAnthropic,
instantiate the async client (e.g., AsyncAnthropic(...)) and change the blocking
call client.messages.create(...) to an awaited call (await
client.messages.create(...)) so the HTTP request does not block the event loop;
apply the same change in enhance_image_prompt_step.py by swapping Anthropic for
AsyncAnthropic, creating the async client, and awaiting its API calls, ensuring
all calls occur inside the async handler/coroutine context and any client
cleanup/close is handled appropriately.
In `@examples/advanced-use-cases/vision-example/types.d.ts`:
- Around line 5-10: The import of MotiaStream is unused in this ambient
declaration—remove the unused import statement for MotiaStream (or change the
generator to omit it) when no streams are declared; update the declarations for
the empty interfaces Streams and Enqueues in types.d.ts so they remain empty
without importing MotiaStream, ensuring there are no unused-import warnings from
the MotiaStream symbol.
---
Outside diff comments:
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/design_planner.step.ts`:
- Around line 38-44: The scope assignment logic makes 'luxury' unreachable
because the avgCost > 300 branch runs before avgCost > 600; update the
conditional order in the block that computes avgCost (which uses costEstimate
and assessment.squareFootage) to check the highest threshold first (avgCost >
600), then avgCost > 300, then avgCost < 100, or refactor to explicit range
checks so 'scope' can be set to 'luxury', 'full', 'moderate' or 'cosmetic'
correctly.
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit_rendering_api.step.ts`:
- Around line 71-77: The current catch around bodySchema.parse(), state.get(),
and enqueue() treats all errors as validation failures; isolate validation by
calling bodySchema.parse() inside its own try/catch that only catches ZodError
(imported from "zod") and returns the 400 + "Invalid request data" response,
while letting other errors from state.get() or enqueue() bubble up or be handled
as server errors (log with logger.error in the outer catch and return 500).
Update the handler so bodySchema.parse() is validated first (or in a small try
that catches ZodError), and ensure logger.error calls (the existing
logger.error('Failed to process edit request', { error: String(error) })) remain
for non-validation failures.
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/visual_assessor.step.ts`:
- Around line 99-102: The conditional ordering in the block that sets scope
based on avgCost makes the 'luxury' branch unreachable; reorder the comparisons
in the conditional that assigns scope (the code using variable scope and
avgCost) so the largest threshold is checked first (e.g., if (avgCost > 600)
scope = 'luxury'; else if (avgCost > 300) scope = 'full'; else if (avgCost <
100) scope = 'cosmetic'; else scope = 'moderate';) to ensure each branch can be
reached.
- Around line 97-98: The calculation using assessment.budget can produce NaN if
budget is non-numeric; in the visual_assessor.step.ts code locate the block that
computes avgCost from assessment.budget and assessment.squareFootage and coerce
assessment.budget to a numeric value (e.g., via Number(...) or a sanitized
parse) then check isFinite/!isNaN before performing the division, and if the
value is invalid handle it (skip avgCost, use a fallback, or log/throw) so
avgCost = assessment.budget / assessment.squareFootage cannot silently become
NaN.
In
`@examples/advanced-use-cases/vision-example/steps/enhance_image_prompt_step.py`:
- Around line 32-33: The model string passed to client.messages.create
("claude-3-sonnet-20240229") is retired and will cause runtime failures; update
the model parameter in the client.messages.create call to a supported model
(e.g., the same "claude-sonnet-4-20250514" used in evaluate_result_step.py) so
requests succeed, ensuring any related code or tests that assume the old model
name are adjusted to the new model identifier.
In
`@examples/advanced-use-cases/vision-example/steps/eval-agent/eval-agent.step.ts`:
- Around line 133-139: Guard against division by zero when computing
confidencePercentage before calling enqueue: check datasetReport.length (or
compute a safe denominator variable) and if it is 0 set confidencePercentage to
0 (or an appropriate default) instead of doing (finalScore.success * 100) /
datasetReport.length; then call enqueue exactly as before with reportPath and
the safe confidencePercentage. Update the code around the enqueue call and any
helpers that compute finalScore to use the safe denominator to avoid Infinity.
- Line 96: The inline comment next to the sleep call is incorrect; in the
eval-agent.step.ts code the statement await sleep(1000) sleeps for 1 second
(1000 ms) but the comment says 30 seconds—update the comment to accurately
reflect the actual delay (e.g., "Sleep for 1 second (1000 ms)") or, if the
intent was 30 seconds, change the argument to sleep(30000) instead; locate the
await sleep(...) call to apply the fix.
In `@examples/advanced-use-cases/vision-example/steps/generate-image.step.ts`:
- Around line 28-40: The getRequestStatus function currently recurses
indefinitely when fal.queue.status never returns 'COMPLETED' (risking stack
overflow); replace the recursive logic in getRequestStatus with an iterative
loop that polls fal.queue.status(FAL_MODEL, { requestId, logs:true }) with a
delay and enforces a max retry count or overall timeout, and if the limit is
reached either throw a descriptive error or return the last status object;
update any callers to handle the thrown error or timeout return.
---
Nitpick comments:
In `@examples/advanced-use-cases/ai-room-renovate/config.yaml`:
- Around line 26-34: The CORS config uses a wildcard origin
(cors.allowed_origins: ['*']), which is too permissive if host is changed;
update cors.allowed_origins in the YAML (the cors section and allowed_origins
key) to a scoped development value such as explicit localhost patterns (e.g.,
http://localhost:3000 or http://localhost:*), or list specific origins you
expect, so the intent is explicit while keeping the current host 127.0.0.1
behavior safe.
In `@examples/advanced-use-cases/ai-room-renovate/pyproject.toml`:
- Around line 10-11: The dependency constraints for "google-adk" and
"google-genai" are too loose (using >=0.1.0 and >=0.2.0); tighten them to ensure
reproducible installs by switching to compatible-release or adding an upper
bound—for example use the ~= operator (e.g., "google-adk~=0.1" and
"google-genai~=0.2") or explicit upper bounds (e.g., "google-adk>=0.1.0,<1.0.0"
and "google-genai>=0.2.0,<1.0.0"); update the entries for the "google-adk" and
"google-genai" dependency lines in pyproject.toml accordingly.
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit_rendering_api.step.ts`:
- Line 42: state.get is called without a type argument causing rendering to be
inferred as unknown; update the call to state.get to include the appropriate
generic (e.g., state.get<any>(sessionId, 'rendering') or a specific interface)
so rendering is typed correctly; modify the invocation in
edit_rendering_api.step.ts (the state.get call that assigns rendering) to pass
the chosen type parameter.
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit-rendering_step.py`:
- Around line 239-242: Replace the broad "except Exception as e" in the error
handling for edit-rendering_step with more specific exception types where
possible (e.g., ValueError, RuntimeError) or, if you cannot enumerate them,
augment the existing handler to include the exception type and stack trace in
logs/state; update the ctx.logger.error call to include type(e).__name__ (and
optionally traceback.format_exc()) and store a richer error value in await
ctx.state.set(session_id, "editError", ...) so both ctx.logger.error and
ctx.state.set include the error type (and stack) alongside the message.
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/generate-rendering_step.py`:
- Around line 230-236: The except block currently swallows the traceback by
using "except Exception as e" and only logging str(e); import the traceback
module and capture the full traceback with traceback.format_exc(), then include
that traceback string (in addition to the error message) when calling
ctx.logger.error and when calling ctx.state.set to preserve diagnostic context;
update the error handling in the except block that surrounds ctx.logger.error /
await ctx.state.set (the block catching Exception e in
generate-rendering_step.py) to record both the error message and the formatted
traceback.
- Around line 164-174: The async handler is performing synchronous streaming
iteration (client.models.generate_content_stream) with a blocking for loop,
which will block the event loop; fix by using the SDK's async client variant
(client.aio.models.generate_content_stream) and iterate with "async for" inside
the async def handler, or alternatively call the sync generator inside
asyncio.to_thread to avoid blocking; update the loop in
generate-rendering_step.py (the async def handler that uses
client.models.generate_content_stream) and make the analogous change in
edit-rendering_step.py where the same pattern occurs.
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/project_coordinator.step.ts`:
- Around line 77-82: The enqueue call is sending session_id (snake_case) while
this step's input schema uses sessionId (camelCase); explicitly map the step
input sessionId to session_id in the enqueue payload (i.e., set data.session_id
= input.sessionId) and add a short comment noting the TS↔Python naming boundary,
or alternatively centralize the mapping in a shared helper to prevent drift;
update the call to enqueue (topic 'renovation.render') and ensure any references
to sessionId/session_id across this step and generate-rendering_step.py are
consistently mapped.
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/start_renovation.step.ts`:
- Line 46: Replace the deprecated String.prototype.substr call when building
sessionId: in the expression that generates the random suffix
(Math.random().toString(36).substr(2, 9)), switch to substring and use the
equivalent end index (Math.random().toString(36).substring(2, 11)) so the
produced sessionId in the const sessionId = `session_${Date.now()}_${...}`
retains the same length and behavior.
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/visual_assessor.step.ts`:
- Around line 34-47: Extract the repeated "if object has 'data' unwrap it" logic
into a small generic helper function, e.g. unwrapState<T>(value: any): T |
undefined, that returns value.data when value is an object containing 'data' and
otherwise returns value; then replace the three manual blocks with calls like
const storedBudget = unwrapState<number>(await state.get<any>(sessionId,
'budget')), const storedRoomType = unwrapState<string>(await
state.get<any>(sessionId, 'roomType')), and const storedStyle =
unwrapState<string>(await state.get<any>(sessionId, 'style')) so the unwrapping
logic is centralized (refer to symbols unwrapState, storedBudget,
storedRoomType, storedStyle, and the state.get(...) calls).
In
`@examples/advanced-use-cases/vision-example/steps/eval-agent/eval-agent-results.api.step.ts`:
- Around line 22-31: Replace the blocking fs.readdirSync call with the async
fs/promises readdir and add error handling for a missing 'tmp' directory:
import/require the promise-based fs, await fs.readdir('tmp'), filter for files
ending with '_report.txt' (same reportFiles variable), and catch errors—if
err.code === 'ENOENT' return a 400 response with a clear message about the
missing 'tmp' directory; for other errors return a 500 or appropriate error
response. Ensure the logic still checks reportFiles.length < 10 and returns the
existing insufficient reports message.
In
`@examples/advanced-use-cases/vision-example/steps/eval-agent/eval-agent.step.ts`:
- Line 20: The review notes outdated terminology: update any inline comment or
log that says "emit" to use the new API name "enqueue" inside the handler
function (the exported handler: Handlers<typeof config>) where you describe
sending the report path (currently referenced as "we only emit the report
path"); locate the comment or string near the code that calls enqueue and
replace "emit" with "enqueue" (and adjust wording if it implies the old emit
behavior) so comments/logs match the actual enqueue(...) call.
In `@examples/advanced-use-cases/vision-example/steps/generate-image.step.ts`:
- Around line 63-109: The error paths in the exported handler use console.error
(three places) instead of the destructured logger; replace all console.error
calls inside handler with logger.error so errors go through the app's structured
logger. Specifically, update the error branches after fal.queue.submit (check
for missing requestId), after checking result.data.images.length (no image
generated), and in the catch block to call logger.error and include the
error/context (e.g., requestId, traceId, prompt) while keeping the existing
returns; locate these in the handler that calls fal.queue.submit,
getRequestStatus, fal.queue.result, and saveBase64Image.
Comment on lines
+39
to
+41
| service_name: ${OTEL_SERVICE_NAME:iii-engine} | ||
| service_version: ${SERVICE_VERSION:0.2.0} | ||
| service_namespace: ${SERVICE_NAMESPACE:production} |
There was a problem hiding this comment.
Default SERVICE_NAMESPACE of "production" is misleading for a dev/example project.
If a developer runs this example with a real OTLP backend without setting SERVICE_NAMESPACE, all traces/metrics will be tagged as belonging to the production namespace.
✏️ Suggested fix
- service_namespace: ${SERVICE_NAMESPACE:production}
+ service_namespace: ${SERVICE_NAMESPACE:development}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| service_name: ${OTEL_SERVICE_NAME:iii-engine} | |
| service_version: ${SERVICE_VERSION:0.2.0} | |
| service_namespace: ${SERVICE_NAMESPACE:production} | |
| service_name: ${OTEL_SERVICE_NAME:iii-engine} | |
| service_version: ${SERVICE_VERSION:0.2.0} | |
| service_namespace: ${SERVICE_NAMESPACE:development} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/advanced-use-cases/ai-room-renovate/config.yaml` around lines 39 -
41, The default SERVICE_NAMESPACE in config.yaml is set to "production" which is
misleading for an example; change the default placeholder for service_namespace
(the SERVICE_NAMESPACE env var) to a non-production value like "example" or
leave it empty/null so developers don't accidentally tag traces as production;
update the service_namespace: ${SERVICE_NAMESPACE:production} entry to use
${SERVICE_NAMESPACE:example} (or remove the default) so service_namespace and
SERVICE_NAME/service_version defaults reflect an example/dev context.
Comment on lines
+71
to
+78
| - class: modules::shell::ExecModule | ||
| config: | ||
| watch: | ||
| - steps/**/*.ts | ||
|
|
||
| exec: | ||
| - npx motia dev | ||
| - bun run --enable-source-maps dist/index-dev.js |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the config.yaml file
find . -name "config.yaml" -path "*/ai-room-renovate/*" | head -5Repository: MotiaDev/motia-examples
Length of output: 125
🏁 Script executed:
# Check the repository structure to understand the project better
ls -la examples/advanced-use-cases/ai-room-renovate/ 2>/dev/null || echo "Directory not found"Repository: MotiaDev/motia-examples
Length of output: 1051
🏁 Script executed:
# Search for Motia documentation or ExecModule references in the repo
find . -type f \( -name "*.md" -o -name "*.ts" -o -name "*.js" \) -exec grep -l "ExecModule\|motia" {} \; | head -20Repository: MotiaDev/motia-examples
Length of output: 31238
🌐 Web query:
Motia ExecModule multiple exec entries sequential parallel execution behavior
💡 Result:
In Motia’s config.yaml, the iii Shell ExecModule treats config.exec as an ordered list: the commands are executed in order (sequentially), not in parallel. This is why the default dev setup runs a build command first (npx motia dev) and then starts the output (node dist/index-dev.js). When watched files change, the exec commands are re-run. [1]
If you need “parallel” behavior, you generally have to do it yourself inside a single exec entry using your shell (e.g., backgrounding with &, concurrently, etc.), because multiple exec: list items are intended to be ordered steps, not concurrent processes. [1]
Sources
[1] Motia Docs — “Configuration (config.yaml)” (Shell Exec module; “exec — commands to execute in order”) https://www.motia.dev/docs/development-guide/motia-config
🏁 Script executed:
# Read the config.yaml file to see the actual content at lines 71-78
cat -n examples/advanced-use-cases/ai-room-renovate/config.yaml | sed -n '60,85p'Repository: MotiaDev/motia-examples
Length of output: 754
🏁 Script executed:
# Read the package.json to understand what these scripts do
cat examples/advanced-use-cases/ai-room-renovate/package.jsonRepository: MotiaDev/motia-examples
Length of output: 705
🏁 Script executed:
# Check if there's any documentation in the repo about ExecModule or the intended behavior
cat examples/advanced-use-cases/ai-room-renovate/README.md | head -100Repository: MotiaDev/motia-examples
Length of output: 3430
The two exec commands are executed sequentially; the second will never start.
Motia's ExecModule runs commands in the exec list in order, not in parallel. Since npx motia dev is a long-running dev/build server that doesn't exit, bun run --enable-source-maps dist/index-dev.js will never execute.
Looking at the rest of the config, there's already a separate ExecModule for Python files (lines 80+), suggesting TypeScript and Python should also be separate. Split these into two ExecModule entries:
- One for
npx motia devwatchingsteps/**/*.ts - One for
bun run ...watching the built output (or handle both in a single shell command using&)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/advanced-use-cases/ai-room-renovate/config.yaml` around lines 71 -
78, The ExecModule currently lists two exec commands under the same module
(exec: - npx motia dev - bun run --enable-source-maps dist/index-dev.js) which
run sequentially so the long-running `npx motia dev` blocks the `bun run`
command; fix it by splitting into two ExecModule entries: one ExecModule with
watch: ["steps/**/*.ts"] and exec: ["npx motia dev"] (for the dev server), and a
separate ExecModule that watches the built output (or appropriate pattern) and
runs exec: ["bun run --enable-source-maps dist/index-dev.js"] so both processes
can run concurrently (or alternatively run both in a single shell command using
`&` if you intend a single module).
| "start": "bun run --enable-source-maps dist/index-dev.js", | ||
| "dev": "iii", | ||
| "dev:py": "uv run motia dev --dir steps --watch", | ||
| "build": "npx motia dev" |
There was a problem hiding this comment.
"build": "npx motia dev" is semantically wrong — this starts a dev server, not a build.
Running npm run build in CI or during deployment will spin up a long-running dev server instead of producing artifacts. If the intent is to compile TypeScript to dist/, this command should be the TypeScript compilation step. If npx motia dev is intentional (i.e., it performs a one-shot code generation step), rename the script to something unambiguous like "dev:ts".
✏️ Suggested fix
- "build": "npx motia dev"
+ "dev:ts": "npx motia dev"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "build": "npx motia dev" | |
| "dev:ts": "npx motia dev" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/advanced-use-cases/ai-room-renovate/package.json` at line 9, The
"build" script currently runs "npx motia dev" which starts a dev server rather
than producing build artifacts; change the "build" script to perform a one-shot
build (for example "tsc -p tsconfig.json" or your project's actual build
command) and move "npx motia dev" to a clearly named dev script such as
"dev:motia" or "motia:dev" so "build" is safe for CI/deploy; update the "build"
and new dev script names in package.json accordingly, ensuring any CI references
use the new "build" script.
Comment on lines
+199
to
207
| edit_history = await ctx.state.get(session_id, "editHistory") | ||
| if not edit_history or not isinstance(edit_history, list): | ||
| edit_history = [] | ||
|
|
||
| # If it's wrapped in data, unwrap it | ||
| if isinstance(edit_history, dict) and "data" in edit_history: | ||
| edit_history = edit_history["data"] | ||
| if not isinstance(edit_history, list): | ||
| edit_history = [] |
There was a problem hiding this comment.
Bug: Edit history unwrapping is in the wrong order — wrapped state data will be silently lost.
Line 200 checks not isinstance(edit_history, list) and resets to [] before line 204 attempts to unwrap a dict with a "data" key. If Motia state wraps the list in {"data": [...]}, the dict fails the isinstance(list) check and gets replaced with [], making the unwrapping on Line 204 unreachable.
🐛 Proposed fix — unwrap before validating
# Store edit history
edit_history = await ctx.state.get(session_id, "editHistory")
+
+ # If it's wrapped in data, unwrap it first
+ if isinstance(edit_history, dict) and "data" in edit_history:
+ edit_history = edit_history["data"]
+
if not edit_history or not isinstance(edit_history, list):
edit_history = []
-
- # If it's wrapped in data, unwrap it
- if isinstance(edit_history, dict) and "data" in edit_history:
- edit_history = edit_history["data"]
- if not isinstance(edit_history, list):
- edit_history = []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@examples/advanced-use-cases/ai-room-renovate/steps/renovation/edit-rendering_step.py`
around lines 199 - 207, The edit history unwrap logic is in the wrong order:
when retrieving edit_history via ctx.state.get(session_id, "editHistory"), check
for a wrapped dict first (if isinstance(edit_history, dict) and "data" in
edit_history) and unwrap edit_history = edit_history["data"] before validating
type; then if not isinstance(edit_history, list) set edit_history = []; update
the code around the edit_history handling (the block using ctx.state.get, the
dict "data" check, and the list validation) so wrapped {"data": [...]} is
unwrapped prior to the list type check.
| "start": "bun run --enable-source-maps dist/index-dev.js", | ||
| "dev": "iii", | ||
| "dev:py": "uv run motia dev --dir steps --watch", | ||
| "build": "npx motia dev" |
There was a problem hiding this comment.
build script runs npx motia dev — semantically misleading.
motia dev starts a dev watch process rather than producing a one-shot build artifact. Users or CI pipelines invoking pnpm build expecting an offline compile will get a long-running watcher. Consider renaming this script (e.g., compile or dev:ts) or replacing it with a proper build command, and update the references in config.yaml's TS ExecModule exec array accordingly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/advanced-use-cases/vision-example/package.json` at line 9, The
package.json "build" script currently runs "npx motia dev" which starts a
long‑running watcher; change the script either by renaming it (e.g., "dev:ts" or
"watch") to reflect dev mode or replace it with a one‑shot build/compile command
(e.g., a TypeScript compiler or bundler invocation) so pnpm build is
non-blocking; then update all references to this script in config.yaml's TS
ExecModule exec array to point to the new script name or command (look for the
"build" script entry in package.json and the TS ExecModule "exec" entries in
config.yaml to make the edits).
| @@ -1,28 +1,38 @@ | |||
| import ultraimport | |||
| from vision_agent.lmm import AnthropicLMM | |||
| from anthropic import Anthropic | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat examples/advanced-use-cases/vision-example/steps/evaluate_result_step.pyRepository: MotiaDev/motia-examples
Length of output: 3579
🏁 Script executed:
fd "enhance_image_prompt_step.py" examples/advanced-use-cases/vision-example --exec cat {}Repository: MotiaDev/motia-examples
Length of output: 1434
🏁 Script executed:
rg "AsyncAnthropic|async def.*handler" examples/advanced-use-cases/vision-example --context 3Repository: MotiaDev/motia-examples
Length of output: 1566
Synchronous Anthropic client blocks the async event loop.
The handler is async def (line 21) but uses the synchronous Anthropic client (line 9). The client.messages.create(...) call on line 50 performs a blocking HTTP request, which will block the event loop and prevent concurrent task processing.
Use AsyncAnthropic and await the API call instead. The same issue exists in enhance_image_prompt_step.py (lines 6, 16, 24).
Suggested fix
-from anthropic import Anthropic
+from anthropic import AsyncAnthropic-client = Anthropic(api_key=os.getenv("ANTHROPIC_API_KEY"))
+client = AsyncAnthropic(api_key=os.getenv("ANTHROPIC_API_KEY"))- response = client.messages.create(
+ response = await client.messages.create(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| from anthropic import Anthropic | |
| from anthropic import AsyncAnthropic |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/advanced-use-cases/vision-example/steps/evaluate_result_step.py` at
line 1, The async handler function (async def handler) is using the blocking
synchronous Anthropic client; replace the import and client with AsyncAnthropic,
instantiate the async client (e.g., AsyncAnthropic(...)) and change the blocking
call client.messages.create(...) to an awaited call (await
client.messages.create(...)) so the HTTP request does not block the event loop;
apply the same change in enhance_image_prompt_step.py by swapping Anthropic for
AsyncAnthropic, creating the async client, and awaiting its API calls, ensuring
all calls occur inside the async handler/coroutine context and any client
cleanup/close is handled appropriately.
| "triggers": [ | ||
| queue("eval-image-result") | ||
| ], | ||
| "enqueues": ["eval-report"], |
There was a problem hiding this comment.
enqueues: ["eval-report"] is declared but never invoked in the handler.
The config declares this step enqueues to "eval-report", but the handler never calls ctx.enqueue(...). The evaluation result is only written to a local file (line 80-88) and logged (lines 90-93), meaning no downstream step will receive the report via the flow.
If downstream processing is intended, an enqueue call is missing. If it's not needed, remove "eval-report" from the config to avoid confusion.
Example fix if enqueue is intended
if score > 90:
ctx.logger.info('image is a good representation, do something with it', score)
else:
ctx.logger.info('image is not a good representation, try again or use a different prompt', score)
+
+ await ctx.enqueue("eval-report", report)Also applies to: 80-93
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/advanced-use-cases/vision-example/steps/evaluate_result_step.py` at
line 17, The step declares enqueues: ["eval-report"] but the step handler never
calls ctx.enqueue, so downstream step won't receive the report; either add a
ctx.enqueue("eval-report", payload) call in the handler after constructing the
evaluation result (the same object you write to file in the handler, around the
code that writes the local file and logs results), or remove "eval-report" from
the step config to avoid confusion—look for the handler function in
evaluate_result_step.py and insert ctx.enqueue("eval-report", { /* evaluation
payload */ }) immediately after the file write/logging (or remove the enqueues
entry if enqueueing is not intended).
Comment on lines
28
to
+29
| try: | ||
| lmm = AnthropicLMM() | ||
| prompt = """Evaluate if the image is a good representation of the following prompt: | ||
| # Read and base64 encode the image |
There was a problem hiding this comment.
Exception handling is too narrow — only ValueError is caught.
The try block includes file I/O (line 30), an external API call (line 50), and response parsing (line 72-74). Only ValueError is caught, so:
FileNotFoundError/OSErrorfromopen(image, ...)will propagate unhandled if the image path is invalid.- Anthropic API errors (
APIError,AuthenticationError, rate limits, etc.) will propagate unhandled. - In the
except ValueErrorblock (line 96),raw_responsecould beUnboundLocalErrorif theValueErroris raised before line 72 assigns it (unlikely with current code but fragile).
Consider broadening the error handling or adding an outer except Exception with proper logging.
Suggested fix
except ValueError:
- ctx.logger.error('Invalid response from vision agent', raw_response)
+ ctx.logger.exception('Invalid response from vision agent: %s', raw_response)
+ except FileNotFoundError:
+ ctx.logger.exception('Image file not found: %s', image)
+ except Exception:
+ ctx.logger.exception('Unexpected error evaluating image')Also applies to: 95-96
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/advanced-use-cases/vision-example/steps/evaluate_result_step.py`
around lines 28 - 29, The current try block in evaluate_result_step.py (around
the image reading, Anthropic API call, and response parsing inside the
function/method handling evaluation) only catches ValueError which leaves
FileNotFoundError, OSError and Anthropic API exceptions unhandled and can leave
raw_response unbound; update the error handling by broadening the excepts:
explicitly catch FileNotFoundError/OSError around the open(image, ...) call (or
add a small try/except there), catch the Anthropic client exceptions (e.g.,
APIError/AuthenticationError/rate limit errors from the Anthropic client) around
the API call, and add a final except Exception that logs the full exception
(including the exception object) and returns or raises a controlled error so
raw_response is never referenced if not set; also adjust the existing except
ValueError branch to only handle parsing/value errors after raw_response is
guaranteed assigned.
Comment on lines
+57
to
+62
| "type": "image", | ||
| "source": { | ||
| "type": "base64", | ||
| "media_type": "image/png", | ||
| "data": image_data, | ||
| }, |
There was a problem hiding this comment.
Hardcoded media_type: "image/png" may be incorrect for non-PNG images.
The image path comes from input_data and could reference a JPEG, WebP, or other format. Sending the wrong media type to the Anthropic API may cause errors or misinterpretation.
Suggested fix: detect media type from the file extension
+ import mimetypes
+
# Read and base64 encode the image
+ media_type = mimetypes.guess_type(image)[0] or "image/png"
with open(image, "rb") as f:
image_data = base64.standard_b64encode(f.read()).decode("utf-8")Then use media_type variable on line 60:
- "media_type": "image/png",
+ "media_type": media_type,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/advanced-use-cases/vision-example/steps/evaluate_result_step.py`
around lines 57 - 62, The payload hardcodes "media_type": "image/png" which
breaks for non-PNG inputs; determine the MIME type from the incoming image
path/filename in input_data (e.g., inspect input_data["image_path"] or the
source filename) and map extensions like .jpg/.jpeg -> "image/jpeg", .webp ->
"image/webp", .png -> "image/png" (fallback to "application/octet-stream" or use
Python's mimetypes.guess_type), assign it to a media_type variable and replace
the hardcoded string in the image source dict so the code uses media_type when
building the payload that includes image_data.
Comment on lines
+5
to
+10
| import { MotiaStream } from 'motia' | ||
|
|
||
| declare module 'motia' { | ||
| interface Streams {} | ||
|
|
||
| interface Enqueues {} |
There was a problem hiding this comment.
MotiaStream is imported but never referenced.
The Streams and Enqueues interfaces are empty, so the MotiaStream import is unused. While this is auto-generated, a stale import in ambient declaration files can produce TypeScript warnings depending on verbatimModuleSyntax settings. If the generator is under your control, suppress this import when no streams are declared.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/advanced-use-cases/vision-example/types.d.ts` around lines 5 - 10,
The import of MotiaStream is unused in this ambient declaration—remove the
unused import statement for MotiaStream (or change the generator to omit it)
when no streams are declared; update the declarations for the empty interfaces
Streams and Enqueues in types.d.ts so they remain empty without importing
MotiaStream, ensuring there are no unused-import warnings from the MotiaStream
symbol.
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 by CodeRabbit
New Features
Chores