Skip to content

Fix custom sidebar function return and file-scope bindings - #9501

Closed
austinywang wants to merge 2 commits into
mainfrom
issue-7945-custom-sidebar-interpreter-return-inside
Closed

austinywang wants to merge 2 commits into
mainfrom
issue-7945-custom-sidebar-interpreter-return-inside

Conversation

@austinywang

@austinywang austinywang commented Aug 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • bind top-level variable declarations into the interpreter's root environment so user functions can resolve file-scope constants
  • propagate explicit return through view-position if, switch, and for evaluation so a view function exits instead of appending fallthrough nodes
  • add focused regression coverage for both reported snippets

Fixes #7945

Testing

  • Red on test-only commit ca520f1703 on fleet builder aws-m4pro-6:
    • swift test --filter returnExitsViewFunctionBeforeFallthrough failed because the rendered child was a nested node containing both return paths
    • swift test --filter topLevelLetIsVisibleInsideFunction failed with a-b-c instead of X:a-b-c
  • Green on fix commit 7b168f0fd7 on the same fleet builder:
    • both focused swift test --filter ... commands passed
    • full swift test passed all 68 tests in CmuxSwiftRender
  • Dev build: cloud fleet path only (no local fallback): reload-cloud.sh --tag sym7945 --builder fleet --pool-tag tag:macbuilder --slot cmuxs-mac-mini-2.1 --launch completed with BUILD_OK
  • Tagged runtime verification over /tmp/cmux-debug-sym7945.sock:
    • sidebar validate, sidebar select, and sidebar reload succeeded for both repro sidebars
    • debug.window.screenshot showed only EARLY after the early return, with no FALLTHROUGH
    • the file-scope constant repro rendered X:a-b-c
    • quit the tagged app with pkill -f "DerivedData/cmux-sym7945" and removed its stale socket

Localization audit: no user-facing strings changed.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes the custom sidebar interpreter in CmuxSwiftRender so explicit returns exit view functions and file-scope constants are visible inside user functions.

  • Bug Fixes
    • Bind top-level let/var declarations into the root eval environment so helper functions can read file-scope constants.
    • Propagate explicit return through view-position if, switch, and for so a view function returns early instead of appending fallthrough nodes.

Written for commit 7b168f0. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes

    • Fixed view helper functions so explicit returns correctly stop further evaluation.
    • Improved return handling across loops, conditionals, switches, and nested view helpers.
    • Top-level constants are now available inside user-defined view functions.
  • Tests

    • Added coverage for early returns and access to file-scope constants.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 50ee0f3c-f8e7-4241-a3ba-2461cb46c6a3

📥 Commits

Reviewing files that changed from the base of the PR and between 9decec5 and 7b168f0.

📒 Files selected for processing (2)
  • Packages/macOS/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift
  • Packages/macOS/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swift

📝 Walkthrough

Walkthrough

The interpreter now binds top-level variables before evaluation and propagates explicit returns through view helpers, loops, conditionals, and switches. Tests cover early view returns and file-scope bindings used inside functions.

Changes

Interpreter semantics

Layer / File(s) Summary
Top-level scope binding
Packages/macOS/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift, Packages/macOS/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swift
The root environment now receives file-scope variable bindings. Tests verify that a top-level MARK binding is visible inside a user-defined function.
View return propagation
Packages/macOS/CmuxSwiftRender/Sources/CmuxSwiftRender/SwiftViewInterpreter.swift, Packages/macOS/CmuxSwiftRender/Tests/CmuxSwiftRenderTests/SwiftViewInterpreterTests.swift
ViewBlockResult distinguishes completed evaluation from explicit returns. User-defined view helpers, loops, conditionals, and switches propagate returned nodes and stop later statements. Tests verify early return behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SwiftViewInterpreter
  participant evalItems
  participant ViewBlockResult
  SwiftViewInterpreter->>evalItems: evaluate user-defined view helper
  evalItems->>ViewBlockResult: evaluate statements
  ViewBlockResult-->>evalItems: returned node or completed nodes
  evalItems-->>SwiftViewInterpreter: render helper result
