feat(flows): Workflows B5a — list page + nav tab (/flows)#4471
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (29)
📝 WalkthroughWalkthroughAdds a new protected ChangesWorkflows feature
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
coderabbitai
Bot
added
the
feature
![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: 9166e31abe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| data-testid={`flow-view-runs-${flow.id}`} | ||
| onClick={() => onViewRuns(flow)}> | ||
| {t('flows.list.viewRuns')} |
There was a problem hiding this comment.
Until the B3b inspector exists, every saved flow shows an enabled “View runs” button, but the callback only stores selectedFlowId and logs it (a repo-wide search for FlowRunInspector/selectedFlowId shows no rendered consumer). Users who click this on /flows get no drawer, navigation, or run list, so this is a dead action; hide/disable it or wire it to listFlowRuns before exposing it.
Useful? React with 👍 / 👎.
| if (mins < 1) return 'just now'; | ||
| if (mins < 60) return `${mins}m ago`; |
There was a problem hiding this comment.
When a user is in any non-English locale and a flow has last_run_at, these branches render English strings like just now and 5m ago directly instead of using useT() or locale-aware formatting. The rest of the new Workflows page has translations, but this run metadata remains untranslated; please route it through i18n or Intl.RelativeTimeFormat.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e55de28 — relativeTime() now routes through useT() with new flows.list.{justNow,minutesAgo,hoursAgo,daysAgo} keys (real translations in all 14 locales, {count} via .replace).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/src/pages/FlowsPage.tsx (2)
82-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winToggle/run failures show raw, untranslated backend error text.
loadFlowsuses a translated generic message (t('flows.page.loadError')), buthandleToggle/handleRunsurface the raw exception message viaerrorMessage(err)directly in the UI (ErrorBanner). This is inconsistent and bypasses i18n for these paths — non-English users will see raw English/backend error strings.Consider using a translated generic fallback (e.g.,
t('flows.list.toggleError')/t('flows.list.runError')) instead of the raw message, or at least translate a wrapper string around it.As per coding guidelines, "All user-facing UI text in the React app must go through
useT()".Also applies to: 109-111
🤖 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 `@app/src/pages/FlowsPage.tsx` around lines 82 - 84, The toggle/run error handling in FlowsPage is surfacing raw backend text via errorMessage(err) instead of translated UI copy, which breaks the i18n rule used by loadFlows. Update handleToggle and handleRun to use useT()-backed translated fallback messages (for example flows.list.toggleError and flows.list.runError), and only include raw error details if they are wrapped inside a translated user-facing string before passing to ErrorBanner.Source: Coding guidelines
43-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStub wiring for
selectedFlowId— acceptable but slightly convoluted.The extra
useEffectexists solely to keepselectedFlowIdfrom being flagged as unused bynoUnusedLocalsuntil the B3b inspector lands. This works but adds an effect + dependency array purely as a lint workaround. Avoid selectedFlowId;comment or a narrower// eslint-disable-next-lineat declaration would achieve the same without a full effect.Given this is explicitly a temporary stub per the PR notes, low priority.
Also applies to: 119-132, 191-201
🤖 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 `@app/src/pages/FlowsPage.tsx` at line 43, The `selectedFlowId` stub in `FlowsPage` is being kept alive with an extra `useEffect` only to satisfy `noUnusedLocals`, which is unnecessarily convoluted. Remove the effect-based workaround around `selectedFlowId` and use a lighter temporary suppression at the declaration site instead, such as a `void selectedFlowId;` or a narrowly scoped eslint disable. Apply the same cleanup to the related stub wiring noted elsewhere in `FlowsPage` so the temporary B3b placeholder stays simple.app/src/components/layout/shell/SidebarNav.tsx (1)
26-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
matchActivelogic across nav components.This
matchActivehelper is duplicated verbatim inCollapsedNavRail.tsx(lines 15-21). Both had to be edited identically to add the/flowsrule, andCollapsedNavRail.tsx's copy is already missing the/agent-worldcase present here — a sign the two can silently drift. Consider extracting a sharedmatchActive(andNavTabtype) into a common module (e.g.navConfig.tsor a newnavMatch.ts) imported by both components.♻️ Suggested extraction
// app/src/config/navMatch.ts export function matchActive(path: string, pathname: string): boolean { if (path === '/chat') return pathname.startsWith('/chat'); if (path === '/settings') return pathname === '/settings' || pathname.startsWith('/settings/'); if (path === '/agent-world') return pathname === '/agent-world' || pathname.startsWith('/agent-world/'); if (path === '/flows') return pathname === '/flows' || pathname.startsWith('/flows/'); if (path === '/home') return pathname === '/home'; return pathname === path; }Then import this in both
SidebarNav.tsxandCollapsedNavRail.tsxinstead of maintaining two copies.🤖 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 `@app/src/components/layout/shell/SidebarNav.tsx` around lines 26 - 34, The `matchActive` helper in `SidebarNav.tsx` is duplicated in `CollapsedNavRail.tsx`, and the copies are already drifting (e.g. `/agent-world` and `/flows` handling). Extract the shared route-matching logic into a common module such as `navMatch.ts` or `navConfig.ts`, and have both `SidebarNav` and `CollapsedNavRail` import the same `matchActive` implementation. If `NavTab` is also shared between these components, move that type alongside the helper so navigation behavior stays consistent in one place.
🤖 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 `@app/src/components/flows/FlowListRow.tsx`:
- Around line 34-43: The user-facing strings in `relativeTime()` are hardcoded
in English and bypass the app’s i18n flow. Update `FlowListRow` to source these
labels through `useT()`/`t()` instead of raw literals, and add the needed keys
for “just now” and the relative-time formats to `en.ts` plus every locale file.
Follow the existing interpolation pattern in this codebase (for example,
translating a template then replacing placeholders) so the `relativeTime()`
helper can render localized minutes, hours, and days consistently.
---
Nitpick comments:
In `@app/src/components/layout/shell/SidebarNav.tsx`:
- Around line 26-34: The `matchActive` helper in `SidebarNav.tsx` is duplicated
in `CollapsedNavRail.tsx`, and the copies are already drifting (e.g.
`/agent-world` and `/flows` handling). Extract the shared route-matching logic
into a common module such as `navMatch.ts` or `navConfig.ts`, and have both
`SidebarNav` and `CollapsedNavRail` import the same `matchActive`
implementation. If `NavTab` is also shared between these components, move that
type alongside the helper so navigation behavior stays consistent in one place.
In `@app/src/pages/FlowsPage.tsx`:
- Around line 82-84: The toggle/run error handling in FlowsPage is surfacing raw
backend text via errorMessage(err) instead of translated UI copy, which breaks
the i18n rule used by loadFlows. Update handleToggle and handleRun to use
useT()-backed translated fallback messages (for example flows.list.toggleError
and flows.list.runError), and only include raw error details if they are wrapped
inside a translated user-facing string before passing to ErrorBanner.
- Line 43: The `selectedFlowId` stub in `FlowsPage` is being kept alive with an
extra `useEffect` only to satisfy `noUnusedLocals`, which is unnecessarily
convoluted. Remove the effect-based workaround around `selectedFlowId` and use a
lighter temporary suppression at the declaration site instead, such as a `void
selectedFlowId;` or a narrowly scoped eslint disable. Apply the same cleanup to
the related stub wiring noted elsewhere in `FlowsPage` so the temporary B3b
placeholder stays simple.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 610c8c35-5d6c-4807-9613-8d6d09f01543
📒 Files selected for processing (29)
app/src/AppRoutes.tsxapp/src/components/flows/FlowListRow.test.tsxapp/src/components/flows/FlowListRow.tsxapp/src/components/layout/shell/CollapsedNavRail.test.tsxapp/src/components/layout/shell/CollapsedNavRail.tsxapp/src/components/layout/shell/SidebarNav.test.tsxapp/src/components/layout/shell/SidebarNav.tsxapp/src/components/layout/shell/navIcons.tsxapp/src/config/__tests__/navConfig.test.tsapp/src/config/navConfig.tsapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/pages/FlowsPage.test.tsxapp/src/pages/FlowsPage.tsxapp/src/services/api/flowsApi.test.tsapp/src/services/api/flowsApi.tsapp/test/e2e/specs/navigation.spec.ts
The discoverable hub: a top-level /flows page listing saved workflows (name, enabled toggle, last run + status, Run button) reachable from a new nav tab. Uses existing RPCs only — no backend change. - pages/FlowsPage.tsx (+test): PanelPage shell; listFlows on mount; loading/ error/empty states (empty has no CTA until the B5b canvas); toggle → setFlowEnabled (optimistic), Run → fire-and-forget runFlow + toast + refetch (refetch only on success so a run error keeps its banner). - components/flows/FlowListRow.tsx (+test): CoreJobList-style row — name, Enabled/Paused badge, last-run line, SettingsSwitch toggle, Run + View-runs. - flowsApi.ts: appended listFlows / setFlowEnabled / runFlow (+ Flow type); B3b (tinyhumansai#4450) doesn't touch this file, so no conflict. - Nav wired in lockstep: navConfig (flows tab, 4th), navIcons, SidebarNav + CollapsedNavRail matchActive, AppRoutes (/flows/*), and the E2E navigation.spec ROUTES + nav-config/shell tests (the CI-gated bits). The /workflows/* Skill routes are untouched. - The run inspector (B3b) is a clearly-marked stub, wired after tinyhumansai#4450 merges. - i18n: flows.* + nav.flows across all 14 locales (distinct namespace). typecheck/lint/prettier clean; 69 Vitest tests pass; i18n parity clean.
…button
- FlowListRow relativeTime() now routes through useT() with flows.list.{justNow,
minutesAgo,hoursAgo,daysAgo} (real translations x14, {count} interpolation via
.replace) instead of hardcoded English. [Codex + CodeRabbit]
- Removed the 'View runs' button + onViewRuns wiring — it was a dead action with
no consumer until B3b's inspector; the B3b integration note is preserved for
the post-tinyhumansai#4450 rebase. [Codex]
- prettier: FlowApprovalCard.test.tsx (pre-existing dirty on main).
typecheck/lint/prettier(repo-wide) clean; 46+8 tests pass; i18n parity clean.
graycyrus
force-pushed
the
feat/flows-b5a-page-tab
branch
from
e55de28 to
8d7fcfd
Compare
1 participant
Summary
The Workflows hub: a top-level
/flowspage listing saved workflows (name, enabled toggle, last run + status, Run button), reachable from a new nav tab. Makes the feature discoverable and gives runs/inspector a home. Uses existing RPCs — no backend change.Changes
pages/FlowsPage.tsx(+test) —PanelPageshell;listFlowson mount; loading/error/empty states (empty has no CTA until the B5b canvas). Toggle →setFlowEnabled(optimistic); Run → fire-and-forgetrunFlow+ "Workflow started" toast + refetch (only on success, so a run error keeps its banner).components/flows/FlowListRow.tsx(+test) —CoreJobList-style row: name, Enabled/Paused badge, last-run line,SettingsSwitchtoggle, Run + View-runs.flowsApi.ts— appendedlistFlows/setFlowEnabled/runFlow(+Flowtype). B3b (feat(flows): Workflows B3b — Run Inspector drawer #4450) doesn't touch this file → no conflict.navConfig(Workflows tab, 4th),navIcons,SidebarNav+CollapsedNavRailmatchActive,AppRoutes(/flows/*), the E2Enavigation.specROUTES table, and the nav-config/shell unit tests. The/workflows/*Skill routes are untouched.flows.*+nav.flowsacross all 14 locales (distinct namespace from B3b/B4).Route decision
New page uses
/flows(matches theflows::domain), leaving the Skills/workflows/new|run+/workflows→/settings/automationsroutes untouched.Sequencing
The run inspector (B3b, #4450) is a clearly-marked stub here — wired after #4450 merges + rebase (inspector is keyed by run id). No
flows_list_all_runsRPC needed for this page (flows_listreturns last-run status per flow).Verification
pnpm typecheck+lint+prettierclean · 69 Vitest tests pass ·pnpm i18n:checkparity clean.Summary by CodeRabbit
New Features
Tests