Refactor execute result analysis to schema transform #96
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis update refactors execution result handling across the browser extension, Discord bot, and shared shapes package, introducing a new phase-based execution result schema. It also updates dependency versions in several package files, simplifies output and tab rendering logic in UI components, and adds new utility functions for generating code sharing URLs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI_Component
participant ExecutionEngine
participant Utility
participant Playground
User->>UI_Component: Submit code (with args/input)
UI_Component->>ExecutionEngine: Execute code (with dynamic filename)
ExecutionEngine-->>UI_Component: Return ExecuteResult (phase-based)
UI_Component->>Utility: Generate code share/edit URL
Utility->>Playground: Construct URL with compressed state
UI_Component-->>User: Display results (tabs, output, links)
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
apps:browser-extension
apps:discord-bot
apps:website
dependencies
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
apps/website/src/components/terminal/index.tsx (1)
150-291: Remove commented-out code.Large blocks of commented code should be removed as version control preserves the history. This improves file readability and maintenance.
🧹 Nitpick comments (3)
packages/scripts/package.json (1)
12-12: Version pinning removes caret – doc comment recommendedHard-pinning
@next/envto15.3.5stops silent minor upgrades, which is sensible after the breakage in15.3.6. Consider leaving a one-liner comment in the README or commit message so the next upgrader knows the rationale.apps/discord-bot/src/utilities/make-url.ts (1)
1-2: Track the code consolidation task.The TODO indicates code duplication between browser extension and Discord bot that should be consolidated.
Would you like me to create an issue to track moving these URL utilities to a shared package?
apps/discord-bot/src/handlers/evaluate.ts (1)
48-54: Consider a cleaner approach for handling cross-user edits.Using
Reflect.setto mutate interaction data is a hack that could lead to maintenance issues. Consider handling this logic more explicitly in the control flow.- // HACK: If trying to edit an existing evaluation owned by another user, create new instead - if ( - interaction.rawData.message && - interaction.rawData.message.interaction_metadata?.user.id !== - interaction.user?.id - ) - Reflect.set(interaction.rawData.data, 'custom_id', 'evaluate,new'); + const isOtherUserEdit = + interaction.rawData.message && + interaction.rawData.message.interaction_metadata?.user.id !== interaction.user?.id; + + const treatAsNew = isNew(interaction) || (isEdit(interaction) && isOtherUserEdit); + const treatAsEdit = isEdit(interaction) && !isOtherUserEdit; - if (isNew(interaction)) await interaction.defer(); - if (isEdit(interaction)) await interaction.acknowledge(); + if (treatAsNew) await interaction.defer(); + if (treatAsEdit) await interaction.acknowledge();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
apps/browser-extension/package.json(1 hunks)apps/browser-extension/src/background/index.ts(2 hunks)apps/browser-extension/src/content-script/execution/dialog.tsx(1 hunks)apps/browser-extension/src/content-script/execution/result.tsx(2 hunks)apps/discord-bot/package.json(1 hunks)apps/discord-bot/src/components/evaluate-modal.ts(2 hunks)apps/discord-bot/src/handlers/evaluate.ts(6 hunks)apps/discord-bot/src/utilities/make-url.ts(1 hunks)apps/website/package.json(2 hunks)apps/website/src/components/terminal/index.tsx(2 hunks)package.json(1 hunks)packages/components/package.json(1 hunks)packages/scripts/package.json(1 hunks)packages/shapes/src/execute.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: apteryxxyz
PR: apteryxxyz/Evaluate#95
File: apps/browser-extension/vite-plugins.ts:50-86
Timestamp: 2025-07-16T23:10:36.498Z
Learning: User apteryxxyz prefers if statements without braces when there's a single statement, formatting them as:
if (condition)
statement;
instead of using curly braces around the statement.
apps/browser-extension/package.json (1)
Learnt from: apteryxxyz
PR: apteryxxyz/Evaluate#95
File: apps/browser-extension/vite-plugins.ts:69-69
Timestamp: 2025-07-16T23:10:33.941Z
Learning: In the Evaluate codebase, apteryxxyz prefers to let errors throw naturally in internal Vite plugins rather than adding explicit error handling, as it's acceptable for development tooling to fail fast and surface errors clearly.
🧬 Code Graph Analysis (4)
apps/website/src/components/terminal/index.tsx (3)
packages/components/src/button.tsx (1)
Button(56-56)apps/website/src/components/editor/readonly.tsx (1)
ReadonlyEditor(8-23)apps/website/src/components/terminal/html-preview.tsx (1)
HtmlPreview(6-45)
apps/browser-extension/src/background/index.ts (2)
packages/engine/src/runtimes/getters.ts (1)
getRuntimeDefaultFileName(42-45)packages/engine/src/execute/fetch.ts (1)
executeCode(17-60)
apps/discord-bot/src/handlers/evaluate.ts (3)
packages/engine/src/runtimes/getters.ts (1)
getRuntimeDefaultFileName(42-45)apps/discord-bot/src/utilities/make-url.ts (1)
makeEditCodeUrl(7-24)apps/discord-bot/src/utilities/discord-formatting.ts (1)
codeBlock(71-89)
apps/discord-bot/src/utilities/make-url.ts (2)
packages/engine/src/runtimes/getters.ts (1)
getRuntimeDefaultFileName(42-45)packages/engine/src/compress/index.ts (1)
compress(11-15)
🔇 Additional comments (16)
packages/components/package.json (1)
40-40: Tailwind versions consistent – manual config build check neededAll
package.jsonfiles specify Tailwind CSS 4.1.x (no mismatched versions found):
- packages/components/package.json → ^4.1.11
- No other
package.jsonfiles reference a different Tailwind CSS versionPlease run your local Tailwind build (for example
npx tailwindcss -c tailwind.config.js build) to confirm the config compiles cleanly without warnings.package.json (1)
23-23: Turbo bump LGTM – remember to regenerate the lock-file
2.5.5only contains bug-fixes, so no red flags. Just ensurepnpm-lock.yamlhas been refreshed so CI picks up the new version.apps/browser-extension/package.json (1)
35-35: @babel/traverse patch bump is safeThe 7.28 line only adds TS 5.4 nodes; nothing in the extension relies on internal Babel symbols, so runtime risk is minimal. ✔️
apps/website/package.json (1)
40-40: LGTM: Dependency version pinning for consistencyPinning Next.js and bundle analyzer to exact versions ensures consistent builds across environments and aligns with the monorepo's version consistency strategy.
Also applies to: 58-58
apps/browser-extension/src/content-script/execution/dialog.tsx (1)
58-78: Good conditional rendering improvementWrapping the tabs in a
results.length > 0condition prevents unnecessary DOM rendering when there are no results and aligns with the dialog's open state logic.apps/discord-bot/src/components/evaluate-modal.ts (1)
75-76: Good UX improvements for optional input fieldsChanging to
TextInputStyle.ShortandminLength = 0makes these optional fields more user-friendly. Single-line style is appropriate for command-line arguments, and allowing empty input aligns with their optional nature.Also applies to: 85-86
apps/browser-extension/src/content-script/execution/result.tsx (3)
18-24: Excellent simplification of output logicThe refactored approach directly uses
result.outputwith simple conditional overrides for timeout scenarios. This is much cleaner than the previous memoized display object approach and aligns well with the new execution result schema.
34-34: Improved placeholder text for better UXThe new placeholder message is more user-friendly, clearly explaining that successful execution without output is normal behaviour.
37-38: Good alignment with new schema propertiesUsing
!result.successandresult.outputdirectly from the execution result schema is cleaner and more intuitive than the previous display object approach.apps/browser-extension/src/background/index.ts (2)
73-77: Excellent improvement with dynamic filename determinationUsing
getRuntimeDefaultFileName(runtime.id)with a fallback to'file.code'is much more accurate than hardcoding filenames. This ensures proper file extensions for different programming languages, which can be important for compilation and execution.
84-85: Good simplification of execution metricsThe new approach using
result.compile?.success ?? nullandresult.successis cleaner and aligns with the new execution result schema. This is more intuitive than checking exit codes directly.apps/website/src/components/terminal/index.tsx (1)
31-85: LGTM! Clean refactor to static tab structure.The simplified static JSX structure improves readability and maintainability compared to the previous dynamic tab generation approach. The indicator dots provide clear visual feedback for output presence.
apps/discord-bot/src/utilities/make-url.ts (1)
7-41: LGTM! Well-structured URL generation utilities.The functions appropriately handle runtime-specific and runtime-agnostic URL generation with consistent state compression logic.
packages/shapes/src/execute.ts (2)
36-47: LGTM! Clean phase result schema with useful derived properties.The transform appropriately trims output and provides derived boolean flags for success and expiry status.
55-60: Verify the output selection logic.The current logic shows compile output when
compile.code > 0(compile failure), otherwise shows run output. Is this the intended behaviour? Typically, compile errors should be shown when compilation fails, but the run output should be shown when execution completes (regardless of compile success).Consider if this logic should be:
- output: (r.compile?.code || 0) > 0 ? r.compile?.output : r.run?.output, + output: r.compile && !r.compile.success ? r.compile.output : r.run.output,apps/discord-bot/src/handlers/evaluate.ts (1)
104-116: LGTM! Clean output handling with the new schema.The simplified logic using the new
expiredflags and unifiedoutputproperty improves readability.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores