ReviewOS

also looking at this

stacks/bench-review

Adopt the hq-apps app conventions

#76
Merged glennmichael123 wants to merge chore/hq-apps-conventions into main
31 files +1656 -395
10 days ago

Brings bench-review in line with the six sibling apps in hq-apps (analyticshq, bughq, commshq, loghq, reportshq, statushq), which share a set of conventions this repo had none of.

What landed

site.config.ts — single source for name / url / SEO, and wired rather than decorative. The brand name was previously hardcoded in three places that could drift:

  • stx.config.ts takes its default title + description from it
  • app/Helpers/seoPages.ts takes the JSON-LD Organization + WebSite nodes, og:site_name, og:type, og:image, and a new og:locale
  • app/Helpers/sitemap.ts exports CANONICAL_BASE

app/Support/health.ts + GET /api/health — answers 503 when the database or cache probe fails, so a plain HTTP check gets the right answer without parsing the body. Declared as a bare /health because routes/api.ts auto-prefixes. Both paths exercised via the injectable check:

failure path -> 503 no-store   {"app":"bench-review","status":"degraded",…}
healthy path -> 200

scripts/ — the fleet's five validators, wired as bun run {docs,ui,visual,release}:validate and deployment:smoke. validate-ui, validate-docs, validate-release and smoke-deployment are byte-identical to the shared copies, with bench-review added to the CONTRACTS map in the latter two. validate-visual is adapted: this app's own 13 routes, light-only themes (bench ships zero dark: utilities, so a dark pass would just re-audit light twice), and statushq's monitor fixture dropped.

DEPLOY.md — gives the two host requirements a canonical home: an /api/* proxy and a /verify-email/:id/:token rewrite. They previously existed only inside a preview.ts comment.

README.md — was still stock Stacks boilerplate claiming this repo is the framework itself.

A11y — validate-ui flagged 14 controls on its first run; all fixed. A placeholder is not a label, and a button whose text arrives only via :text ships with no accessible name at all.

Two upstream validate-visual bugs, fixed here

Worth pushing back to the other five apps — both are live for them too.

1. The contrast check is broken on any Tailwind-v4 / Crosswind palette. parseRgb only matched rgba?(), but Chrome preserves the authored color space in computed values. Measured directly against this app's dist/ over CDP:

computedBackground:   "oklch(0.546 0.245 262.881)"   ← .bg-blue-600
computedColor:        "rgb(255, 255, 255)"
parseRgbOnBackground: null

So the background was unparseable, background() fell through to its white default, and every white-on-brand button reported a bogus 1.00:1. The worse half: a genuine low-contrast pair sitting on an oklch background was also measured against white, so real failures were silently missed. Now falls back to a 1×1 canvas round-trip, which delegates parsing to the engine and so covers oklch / lab / lch / color() / hsl / hwb / named.

2. label() ignored an image's alt text, so the footer brand link <a href="/"><img alt="Bench Review"></a> was reported as an unnamed control on every page, though a screen reader announces it correctly (accname step 2F).

Together these took the visual audit from ~100 findings to 1.

Deliberately not adopted

  • locales/ — langPath() resolves resources/lang on this framework version and locales/ only on 0.75.x, so moving it now would break i18n the moment it is switched on. It belongs with the version bump, not ahead of it.
  • content/ — byte-identical Stacks marketing boilerplate (introducing-stacks.md, the-road-to-v1.md) copied into all six apps, not per-app content, and this app's blog is stx-based.

Verification

lintPASS
typecheck:appPASS (0 errors)
ui:validatePASS
release:validatePASS on a production build — 41 pages, 4 routes, metadata, assets, migrations
visual:validate1 finding (below)
gate16/17 — unchanged; same pre-existing empty-database failure

release:validate correctly fails on a dev build, because sitemap.xml then names localhost. That is the check earning its keep, so it only passes with a production APP_URL.

Known, left open

ConfirmHost's confirm button takes its background only from x-class="acceptClass()", and the visual audit runs with script execution disabled, so pre-hydration it genuinely is white-on-white. The overlay root has no static class either — deliberately, per the comment there, to avoid a full-screen overlay flash — so the dialog paints as a visible in-flow box before hydration. x-cloak on the overlay root would fix both, and [x-cloak]{display:none !important} does ship inline in the built pages, but verifying a destructive-action dialog still opens needs an authenticated session with review data.

docs:validate is also red at 174 issues — all of it inherited stock Stacks documentation plus config/docs.ts nav pointing at pages this app does not have. The script is wired; curating or deleting docs/ is a separate decision.

Not touched

Everything already uncommitted before this branch: bun.lock, gate.ts, ProfileView.stx, the migration churn (2 deleted, 39 untracked), .agents/, AGENTS.md, docs/SYMLINK-STX-FORK.md, and the generated storage/framework/{database,runtime}/.

🤖 Generated with Claude Code

31 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.