Skip to content

Refine observability UI for v0.2.0 - #1

Merged
indrazm merged 1 commit into
mainfrom
agent/refine-observability-ui-v0-2-0
Aug 7, 2026
Merged

Refine observability UI for v0.2.0#1
indrazm merged 1 commit into
mainfrom
agent/refine-observability-ui-v0-2-0

Conversation

@indrazm

@indrazm indrazm commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

What changed

  • align the Evals navigation, tables, timestamps, status chips, run detail summary, and quality-gate management with the observability UI
  • simplify login, registration, Connect, and Project Settings layouts and constrain page headers to their content width
  • remove project redaction configuration across contracts, API, database schema, seed data, and telemetry normalization
  • add the PostgreSQL migration for the removed redaction settings column
  • bump the root release version from 0.1.0 to 0.2.0

Why

Several evaluation and project-management screens used different visual patterns from Traces and Sessions. Quality-gate rules also remounted while typing because their React keys were derived from editable JSON, making the modal difficult to control. Redaction controls were no longer wanted in the product.

Impact

The primary observability workflows now share compact timestamps, semantic status badges, table behavior, and clearer responsive layouts. Authentication and project setup are simpler. Existing deployments will apply migration 0007_remarkable_the_fallen.sql, which removes the obsolete project redaction settings column.

Validation

  • pnpm check
  • pnpm typecheck
  • pnpm test
  • production Docker build through docker-compose.dev.yml
  • web and API readiness endpoints verified after container recreation

Summary by CodeRabbit

  • New Features

    • Redesigned project connection setup with clearer credential and instrumentation steps.
    • Added improved evaluation run, comparison, quality gate, status, sorting, and timestamp displays.
    • Added branded authentication and invitation cards.
  • Enhancements

    • Improved project settings with separate API key and retention cards.
    • Added key status details, revoke actions, validation, and save feedback.
    • Evaluation breadcrumbs now reflect nested pages and run details.
  • Changes

    • Removed configurable attribute-redaction settings from project configuration and ingestion.
    • Updated the application version to 0.2.0.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change removes configurable redaction patterns across contracts, telemetry, APIs, and database storage. It adds observability UI components and quality-gate workflows. It redesigns project and authentication screens, updates fixtures and tests, and increments the package version.

Changes

Redaction configuration removal