Loading
🚥 Pre-merge checks | ✅ 25
✅ Passed checks (25 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the fixes for custom sidebar function returns and file-scope bindings.
Description check ✅ Passed The description provides a clear summary, detailed testing results, runtime verification, and issue linkage.
Linked Issues check ✅ Passed The changes implement both requirements from issue #7945 and add regression tests for each reported interpreter defect.
Out of Scope Changes check ✅ Passed The changes are limited to interpreter behavior and focused regression tests related to issue #7945.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Cmux Swift Actor Isolation ✅ Passed placeholder
Cmux Swift Blocking Runtime ✅ Passed The PR adds no blocking or timing primitives. The existing DispatchSemaphore thread join is unchanged, and all added production and test lines contain no flagged synchronization.
Cmux Browser Automation Off-Main ✅ Passed The two-commit PR changes only SwiftViewInterpreter.swift and its tests; no browser socket command, WebKit wait, worker router, or policy file changed, so this check is not applicable.
Cmux Expensive Synchronous Load ✅ Passed The two-commit diff adds only interpreter semantics and tests; it adds no agent-history loader, filesystem/JSON parse, or interactive-path load. Existing onLargeStack code is unchanged.
Cmux Cache Substitution Correctness ✅ Passed The PR only changes SwiftViewInterpreter scope/return evaluation and tests; it introduces no cached substitution or persistence, history, undo, or snapshot read.
Cmux No Hacky Sleeps ✅ Passed The cumulative PR diff changes only two Swift files; no covered non-Swift runtime files or added sleep, timer, polling, or fixed-delay synchronization patterns were found.
Cmux Algorithmic Complexity ✅ Passed The production diff adds one linear top-level declaration scan and return-state propagation; it adds no nested collection scan, repeated sorting/filtering, join, or slower batch algorithm.
Cmux Swift Concurrency ✅ Passed The two-commit diff adds scope/return logic and regression tests only; it adds no Dispatch, Combine, completion-handler, or fire-and-forget Task patterns.
Cmux Swift @Concurrent ✅ Passed The PR adds only synchronous interpreter logic; the diff has no async/await, nonisolated, @concurrent, actor-isolation, or production call-site changes. Existing onLargeStack remains explicit synch...
Cmux Swift Package Boundaries ✅ Passed The production change is in the existing CmuxSwiftRender SwiftPM target, with a dedicated CmuxSwiftRenderTests target and no AppKit, SwiftUI, Ghostty, or app-global dependency.
Cmux Swiftpm Lockfiles ✅ Passed PR diff contains only CmuxSwiftRender source and test files; no Package.swift, Package.resolved, .gitignore, workflow, or Xcode project changes. The sole ignore is vendored bonsplit policy.
Cmux Swift Logging ✅ Passed The production Swift diff only adds scope and return propagation; it adds no print, debugPrint, dump, NSLog, file/stdout logging, Logger, or sensitive-data logging.
Cmux User-Facing Error Privacy ✅ Passed The cumulative diff changes interpreter scope/return control flow and adds regression tests; it adds no user-facing errors, alerts, output, recovery copy, or sensitive diagnostics.
Cmux Full Internationalization ✅ Passed The diff changes interpreter logic and adds regression-test fixtures only; it adds no production user-facing text, localization keys, catalogs, web messages, or locale data.
Cmux Swiftui State Layout ✅ Passed The diff changes interpreter environments and return propagation only; it adds no SwiftUI state wrappers, GeometryReader, row store references, or render-time SwiftUI state writes.
Cmux Architecture Rethink ✅ Passed The PR adds only local environment binding and explicit ViewBlockResult return propagation; it adds no timing, locks, observers, side channels, duplicate wiring, or UI lifecycle owners. The existin...
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed The PR changes only SwiftViewInterpreter logic and interpreter tests; the diff adds no NSWindow, NSPanel, WindowGroup, window identifier, or close-shortcut ownership code.
Cmux Source Artifacts ✅ Passed The PR changes only the intentional Swift interpreter source and its Swift test file; no logs, caches, screenshots, temp directories, build output, or other artifact paths are added.
Cmux No Test Or Debug Seam In Production Source ✅ Passed The production diff adds only private interpreter logic in SwiftViewInterpreter.swift; no DEBUG guard, debug/test-named member, visibility widening, or test accessor was added.
Cmux No Ambient Global State ✅ Passed The production diff adds only private instance methods and a nested result enum inside existing SwiftViewInterpreter; it adds no new top-level API, mutable global, static namespace, or singleton.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-7945-custom-sidebar-interpreter-return-inside

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lawrencecchen lawrencecchen added the stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening. label Sep 23, 2026
@github-project-automation github-project-automation Bot moved this from Todo to Done in cmux backlog Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale-revisit Closed after 30+ days without activity; preserved for possible revisit or reopening.

Projects

None yet

3 participants