fix(FUX-039): staff-engineer review fixes for FUX-022 runtime-fallback UI patch#1963
Conversation
…k UI patch Addresses 3 real structural issues flagged in the FUX-022 pre-landing review of PR Runfusion#1957 (finding 4, a lint-breaking eslint-disable comment, was independently confirmed already resolved on main — no code change needed): 1. resolvePluginRuntime(): the found-but-failed-to-init branch (plugin registration exists, but pluginContext/createRuntimeContext(...) comes back falsy) mislabeled the failure as reason: "not_found" (indistinguishable from "never registered"), instead of reason: "init_error". Fixed the return value and updated/extended the existing unit test suite to assert all 3 FallbackReason values are independently reachable and pairwise-distinct. 2. ActiveAgentsPanel and AgentsView hardcoded isInViewport={true} when rendering RuntimeFallbackBadge on their card surfaces, defeating the viewport-gated polling the hook was designed for (every card would poll regardless of visibility). Wired real IntersectionObserver-based viewport gating: ActiveAgentsPanel's LiveAgentCard now tracks its own visibility; AgentsView uses one shared IntersectionObserver instance keyed per card ("board:{id}" / "list:{id}") so the two surfaces never share visibility state. Added desktop + mobile regression tests asserting polling starts/stops correctly as cards enter/leave the viewport, plus a pre-existing bug fix (RuntimeFallbackBadge's useToast() call needed a ToastProvider ancestor in tests that lacked one). 3. useRuntimeFallbackStatus's toast-dedupe key lived only in a per-hook-instance useRef, so multiple simultaneously-mounted card surfaces polling the same taskId (e.g. a task's board card and list card both visible at once) would each independently fire their own toast for the same fallback session. Replaced with a module-level shared claim store (claimToastOnce, bounded to 500 entries with oldest-first eviction) so only the first instance to observe a given eventId, process-wide, fires the toast. Added a cross-instance regression test asserting exactly one toast fires across two simultaneously-mounted badge instances for the same task. All findings were independently re-verified against a fresh bootstrap of main (not trusting a prior unmerged branch attempt) before fixing.
📝 WalkthroughWalkthroughAdds IntersectionObserver-based viewport gating so RuntimeFallbackBadge only polls/renders when its card is visible in ActiveAgentsPanel and AgentsView (board/list). Introduces cross-instance toast deduplication in useRuntimeFallbackStatus. Separately, corrects runtime-resolution's fallback reason from "not_found" to "init_error" for failed context creation. ChangesRuntimeFallbackBadge viewport gating and toast dedupe
Estimated code review effort: 3 (Moderate) | ~25 minutes Runtime Resolution Fallback Reason Correction
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR addresses four staff-engineer review findings from the FUX-022 runtime-fallback feature. The bug fixes are well-targeted and correctly implemented:
Confidence Score: 4/5Safe to merge — all four bug fixes are logically correct, well-isolated, and backed by targeted regression tests covering both desktop and mobile surfaces. The core logic in all changed files is sound. The No files require special attention for correctness. The style findings touch Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Card as LiveAgentCard / AgentCard
participant IO as IntersectionObserver
participant Hook as useRuntimeFallbackStatus
participant Claim as claimToastOnce (module-level)
participant API as /api/tasks/:id/runtime-fallback
participant Toast as useToast
Card->>IO: observe(cardRef)
IO-->>Card: "isIntersecting=false (no poll yet)"
IO-->>Card: "isIntersecting=true"
Card->>Hook: "enabled=true, taskId"
Hook->>API: fetchTaskRuntimeFallback(taskId)
API-->>Hook: "showFallbackBadge=true, eventId=audit-1, runtimeHint=hermes"
Hook->>Claim: claimToastOnce(audit-1)
alt First instance to observe eventId
Claim-->>Hook: true (claim won)
Hook->>Toast: addToast(hermes unavailable)
else Sibling card instance races same eventId
Claim-->>Hook: false (already claimed)
Hook-->>Card: "shouldToastNow=false, badge still shown"
end
Note over Hook,API: Every 30s while enabled=true
Hook->>API: fetchTaskRuntimeFallback(taskId)
API-->>Hook: same eventId
Hook->>Claim: claimToastOnce(audit-1) returns false, no duplicate toast
IO-->>Card: "isIntersecting=false"
Hook->>Hook: setStatus(IDLE), clearInterval
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Card as LiveAgentCard / AgentCard
participant IO as IntersectionObserver
participant Hook as useRuntimeFallbackStatus
participant Claim as claimToastOnce (module-level)
participant API as /api/tasks/:id/runtime-fallback
participant Toast as useToast
Card->>IO: observe(cardRef)
IO-->>Card: "isIntersecting=false (no poll yet)"
IO-->>Card: "isIntersecting=true"
Card->>Hook: "enabled=true, taskId"
Hook->>API: fetchTaskRuntimeFallback(taskId)
API-->>Hook: "showFallbackBadge=true, eventId=audit-1, runtimeHint=hermes"
Hook->>Claim: claimToastOnce(audit-1)
alt First instance to observe eventId
Claim-->>Hook: true (claim won)
Hook->>Toast: addToast(hermes unavailable)
else Sibling card instance races same eventId
Claim-->>Hook: false (already claimed)
Hook-->>Card: "shouldToastNow=false, badge still shown"
end
Note over Hook,API: Every 30s while enabled=true
Hook->>API: fetchTaskRuntimeFallback(taskId)
API-->>Hook: same eventId
Hook->>Claim: claimToastOnce(audit-1) returns false, no duplicate toast
IO-->>Card: "isIntersecting=false"
Hook->>Hook: setStatus(IDLE), clearInterval
Reviews (1): Last reviewed commit: "fix(FUX-039): staff-engineer review fixe..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const cardRef = useRef<HTMLDivElement>(null); | ||
| // FUX-039: real viewport gating (matching TaskCard.tsx's pattern) so | ||
| // RuntimeFallbackBadge's 30s poll only runs while this card is actually | ||
| // visible, rather than hardcoding isInViewport={true} for every mounted | ||
| // card regardless of scroll position. | ||
| const [isInViewport, setIsInViewport] = useState(false); |
There was a problem hiding this comment.
FNXC:Area-of-product YYYY-MM-dd-hh:mm: prefix so they are greppable. The same pattern appears in AgentsView.tsx lines 347–357.
| const cardRef = useRef<HTMLDivElement>(null); | |
| // FUX-039: real viewport gating (matching TaskCard.tsx's pattern) so | |
| // RuntimeFallbackBadge's 30s poll only runs while this card is actually | |
| // visible, rather than hardcoding isInViewport={true} for every mounted | |
| // card regardless of scroll position. | |
| const [isInViewport, setIsInViewport] = useState(false); | |
| const cardRef = useRef<HTMLDivElement>(null); | |
| /* | |
| FNXC:RuntimeFallback 2026-07-08-00:00: | |
| FUX-039: real viewport gating (matching TaskCard.tsx's pattern) so | |
| RuntimeFallbackBadge's 30s poll only runs while this card is actually | |
| visible, rather than hardcoding isInViewport={true} for every mounted | |
| card regardless of scroll position. | |
| */ | |
| const [isInViewport, setIsInViewport] = useState(false); |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| // FUX-039: real viewport gating for RuntimeFallbackBadge on the board and | ||
| // list card surfaces. Both surfaces render inline inside .map() calls, so | ||
| // a naive per-item useState/useEffect would violate the rules of hooks. | ||
| // Instead, a single shared IntersectionObserver instance observes every | ||
| // registered card element, keyed distinctly per surface ("board:{id}" vs | ||
| // "list:{id}") so the two surfaces never share visibility state. The | ||
| // registration function below MUST return a cached, stable callback per | ||
| // key (never a fresh closure per render) — a fresh closure would look like | ||
| // an unmount+remount to React on every render, and in environments without | ||
| // IntersectionObserver support that reproduces a real infinite re-render | ||
| // loop (including jsdom test environments). |
There was a problem hiding this comment.
ActiveAgentsPanel.tsx — the FUX-039 comment block describing the shared-observer requirement needs the FNXC:Area-of-product YYYY-MM-dd-hh:mm: prefix and date stamp so it is greppable alongside the rest of the codebase's requirement annotations.
| // FUX-039: real viewport gating for RuntimeFallbackBadge on the board and | |
| // list card surfaces. Both surfaces render inline inside .map() calls, so | |
| // a naive per-item useState/useEffect would violate the rules of hooks. | |
| // Instead, a single shared IntersectionObserver instance observes every | |
| // registered card element, keyed distinctly per surface ("board:{id}" vs | |
| // "list:{id}") so the two surfaces never share visibility state. The | |
| // registration function below MUST return a cached, stable callback per | |
| // key (never a fresh closure per render) — a fresh closure would look like | |
| // an unmount+remount to React on every render, and in environments without | |
| // IntersectionObserver support that reproduces a real infinite re-render | |
| // loop (including jsdom test environments). | |
| /* | |
| FNXC:RuntimeFallback 2026-07-08-00:00: | |
| FUX-039: real viewport gating for RuntimeFallbackBadge on the board and | |
| list card surfaces. Both surfaces render inline inside .map() calls, so | |
| a naive per-item useState/useEffect would violate the rules of hooks. | |
| Instead, a single shared IntersectionObserver instance observes every | |
| registered card element, keyed distinctly per surface ("board:{id}" vs | |
| "list:{id}") so the two surfaces never share visibility state. The | |
| registration function below MUST return a cached, stable callback per | |
| key (never a fresh closure per render) — a fresh closure would look like | |
| an unmount+remount to React on every render, and in environments without | |
| IntersectionObserver support that reproduces a real infinite re-render | |
| loop (including jsdom test environments). | |
| */ |
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export function __resetRuntimeFallbackToastClaimsForTests(): void { | ||
| claimedEventIds = new Set<string>(); | ||
| } |
There was a problem hiding this comment.
__resetRuntimeFallbackToastClaimsForTests is exported from the production module and will be included in every client bundle. The comment says "Not used by production code" but there is no build-time guard to enforce that. Consider placing the reset behind a process.env.NODE_ENV === "test" check, or exporting it from a companion useRuntimeFallbackStatus.test-utils.ts file that the test suite imports directly. As-is, the unused export is harmless but it is a surface that could be accidentally called by future callers.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/dashboard/app/components/__tests__/AgentsView.test.tsx (1)
19-70: 🚀 Performance & Scalability | 🔵 TrivialConsider extracting shared test scaffolding.
MockIntersectionObserver, thelegacyMocks/fetchTaskRuntimeFallbackhoisted mock, and therenderView/renderPanel-style ToastProvider wrapper are duplicated near-verbatim between this file andActiveAgentsPanel.test.tsx. Extracting these into a shared test utility module would reduce duplication and keep future gating-test changes in sync across both files.Also applies to: 2898-2980
🤖 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 `@packages/dashboard/app/components/__tests__/AgentsView.test.tsx` around lines 19 - 70, Extract the duplicated test scaffolding used by AgentsView.test.tsx and ActiveAgentsPanel.test.tsx into a shared test utility module, including MockIntersectionObserver, the hoisted legacyMocks/fetchTaskRuntimeFallback setup, and the ToastProvider render helper (renderView/renderPanel equivalents). Update the tests to import and reuse these shared helpers so the visibility/gating behavior stays consistent across both suites and future changes only need to be made once.packages/dashboard/app/components/AgentsView.tsx (1)
404-429: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winCached ref callbacks are never evicted from
agentCardRefCallbacksRef.On unregister (
el === null) you correctly clean upagentCardElementsRefandvisibleAgentCardKeys, but the cached closure inagentCardRefCallbacksRefis left behind forever. In a long-running dashboard session with many ephemeral/created-and-deleted agents, this Map grows unboundedly for the life of the component.♻️ Proposed fix
} else { agentCardElementsRef.current.delete(key); + agentCardRefCallbacksRef.current.delete(key); setVisibleAgentCardKeys((prev) => { if (!prev.has(key)) return prev; const next = new Set(prev); next.delete(key); return next; }); }
🤖 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.
Nitpick comments:
In `@packages/dashboard/app/components/__tests__/AgentsView.test.tsx`:
- Around line 19-70: Extract the duplicated test scaffolding used by
AgentsView.test.tsx and ActiveAgentsPanel.test.tsx into a shared test utility
module, including MockIntersectionObserver, the hoisted
legacyMocks/fetchTaskRuntimeFallback setup, and the ToastProvider render helper
(renderView/renderPanel equivalents). Update the tests to import and reuse these
shared helpers so the visibility/gating behavior stays consistent across both
suites and future changes only need to be made once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e84346d2-19cd-4065-9a6b-34667fb590e1
📒 Files selected for processing (8)
packages/dashboard/app/components/ActiveAgentsPanel.tsxpackages/dashboard/app/components/AgentsView.tsxpackages/dashboard/app/components/__tests__/ActiveAgentsPanel.test.tsxpackages/dashboard/app/components/__tests__/AgentsView.test.tsxpackages/dashboard/app/components/__tests__/RuntimeFallbackBadge.test.tsxpackages/dashboard/app/hooks/useRuntimeFallbackStatus.tspackages/engine/src/__tests__/runtime-resolution.test.tspackages/engine/src/runtime-resolution.ts
Closing as superseded by
|
| Finding | Landed on main |
|---|---|
resolvePluginRuntime() → init_error reason |
c3c726cff |
Real IntersectionObserver viewport gating on agent cards |
8a35b0bc4 |
| Cross-instance toast dedupe + regression test | 247908141 |
init_error/not_found/factory_error three-way coverage |
0bed997af / c3c726cff |
Review comments — all addressed on main by 003948033 ("harden runtime-fallback agent-card viewport gating")
- Greptile (FNXC comment format) —
ActiveAgentsPanel.tsxandAgentsView.tsxgating comments are nowFNXC:RuntimeFallback-prefixed and greppable. ✅ - Greptile (test-only reset leaks into prod bundle) —
__resetRuntimeFallbackToastDedupeStoreForTestsis now guarded withimport.meta.env.MODE !== "test"(the Vite convention used inApp.tsx). ✅ - CodeRabbit (cached ref callbacks never evicted) —
registerAgentCardRefnow caches one stable callback per viewport key (which also closes the fresh-closure-per-render re-render loop) and deletes the cache entry onel === null. ✅ - CodeRabbit (shared test scaffolding) — optional nit, not required.
Diff vs main
Empty. Resetting this branch to origin/main produces zero code delta — every file the PR touched is identical to main.
One follow-up
003948033 landed without a changeset, and it affects published @runfusion/fusion. A separate tiny PR will carry the missing patch changeset so the fix shows up in release notes.
Thanks for the review work — it directly shaped 003948033. 🤖
…dening (#1966) ## Summary `003948033` ("harden runtime-fallback agent-card viewport gating") landed on `main` **without a changeset**, but it affects published `@runfusion/fusion`. This PR adds the missing patch changeset so the fix shows up in release notes. ## Context Supersedes the now-closed #1963, whose code was fully redundant with `main` — all four FUX-039 findings plus every Greptile/CodeRabbit review comment already shipped via `003948033` and the preceding FUX-039 commits. The only remaining gap was this release-notes entry. ## What the changeset documents - **summary (user-facing):** Prevent redundant polling and a re-render loop in agent-card runtime-fallback badges. - **category:** `fix` - **dev:** `AgentsView` caches one stable ref callback per viewport key (avoids an infinite re-render loop when `IntersectionObserver` is unavailable) and evicts it on unmount; the test-only toast-dedupe reset is guarded to a no-op in production builds. ## Status - Diff: a single new file under `.changeset/`. No production code changes. - Gate: ✅ green — Lint, Typecheck, Build, and Gate all pass on `d8ce3f408`. - An earlier revision also trimmed `fn-7692`'s over-length summary to unblock the gate, but `main` since fixed that itself in `5815cd170`, so this branch was rebased to drop the now-redundant commit. The diff is now purely the FUX-039 changeset. 🤖 Generated with an autonomous coding agent
2 participants
Summary
Addresses the Staff Engineer pre-landing review findings on merged PR #1957 (FUX-022 runtime-fallback badge/toast feature). All 4 findings were independently re-verified against a fresh bootstrap of
main(not trusting a prior unmerged branch attempt) before fixing.resolvePluginRuntime()mislabeled found-but-failed-to-init as"not_found". The branch where a plugin's registration exists butcreateRuntimeContext(...)returns falsy now correctly returnsreason: "init_error"instead of"not_found"(which was indistinguishable from "never registered"). Extended the existing unit test suite to assert all 3FallbackReasonvalues are independently reachable and pairwise-distinct.Viewport gating was defeated on card surfaces.
ActiveAgentsPanelandAgentsViewboth hardcodedisInViewport={true}when renderingRuntimeFallbackBadge, so every rendered card polled regardless of actual visibility — defeating the entire purpose of theisInViewportgate. Wired realIntersectionObserver-based viewport gating on both surfaces (ActiveAgentsPanel'sLiveAgentCardtracks its own visibility;AgentsViewuses one shared observer instance keyed per card so the board and list surfaces never share visibility state). Added desktop + mobile regression tests.Toast-dedupe was per-component-instance, not shared.
useRuntimeFallbackStatus's dedupe key lived only in a per-hook-instanceuseRef, so two simultaneously-mounted card surfaces polling the same task (e.g. its board card and list card both visible at once) would each independently fire their own toast for the same fallback session. Replaced with a module-level shared claim store (bounded, with a test-only reset hook) so only the first instance to observe a given event, process-wide, fires the toast. Added a cross-instance regression test.Lint-breaking
eslint-disablecomment — independently confirmed already resolved onmainprior to this PR; no code change included.Test plan
pnpm --filter @fusion/engine exec vitest run src/__tests__/runtime-resolution.test.ts— 25/25 passpnpm --filter @fusion/dashboard exec vitest run app/components/__tests__/ActiveAgentsPanel.test.tsx— 17/17 passpnpm --filter @fusion/dashboard exec vitest run app/components/__tests__/RuntimeFallbackBadge.test.tsx— 9/9 passpnpm --filter @fusion/dashboard exec vitest run app/components/__tests__/AgentsView.test.tsx— 132/134 pass (1 pre-existing, unrelated CSS-fixture failure confirmed viagit stashprior to this change)pnpm --filter @fusion/engine exec tsc --noEmit— cleanpnpm --filter @fusion/dashboard exec tsc --noEmit— cleaneslintscoped to all changed files — 0 errors🤖 Generated with an autonomous coding agent
Summary by CodeRabbit
New Features
Bug Fixes