Layer / File(s) Summary
Redaction contracts and normalization
packages/contracts/src/projects.ts, packages/contracts/test/contracts.test.ts, packages/telemetry/src/*, packages/telemetry/test/telemetry.test.ts
Project settings no longer include redactionPatterns. Telemetry uses default patterns only.
Persistence and ingestion wiring
packages/db/..., apps/api/src/modules/ingestion/..., apps/api/src/modules/projects/...
Database persistence, project settings updates, project mapping, and ingestion normalization no longer use redaction patterns.
Settings fixture alignment
apps/web/src/components/app-shell.test.tsx, apps/web/src/components/project-rail.test.tsx, apps/web/src/modules/observability/components/trace-data-table.test.tsx
Project fixtures omit the removed settings field.

Observability interface updates

Layer / File(s) Summary
Evaluation navigation and shared display components
apps/web/src/components/app-header.tsx, apps/web/src/components/app-header.test.tsx, apps/web/src/modules/observability/components/status-badge.tsx, apps/web/src/modules/observability/components/evaluation-status-badge.tsx, apps/web/src/modules/observability/components/table-timestamp.tsx, apps/web/src/modules/observability/components/*test.tsx
Evaluation breadcrumbs, semantic status badges, and shared timestamp rendering are added and tested.
Evaluation tables and run details
apps/web/src/modules/observability/components/evaluations-view.tsx, apps/web/src/modules/observability/components/evaluation-runs-view.tsx, apps/web/src/modules/observability/components/evaluation-compare-view.tsx, apps/web/src/modules/observability/components/evaluation-run-detail-view.tsx, apps/web/src/modules/observability/utils/observability-view.tsx
Tables gain accessible sorting and sticky headers. Evaluation outcomes and timestamps use shared components. Run details use summary metrics and reusable badges.
Quality-gate management flow
apps/web/src/modules/observability/components/quality-gates-view.tsx, apps/web/src/modules/observability/components/quality-gates-view.test.tsx
Quality gates use a table, structured editor, validation, percentage conversion, stable draft rule IDs, and deletion confirmation.

Project and authentication surfaces

Layer / File(s) Summary
Branded authentication cards
apps/web/src/components/centered-card.tsx, apps/web/src/modules/auth/components/auth-form.tsx, apps/web/src/modules/auth/components/invitation-card.tsx
Centered cards support branded presentation. Authentication forms and invitation cards use the branded layout and updated copy.
Project connection and settings redesign
apps/web/src/components/page.tsx, apps/web/src/modules/projects/components/connect-content.tsx, apps/web/src/modules/projects/components/project-settings.tsx, apps/web/src/modules/projects/hooks/use-project-settings.ts
Connection setup uses credential and instrumentation cards. Project settings use ingestion-key and retention cards.
Project UI validation
apps/web/src/modules/projects/components/project-pages.test.tsx, apps/web/src/components/app-shell.test.tsx, apps/web/src/components/project-rail.test.tsx
Tests cover the revised project pages, layout classes, and settings fixtures.
Package release metadata
package.json
The package version changes from 0.1.0 to 0.2.0.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant AppHeader
  participant EvaluationRunsView
  participant EvaluationRunDetailView
  participant EvaluationStatusBadge
  User->>AppHeader: Open an evaluation route
  AppHeader->>EvaluationRunsView: Link evaluation breadcrumb to runs
  User->>EvaluationRunsView: Select a run
  EvaluationRunsView->>EvaluationRunDetailView: Open run details
  EvaluationRunDetailView->>EvaluationStatusBadge: Render run and outcome statuses
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary observability UI refinements and identifies the v0.2.0 release context.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch agent/refine-observability-ui-v0-2-0

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@indrazm
indrazm marked this pull request as ready for review August 7, 2026 17:19
@cursor

cursor Bot commented Aug 7, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (6)
packages/db/migrations/postgres/0007_remarkable_the_fallen.sql (1)

1-1: 🩺 Stability & Availability | 🔵 Trivial

Verify the rollout order before dropping the column.

ALTER TABLE "projects" DROP COLUMN "redaction_patterns" is destructive. If a v0.1.0 API instance still runs after migration 0007, project queries or writes can fail because the column no longer exists. Apply this migration after old instances stop using the column, or use an expand/contract rollout.

#!/bin/bash
set -euo pipefail

# Historical migrations may match. Inspect all remaining runtime references.
rg -n --hidden --glob '!.git/**' --glob '!node_modules/**' \
  'redaction_patterns|redactionPatterns' . || true

# Confirm that deployment upgrades or stops old API instances before migration 0007.
rg -n --hidden --glob '!.git/**' --glob '!node_modules/**' \
  '0007_remarkable_the_fallen|drizzle|migration|migrate|rollout|deploy|docker|compose' . || true
🤖 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/db/migrations/postgres/0007_remarkable_the_fallen.sql` at line 1,
Verify the rollout sequence for migration 0007 before applying the destructive
redaction_patterns drop: confirm no v0.1.0 API instances or other runtime
references still read or write redaction_patterns/redactionPatterns, and ensure
deployment stops or upgrades those instances before this migration runs;
otherwise use an expand/contract migration sequence.
apps/web/src/modules/observability/components/evaluation-runs-view.tsx (1)

247-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Show the sort direction visually.

The icon only changes color when the column is active. Sighted users cannot tell ascending from descending. Screen-reader users get the direction from aria-sort. Render a direction-specific icon (for example ArrowUp / ArrowDown) when the column is active, and keep ArrowsDownUp for inactive columns.

🤖 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 `@apps/web/src/modules/observability/components/evaluation-runs-view.tsx`
around lines 247 - 261, Update the sort icon rendering in the Button around the
sort(field) handler: show ArrowUp or ArrowDown based on props.filters.order when
column is active, and use ArrowsDownUp for inactive columns. Preserve the
existing active-column styling and aria-label behavior.
apps/web/src/modules/observability/components/quality-gates-view.tsx (3)

446-461: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

Changing the measure reinterprets the stored value.

percentage depends on rule.measure. The measure select changes only measure and keeps the stored number. A pass rate of 0.9 shown as "90" becomes an average score shown as "0.9" after the user picks "Average score". The displayed number changes without the user editing it. Reset the value to the type default when the measure crosses the percentage boundary, or state the unit next to the field so the change is expected.

🤖 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 `@apps/web/src/modules/observability/components/quality-gates-view.tsx` around
lines 446 - 461, Update the measure change handler in the rule editor so
switching between “pass_rate” and “average_score” also resets the stored value
to the appropriate type default when crossing the percentage boundary,
preventing the existing value from being reinterpreted under the new measure.

366-382: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Switching the check type discards the metric name.

ruleForType builds a fresh rule and resets metricName to "". evaluation_threshold and evaluation_regression both carry metricName and measure. A user who switches between these two types loses the values already typed. Carry the shared fields across the change.

♻️ Proposed fix
-function ruleForType(type: QualityGateRule["type"]): QualityGateRule {
+function ruleForType(type: QualityGateRule["type"], previous?: QualityGateRule): QualityGateRule {
+  const carried =
+    previous && previous.type !== "operational_regression"
+      ? { metricName: previous.metricName, measure: previous.measure }
+      : {};
   if (type === "evaluation_regression") {
     return {
       type,
       metricName: "",
       measure: "pass_rate",
+      ...carried,
       direction: "decrease",
       maxAbsoluteChange: 0.05,
     };
   }
   if (type === "operational_regression") {
     return { type, measure: "p95_latency_ms", maxIncreasePercent: 15 };
   }
-  return defaultRule();
+  return { ...defaultRule(), ...carried };
 }

Then pass the current rule at the call site:

-            props.onChange(ruleForType(event.target.value as QualityGateRule["type"]))
+            props.onChange(ruleForType(event.target.value as QualityGateRule["type"], rule))
🤖 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 `@apps/web/src/modules/observability/components/quality-gates-view.tsx` around
lines 366 - 382, Update the type-change handler in the NativeSelect to pass the
current rule into ruleForType, and modify ruleForType to preserve the shared
metricName and measure fields when switching between evaluation_threshold and
evaluation_regression. Keep type-specific fields reset as appropriate while
retaining the existing values for these shared fields.

615-636: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Tell the user which field blocks the save.

isValidGate returns a single boolean. Line 321 disables the submit button when it is false. No field shows an error, and no input receives aria-invalid. A user with an empty metric name or an out-of-range target sees a disabled button and no reason.

Return per-field issues from the validator, mark the offending inputs with aria-invalid, and render the messages with the FieldError slot from @lens/ui/components/field. Keep the submit button enabled and report the errors on submit, so keyboard and screen-reader users get the feedback.

🤖 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 `@apps/web/src/modules/observability/components/quality-gates-view.tsx` around
lines 615 - 636, Replace the boolean-only isValidGate validation with per-field
issues that identify invalid gate and rule inputs, and keep the submit button
enabled so submission surfaces those issues. In the quality-gates form, apply
aria-invalid to each offending input and render its message through the
FieldError slot from `@lens/ui/components/field`; ensure submit focuses or
otherwise exposes the first error for keyboard and screen-reader users.
apps/web/src/modules/observability/components/table-timestamp.test.tsx (1)

10-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Strengthen the assertions in both tests.

Line 17 compares the rendered text to formatTableTimestamp(value), the same function the component calls. The assertion passes for any formatting change, so it cannot detect a regression. Use a literal expected string and pin the time zone (for example process.env.TZ or a Vitest env option) to make the format observable.

The second test claims the component keeps an invalid timestamp visible, but it never renders TableTimestamp. Render the component to cover that path.

♻️ Proposed test change
   it("keeps an invalid timestamp visible", () => {
-    expect(formatTableTimestamp("unknown")).toBe("unknown");
+    render(<TableTimestamp value="unknown" />);
+
+    expect(screen.getByTitle("unknown").textContent).toBe("unknown");
   });
🤖 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 `@apps/web/src/modules/observability/components/table-timestamp.test.tsx`
around lines 10 - 22, Strengthen both tests around TableTimestamp: in the valid
timestamp test, pin the test time zone and assert against a literal formatted
string instead of calling formatTableTimestamp; in the invalid timestamp test,
render TableTimestamp with "unknown" and assert the rendered output remains
visible. Preserve the existing class assertions and use the component’s rendered
element for both content checks.
🤖 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 `@apps/web/src/components/centered-card.tsx`:
- Around line 40-47: Update the centered card rendering around props.icon so the
bordered icon wrapper is rendered only when an icon exists, and apply the
badge’s top margin only in that case. Preserve the badge rendering and secondary
variant for cards without an icon.

In `@apps/web/src/modules/observability/components/quality-gates-view.tsx`:
- Around line 76-117: Surface mutation errors in the matching create/edit and
delete dialogs by rendering state.createGate.error, state.updateGate.error, and
state.deleteGate.error near their respective actions, unless
use-evaluation-workspace already reports these errors globally. Preserve the
existing onSuccess close behavior and show actionable error text while each
mutation is pending or failed.
- Around line 267-276: In
apps/web/src/modules/observability/components/quality-gates-view.tsx#L267-L276,
add one shared number-input helper that maps an empty string to NaN and use it
for minimumCaseCount, rendering NaN as an empty input value. Apply the same
helper at `#L405-L414` for maxIncreasePercent and at `#L535-L543` inside
RuleValueField.onChange, also rendering its NaN value as empty; preserve
isValidGate’s finite-number validation so cleared fields cannot be saved.
- Around line 167-169: Update the Edit button in the quality-gate row rendering
to include an accessible label containing the gate name, matching the Delete
button’s `aria-label` pattern while preserving the visible “Edit” text. Update
the corresponding expectation in quality-gates-view.test.tsx to query the newly
specific accessible name.

---

Nitpick comments:
In `@apps/web/src/modules/observability/components/evaluation-runs-view.tsx`:
- Around line 247-261: Update the sort icon rendering in the Button around the
sort(field) handler: show ArrowUp or ArrowDown based on props.filters.order when
column is active, and use ArrowsDownUp for inactive columns. Preserve the
existing active-column styling and aria-label behavior.

In `@apps/web/src/modules/observability/components/quality-gates-view.tsx`:
- Around line 446-461: Update the measure change handler in the rule editor so
switching between “pass_rate” and “average_score” also resets the stored value
to the appropriate type default when crossing the percentage boundary,
preventing the existing value from being reinterpreted under the new measure.
- Around line 366-382: Update the type-change handler in the NativeSelect to
pass the current rule into ruleForType, and modify ruleForType to preserve the
shared metricName and measure fields when switching between evaluation_threshold
and evaluation_regression. Keep type-specific fields reset as appropriate while
retaining the existing values for these shared fields.
- Around line 615-636: Replace the boolean-only isValidGate validation with
per-field issues that identify invalid gate and rule inputs, and keep the submit
button enabled so submission surfaces those issues. In the quality-gates form,
apply aria-invalid to each offending input and render its message through the
FieldError slot from `@lens/ui/components/field`; ensure submit focuses or
otherwise exposes the first error for keyboard and screen-reader users.

In `@apps/web/src/modules/observability/components/table-timestamp.test.tsx`:
- Around line 10-22: Strengthen both tests around TableTimestamp: in the valid
timestamp test, pin the test time zone and assert against a literal formatted
string instead of calling formatTableTimestamp; in the invalid timestamp test,
render TableTimestamp with "unknown" and assert the rendered output remains
visible. Preserve the existing class assertions and use the component’s rendered
element for both content checks.

In `@packages/db/migrations/postgres/0007_remarkable_the_fallen.sql`:
- Line 1: Verify the rollout sequence for migration 0007 before applying the
destructive redaction_patterns drop: confirm no v0.1.0 API instances or other
runtime references still read or write redaction_patterns/redactionPatterns, and
ensure deployment stops or upgrades those instances before this migration runs;
otherwise use an expand/contract migration sequence.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 3227477a-9374-40d3-a86a-e5c5a6e2bf49

📥 Commits

Reviewing files that changed from the base of the PR and between afead9d and 17462e1.

📒 Files selected for processing (41)
  • apps/api/src/modules/ingestion/logs-router.ts
  • apps/api/src/modules/ingestion/router.ts
  • apps/api/src/modules/projects/router.ts
  • apps/api/src/modules/projects/services.ts
  • apps/web/src/components/app-header.test.tsx
  • apps/web/src/components/app-header.tsx
  • apps/web/src/components/app-shell.test.tsx
  • apps/web/src/components/centered-card.tsx
  • apps/web/src/components/page.tsx
  • apps/web/src/components/project-rail.test.tsx
  • apps/web/src/modules/auth/components/auth-form.tsx
  • apps/web/src/modules/auth/components/invitation-card.tsx
  • apps/web/src/modules/observability/components/evaluation-compare-view.tsx
  • apps/web/src/modules/observability/components/evaluation-run-detail-view.tsx
  • apps/web/src/modules/observability/components/evaluation-runs-view.tsx
  • apps/web/src/modules/observability/components/evaluation-status-badge.test.tsx
  • apps/web/src/modules/observability/components/evaluation-status-badge.tsx
  • apps/web/src/modules/observability/components/evaluations-view.tsx
  • apps/web/src/modules/observability/components/quality-gates-view.test.tsx
  • apps/web/src/modules/observability/components/quality-gates-view.tsx
  • apps/web/src/modules/observability/components/status-badge.tsx
  • apps/web/src/modules/observability/components/table-timestamp.test.tsx
  • apps/web/src/modules/observability/components/table-timestamp.tsx
  • apps/web/src/modules/observability/components/trace-data-table.test.tsx
  • apps/web/src/modules/observability/utils/observability-view.tsx
  • apps/web/src/modules/projects/components/connect-content.tsx
  • apps/web/src/modules/projects/components/project-pages.test.tsx
  • apps/web/src/modules/projects/components/project-settings.tsx
  • apps/web/src/modules/projects/hooks/use-project-settings.ts
  • package.json
  • packages/contracts/src/projects.ts
  • packages/contracts/test/contracts.test.ts
  • packages/db/migrations/postgres/0007_remarkable_the_fallen.sql
  • packages/db/migrations/postgres/meta/0007_snapshot.json
  • packages/db/migrations/postgres/meta/_journal.json
  • packages/db/src/schema.ts
  • packages/db/src/seed-runner.ts
  • packages/telemetry/src/normalization.ts
  • packages/telemetry/src/normalize-evaluations.ts
  • packages/telemetry/src/normalize-traces.ts
  • packages/telemetry/test/telemetry.test.ts
💤 Files with no reviewable changes (9)
  • apps/api/src/modules/ingestion/router.ts
  • apps/api/src/modules/ingestion/logs-router.ts
  • packages/db/src/schema.ts
  • packages/contracts/src/projects.ts
  • apps/api/src/modules/projects/router.ts
  • packages/telemetry/src/normalization.ts
  • packages/db/src/seed-runner.ts
  • apps/api/src/modules/projects/services.ts
  • apps/web/src/modules/projects/hooks/use-project-settings.ts

Comment on lines +40 to +47
) : (
<>
<span className="mx-auto flex size-10 items-center justify-center rounded-full border bg-background text-foreground">
{props.icon}
</span>
<Badge className="mx-auto mt-2" variant="secondary">
{props.eyebrow}
</Badge>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Do not render an empty icon container.

icon is optional, but the standard layout always renders its wrapper. A caller without icon shows an empty bordered circle above the badge. Render the wrapper only when props.icon exists. Remove the badge top margin when no icon exists.

Proposed fix
-                <span className="mx-auto flex size-10 items-center justify-center rounded-full border bg-background text-foreground">
-                  {props.icon}
-                </span>
-                <Badge className="mx-auto mt-2" variant="secondary">
+                {props.icon ? (
+                  <span className="mx-auto flex size-10 items-center justify-center rounded-full border bg-background text-foreground">
+                    {props.icon}
+                  </span>
+                ) : null}
+                <Badge className={cn("mx-auto", props.icon && "mt-2")} variant="secondary">
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
) : (
<>
<span className="mx-auto flex size-10 items-center justify-center rounded-full border bg-background text-foreground">
{props.icon}
</span>
<Badge className="mx-auto mt-2" variant="secondary">
{props.eyebrow}
</Badge>
) : (
<>
{props.icon ? (
<span className="mx-auto flex size-10 items-center justify-center rounded-full border bg-background text-foreground">
{props.icon}
</span>
) : null}
<Badge className={cn("mx-auto", props.icon && "mt-2")} variant="secondary">
{props.eyebrow}
</Badge>
🤖 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 `@apps/web/src/components/centered-card.tsx` around lines 40 - 47, Update the
centered card rendering around props.icon so the bordered icon wrapper is
rendered only when an icon exists, and apply the badge’s top margin only in that
case. Preserve the badge rendering and secondary variant for cards without an
icon.

Comment on lines 76 to +117
<GateDialog
item={editing}
saving={saving}
onClose={() => setEditing(null)}
onSave={(input) => {
if (editing === "new")
if (editing === "new") {
state.createGate.mutate(input, { onSuccess: () => setEditing(null) });
else if (editing)
} else if (editing) {
state.updateGate.mutate(
{ id: editing.id, input },
{ onSuccess: () => setEditing(null) },
);
}
}}
/>

<AlertDialog open={deleting !== null} onOpenChange={(open) => !open && setDeleting(null)}>
<AlertDialogContent>
<AlertDialogHeader>
<AlertDialogTitle>Delete this quality gate?</AlertDialogTitle>
<AlertDialogDescription>
{deleting
? `“${deleting.name}” will no longer be available when comparing evaluation runs.`
: "This gate will no longer be available when comparing evaluation runs."}
</AlertDialogDescription>
</AlertDialogHeader>
<AlertDialogFooter>
<AlertDialogCancel>Cancel</AlertDialogCancel>
<AlertDialogAction
variant="destructive"
disabled={state.deleteGate.isPending}
onClick={() => {
if (deleting) {
state.deleteGate.mutate(deleting.id, { onSuccess: () => setDeleting(null) });
}
}}
>
Delete gate
</AlertDialogAction>
</AlertDialogFooter>
</AlertDialogContent>
</AlertDialog>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Surface create, update, and delete errors.

Line 61 renders only state.gates.error. The gate dialog and the delete dialog close only in onSuccess. If a mutation fails, the dialog stays open, the button re-enables, and the user gets no reason for the failure. Render state.createGate.error, state.updateGate.error, and state.deleteGate.error in the matching dialog, unless a global error handler already reports them.

#!/bin/bash
# Description: Check whether mutations in the evaluation workspace hook report errors globally (toast, onError).
fd -t f 'use-evaluation-workspace.ts' | xargs rg -n -C4 'onError|toast|useMutation'
🤖 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 `@apps/web/src/modules/observability/components/quality-gates-view.tsx` around
lines 76 - 117, Surface mutation errors in the matching create/edit and delete
dialogs by rendering state.createGate.error, state.updateGate.error, and
state.deleteGate.error near their respective actions, unless
use-evaluation-workspace already reports these errors globally. Preserve the
existing onSuccess close behavior and show actionable error text while each
mutation is pending or failed.

Comment on lines +167 to +169
<Button size="sm" variant="ghost" onClick={() => props.onEdit(gate)}>
<PencilSimple /> Edit
</Button>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Give each Edit button a unique accessible name.

Every row renders an Edit button with the accessible name "Edit". With more than one gate, screen-reader users cannot tell the rows apart. The Delete button already uses aria-label={\Delete ${gate.name}`}`. Apply the same pattern to Edit.

♿ Proposed fix
-                    <Button size="sm" variant="ghost" onClick={() => props.onEdit(gate)}>
+                    <Button
+                      aria-label={`Edit ${gate.name}`}
+                      size="sm"
+                      variant="ghost"
+                      onClick={() => props.onEdit(gate)}
+                    >
                       <PencilSimple /> Edit
                     </Button>

This changes the accessible name, so update quality-gates-view.test.tsx line 16 accordingly.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button size="sm" variant="ghost" onClick={() => props.onEdit(gate)}>
<PencilSimple /> Edit
</Button>
<Button
aria-label={`Edit ${gate.name}`}
size="sm"
variant="ghost"
onClick={() => props.onEdit(gate)}
>
<PencilSimple /> Edit
</Button>
🤖 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 `@apps/web/src/modules/observability/components/quality-gates-view.tsx` around
lines 167 - 169, Update the Edit button in the quality-gate row rendering to
include an accessible label containing the gate name, matching the Delete
button’s `aria-label` pattern while preserving the visible “Edit” text. Update
the corresponding expectation in quality-gates-view.test.tsx to query the newly
specific accessible name.

Comment on lines 267 to 276
<Input
id="gate-minimum-cases"
min={1}
max={1_000_000}
type="number"
value={value.minimumCaseCount}
onChange={(event) =>
setValue({ ...value, minimumCaseCount: Number(event.target.value) })
}
/>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Number inputs turn a cleared field into 0. All three numeric inputs call Number(event.target.value). A cleared input[type=number] reports "", and Number("") returns 0, so the draft receives 0 and the field re-renders as "0". The user must delete the "0" before typing. For maxIncreasePercent and the rule value, 0 also passes isValidGate, so a cleared field can be saved as a real 0 threshold.

Add one shared helper and use it at each site, for example const numberInput = (raw: string) => (raw === "" ? Number.NaN : Number(raw));. isValidGate already rejects NaN through Number.isFinite, so the save stays blocked while the field is empty. Render the field with Number.isNaN(value) ? "" : value so the input stays empty.

  • apps/web/src/modules/observability/components/quality-gates-view.tsx#L267-L276: apply the helper to minimumCaseCount and render an empty string when the value is NaN.
  • apps/web/src/modules/observability/components/quality-gates-view.tsx#L405-L414: apply the helper to maxIncreasePercent; isValidGate then rejects the empty field instead of accepting 0.
  • apps/web/src/modules/observability/components/quality-gates-view.tsx#L535-L543: apply the helper inside RuleValueField.onChange and guard the value prop for NaN.
📍 Affects 1 file
  • apps/web/src/modules/observability/components/quality-gates-view.tsx#L267-L276 (this comment)
  • apps/web/src/modules/observability/components/quality-gates-view.tsx#L405-L414
  • apps/web/src/modules/observability/components/quality-gates-view.tsx#L535-L543
🤖 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 `@apps/web/src/modules/observability/components/quality-gates-view.tsx` around
lines 267 - 276, In
apps/web/src/modules/observability/components/quality-gates-view.tsx#L267-L276,
add one shared number-input helper that maps an empty string to NaN and use it
for minimumCaseCount, rendering NaN as an empty input value. Apply the same
helper at `#L405-L414` for maxIncreasePercent and at `#L535-L543` inside
RuleValueField.onChange, also rendering its NaN value as empty; preserve
isValidGate’s finite-number validation so cleared fields cannot be saved.

@indrazm
indrazm merged commit d4c5652 into main Aug 7, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant