Skip to content

Hub display fixes: complete run ids + humanized durations, [hidden] guard, hero dedup (#491, #492, #493) - #508

Merged
richard-devbot merged 4 commits into
mainfrom
claude/hub-display-bugs-491-493
Jul 31, 2026
Merged

Hub display fixes: complete run ids + humanized durations, [hidden] guard, hero dedup (#491, #492, #493)#508
richard-devbot merged 4 commits into
mainfrom
claude/hub-display-bugs-491-493

Conversation

@richard-devbot

Copy link
Copy Markdown
Owner

Three root-caused display bugs from the 2026-07-30 Hub tour, one bisected commit per bug, TDD throughout (every test verified failing on unfixed code first — RED transcripts in this session's evidence).

Closes #491
Closes #492
Closes #493

Commit 1 — #491: alert copy names the complete run id and humanizes durations

alerts/engine.js built stall details from run.runId.slice(-12) ("run-fx-blocked" → "n-fx-blocked") and printed raw minutes ("42431 min" for a 29-day stall); the cost alert had the same slice mangle; the studio3d topbar chip had the sibling problem ("Observed 42433m ago"). Details now carry the full id, durations render through humanizeDurationMs ("45m" / "1h 30m" / "29d 11h"), and formatSnapshotAge gains matching hours/days tiers — the two formatters cross-reference each other. Pinned by 5 unit tests including a "no \d{3,} min" pattern gate.

Commit 2 — #493: the hero never repeats its headline

With a focused run, overview.title already IS readiness.summary, and the page rendered the rationale as readiness.summary || title — the same sentence twice, always. The projection now owns rationale (server-side, null when it would echo the title, kept when it genuinely adds information — e.g. the no-run state); the page consumes overview.rationale only and hides the element when empty. Projection invariants pinned across run / no-run / no-summary states + a browser assertion.

Commit 3 — #492: the hidden attribute always wins

empty.hidden = true was correct in JS but .run-workspace-empty's display:grid defeats the UA [hidden] rule, so the "Select one run" prompt floated above every opened workspace. Rather than adding a third hand-written per-class twin (two already existed), the hub stylesheet gains the global [hidden] { display:none !important; } guard — the identical pattern studio3d's stylesheet has carried since #431 — closing the whole class. Audit of all hidden-toggle sites (commit message has the full table): secondary-nav ✓, run-workspace panels/empty/content ✓, overview-rationale ✓, studio3d ✓ (own guard), mobile nav (aria-hidden, unaffected); the two inline-style.display elements (#err, #studio-inspector) never carry the attribute.

Verification

  • Core suite 1717/1717 (exit 0) · browser regression 7/7 (2 new + dashboard-96's 5 incl. axe + responsive) · lint 0 errors · typecheck 0 · validate 196 agents · schemas 34 PASS · security gate green · whitespace clean.
  • Live-verified on the fixture-seeded hub after a server restart: Alerts page reads "No activity in run-fx-blocked for 30d 0h" (all five ids complete), the Command Center hero renders its sentence once, and the scoped Run Workspace opens with the phantom prompt gone.

Branch is off current main — independent of PR #504 (no shared files, no stacked base).

Held for Richardson's review — please verify before merge. Note the three separate Closes lines above (the "Closes X and Y" single-keyword form only closes the first — the #449 lesson); still worth confirming all three issues auto-closed after merge.

🤖 Generated with Claude Code

richardsongunde and others added 3 commits July 31, 2026 09:44
The stalled-run alert built its detail from run.runId.slice(-12), clipping
every id longer than 12 chars into something unsearchable (run-fx-blocked
→ "n-fx-blocked", run-fx-active → "un-fx-active"), and printed raw minute
math ("42431 min" for a 29-day stall). The cost alert had the same
slice(-12) mangle. The studio3d freshness chip had the sibling problem —
"Observed 42433m ago" with no hours/days tiers.

Alert details now name the complete run id, and durations render through
humanizeDurationMs ("45m", "1h 30m", "29d 11h"). formatSnapshotAge gains
matching hours/days tiers ("Observed 29d 11h ago"); the two formatters are
cross-referenced so the shapes stay in step.

Closes #491.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With a focused run, the overview projection's title already IS the
readiness summary (state/overview.js), and the page rendered the rationale
as readiness.summary || title — so the hero painted the identical sentence
twice ("Release is blocked by 6 source-backed conditions." as both
headline and sub-line), adding zero information.

The projection now owns a server-side `rationale` that is null whenever it
would merely repeat the title (kept only when readiness.summary genuinely
differs, e.g. the no-run state); the page consumes overview.rationale
alone — the readiness.summary fallback that caused the echo is gone — and
hides the element when there is nothing distinct to say. Honest empties
preserved: no rationale is ever invented.

Pinned by projection invariants across run/no-run/no-summary states and a
browser assertion that a visible rationale never equals the headline.

Closes #493.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
renderRunWorkspace correctly sets empty.hidden = true when a run is
scoped, but .run-workspace-empty's own display:grid (an author rule)
overrides the UA stylesheet's [hidden] { display:none }, so the "Select
one run to open its workspace" prompt stayed visible above the opened
workspace. Two classes already carried hand-written [hidden] twins
(.secondary-nav, .run-workspace-panel); this class was the one that
didn't — a whack-a-mole class of bug.

Close the class instead of adding a third twin: a global
[hidden] { display:none !important; } guard, the identical pattern the
studio3d stylesheet has carried since #431 (studio3d/styles.css). Audit of
every hidden-attribute toggle site (secondary-nav, run-workspace panels /
empty / content, overview-rationale, studio3d, mobile nav) confirmed
nothing relies on being visible while carrying hidden; the two elements
shown via inline style.display (#err, #studio-inspector) never carry the
attribute. The existing per-class twins stay (now redundant, harmless).

Pinned by a browser assertion that the scoped workspace renders with the
empty prompt taking zero space.

Closes #492.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@strix-security

strix-security Bot commented Jul 31, 2026

Copy link
Copy Markdown

Strix Security Review

No security issues found.

Updated for 502c5c5.


Reviewed by Strix
Re-run review · Configure security review settings

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@richard-devbot, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 20 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f7565048-d74e-40a1-a2cf-659cd53b331a

📥 Commits

Reviewing files that changed from the base of the PR and between 7c690f1 and 5b211aa.

📒 Files selected for processing (9)
  • src/observability/alerts/engine.js
  • src/observability/dashboard/state/overview.js
  • src/observability/dashboard/ui/pages/command-center.js
  • src/observability/dashboard/ui/studio3d/model.js
  • src/observability/dashboard/ui/styles.js
  • tests/browser/dashboard-492-workspace-empty.test.js
  • tests/browser/dashboard-493-hero.test.js
  • tests/hub-alerts-copy-491.test.js
  • tests/hub-overview-rationale-493.test.js

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.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Hub display fixes: full run IDs, humanized durations, [hidden] guard

🐞 Bug fix 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Stop truncating run IDs in alerts; render stalls/snapshot ages with hours/days tiers.
• Prevent Command Center hero from duplicating headline as rationale via server-owned field.
• Make [hidden] reliably hide elements by enforcing a global display:none !important rule.
Diagram

graph TD
  A["alerts/engine.js"] --> B["Alert copy"]
  C["studio3d/model.js"] --> D["Snapshot age chip"]
  E["state/overview.js"] --> F["overview projection"] --> G["ui/pages/command-center.js"]
  H["ui/styles.js"] --> G
  I["New unit + browser tests"] --> A --> C
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Extract shared duration formatter module
  • ➕ Eliminates duplicated hours/days/minutes tier logic between alerts and Studio3D
  • ➕ Provides a single contract for future formatting changes (tests can target one entrypoint)
  • ➖ Introduces a new shared dependency boundary that may not exist today
  • ➖ Requires minor refactor churn beyond the immediate bug fixes
2. CSS: use `:not([hidden])` selectors on specific components instead of global guard
  • ➕ Avoids a global !important rule affecting all dashboard elements
  • ➕ Keeps intent localized to the few components known to toggle hidden
  • ➖ Easy to miss future components that toggle hidden
  • ➖ Requires touching multiple component selectors (the issue this PR is avoiding)
3. Compute hero rationale purely client-side with stricter equality rules
  • ➕ No server projection change; UI owns presentation decisions
  • ➕ Can incorporate richer heuristics (e.g., normalized whitespace)
  • ➖ Reintroduces risk of divergent logic across pages consuming the projection
  • ➖ Harder to guarantee invariants; server-owned field is easier to audit and test

Recommendation: The PR’s approach is the best short-term fix set: it keeps the hero de-dup invariant server-owned (preventing multiple clients from re-deriving), applies a single global [hidden] guard to close the entire class of CSS precedence bugs, and adds regression tests. Consider a follow-up to extract a shared duration-formatting helper to remove the remaining duplication between alerts and Studio3D.

Files changed (9) +410 / -5

Bug fix (5) +42 / -5
engine.jsHumanize stall durations and stop truncating run IDs in alert details +14/-2

Humanize stall durations and stop truncating run IDs in alert details

• Adds 'humanizeDurationMs' and uses it for stalled-run alert durations to avoid unbounded minute dumps. Removes 'slice(-12)' from cost and stalled-run alert copy so details contain the complete run ID.

src/observability/alerts/engine.js

overview.jsAdd server-owned 'rationale' field to prevent hero text duplication +8/-1

Add server-owned 'rationale' field to prevent hero text duplication

• Refactors overview projection to compute 'title' and 'summary' variables and introduces 'rationale', which is null when it would repeat the title. This ensures the UI can render a distinct sub-line only when it adds information.

src/observability/dashboard/state/overview.js

command-center.jsRender overview rationale only when present and hide empty element +7/-1

Render overview rationale only when present and hide empty element

• Switches from 'readiness.summary || title' to consuming 'overview.rationale' directly. Explicitly toggles the rationale element’s 'hidden' state when the rationale is empty to avoid rendering duplicate/blank UI.

src/observability/dashboard/ui/pages/command-center.js

model.jsFormat snapshot ages with minutes/hours/days tiers +8/-1

Format snapshot ages with minutes/hours/days tiers

• Extends 'formatSnapshotAge' to include hours and days formatting so old snapshots don’t display as huge minute counts. Keeps formatting shape aligned with the alert-duration humanization logic.

src/observability/dashboard/ui/studio3d/model.js

styles.jsEnforce '[hidden]' as 'display:none !important' globally +5/-0

Enforce '[hidden]' as 'display:none !important' globally

• Adds a global CSS guard so author 'display:*' rules cannot override the browser’s '[hidden]' behavior. Fixes cases like '.run-workspace-empty' remaining visible despite 'el.hidden = true'.

src/observability/dashboard/ui/styles.js

Tests (4) +368 / -0
dashboard-492-workspace-empty.test.jsBrowser regression: scoped run hides Run Workspace empty prompt +132/-0

Browser regression: scoped run hides Run Workspace empty prompt

• Adds a Playwright-based test that scopes to a run and asserts the empty prompt both has 'hidden' set and is not visibly rendered. Protects against CSS overriding '[hidden]' via component display rules.

tests/browser/dashboard-492-workspace-empty.test.js

dashboard-493-hero.test.jsBrowser regression: hero rationale never repeats the headline +120/-0

Browser regression: hero rationale never repeats the headline

• Adds a Playwright test that reads the hero title and rationale and asserts they are not identical when the rationale is visible. Ensures the headline duplication bug can’t regress via client-side fallback logic.

tests/browser/dashboard-493-hero.test.js

hub-alerts-copy-491.test.jsUnit tests: full run IDs and humanized durations in alerts + Studio3D chip +75/-0

Unit tests: full run IDs and humanized durations in alerts + Studio3D chip

• Adds unit coverage ensuring stalled-run and cost alerts include the complete run ID and format durations as m/h/d tiers. Also asserts Studio3D snapshot age formatting avoids unbounded minute outputs.

tests/hub-alerts-copy-491.test.js

hub-overview-rationale-493.test.jsUnit tests: overview 'rationale' invariant across run/no-run states +41/-0

Unit tests: overview 'rationale' invariant across run/no-run states

• Adds tests verifying 'buildOverviewProjection' sets 'rationale' to null when it would duplicate the title, preserves distinct summaries when appropriate, and does not invent rationale when absent.

tests/hub-overview-rationale-493.test.js

@qodo-code-review

qodo-code-review Bot commented Jul 31, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (3)

Context used
✅ Compliance rules (platform): 300 rules
✅ Skills: 17 invoked
  code-review-pr
  claude-api
  documentation-writing
  pptx
  docx
  performance-monitoring
  security-compliance
  cso
  plan-eng-review
  design-review
  prompt-engineering
  mcp-builder
  qa-testing
  code-patterns
  xlsx
  security-owasp
  testing-qa

Grey Divider


Remediation recommended

1. Browser test timeouts are magic ✓ Resolved 📜 Skill insight ⚙ Maintainability
Description
The new browser test hardcodes multiple timeout values (e.g., 15_000, 10_000, 5000) rather
than named constants, which makes harness tuning and consistency across tests harder.
Code

tests/browser/dashboard-492-workspace-empty.test.js[R52-55]

+    const timer = setTimeout(() => {
+      if (!settled) { settled = true; child.kill('SIGKILL'); rejectPromise(new Error(`server did not boot\n${out}`)); }
+    }, 15_000);
+    child.stdout.on('data', (chunk) => {
Relevance

●● Moderate

No close precedent found requiring named constants for timeout literals in browser tests.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1400588 requires replacing literal numeric values used in timeouts/configuration
with named constants. The added test uses literal millisecond timeout values directly in
setTimeout and Playwright wait calls.

tests/browser/dashboard-492-workspace-empty.test.js[52-55]
Skill: code-patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new browser test uses hardcoded timeout values instead of named constants, violating the magic-number rule.

## Issue Context
These tests share a harness pattern (noted in comments). Centralizing timeouts as constants improves maintainability and keeps similar tests aligned.

## Fix Focus Areas
- tests/browser/dashboard-492-workspace-empty.test.js[52-55]
- tests/browser/dashboard-492-workspace-empty.test.js[101-103]
- tests/browser/dashboard-492-workspace-empty.test.js[113-117]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. formatSnapshotAge uses magic numbers ✓ Resolved 📜 Skill insight ⚙ Maintainability
Description
formatSnapshotAge and humanizeDurationMs hardcode minute/hour/day conversion and threshold
values (e.g., 60_000, 60, 24) instead of using named constants, making the logic harder to
read and more error-prone to change. This also increases the risk the two related duration
formatters drift out of sync over time.
Code

src/observability/dashboard/ui/studio3d/model.js[R45-49]

+  const minutes = Math.floor(age / 60_000);
+  if (minutes < 60) return `Observed ${minutes}m ago`;
+  const hours = Math.floor(minutes / 60);
+  if (hours < 24) return `Observed ${hours}h ${minutes % 60}m ago`;
+  return `Observed ${Math.floor(hours / 24)}d ${hours % 24}h ago`;
Relevance

●● Moderate

No close precedent found on extracting duration-conversion literals into named constants in this
area.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1400588 requires that numeric literals used in conditions and calculations be
replaced with named constants. The cited code locations show the formatters implementing
minutes/hours/days tiering and conversions directly with raw literals like 60_000, 60, and 24,
meaning the conversion factors and branching thresholds are embedded in logic rather than extracted
into reusable, named constants as required.

src/observability/dashboard/ui/studio3d/model.js[45-49]
src/observability/alerts/engine.js[16-21]
Skill: code-patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Both `formatSnapshotAge` and `humanizeDurationMs` rely on magic numbers for time conversions and branching thresholds (e.g., `60_000`, `60`, `24`). Update the implementation to extract these literals into named constants (ideally shared) to satisfy the compliance rule and reduce the risk of inconsistent future updates.

## Issue Context
PR Compliance ID 1400588 requires numeric literals used in conditions/calculations to be replaced with named constants. These two formatters are explicitly intended to stay in sync with each other (`formatSnapshotAge` in the Studio 3D model and `humanizeDurationMs` in the alerts engine), so using shared constants (or imported constants) will reduce drift and make future changes safer.

## Fix Focus Areas
- src/observability/dashboard/ui/studio3d/model.js[45-49]
- src/observability/alerts/engine.js[16-21]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Regression tests lack attribution fields 📜 Skill insight ⚙ Maintainability
Description
New regression tests include issue context but omit required attribution details (date found and QA
report path). This reduces traceability for future audits and debugging of regressions.
Code

tests/browser/dashboard-492-workspace-empty.test.js[R1-11]

+/**
+ * #492 — with a run scoped, the Run Workspace "Select one run" empty prompt
+ *
+ * must actually disappear — the `hidden` attribute was defeated by the
+ * class's own `display:grid` (author styles beat the UA [hidden] rule).
+ *
+ * Same harness as dashboard-96: real server on an ephemeral port, canonical
+ * fixtures, real Chromium via playwright-core; skips cleanly with no browser.
+ *
+ * owner: RStack developed by Richardson Gunde
+ */
Relevance

●● Moderate

No historical suggestions found enforcing regression-test attribution fields (date found, QA report
path).

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1400067 requires regression tests to include an attribution block with issue ID,
description, date found, and QA report path. The new test-file headers mention the issue and
description but do not include a date or QA report reference.

tests/browser/dashboard-492-workspace-empty.test.js[1-11]
tests/browser/dashboard-493-hero.test.js[1-10]
tests/hub-alerts-copy-491.test.js[1-7]
tests/hub-overview-rationale-493.test.js[1-7]
Skill: qa-testing

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Regression tests added in this PR do not include the full attribution comment block required by policy (issue ID, what broke, date found, and QA report path).

## Issue Context
These tests already mention `#491/#492/#493` and a brief description, but they do not include the "Found by ... on YYYY-MM-DD" and "Report: ..." lines.

## Fix Focus Areas
- tests/browser/dashboard-492-workspace-empty.test.js[1-11]
- tests/browser/dashboard-493-hero.test.js[1-10]
- tests/hub-alerts-copy-491.test.js[1-7]
- tests/hub-overview-rationale-493.test.js[1-7]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. humanizeDurationMs missing JSDoc 📜 Skill insight ⚙ Maintainability
Description
The newly exported function humanizeDurationMs is introduced without the required JSDoc block
including @param, @returns, and @throws. This makes the public API less self-documenting and
violates the exported-function documentation requirement.
Code

src/observability/alerts/engine.js[R16-22]

+export function humanizeDurationMs(ms) {
+  const minutes = Math.floor(Math.max(0, Number(ms) || 0) / 60_000);
+  if (minutes < 60) return `${minutes}m`;
+  const hours = Math.floor(minutes / 60);
+  if (hours < 24) return `${hours}h ${minutes % 60}m`;
+  return `${Math.floor(hours / 24)}d ${hours % 24}h`;
+}
Relevance

● Weak

Prior review rejected adding JSDoc/docstrings for exported functions to satisfy coverage thresholds.

PR-#317

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1400405 requires exported functions to include JSDoc with @param, @returns, and
@throws. The added exported function humanizeDurationMs(ms) appears without any JSDoc block.

src/observability/alerts/engine.js[12-22]
Skill: code-patterns

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new exported function `humanizeDurationMs` was added without the required JSDoc tags (`@param`, `@returns`, `@throws`).

## Issue Context
Compliance requires exported/public functions to have JSDoc that includes these tags.

## Fix Focus Areas
- src/observability/alerts/engine.js[12-22]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Duplicated browser test harness 📜 Skill insight ⚙ Maintainability
Description
The Playwright server/bootstrap harness (startServer, browserTest, gotoDashboard, and related
setup) is duplicated across two new browser tests, increasing maintenance cost and the risk of
future drift. This violates the DRY refactoring requirement for repeated logic blocks.
Code

tests/browser/dashboard-492-workspace-empty.test.js[R33-103]

+function startServer(root) {
+  return new Promise((resolvePromise, rejectPromise) => {
+    const child = spawn(process.execPath, [SERVER_PATH, '--port', '0', '--no-browser', '--project', root], {
+      cwd: root,
+      env: {
+        ...process.env,
+        RSTACK_APPROVAL_TOKEN: undefined,
+        RSTACK_DASHBOARD_READ_TOKEN: undefined,
+        RSTACK_TLS_CERT: undefined,
+        RSTACK_TLS_KEY: undefined,
+        RSTACK_BUSINESS_PORT: undefined,
+        RSTACK_PROJECT_ROOT: undefined,
+        RSTACK_NO_BROWSER: '1',
+        RSTACK_REGISTRY_DIR: join(root, '.registry'),
+      },
+      stdio: ['ignore', 'pipe', 'pipe'],
+    });
+    let out = '';
+    let settled = false;
+    const timer = setTimeout(() => {
+      if (!settled) { settled = true; child.kill('SIGKILL'); rejectPromise(new Error(`server did not boot\n${out}`)); }
+    }, 15_000);
+    child.stdout.on('data', (chunk) => {
+      out += chunk;
+      const match = out.match(/Dashboard: http:\/\/localhost:(\d+)/);
+      if (match && !settled) {
+        settled = true;
+        clearTimeout(timer);
+        resolvePromise({ baseUrl: `http://127.0.0.1:${match[1]}`, stop: () => child.kill('SIGKILL') });
+      }
+    });
+    child.stderr.on('data', (chunk) => { out += chunk; });
+    child.on('exit', () => { if (!settled) { settled = true; clearTimeout(timer); rejectPromise(new Error(`server exited\n${out}`)); } });
+  });
+}
+
+let browser = null;
+test.before(async () => {
+  if (!chromium) return;
+  try { browser = await chromium.launch({ headless: true }); } catch { browser = null; }
+});
+test.after(async () => { if (browser) await browser.close(); });
+
+function browserTest(name, seed, body) {
+  test(name, async (t) => {
+    if (!browser) { t.skip('Chromium not installed — run `npx playwright-core install chromium`'); return; }
+    const root = mkdtempSync(join(tmpdir(), 'rstack-browser-display-'));
+    let server = null;
+    const context = await browser.newContext();
+    const page = await context.newPage();
+    const pageErrors = [];
+    page.on('pageerror', (err) => pageErrors.push(String(err)));
+    try {
+      await seed(root);
+      server = await startServer(root);
+      await body({ page, server, t });
+      assert.deepEqual(pageErrors, [], 'no uncaught page errors');
+    } finally {
+      await context.close();
+      if (server) server.stop();
+      rmSync(root, { recursive: true, force: true });
+    }
+  });
+}
+
+const seedRichProject = async (root) => { await fixtureReadyRun(root); await fixtureBlockedRun(root); };
+
+async function gotoDashboard(page, server) {
+  await page.goto(server.baseUrl, { waitUntil: 'networkidle' });
+  await page.waitForFunction(() => window.STATE != null, null, { timeout: 10_000 });
+}
Relevance

● Weak

Team previously rejected DRY refactors consolidating duplicated spawn/harness logic into shared
helpers.

PR-#471

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1399536 requires refactoring duplicated logic. The same harness implementation
(server spawn/boot parsing, Playwright setup, and shared navigation helper) appears verbatim in both
new browser tests.

tests/browser/dashboard-492-workspace-empty.test.js[33-103]
tests/browser/dashboard-493-hero.test.js[32-103]
Skill: plan-eng-review

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The browser test harness code (server boot, chromium setup, `browserTest` wrapper, `gotoDashboard`) is duplicated across multiple browser tests, creating a DRY violation and increasing the chance of inconsistent behavior when the harness needs updates.

## Issue Context
Both `tests/browser/dashboard-492-workspace-empty.test.js` and `tests/browser/dashboard-493-hero.test.js` contain near-identical harness code.

## Fix Focus Areas
- tests/browser/dashboard-492-workspace-empty.test.js[33-103]
- tests/browser/dashboard-493-hero.test.js[32-103]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment on lines +1 to +11
/**
* #492 — with a run scoped, the Run Workspace "Select one run" empty prompt
*
* must actually disappear — the `hidden` attribute was defeated by the
* class's own `display:grid` (author styles beat the UA [hidden] rule).
*
* Same harness as dashboard-96: real server on an ephemeral port, canonical
* fixtures, real Chromium via playwright-core; skips cleanly with no browser.
*
* owner: RStack developed by Richardson Gunde
*/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Informational

3. Regression tests lack attribution fields 📜 Skill insight ⚙ Maintainability

New regression tests include issue context but omit required attribution details (date found and QA
report path). This reduces traceability for future audits and debugging of regressions.
Agent Prompt
## Issue description
Regression tests added in this PR do not include the full attribution comment block required by policy (issue ID, what broke, date found, and QA report path).

## Issue Context
These tests already mention `#491/#492/#493` and a brief description, but they do not include the "Found by ... on YYYY-MM-DD" and "Report: ..." lines.

## Fix Focus Areas
- tests/browser/dashboard-492-workspace-empty.test.js[1-11]
- tests/browser/dashboard-493-hero.test.js[1-10]
- tests/hub-alerts-copy-491.test.js[1-7]
- tests/hub-overview-rationale-493.test.js[1-7]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread src/observability/dashboard/ui/studio3d/model.js Outdated
Comment thread tests/browser/dashboard-492-workspace-empty.test.js
Qodo (convention, PR #490 precedent): humanizeDurationMs and
formatSnapshotAge now branch on named constants (MS_PER_MINUTE,
MINUTES_PER_HOUR, HOURS_PER_DAY) — the two formatters are meant to stay
in step, and named thresholds make drift visible at a glance. Browser-test
timeouts are named constants in both new test files.

Refs #491 #492 #493 (PR #508 review follow-up).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@richard-devbot

Copy link
Copy Markdown
Owner Author

Qodo's convention findings addressed in the follow-up commit (all were skill-insights; it reported 0 bugs on this PR): humanizeDurationMs and formatSnapshotAge branch on named time constants so the paired formatters can't drift silently, and both new browser tests use named timeout constants per the #490 convention. Suite 1717/1717, lint clean. — Claude Main

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants