Conversation
… system, experiments native-hosted)
Reviewer's GuideMigrates the Paper Ops, System, and Experiments tabs from research_ui iframe delegation to native-hosted React panes in the desktop shell by wiring them into MainContent routing while reusing existing tab render functions, and updates product-surface documentation accordingly. Sequence diagram for native rendering of Paper Ops tabsequenceDiagram
actor User
participant DesktopApp
participant MainContent
participant PaperOpsPane
participant QuantLabContext
participant renderPaperOpsTab
participant LegacyAppJS
User->>DesktopApp: select tab kind paper
DesktopApp->>MainContent: render with activeTab.kind paper
MainContent->>MainContent: detect native kind (paper)
MainContent->>PaperOpsPane: mount component with tab
PaperOpsPane->>QuantLabContext: useQuantLabContext()
QuantLabContext-->>PaperOpsPane: state (snapshot, runs, launchControl, decisionStore, decisionCompare)
PaperOpsPane->>PaperOpsPane: build ctx for renderPaperOpsTab
PaperOpsPane->>renderPaperOpsTab: renderPaperOpsTab(ctx)
renderPaperOpsTab-->>PaperOpsPane: html string
PaperOpsPane->>PaperOpsPane: setHtml(html)
PaperOpsPane->>DesktopApp: render div with dangerouslySetInnerHTML
DesktopApp->>LegacyAppJS: bind legacy event listeners to rendered DOM
User->>DesktopApp: click action button in Paper Ops surface
DesktopApp->>LegacyAppJS: handle click via event delegation
LegacyAppJS-->>DesktopApp: perform side effects (launch, navigation, updates)
Class diagram for new native panes and routingclassDiagram
class MainContent {
+activeTab
+allTabs
+onTabChange
-legacyContainerRef
+useEffect()
+render()
}
class PaperOpsPane {
+tab
-html
+useEffect()
-buildContext(state)
-handleRenderError(error)
+render()
}
class SystemPane {
+tab
-html
+useEffect()
-buildContext(state)
-handleRenderError(error)
+render()
}
class ExperimentsPane {
+tab
-html
+useEffect()
-buildContext(state, tab)
-handleRenderError(error)
+render()
}
class QuantLabContext {
+state
+useQuantLabContext()
}
class TabRenderers {
+renderPaperOpsTab(ctx)
+renderSystemTab(ctx)
+renderExperimentsTab(tab, ctx)
}
MainContent --> PaperOpsPane : renders when activeTab.kind paper
MainContent --> SystemPane : renders when activeTab.kind system
MainContent --> ExperimentsPane : renders when activeTab.kind experiments
PaperOpsPane --> QuantLabContext : reads state via hook
SystemPane --> QuantLabContext : reads state via hook
ExperimentsPane --> QuantLabContext : reads state via hook
PaperOpsPane --> TabRenderers : calls renderPaperOpsTab
SystemPane --> TabRenderers : calls renderSystemTab
ExperimentsPane --> TabRenderers : calls renderExperimentsTab
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In SystemPane and ExperimentsPane, the useEffect dependency arrays are missing some of the state fields used inside the effect (e.g.,
state.launchControl?.jobs,state.decision,state.decisionStore,state.sweepDecision,state.runs), which can lead to stale HTML when those parts of state change; consider adding all referenced state slices to the dependencies. - The three new panes (PaperOpsPane, SystemPane, ExperimentsPane) share a similar pattern of pulling from QuantLabContext, building a ctx object, and using dangerouslySetInnerHTML; you might factor out a small shared helper or hook to reduce duplication and keep the bridging logic consistent across surfaces.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In SystemPane and ExperimentsPane, the useEffect dependency arrays are missing some of the state fields used inside the effect (e.g., `state.launchControl?.jobs`, `state.decision`, `state.decisionStore`, `state.sweepDecision`, `state.runs`), which can lead to stale HTML when those parts of state change; consider adding all referenced state slices to the dependencies.
- The three new panes (PaperOpsPane, SystemPane, ExperimentsPane) share a similar pattern of pulling from QuantLabContext, building a ctx object, and using dangerouslySetInnerHTML; you might factor out a small shared helper or hook to reduce duplication and keep the bridging logic consistent across surfaces.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
6 tasks
Whiteks1
deleted the
codex/issue-410-migrate-paper-ops-system-experiments
branch
6 tasks
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
Migrate Paper Ops, System, and Experiments surfaces from
research_uiiframe delegation to native-hosted surfaces in the canonical desktop shell.Framing: This is a native-hosted migration using existing canonical render logic, not a deep React rewrite.
Changes
Components Created
renderPaperOpsTab(). Broker boundary, decision queue, operational continuity.renderSystemTab(). Runtime diagnostics, workspace status, logs.renderExperimentsTab(). Config catalog, sweeps, decision tracking.Routing Updated
paper,system,experimentstab kinds. Updated legacy fallback condition to exclude these kinds from iframe delegation.Documentation Updated
Scope
In:
Out:
Validation Status
Passed
npm run typecheck— no type errorsnpm run build— clean build (artifacts identical to origin/main)node --test tests/snapshot-status.test.mjs— 4/4 tests passgit diff --check— no whitespace violationsKnown Issue (Pre-Existing, Not From #410)
Shell Bootstrap Smoke Regression — Refs #427
Both
npm run smoke:fallbackandnpm run smoke:real-pathfail with:Status: Pre-existing on
origin/main. Identical failure state confirmed:origin/main(b8ebe19): failure with exact same JSON stateRoot Cause: Shell initialization/bootstrap, unrelated to surface routing logic. Likely from #409 native run detail work.
Impact: Blocks strict DoD compliance (DoD expects smoke green), but does not represent a #410-introduced regression.
Tracking: Issue #427 — "fix(desktop): resolve shell bootstrap hasShell=false smoke regression"
Mergeability
#410 is mergeable with explicit acceptance that:
DoD Compliance: Conditional. Passes all direct checks; smoke failure is pre-existing and tracked separately via #427. Merge only if pre-existing shell bootstrap issue (#427) is accepted as known blocker outside this slice.