Repository navigation
[fix] Stop the Use API drawer render loop with two variants [#6708] - #6711
Conversation
The variant-mode "How to use API" drawer kept a variant id and a revision id in state and reconciled them with three effects that each wrote what another read. With more than one variant the reconciliation had no fixed point: the drawer alternated between the two variants forever, and the first click on a language tab turned the loop synchronous and froze the tab with React #185. Make the revision the only selection state and derive the variant from it. The variant now comes from the selected revision entity, which already carries workflow_variant_id, so no list has to arrive before the drawer knows which variant it is showing. One effect remains, and it writes only while nothing is selected, so it runs at most once. Fixes #6708
|
@coderabbitai review |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 SummarySummary by CodeRabbit
Walkthrough
ChangesVariant API drawer
Priority: ⬆️ High Estimated code review effort: 3 (Moderate) | ~20 minutes Severity of issue fixed: High Merge Risk: 🔵 Low · up to The drawer now uses revision-driven selection to prevent multi-variant render loops and includes regression coverage for delayed and missing revisions. The remaining risk is limited to a potentially brittle test assertion that could cause intermittent CI failures without affecting runtime drawer behavior. Sequence Diagram(s)sequenceDiagram
participant VariantUseApiContent
participant WorkflowMolecule
participant QueryAPI
participant LanguageCodeBlock
VariantUseApiContent->>WorkflowMolecule: Read selected revision
VariantUseApiContent->>QueryAPI: Load variants and revisions
QueryAPI-->>VariantUseApiContent: Return revision and variant data
VariantUseApiContent->>VariantUseApiContent: Derive selected variant
VariantUseApiContent->>LanguageCodeBlock: Render selected variant snippet
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The implementation fixes the underlying product bug, but linked issue [ Full details: Out of Scope Changes checkExplanation The product-code refactor and new unit test are outside the explicit scope of [
✨ Finishing Touches📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx (1)
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueShorten the added comments to one line each.
The added blocks use three or four lines per comment. The coding guidelines allow at most one short line per comment. Move the rationale for issue
#6708into the PR description or the test file header, and keep one line here.♻️ Proposed comment trimming
- // The revision is the ONLY selection state here; the variant is DERIVED from it and is - // never stored. Two states that must agree cannot be reconciled by effects that each - // write what another reads: with more than one variant that reconciliation had no fixed - // point and the drawer re-rendered forever (issue `#6708`). + // Revision is the only selection state; the variant is derived (issue `#6708`). const [selectedRevisionId, setSelectedRevisionId] = useState<string | undefined>( initialRevisionId, )- // The drawer is destroyed when it closes, so this only fires when the host swaps the - // opened row while the drawer stays mounted. It is keyed on the prop, not on derived - // state, so it writes once per prop value. + // Adopts a new opened revision while the drawer stays mounted. useEffect(() => {- // With no revision selected yet the drawer defaults to the first variant, and the effect - // below then adopts that variant's latest revision. + // No revision selected yet: default to the first variant. const selectedVariantId = derivedVariantId ?? (selectedRevisionId ? undefined : variants[0]?.id)As per coding guidelines: "Hard rule. At most ONE short line per comment."
Also applies to: 50-52, 94-95
Source: Coding guidelines
web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.loop.test.tsx (1)
216-216: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider relaxing the exact commit budget.
MAX_COMMITSat line 122 already fails the test on a render loop. The extratoBeLessThanOrEqual(6)bound asserts an exact commit count, so any benign added render — a new atom subscription, for example — fails the test without a loop. Raise the bound or drop it and keep the final-slug assertions.♻️ Proposed change
- expect(trajectory.length).toBeLessThanOrEqual(6) + expect(trajectory.length).toBeLessThan(MAX_COMMITS)Also applies to: 230-230
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: 309ada7d-e749-4da1-8ed0-e5f38527f3c7
📒 Files selected for processing (2)
web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.loop.test.tsxweb/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
|
Railway Preview Environment
Updated at 2026-09-09T01:05:33.116Z |
…rable Two defects Codex found in the first pass of the fix. The prop effect and the fallback effect could both see an empty selection in one pass, so the fallback overwrote a revision the prop had just supplied. The fallback now writes through a functional update that keeps whatever is there. A revision id that resolves to nothing, a deleted revision behind a shared link, left the drawer spinning for ever. Read the revision query so pending is told apart from unresolvable, and fall back to the first variant's latest revision when the requested one cannot be resolved. Also read the thin revision refs for the variant instead of resolving every revision entity, and accept the variant_slug alias the schema allows.
|
Pushed two follow-up fixes from a Codex xhigh review, with tests that fail on the previous commit. Re-review please @coderabbitai |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx (1)
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShorten the added comments to one short line each.
The coding guidelines allow at most one short line per comment. The added blocks at Lines 40-43, 97-98, 109-110, 115-116, 129-130 and 144-145 each use several lines of explanation. Move the reasoning to the PR description or the issue, and keep one line at each site.
As per coding guidelines: "Hard rule. At most ONE short line per comment."
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: ad42f5ac-5ab7-4a6e-85d6-7e9b6d8d47ae
📒 Files selected for processing (2)
web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.loop.test.tsxweb/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx (1)
40-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce each inline comment to one short line. The changed code adds multi-line explanatory comments. The coding guidelines set a hard rule of at most one short line per comment. Move the longer rationale to the PR description or the issue link.
web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx#L40-L43: replace the four-line rationale with one line, for example// Revision is authoritative; variant is derived (issue#6708).web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx#L50-L52: reduce to one line describing the prop-keyed initialization.web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx#L82-L83: reduce to one line.web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx#L97-L98: reduce to one line.web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx#L109-L110: reduce to one line.web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx#L115-L116: reduce to one line.web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx#L129-L130: reduce to one line.web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx#L144-L145: reduce to one line.web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.loop.test.tsx#L76-L81: reduce the block comment to one line.web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.loop.test.tsx#L144-L147: reduce the block comment to one line.As per coding guidelines: "Hard rule. At most ONE short line per comment."
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Advanced
Run ID: 9185895a-c0ef-4a1d-8e7c-20b951e2ebfb
📒 Files selected for processing (2)
web/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.loop.test.tsxweb/oss/src/components/DeploymentsDashboard/assets/VariantUseApiContent.tsx
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.
Closes #6708
Context
Open an app that has more than one variant, go to the Variants registry, select a row and click Use API. The "How to use API" drawer enters an infinite React render loop the moment it opens. The snippet flips between the two variants many times a second. Click any language tab and the loop turns synchronous: React throws #185 continuously and the tab is frozen until it is reloaded. The drawer opened from the Deployments tab is not affected. Issue: #6708
VariantUseApiContentkept two pieces of state that must agree,selectedVariantIdandselectedRevisionId, and reconciled them with three effects that each wrote what another read. The three effects run in one pass, so each one sees a snapshot where the others have not applied yet, and each re-asserts its own view. With one variant all three agree and it settles. With two variants the branchinitialRevision.workflow_variant_id !== selectedVariantIdbecomes reachable and they oscillate.Here is the cycle, logged from the reproduction:
This is not a v0.115.3 regression. The three effects have been in the file since March 2026 and last changed in May 2026. What is new is the acceptance test's drawer wait, which made the freeze reachable in CI instead of failing earlier on a stability check.
Changes
The revision is now the authoritative selection. The parent variant is derived from it and never stored.
Before, the variant came from a workflow-wide revision list that arrives late, and three effects pushed the two states at each other:
After, the drawer reads the selected revision entity, which already carries
workflow_variant_id, so nothing has to arrive before the drawer knows which variant it is showing:Two writers remain and neither can fight the other. One is keyed on the
initialRevisionIdprop. The other runs only while there is no usable selection, and it writes through a functional update so it cannot overwrite a revision the prop supplied in the same pass.The drawer also tells "still fetching" apart from "this revision does not exist". A stale id, for example a deleted revision behind a shared link, now falls back to the first variant's latest revision instead of spinning for ever.
Two smaller consequences. The drawer no longer mounts the workflow-wide revisions query, and it reads the thin revision refs for the variant rather than resolving every revision entity, so it opens with fewer queries than before. While the opened revision is still being fetched the drawer shows its spinner.
Tests
VariantUseApiContent.loop.test.tsxmounts the drawer content against the real workflow atoms and real react-query, with two variants and the arrival order that opens the race. On the old component it records 62 renders alternatingmyapp.defaultandmyapp.e2eand React reports "Maximum update depth exceeded". On the new component the trajectory is two renders, it never builds a snippet for the variant the user did not open, and rendering stops once the queries resolve. Four cases: the loop, the drawer opened without a revision, a revision the prop supplies in the same pass as the fallback list, and a revision that does not exist. The last two fail on the first pass of this fix and pass now.pnpm exec tsc --noEmitinweb/oss: 0 errors.pnpm run lint-fixinweb/oss: clean. Fullweb/ossunit suite: 62 files, 531 tests, all pass.--workers=1 --retries=0. "Registry: use API snippets > should show variant TypeScript snippet" passed all three times, in 5.2 s, 3.4 s and 3.2 s. It used to hang for the full 60 s timeout. The one failure in each run is "Should run single view variant for chat", which times out waiting for an LLM response because that stack has no provider key. It is unrelated and fails identically on every run. Both use-API tests passed again on the final code.PR #6709 quarantines the acceptance test for this bug. It is still open and not merged, so there is no
fixmeonrelease/v0.115.3to revert. Close #6709 rather than merging it.A Codex review at xhigh found the two selection defects listed above, which are now fixed and covered. It also reported a separate pre-existing oscillation in
TraceDrawerContent.tsxin the observability package, where a localselectedspan and the globalactiveSpanIdcopy into each other. That is not this component and not these atoms, so it is out of scope here and worth its own issue.What to QA