Repository navigation
feat(playground): sort functions by workflow size with numeric badges - #4302
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughFunction metadata now includes rendered signatures and source positions across Rust and TypeScript interfaces. The playground correlates CFG responses with request IDs, computes workflow-node counts, and supports stable function sorting with metadata tooltips. ChangesFunction metadata and sidebar enhancements
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ExecutionPanel
participant WorkerProtocol
participant BamlWasmRuntime
participant PlaygroundServer
participant FunctionSidebar
ExecutionPanel->>WorkerProtocol: send CFG request with requestId
WorkerProtocol->>BamlWasmRuntime: forward function and requestId
BamlWasmRuntime->>PlaygroundServer: request CFG with requestId
PlaygroundServer-->>WorkerProtocol: return CFG result with requestId
WorkerProtocol-->>ExecutionPanel: deliver correlated CFG result
ExecutionPanel->>FunctionSidebar: pass current workflowNodeCounts
FunctionSidebar->>FunctionSidebar: sort and display function metadata
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
sxlijin
force-pushed
the
agent/sort-functions-with-numeric-node-badges
branch
from
July 31, 2026 19:57
3912667 to
d12af66
Compare
sxlijin
changed the title
sxlijin
marked this pull request as ready for review
July 31, 2026 19:58
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d12af66f7c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Binary size checks passed✅ 7 passed
Generated by |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
typescript2/pkg-playground/src/FunctionSidebar.tsx (1)
367-425: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider hoisting
TooltipProviderabove the per-row and per-button instances.Each function leaf row (Line 372) and the sort button (Line 541) wraps itself in its own
TooltipProvider. Radix UI's docs describeTooltipProvideras intended to wrap the app, or at least a section, once — not per individual tooltip. Wrapping every row individually works, but it forfeits the shared cross-tooltip delay/skip-delay behavior and creates one extra context provider per rendered function, which is measurable for projects with many functions.Move a single
TooltipProviderup to wrap the whole functions list (and the sort control) once, and remove the per-instance providers.Also applies to: 523-613
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@typescript2/pkg-playground/src/FunctionSidebar.tsx` around lines 367 - 425, Hoist a single TooltipProvider to wrap the entire functions list and sort control, then remove the per-row provider around the function leaf rendering and the per-button provider around the sort control. Keep each Tooltip and TooltipTrigger/TooltipContent structure unchanged while ensuring both FunctionSidebar leaf rows and the sort button share the provider context.baml_language/crates/baml_project/src/symbols.rs (1)
233-257: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider caching the per-file
LineIndexinstead of rebuilding it per function.
function_source_positionbuilds a freshLineIndex::new(source_file.text(db))on every call. When a file contains many functions,list_functions_with_metadatarecomputes the same line index once per function in that file. Building a per-file cache (or making this a salsa-tracked query keyed on the file) avoids the repeated linear scan.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_project/src/symbols.rs` around lines 233 - 257, Update function_source_position to reuse a cached or Salsa-tracked LineIndex keyed by source_file instead of constructing LineIndex::new(source_file.text(db)) for every function. Preserve the existing offset-to-position and fallback behavior while ensuring functions from the same file share one index.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@typescript2/pkg-playground/src/FunctionSidebar.tsx`:
- Line 338: Add aria-hidden="true" to the isInternal origin badge span in the
FunctionSidebar button rendering, matching the existing workflowNodeCount badge
treatment so the visible function label remains the button’s sole accessible
name.
---
Nitpick comments:
In `@baml_language/crates/baml_project/src/symbols.rs`:
- Around line 233-257: Update function_source_position to reuse a cached or
Salsa-tracked LineIndex keyed by source_file instead of constructing
LineIndex::new(source_file.text(db)) for every function. Preserve the existing
offset-to-position and fallback behavior while ensuring functions from the same
file share one index.
In `@typescript2/pkg-playground/src/FunctionSidebar.tsx`:
- Around line 367-425: Hoist a single TooltipProvider to wrap the entire
functions list and sort control, then remove the per-row provider around the
function leaf rendering and the per-button provider around the sort control.
Keep each Tooltip and TooltipTrigger/TooltipContent structure unchanged while
ensuring both FunctionSidebar leaf rows and the sort button share the provider
context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9300f077-71e3-4486-a992-655f471b6033
⛔ Files ignored due to path filters (39)
typescript2/pkg-playground/docs/workflow-node-count-number-badges/aie-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/aie-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/aie-wide-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/aie-wide-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-c-compiler-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-c-compiler-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-c-compiler-wide-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-c-compiler-wide-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-deep-research-demo-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-deep-research-demo-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-deep-research-demo-wide-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-deep-research-demo-wide-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-email-agent-demo-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-email-agent-demo-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-email-agent-demo-wide-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-email-agent-demo-wide-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-text-to-sql-demo-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-text-to-sql-demo-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-text-to-sql-demo-wide-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/baml-text-to-sql-demo-wide-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/bamlcode-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/bamlcode-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/bamlcode-wide-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/bamlcode-wide-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/claude-session-quality-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/claude-session-quality-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/claude-session-quality-wide-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/claude-session-quality-wide-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/function-hover-details-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/function-sort-menu-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/google-adk-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/google-adk-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/google-adk-wide-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/google-adk-wide-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/namespace-rows-no-folder-icons-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/python-discord-bot-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/python-discord-bot-before.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/python-discord-bot-wide-after.pngis excluded by!**/*.pngtypescript2/pkg-playground/docs/workflow-node-count-number-badges/python-discord-bot-wide-before.pngis excluded by!**/*.png
📒 Files selected for processing (11)
baml_language/crates/baml_project/src/lib.rsbaml_language/crates/baml_project/src/symbols.rsbaml_language/crates/bex_project/src/bex_lsp/mod.rsbaml_language/crates/bex_project/src/bex_lsp/multi_project/mod.rsbaml_language/crates/bridge_wasm/src/wasm_playground.rstypescript2/app-vscode-webview/src/FunctionSidebar.interaction.test.tsxtypescript2/pkg-playground/src/ExecutionPanel.tsxtypescript2/pkg-playground/src/FunctionSidebar.tsxtypescript2/pkg-playground/src/function-sidebar-tree.tstypescript2/pkg-playground/src/function-sidebar-workflow-size.test.tstypescript2/pkg-playground/src/worker-protocol.ts
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
baml_language/crates/bridge_wasm/src/wasm_playground.rs (1)
359-366: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winMake optional playground fields compatible with older wire shapes.
wasm_playground::FunctionInfodeclaressignature: Stringandsource_position: FunctionSourcePosition, while the mapping readsf.signatureandf.source_position.*directly. Mark these fields asOption<T>with serialization defaults/defaults so payloads from older toolchains or runtimes that omit metadata still deserialize; treatNonein the UI as “schema not available”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/bridge_wasm/src/wasm_playground.rs` around lines 359 - 366, Update wasm_playground::FunctionInfo and its mapping to make signature and source_position optional, including serde defaults and Rust defaults so older payloads missing metadata deserialize successfully. Adjust the source-position field mapping and UI consumers to handle None as “schema not available,” while preserving existing behavior when metadata is present.
🧹 Nitpick comments (1)
typescript2/pkg-playground/src/ports/WebSocketRuntimePort.test.ts (1)
238-287: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd coverage for the omitted-
requestIdcompatibility path.The new test only covers the case where
requestIdis present. Add a case that postsrequestControlFlowGraphwithoutrequestIdand confirms the serialized frame omits the field, and a case where the server sendscontrolFlowGraphResultwithoutrequestIdand confirms it's still delivered to the consumer. This is the actual backward-compatibility scenario the optional field exists to support.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@typescript2/pkg-playground/src/ports/WebSocketRuntimePort.test.ts` around lines 238 - 287, Extend the WebSocketRuntimePort control-flow graph tests around the existing optional request ID case to cover omitted requestId compatibility: verify a requestControlFlowGraph posted without requestId serializes without that field, and verify a controlFlowGraphResult received without requestId is delivered unchanged to the consumer. Reuse the existing FakeWebSocket setup and assertions in the test block.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@baml_language/crates/bridge_wasm/src/wasm_playground.rs`:
- Around line 359-366: Update wasm_playground::FunctionInfo and its mapping to
make signature and source_position optional, including serde defaults and Rust
defaults so older payloads missing metadata deserialize successfully. Adjust the
source-position field mapping and UI consumers to handle None as “schema not
available,” while preserving existing behavior when metadata is present.
---
Nitpick comments:
In `@typescript2/pkg-playground/src/ports/WebSocketRuntimePort.test.ts`:
- Around line 238-287: Extend the WebSocketRuntimePort control-flow graph tests
around the existing optional request ID case to cover omitted requestId
compatibility: verify a requestControlFlowGraph posted without requestId
serializes without that field, and verify a controlFlowGraphResult received
without requestId is delivered unchanged to the consumer. Reuse the existing
FakeWebSocket setup and assertions in the test block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d3a8c50b-fd64-4e11-a26a-624161ba58d5
📒 Files selected for processing (13)
baml_language/crates/baml_lsp_server/src/playground_server.rsbaml_language/crates/baml_lsp_server/src/playground_ws.rsbaml_language/crates/bex_project/src/bex_lsp/mod.rsbaml_language/crates/bex_project/src/bex_lsp/multi_project/mod.rsbaml_language/crates/bridge_wasm/src/lib.rsbaml_language/crates/bridge_wasm/src/wasm_playground.rstypescript2/app-promptfiddle/src/playground/baml-lsp-worker.tstypescript2/app-vscode-webview/src/ExecutionPanel.strict-mode.test.tsxtypescript2/app-website/playground/baml-worker.tstypescript2/pkg-playground/src/ExecutionPanel.tsxtypescript2/pkg-playground/src/ports/WebSocketRuntimePort.test.tstypescript2/pkg-playground/src/ports/WebSocketRuntimePort.tstypescript2/pkg-playground/src/worker-protocol.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- baml_language/crates/bex_project/src/bex_lsp/multi_project/mod.rs
- baml_language/crates/bex_project/src/bex_lsp/mod.rs
- typescript2/pkg-playground/src/worker-protocol.ts
Contributor
Author
|
Follow-up on the two top-level CodeRabbit notes from the previous review:
|
This branch was previously deployed
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
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
Call graph node countand naturalAlphanumericordering, so numeric segments compare numerically (Function2beforeFunction10) and case-equivalent ties retain source order. The selection persists while the sidebar remains mounted.file at line:column, and call-graph node count.Linear: B-1109 — how to navigate the function list?
Testing
pnpm format:ci --since=origin/canary --reporter=github— passed (warnings only).pnpm --filter @b/pkg-playground test— 155 tests passed.pnpm --filter @b/pkg-playground typecheck— passed.pnpm --filter app-vscode-webview test:unit:run— 30 tests passed.pnpm --filter app-vscode-webview test:browser:run— 2 tests passed.pnpm --filter app-vscode-webview typecheck— passed.pnpm --filter app-vscode-webview build— passed.baml playgroundverification confirmedAlphanumeric, omission of the one-node inline badge, and retention ofCall graph nodes: 1in the tooltip.Screenshots
The original PR description contained broad before/after coverage for nine projects and two sidebar widths. These focused captures cover the requested follow-up behavior using the production playground component.
Real
baml-demos/aieprojectCaptured from the real
~/work-repos/baml-demos/aieproject using this PR's Rust playground server and built webview. It shows all 94 functions, theagentnamespace expanded in the default node-count order, numeric badges only for functions with more than one node, one-node rows without badges, namespace chevrons without folder icons, and the open sort menu.Function row hover details
The one-node function has no inline count badge, while hover still exposes the full compiler signature, source location, and
Call graph nodes: 1.Sort order control
Namespace rows without folder icons
The updated rows retain only their disclosure chevrons:
Summary by CodeRabbit
New Features
UI Improvements
Bug Fixes