fix: Frontend demo polish - broken pages, navigation, UX#1430
Merged
fix: Frontend demo polish - broken pages, navigation, UX#1430
Conversation
- Add HTTP annotations to manifest history and apply protos - Fix positions detail BigInt crash when rawAmount is a number - Group sidebar navigation into Operations/Configuration/Admin sections - Wire audit trail to Connect client instead of raw fetch - Add party type/status enum labels on party overview - Add party column to accounts list table - Add empty state messaging for payments and reconciliation - Replace saga script textarea with StarlarkEditor (CodeMirror) - Make saga dialog footer sticky for better scroll UX - Update tests for all changed components
Contributor
📝 WalkthroughWalkthroughAdds HTTP REST bindings to protobuf RPCs, introduces an AuditService client, restructures sidebar navigation into groups, surfaces party IDs in accounts, updates party/status formatting, adds empty-table states, fixes BigInt balance handling, swaps a textarea for StarlarkEditor, and integrates the audit trail with real API clients. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/src/features/sagas/pages/create-saga-draft-dialog.tsx (1)
260-271:⚠️ Potential issue | 🟠 MajorStarlarkEditor component lacks accessible label/error association support.
The
StarlarkEditorcomponent (lines 263–267) does not acceptid,aria-labelledby, oraria-describedbyprops and does not forward them to its underlying elements. This prevents screen readers from reliably associating the Script label and validation error with the editor control.To fix this accessibility issue, the
StarlarkEditorcomponent needs to be extended to:
- Accept
id,aria-labelledby, andaria-describedbyin itsStarlarkEditorPropsinterface- Forward these attributes to the editable element (the ref div at line 194–197)
Then the parent form can properly link the label and error messaging to the editor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/sagas/pages/create-saga-draft-dialog.tsx` around lines 260 - 271, The StarlarkEditor lacks accessibility attribute passing; update its StarlarkEditorProps to include id, aria-labelledby, and aria-describedby (optional strings), update the StarlarkEditor component signature to accept and type those props, and forward them onto the editable element (the inner editable div/element referenced by the component's ref) so the parent can link the label and error via id/aria-describedby; ensure TypeScript types and any prop spreading or ref-forwarding is adjusted so these attributes are applied to the underlying DOM element.frontend/src/test/api/clients.test.ts (1)
23-27:⚠️ Potential issue | 🟡 MinorUpdate the test title to match the new expected count.
The test name says “18 service clients” while the assertion now expects
19, which makes the spec misleading.Proposed fix
- it('returns an object with all 18 service clients', () => { + it('returns an object with all 19 service clients', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/test/api/clients.test.ts` around lines 23 - 27, Update the test title string to reflect the new expected count: change the description in the it(...) call currently reading "returns an object with all 18 service clients" to state "returns an object with all 19 service clients" so it matches the assertion that Object.keys(clients) has length 19; the test references createServiceClients and makeTransport so only the title text needs updating.
🧹 Nitpick comments (4)
frontend/src/features/payments/pages/index.tsx (1)
86-91: Consider a sharedTableEmptyStatecomponent to avoid repeated markup.This pattern now exists in multiple pages and is likely to grow; extracting a small reusable component will keep copy/layout consistent and reduce drift.
♻️ Example refactor
- emptyState={ - <div className="flex flex-col items-center gap-2 py-12 text-muted-foreground"> - <span className="text-sm font-medium">No payments yet</span> - <span className="text-xs">Payments will appear here once initiated.</span> - </div> - } + emptyState={ + <TableEmptyState + title="No payments yet" + description="Payments will appear here once initiated." + /> + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/payments/pages/index.tsx` around lines 86 - 91, Extract the repeated empty-state markup into a reusable React component named TableEmptyState that accepts props like title and subtitle; implement TableEmptyState to render the exact JSX (container div with classes "flex flex-col items-center gap-2 py-12 text-muted-foreground" and two spans for title/subtitle) and replace the inline emptyState in the payments page with <TableEmptyState title="No payments yet" subtitle="Payments will appear here once initiated." />; update other pages using the same pattern to use TableEmptyState to keep layout/strings consistent and export the component for reuse.frontend/src/features/positions/pages/detail.tsx (1)
36-48: Good fix for the BigInt arithmetic crash.The change correctly ensures all numeric values are converted to BigInt before arithmetic operations, preventing the TypeError that occurs when mixing
numberandbiginttypes.One edge case to be aware of:
BigInt(rawAmount)will throw aRangeErrorifrawAmountis a floating-point number (e.g.,123.45), or aSyntaxErrorfor non-numeric strings. If the upstream data could ever contain decimals, consider wrapping in a try-catch or truncating:🛡️ Optional: Defensive conversion with fallback
const rawAmount = entry.amount?.amount if (rawAmount === undefined || rawAmount === null) continue - const amt = typeof rawAmount === 'bigint' ? rawAmount : BigInt(rawAmount) + let amt: bigint + if (typeof rawAmount === 'bigint') { + amt = rawAmount + } else { + try { + amt = BigInt(Math.trunc(Number(rawAmount))) + } catch { + continue // Skip malformed entries + } + } const signed = entry.direction === 'CREDIT' ? amt : -amt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/positions/pages/detail.tsx` around lines 36 - 48, The conversion to BigInt for rawAmount (in the loop over entries that computes provisionalTotal and availableTotal) can throw for floating-point or invalid string inputs; update the conversion logic around rawAmount/amt (where you currently call BigInt(rawAmount)) to defensively handle decimals/non-numeric values: either attempt a safe parse inside a try/catch and skip or coerce floats via truncation (e.g., Math.trunc or string split) before calling BigInt, and ensure the existing signed, provisionalTotal and availableTotal updates still only run when the conversion succeeded; keep checks for null/undefined and preserve behavior tied to entry.direction and entry.qualityLevel.frontend/src/components/layout/sidebar.tsx (1)
128-149: Consider extracting shared nav-item rendering.The
<Link>item markup is duplicated for tenant and platform items. Pulling it into a helper reduces maintenance overhead and avoids style drift.Refactor sketch
+ function renderNavItem(item: NavItem) { + const Icon = item.icon + const isActive = currentPath === item.href + return ( + <li key={item.href}> + <Link + to={item.href} + aria-current={isActive ? 'page' : undefined} + className={cn( + 'flex items-center gap-3 rounded-md px-3 py-2 text-sm font-medium transition-colors', + isActive ? 'bg-gray-700 text-white' : 'text-gray-300 hover:bg-gray-700 hover:text-white', + )} + > + <Icon className="size-4 shrink-0" /> + {item.label} + </Link> + </li> + ) + } ... - {visibleItems.map((item) => { - const Icon = item.icon - const isActive = currentPath === item.href - return ( - <li key={item.href}> - <Link ...> - <Icon className="size-4 shrink-0" /> - {item.label} - </Link> - </li> - ) - })} + {visibleItems.map(renderNavItem)} ... - {PLATFORM_NAV_ITEMS.map((item) => { - const Icon = item.icon - const isActive = currentPath === item.href - return ( - <li key={item.href}> - <Link ...> - <Icon className="size-4 shrink-0" /> - {item.label} - </Link> - </li> - ) - })} + {PLATFORM_NAV_ITEMS.map(renderNavItem)}Also applies to: 162-183
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/layout/sidebar.tsx` around lines 128 - 149, The Link/LI markup for menu entries is duplicated; extract it into a single helper (e.g., a NavItem component or renderNavItem function) that accepts an item prop (with href, label, icon), currentPath, and uses cn to build the class string, sets aria-current, renders Icon (item.icon) and label, and returns the <li><Link...>…</Link></li> structure; replace the two duplicated blocks that map visibleItems (and the other list at the other location) to call this helper to avoid style drift and centralize rendering.frontend/src/features/sagas/pages/create-saga-draft-dialog.test.tsx (1)
154-162: Test intent says “submits successfully” but doesn’t assert submit side effect.Right now this can pass even if the submit path breaks before mutation. Add a submit-side assertion to match the test name.
Suggested test tightening
await user.click(screen.getByRole('button', { name: /create saga draft/i })) await waitFor(() => { expect(screen.queryByText(/script is required/i)).not.toBeInTheDocument() + expect(mockCreateSagaDraft).toHaveBeenCalledOnce() })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/sagas/pages/create-saga-draft-dialog.test.tsx` around lines 154 - 162, The test currently only checks that the validation message is absent but doesn't assert that the submit path was executed; update the "submits with pre-filled script successfully (script validation)" test to assert a submit-side effect: either spy/mock the saga-draft creation mutation or submit handler used by renderDialog() (e.g., the createSagaDraft mutation or onSubmit) and expect it to have been called with the expected payload after clicking the "create saga draft" button, or assert a visible post-submit outcome (dialog closed, success toast/message present). Ensure you reference the existing setup calls (renderDialog(), user.click(screen.getByRole('button', { name: /create saga draft/i })), and the form fields like screen.getByLabelText(/^name$/i)) when wiring the spy/assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/layout/sidebar.tsx`:
- Around line 123-125: The `first:mt-0` utility is ineffective on the inner div
because that div is the first child of every `<li>`, so every group's top margin
is being reset; move the `first:mt-0` Tailwind class from the inner div to the
`<li>` element (the element keyed by {group.label}) so only the very first group
`<li>` gets its top margin removed, leaving subsequent groups with the intended
spacing.
In `@frontend/src/features/accounts/pages/index.tsx`:
- Around line 43-49: The Party column's cell renderer (accessorKey: 'partyId',
cell: ({ row }) => ...) currently renders <EntityLink> which bubbles clicks to
the DataTable's row onClick (TableRow) and triggers
navigate('/accounts/{accountId}'); update the cell renderer to stop propagation
on interactive element clicks by wrapping EntityLink with an onClick handler
that calls event.stopPropagation(), or alternatively ensure the DataTable row
onClick ignores events originating from anchors/buttons; modify the cell for
'partyId' (where EntityLink is returned) to add an event handler that calls
event.stopPropagation() before letting the link navigate.
In `@frontend/src/features/parties/pages/tabs/overview-tab.tsx`:
- Around line 23-33: Change the string branch in formatPartyType and
formatPartyStatus to normalize enum-like strings (e.g. "PARTY_TYPE_ORGANIZATION"
→ "ORGANIZATION", "PARTY_STATUS_ACTIVE" → "ACTIVE") instead of returning them
unchanged: when typeof value === 'string', strip the "PARTY_TYPE_" or
"PARTY_STATUS_" prefix (or otherwise extract the portion after the last
underscore) and return that normalized label; fall back to the existing
PARTY_TYPE_LABELS[Number(value)]/PARTY_STATUS_LABELS[Number(value)] or
String(value) if no prefix match. Apply the same normalization to the other
occurrences of these helpers/branches referenced in the file.
In `@frontend/src/shared/audit-trail.tsx`:
- Around line 40-43: The toOperationName function currently casts any string to
AuditOperation blindly; update it to validate string inputs against the allowed
AuditOperation values before returning them (e.g., check membership in the
AuditOperation union or a set/enum of allowed names) and only return the string
if it matches, otherwise fall back to OPERATION_NAMES[op] or 'UPDATE'; reference
toOperationName, OPERATION_NAMES, and the AuditOperation type when making the
guard.
---
Outside diff comments:
In `@frontend/src/features/sagas/pages/create-saga-draft-dialog.tsx`:
- Around line 260-271: The StarlarkEditor lacks accessibility attribute passing;
update its StarlarkEditorProps to include id, aria-labelledby, and
aria-describedby (optional strings), update the StarlarkEditor component
signature to accept and type those props, and forward them onto the editable
element (the inner editable div/element referenced by the component's ref) so
the parent can link the label and error via id/aria-describedby; ensure
TypeScript types and any prop spreading or ref-forwarding is adjusted so these
attributes are applied to the underlying DOM element.
In `@frontend/src/test/api/clients.test.ts`:
- Around line 23-27: Update the test title string to reflect the new expected
count: change the description in the it(...) call currently reading "returns an
object with all 18 service clients" to state "returns an object with all 19
service clients" so it matches the assertion that Object.keys(clients) has
length 19; the test references createServiceClients and makeTransport so only
the title text needs updating.
---
Nitpick comments:
In `@frontend/src/components/layout/sidebar.tsx`:
- Around line 128-149: The Link/LI markup for menu entries is duplicated;
extract it into a single helper (e.g., a NavItem component or renderNavItem
function) that accepts an item prop (with href, label, icon), currentPath, and
uses cn to build the class string, sets aria-current, renders Icon (item.icon)
and label, and returns the <li><Link...>…</Link></li> structure; replace the two
duplicated blocks that map visibleItems (and the other list at the other
location) to call this helper to avoid style drift and centralize rendering.
In `@frontend/src/features/payments/pages/index.tsx`:
- Around line 86-91: Extract the repeated empty-state markup into a reusable
React component named TableEmptyState that accepts props like title and
subtitle; implement TableEmptyState to render the exact JSX (container div with
classes "flex flex-col items-center gap-2 py-12 text-muted-foreground" and two
spans for title/subtitle) and replace the inline emptyState in the payments page
with <TableEmptyState title="No payments yet" subtitle="Payments will appear
here once initiated." />; update other pages using the same pattern to use
TableEmptyState to keep layout/strings consistent and export the component for
reuse.
In `@frontend/src/features/positions/pages/detail.tsx`:
- Around line 36-48: The conversion to BigInt for rawAmount (in the loop over
entries that computes provisionalTotal and availableTotal) can throw for
floating-point or invalid string inputs; update the conversion logic around
rawAmount/amt (where you currently call BigInt(rawAmount)) to defensively handle
decimals/non-numeric values: either attempt a safe parse inside a try/catch and
skip or coerce floats via truncation (e.g., Math.trunc or string split) before
calling BigInt, and ensure the existing signed, provisionalTotal and
availableTotal updates still only run when the conversion succeeded; keep checks
for null/undefined and preserve behavior tied to entry.direction and
entry.qualityLevel.
In `@frontend/src/features/sagas/pages/create-saga-draft-dialog.test.tsx`:
- Around line 154-162: The test currently only checks that the validation
message is absent but doesn't assert that the submit path was executed; update
the "submits with pre-filled script successfully (script validation)" test to
assert a submit-side effect: either spy/mock the saga-draft creation mutation or
submit handler used by renderDialog() (e.g., the createSagaDraft mutation or
onSubmit) and expect it to have been called with the expected payload after
clicking the "create saga draft" button, or assert a visible post-submit outcome
(dialog closed, success toast/message present). Ensure you reference the
existing setup calls (renderDialog(), user.click(screen.getByRole('button', {
name: /create saga draft/i })), and the form fields like
screen.getByLabelText(/^name$/i)) when wiring the spy/assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3fa767c4-5d19-4a17-accc-46b7c16e1d35
📒 Files selected for processing (17)
api/proto/meridian/control_plane/v1/apply_manifest_service.protoapi/proto/meridian/control_plane/v1/manifest_history_service.protofrontend/src/api/clients.tsfrontend/src/components/layout/sidebar.tsxfrontend/src/features/accounts/hooks/use-accounts.tsfrontend/src/features/accounts/pages/index.tsxfrontend/src/features/parties/pages/tabs/overview-tab.test.tsxfrontend/src/features/parties/pages/tabs/overview-tab.tsxfrontend/src/features/payments/pages/index.tsxfrontend/src/features/positions/pages/detail.tsxfrontend/src/features/reconciliation/pages/index.tsxfrontend/src/features/sagas/pages/create-saga-draft-dialog.test.tsxfrontend/src/features/sagas/pages/create-saga-draft-dialog.tsxfrontend/src/shared/audit-trail.test.tsxfrontend/src/shared/audit-trail.tsxfrontend/src/shared/index.tsfrontend/src/test/api/clients.test.ts
Claude Code ReviewCommit: SummaryWell-scoped demo polish PR fixing real user-facing issues (BigInt crash, broken pages, raw enum display, flat nav). The audit trail refactor from raw Risk Assessment
Findings
Bot Review NotesAll 4 CodeRabbit threads on this PR have been addressed:
Previously Flagged
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Sidebar: fix first:mt-0 ineffective on group headers, use reduce with index tracking instead - Accounts: add stopPropagation on EntityLink to prevent row click hijacking party navigation - Party overview: strip PARTY_TYPE_/PARTY_STATUS_ prefixes from string enums for consistent display - Audit trail: validate string operations against known set before casting
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/shared/audit-trail.tsx (1)
275-283:⚠️ Potential issue | 🟡 MinorSort order can be incorrect for entries in the same second.
Line 276 and Line 279 only compare
timestamp.seconds;nanosis ignored. Two events in the same second can render out of order.🛠️ Proposed fix
const sorted = [...entries].sort((a, b) => { const aTime = typeof a.timestamp?.seconds === 'bigint' ? Number(a.timestamp.seconds) : (a.timestamp?.seconds ?? 0); const bTime = typeof b.timestamp?.seconds === 'bigint' ? Number(b.timestamp.seconds) : (b.timestamp?.seconds ?? 0); - return bTime - aTime; + if (bTime !== aTime) return bTime - aTime; + const aNanos = a.timestamp?.nanos ?? 0; + const bNanos = b.timestamp?.nanos ?? 0; + return bNanos - aNanos; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/shared/audit-trail.tsx` around lines 275 - 283, The sort currently only compares timestamp.seconds (in the sorting of sorted created from entries) so events within the same second can be misordered; update the comparator in the sorted = [...entries].sort(...) function to also compare timestamp.nanos (handling possible bigint or undefined values the same way you handle seconds) by converting both seconds and nanos to numbers (or otherwise normalizing types), then return a descending order by first comparing seconds and, if equal, comparing nanos (use bSec - aSec, then bNanos - aNanos) so events in the same second are correctly ordered.
🧹 Nitpick comments (2)
frontend/src/features/parties/pages/tabs/overview-tab.tsx (1)
23-33: Consider extracting a shared enum-format helper to reduce drift.Both formatters now share the same pattern (prefix-strip + numeric map + fallback). A small helper would keep future behavior changes in one place.
Refactor sketch
+function formatEnumValue( + value: unknown, + prefix: string, + labels: Record<number, string>, +): string { + if (typeof value === 'string') return value.replace(new RegExp(`^${prefix}`), '') + if (typeof value === 'number') return labels[value] ?? String(value) + return String(value ?? '') +} + function formatPartyType(value: unknown): string { - if (typeof value === 'string') return value.replace(/^PARTY_TYPE_/, '') - if (typeof value === 'number') return PARTY_TYPE_LABELS[value] ?? String(value) - return String(value ?? '') + return formatEnumValue(value, 'PARTY_TYPE_', PARTY_TYPE_LABELS) } function formatPartyStatus(value: unknown): string { - if (typeof value === 'string') return value.replace(/^PARTY_STATUS_/, '') - if (typeof value === 'number') return PARTY_STATUS_LABELS[value] ?? String(value) - return String(value ?? '') + return formatEnumValue(value, 'PARTY_STATUS_', PARTY_STATUS_LABELS) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/features/parties/pages/tabs/overview-tab.tsx` around lines 23 - 33, Extract a shared helper (e.g., formatEnumValue or formatPrefixedEnum) that encapsulates the common logic (strip a prefix from string values, map numeric values via a labels map, and fallback to String(value ?? '')), then replace the bodies of formatPartyType and formatPartyStatus to call that helper with the appropriate prefix regex (e.g., /^PARTY_TYPE_/ and /^PARTY_STATUS_/) and maps (PARTY_TYPE_LABELS, PARTY_STATUS_LABELS); ensure the helper signature accepts (value: unknown, prefix: RegExp, labels?: Record<number,string>) so both functions become thin wrappers delegating to it.frontend/src/components/layout/sidebar.tsx (1)
127-149: Extract repeated nav-link markup into a shared renderer.The tenant and platform item loops duplicate the same
<li><Link>...</Link></li>structure. This is likely to drift over time (active-state, classes, aria handling). Consider centralizing it in a small helper/component.♻️ Proposed refactor
+function NavLinkItem({ + item, + currentPath, +}: { + item: NavItem + currentPath: string +}) { + const Icon = item.icon + const isActive = currentPath === item.href + return ( + <li key={item.href}> + <Link + to={item.href} + aria-current={isActive ? 'page' : undefined} + className={cn( + 'flex items-center gap-3 rounded-md px-3 py-2 text-sm font-medium transition-colors', + isActive ? 'bg-gray-700 text-white' : 'text-gray-300 hover:bg-gray-700 hover:text-white', + )} + > + <Icon className="size-4 shrink-0" /> + {item.label} + </Link> + </li> + ) +} ... - {visibleItems.map((item) => { - const Icon = item.icon - const isActive = currentPath === item.href - return ( ... ) - })} + {visibleItems.map((item) => ( + <NavLinkItem key={item.href} item={item} currentPath={currentPath} /> + ))} ... - {PLATFORM_NAV_ITEMS.map((item) => { - const Icon = item.icon - const isActive = currentPath === item.href - return ( ... ) - })} + {PLATFORM_NAV_ITEMS.map((item) => ( + <NavLinkItem key={item.href} item={item} currentPath={currentPath} /> + ))}Also applies to: 163-185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/layout/sidebar.tsx` around lines 127 - 149, The repeated list-item/link markup should be centralized into a small reusable component (e.g., SidebarNavItem or NavLinkRenderer) to avoid duplication between the tenant and platform loops: create a component that accepts props (item, currentPath) or (href, label, icon, isActive) and encapsulates the Link, aria-current logic, cn class composition, Icon rendering and key handling; then replace the map body over visibleItems (and the similar loop at lines 163-185) to render <SidebarNavItem .../> passing item and currentPath so active-state, classes and accessibility remain consistent and maintained in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@frontend/src/shared/audit-trail.tsx`:
- Around line 275-283: The sort currently only compares timestamp.seconds (in
the sorting of sorted created from entries) so events within the same second can
be misordered; update the comparator in the sorted = [...entries].sort(...)
function to also compare timestamp.nanos (handling possible bigint or undefined
values the same way you handle seconds) by converting both seconds and nanos to
numbers (or otherwise normalizing types), then return a descending order by
first comparing seconds and, if equal, comparing nanos (use bSec - aSec, then
bNanos - aNanos) so events in the same second are correctly ordered.
---
Nitpick comments:
In `@frontend/src/components/layout/sidebar.tsx`:
- Around line 127-149: The repeated list-item/link markup should be centralized
into a small reusable component (e.g., SidebarNavItem or NavLinkRenderer) to
avoid duplication between the tenant and platform loops: create a component that
accepts props (item, currentPath) or (href, label, icon, isActive) and
encapsulates the Link, aria-current logic, cn class composition, Icon rendering
and key handling; then replace the map body over visibleItems (and the similar
loop at lines 163-185) to render <SidebarNavItem .../> passing item and
currentPath so active-state, classes and accessibility remain consistent and
maintained in one place.
In `@frontend/src/features/parties/pages/tabs/overview-tab.tsx`:
- Around line 23-33: Extract a shared helper (e.g., formatEnumValue or
formatPrefixedEnum) that encapsulates the common logic (strip a prefix from
string values, map numeric values via a labels map, and fallback to String(value
?? '')), then replace the bodies of formatPartyType and formatPartyStatus to
call that helper with the appropriate prefix regex (e.g., /^PARTY_TYPE_/ and
/^PARTY_STATUS_/) and maps (PARTY_TYPE_LABELS, PARTY_STATUS_LABELS); ensure the
helper signature accepts (value: unknown, prefix: RegExp, labels?:
Record<number,string>) so both functions become thin wrappers delegating to it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a199103-91dc-49fd-a068-e8b3bcc8edd2
📒 Files selected for processing (5)
frontend/src/components/layout/sidebar.tsxfrontend/src/features/accounts/pages/index.tsxfrontend/src/features/parties/pages/tabs/overview-tab.test.tsxfrontend/src/features/parties/pages/tabs/overview-tab.tsxfrontend/src/shared/audit-trail.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/features/accounts/pages/index.tsx
- frontend/src/features/parties/pages/tabs/overview-tab.test.tsx
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
Fixes ~15 issues found during demo.meridianhub.cloud walkthrough (build 87C4620) to make the frontend presentable before showcasing Starlark event triggers.
Changes
Backend (Proto)
google.api.httpannotations toManifestHistoryServiceandApplyManifestServiceprotos, enabling the manifest current view and history tabs to work via Connect-WebBroken Pages Fixed
cannot mix bigint and other types) by safely convertingnumbertoBigIntbefore arithmeticSidebar Navigation
Audit Trail
fetch()with typed Connect-RPC client (clients.audit.listAuditEntries)Data Display
StatusBadgeEntityLinkUX Polish
<textarea>withStarlarkEditor(CodeMirror with Python mode, syntax highlighting, linting)Test plan
npx tsc --noEmit- zero errorsnpm run test -- --run- 1580/1580 tests passing (120 test files)npx vite build- production build succeedsgo build ./...- Go backend compiles cleanlyOut of scope