Add datetime filter for Workflows Page#693
Open
fernandol-nvidia wants to merge 5 commits intomainfrom
Open
Conversation
fernandol-nvidia
temporarily deployed
to
internal-ci
GitHub Actions
Inactive
📝 WalkthroughWalkthroughAdds date-range filtering UI and plumbing (picker, types, keyboard flow, parsing, API adapter), introduces a production-first server auth module with dev fallbacks, minor config aliasing and export changes, and documents a new "Reasoning Integrity" section in CLAUDE.md. Changes
sequenceDiagram
actor User
participant UI as FilterBar UI
participant FBP as FilterBarDatePicker
participant KB as useFilterKeyboard
participant State as useFilterState / FilterBar
participant API as Workflows Adapter
participant BE as Backend API
User->>UI: open filter, focus submitted
UI->>FBP: render date picker (presets/custom)
User->>FBP: select preset or enter From/To
FBP->>FBP: validate range (To > From)
FBP->>State: onCommit(value)
State->>State: handleDateCommit -> addChip(parsed.field, value)
User->>KB: Tab / Shift-Tab
KB->>FBP: stepCycle(direction, fromValue)
State->>API: build query params (include submitted_after/submitted_before)
API->>BE: GET /workflows?submitted_after=...&submitted_before=...
BE-->>API: return workflows
API-->>State: results
State-->>UI: render filtered workflows
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 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 |
fernandol-nvidia
temporarily deployed
to
internal-ci
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/ui/src/components/filter-bar/hooks/use-suggestions.ts (1)
115-136: Variable shadowing:queryredeclared on line 118.The
queryvariable on line 118 shadows the outerqueryfrom line 82. While both represent lowercased queries, the shadowing can cause confusion during maintenance.♻️ Suggested fix: rename inner variable
if (isDateRangeField(parsedInput.field)) { - const query = parsedInput.query.toLowerCase(); + const presetQuery = parsedInput.query.toLowerCase(); for (const preset of DATE_RANGE_PRESETS) { - if (!query || preset.label.toLowerCase().includes(query)) { + if (!presetQuery || preset.label.toLowerCase().includes(presetQuery)) { items.push({ type: "value", field: parsedInput.field, value: preset.label, label: preset.label, }); } } // Custom range inputs are always shown (only when not filtering presets) - if (!query) { + if (!presetQuery) { items.push({ type: "value", field: parsedInput.field, value: DATE_CUSTOM_FROM, label: "From date" });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/src/components/filter-bar/hooks/use-suggestions.ts` around lines 115 - 136, The inner lowercase query shadows the outer variable; in the isDateRangeField block (around isDateRangeField(parsedInput.field) and DATE_RANGE_PRESETS) rename the inner variable (e.g., to lowerQuery or fieldQuery) instead of reusing query, and update all uses in that block (the if check and preset.label.toLowerCase().includes(...) call and the subsequent if (!...) that decides showing custom inputs) so the outer parsedInput.query remains unchanged and shadowing is removed.src/ui/src/components/filter-bar/filter-bar-date-picker.tsx (1)
151-159: Minor: Redundant aria-label.The
<label htmlFor="fb-date-from">From</label>already provides an accessible name for the input. The additionalaria-label="From date"is redundant (and slightly differs from the visible label text).🔧 Suggested simplification
<input ref={fromRef} id="fb-date-from" type="datetime-local" value={fromDate} onChange={(e) => handleFromChange(e.target.value)} className="fb-date-input" - aria-label="From date" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/src/components/filter-bar/filter-bar-date-picker.tsx` around lines 151 - 159, The input element using ref fromRef with id "fb-date-from" already has an associated visible label (<label htmlFor="fb-date-from">From</label>), so remove the redundant aria-label="From date" from that input; keep the other props (type="datetime-local", value={fromDate}, onChange={e => handleFromChange(e.target.value)}, className="fb-date-input") unchanged to preserve behavior and accessibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ui/CLAUDE.md`:
- Around line 33-36: The two markdown code fences containing the examples
starting with "❌ A strips time from a datetime-local input → downgrade input to
type=\"date\" for \"consistency\"" and the block beginning "Fact: backend
accepts datetime.datetime" are missing fence languages and trigger MD040; update
both triple-backtick fences to use a language token (e.g., ```text) so they read
```text ... ``` and apply the same fix to the other pair of example blocks
referenced (the ones around the "Fact: backend accepts datetime.datetime" area
reported as also affected).
In `@src/ui/src/lib/auth/server.production.ts`:
- Around line 1-15: Update the top-of-file copyright header so the line
containing the copyright holder reads "Copyright (c) 2026 NVIDIA CORPORATION &
AFFILIATES. All rights reserved." (i.e., keep "All rights reserved." on the same
line as "NVIDIA CORPORATION & AFFILIATES"); modify the header block at the very
top of server.production.ts to restore the exact required NVIDIA copyright
header text and SPDX identifier.
In `@src/ui/src/lib/auth/server.ts`:
- Line 41: The re-export always pulls hasServerAdminRole from the
production-only implementation so it bypasses the DEV_USER_ROLES dev fallback;
change the export to use the environment-aware implementation (or conditionally
delegate to the production implementation only in production). Concretely:
import the production and the environment-aware/dev implementations of
hasServerAdminRole (the current hasServerAdminRole from server.production and
the one that honors DEV_USER_ROLES used by getServerUserRoles/getServerUser) and
re-export a single hasServerAdminRole that selects the correct implementation
based on the runtime environment (e.g. use the production implementation only
when NODE_ENV === 'production', otherwise use the dev/environment-aware
version).
In `@src/ui/src/lib/date-range-utils.ts`:
- Around line 154-168: parseIsoDate currently treats "YYYY-MM-DDTHH:mm" by
calling new Date(str) which interprets the value in the runtime's local timezone
and can cause SSR hydration mismatches; change the implementation so the
datetime-local branch only parses on the client (e.g. guard with a window check
or isBrowser helper) or else parse deterministically (construct a Date from
numeric year/month/day/hour/minute using Date.UTC if you want UTC). Update the
branch matching /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}$/ to either skip/return null on
the server or replace new Date(str) with an explicit, timezone-agnostic
construction; add a short comment in parseIsoDate explaining the chosen
approach.
---
Nitpick comments:
In `@src/ui/src/components/filter-bar/filter-bar-date-picker.tsx`:
- Around line 151-159: The input element using ref fromRef with id
"fb-date-from" already has an associated visible label (<label
htmlFor="fb-date-from">From</label>), so remove the redundant aria-label="From
date" from that input; keep the other props (type="datetime-local",
value={fromDate}, onChange={e => handleFromChange(e.target.value)},
className="fb-date-input") unchanged to preserve behavior and accessibility.
In `@src/ui/src/components/filter-bar/hooks/use-suggestions.ts`:
- Around line 115-136: The inner lowercase query shadows the outer variable; in
the isDateRangeField block (around isDateRangeField(parsedInput.field) and
DATE_RANGE_PRESETS) rename the inner variable (e.g., to lowerQuery or
fieldQuery) instead of reusing query, and update all uses in that block (the if
check and preset.label.toLowerCase().includes(...) call and the subsequent if
(!...) that decides showing custom inputs) so the outer parsedInput.query
remains unchanged and shadowing is removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6d3bafb1-a923-4b49-b4ba-1ee67cdf85bb
📒 Files selected for processing (18)
src/ui/CLAUDE.mdsrc/ui/next.config.tssrc/ui/src/components/filter-bar/filter-bar-date-picker.tsxsrc/ui/src/components/filter-bar/filter-bar-dropdown.tsxsrc/ui/src/components/filter-bar/filter-bar.csssrc/ui/src/components/filter-bar/filter-bar.tsxsrc/ui/src/components/filter-bar/hooks/use-filter-keyboard.tssrc/ui/src/components/filter-bar/hooks/use-filter-state.tssrc/ui/src/components/filter-bar/hooks/use-suggestions.tssrc/ui/src/components/filter-bar/lib/types.tssrc/ui/src/features/workflows/list/components/workflows-toolbar.tsxsrc/ui/src/features/workflows/list/lib/workflow-search-fields.test.tssrc/ui/src/features/workflows/list/lib/workflow-search-fields.tssrc/ui/src/lib/api/adapter/workflows-shim.tssrc/ui/src/lib/auth/server.production.tssrc/ui/src/lib/auth/server.tssrc/ui/src/lib/date-range-utils.tssrc/ui/src/lib/format-date.ts
fernandol-nvidia
temporarily deployed
to
internal-ci
GitHub Actions
Inactive
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/ui/src/lib/auth/server.ts (1)
68-75: Dev fallback user construction is functional but differs slightly from production.The production
getServerUserusesderiveDisplayName(username)for the name andgetInitials(name)for initials, while the dev fallback uses the raw username as the name and a simpleslice(0, 2).toUpperCase()for initials. This is acceptable for development purposes, but if you want exact parity with production behavior, consider importing and using these helper functions.♻️ Optional: match production behavior
+import { deriveDisplayName, getInitials } from "@/lib/auth/server.production"; // ... return { id: username, - name: username, + name: deriveDisplayName(username), email, username, isAdmin: hasAdminRole(roles), - initials: username.slice(0, 2).toUpperCase(), + initials: getInitials(deriveDisplayName(username)), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/src/lib/auth/server.ts` around lines 68 - 75, The dev fallback user construction returns the raw username and uses slice for initials which differs from production; update the fallback in the server-side auth (the object returned where id/name/email/username/isAdmin/initials are set) to call deriveDisplayName(username) for the name and getInitials(name) (or getInitials(deriveDisplayName(username))) for initials, and add the necessary imports for deriveDisplayName and getInitials so the dev fallback matches getServerUser's display-name and initials logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/ui/src/lib/auth/server.ts`:
- Around line 68-75: The dev fallback user construction returns the raw username
and uses slice for initials which differs from production; update the fallback
in the server-side auth (the object returned where
id/name/email/username/isAdmin/initials are set) to call
deriveDisplayName(username) for the name and getInitials(name) (or
getInitials(deriveDisplayName(username))) for initials, and add the necessary
imports for deriveDisplayName and getInitials so the dev fallback matches
getServerUser's display-name and initials logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5f2958d4-335f-429a-b518-e34ccb9cbf04
📒 Files selected for processing (3)
src/ui/CLAUDE.mdsrc/ui/src/lib/auth/server.tssrc/ui/src/lib/date-range-utils.ts
RyaliNvidia
previously approved these changes
Mar 11, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ui/src/components/filter-bar/filter-bar-date-picker.tsx`:
- Around line 116-121: The fb-date-picker element's onKeyDown currently calls
e.stopPropagation() unconditionally, which prevents the parent's
onEscape/closeDropdown from receiving Escape; update the onKeyDown handler in
filter-bar-date-picker.tsx (the element with className "fb-date-picker") to only
stop propagation for keys other than Escape so Escape can bubble up to the
parent's onEscape and allow closeDropdown() to run.
In `@src/ui/src/lib/auth/server.ts`:
- Around line 43-49: prodGetServerUserRoles currently conflates "no auth
headers" and "authenticated user with empty roles" by returning an empty array;
update the logic so prodGetServerUserRoles returns null (or undefined) when the
required auth headers are missing, and keep returning an empty array for an
authenticated user with no roles. Then change getServerUserRoles to only fall
back to process.env.DEV_USER_ROLES when prodGetServerUserRoles() returns
null/undefined; if it returns an empty array, return that empty array unchanged.
Update references: prodGetServerUserRoles and getServerUserRoles to implement
this sentinel behavior so DEV_USER_ROLES is used strictly when auth headers are
absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 413bc287-cba4-45ee-8a29-16261cbb584f
📒 Files selected for processing (4)
src/ui/src/components/filter-bar/filter-bar-date-picker.tsxsrc/ui/src/components/filter-bar/hooks/use-suggestions.tssrc/ui/src/lib/auth/server.production.tssrc/ui/src/lib/auth/server.ts
Comment on lines
+116
to
+121
| return ( | ||
| <div | ||
| className="fb-date-picker" | ||
| role="none" | ||
| onKeyDown={(e) => e.stopPropagation()} | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how parent components handle keyboard events and whether Escape is expected to bubble
rg -n "onKeyDown|handleKeyDown" --type=ts --type=tsx -C3 src/ui/src/components/filter-bar/ | head -80Repository: NVIDIA/OSMO
Length of output: 82
🏁 Script executed:
#!/bin/bash
# Search for onKeyDown handlers in filter-bar components
rg -n "onKeyDown|handleKeyDown" -C3 src/ui/src/components/filter-bar/Repository: NVIDIA/OSMO
Length of output: 10776
🏁 Script executed:
#!/bin/bash
# List files in filter-bar directory
find src/ui/src/components/filter-bar -type f -name "*.ts" -o -name "*.tsx" | head -20Repository: NVIDIA/OSMO
Length of output: 1180
🏁 Script executed:
#!/bin/bash
# Read the filter-bar-date-picker.tsx file to understand the context
wc -l src/ui/src/components/filter-bar/filter-bar-date-picker.tsxRepository: NVIDIA/OSMO
Length of output: 118
🏁 Script executed:
#!/bin/bash
# Read the date picker file
cat -n src/ui/src/components/filter-bar/filter-bar-date-picker.tsx | head -230Repository: NVIDIA/OSMO
Length of output: 9873
🏁 Script executed:
#!/bin/bash
# Check the Escape key handling in use-filter-keyboard.ts
sed -n '169,202p' src/ui/src/components/filter-bar/hooks/use-filter-keyboard.tsRepository: NVIDIA/OSMO
Length of output: 1020
🏁 Script executed:
#!/bin/bash
# Search for onEscape implementation in the keyboard hook
rg -n "onEscape" src/ui/src/components/filter-bar/hooks/use-filter-keyboard.ts -A 10Repository: NVIDIA/OSMO
Length of output: 674
🏁 Script executed:
#!/bin/bash
# Look at FilterBarInput which seems to manage the date picker visibility
cat -n src/ui/src/components/filter-bar/filter-bar-input.tsx | sed -n '1,150p'Repository: NVIDIA/OSMO
Length of output: 5741
🏁 Script executed:
#!/bin/bash
# Get the full onEscape function
sed -n '338,360p' src/ui/src/components/filter-bar/hooks/use-filter-keyboard.tsRepository: NVIDIA/OSMO
Length of output: 567
🏁 Script executed:
#!/bin/bash
# Look for the date picker rendering in FilterBar
rg -n "FilterBarDatePicker\|showDropdown\|activeDateRangeField" src/ui/src/components/filter-bar/filter-bar.tsx -B 2 -A 2Repository: NVIDIA/OSMO
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Get more of the onEscape function
sed -n '338,375p' src/ui/src/components/filter-bar/hooks/use-filter-keyboard.tsRepository: NVIDIA/OSMO
Length of output: 830
🏁 Script executed:
#!/bin/bash
# Search for FilterBarDatePicker in FilterBar component
cat -n src/ui/src/components/filter-bar/filter-bar.tsx | sed -n '1,200p'Repository: NVIDIA/OSMO
Length of output: 7204
🏁 Script executed:
#!/bin/bash
# Check what closeDropdown does
rg -n "closeDropdown" src/ui/src/components/filter-bar/hooks/use-filter-state.ts -B 2 -A 5Repository: NVIDIA/OSMO
Length of output: 394
Remove stopPropagation() or allow Escape to bubble to parent's keyboard handler.
The onKeyDown={(e) => e.stopPropagation()} blocks all keyboard events, including Escape, from bubbling to the parent's keyboard navigation system. The parent's onEscape handler expects Escape to propagate and calls closeDropdown() to close the date picker. Since the date picker does not handle Escape itself, pressing Escape has no effect. Only Tab has custom handling on the Apply button (intentional), so consider replacing the blanket stopPropagation() with a selective check: if (e.key !== "Escape") e.stopPropagation().
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ui/src/components/filter-bar/filter-bar-date-picker.tsx` around lines 116
- 121, The fb-date-picker element's onKeyDown currently calls
e.stopPropagation() unconditionally, which prevents the parent's
onEscape/closeDropdown from receiving Escape; update the onKeyDown handler in
filter-bar-date-picker.tsx (the element with className "fb-date-picker") to only
stop propagation for keys other than Escape so Escape can bubble up to the
parent's onEscape and allow closeDropdown() to run.
Comment on lines
43
to
49
| export async function getServerUserRoles(): Promise<string[]> { | ||
| const headersList = await headers(); | ||
| const rolesHeader = headersList.get("x-osmo-roles") || ""; | ||
|
|
||
| // Parse roles (comma-separated or space-separated) | ||
| const roles = rolesHeader | ||
| const roles = await prodGetServerUserRoles(); | ||
| if (roles.length > 0) return roles; | ||
| return (process.env.DEV_USER_ROLES ?? "") | ||
| .split(/[,\s]+/) | ||
| .map((role) => role.trim()) | ||
| .map((r) => r.trim()) | ||
| .filter(Boolean); |
There was a problem hiding this comment.
Only use DEV_USER_ROLES when auth headers are actually missing.
prodGetServerUserRoles() normalizes both “no x-osmo-roles header” and “authenticated user with an empty role list” to [] in src/ui/src/lib/auth/server.production.ts Lines 28-34. With the current roles.length === 0 check, a proxied user who legitimately has no roles will inherit DEV_USER_ROLES and can appear admin in local testing.
Suggested fix
export async function getServerUserRoles(): Promise<string[]> {
const roles = await prodGetServerUserRoles();
if (roles.length > 0) return roles;
+ if (await prodGetServerUsername()) return roles;
return (process.env.DEV_USER_ROLES ?? "")
.split(/[,\s]+/)
.map((r) => r.trim())
.filter(Boolean);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ui/src/lib/auth/server.ts` around lines 43 - 49, prodGetServerUserRoles
currently conflates "no auth headers" and "authenticated user with empty roles"
by returning an empty array; update the logic so prodGetServerUserRoles returns
null (or undefined) when the required auth headers are missing, and keep
returning an empty array for an authenticated user with no roles. Then change
getServerUserRoles to only fall back to process.env.DEV_USER_ROLES when
prodGetServerUserRoles() returns null/undefined; if it returns an empty array,
return that empty array unchanged. Update references: prodGetServerUserRoles and
getServerUserRoles to implement this sentinel behavior so DEV_USER_ROLES is used
strictly when auth headers are absent.
cypres
approved these changes
Mar 12, 2026
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
3 participants
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.
Description
Issue #682
Checklist
Summary by CodeRabbit
New Features
Style
Documentation
Chores