Railway-only deployment docs + document-viewer UX cleanup#511
Conversation
Remove Fly.io and Cloud Run from the deployment docs and CI. Rewrite docs/deployment-architecture.md Railway-first (now decided + live), add a measured Singapore->Sydney connection section with optimisations and Railway-native mitigations, point staging at a Railway environment, and add reproducible per-service config-as-code (railway.app.json, railway.worker.json) mirroring the live services. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Twenty focused UX fixes to the document viewer:
- one clear back affordance (ArrowLeft on all breakpoints) and drop the
redundant Answer|Documents mode toggle (the global shell owns mode)
- remove the misleading chevrons from the inert overview cards; replace
fake wireframe thumbnails with real "Jump to p.N" chips
- show the overview sentence once (landing card), not again in the rail
- derive the document-type eyebrow from labels instead of hardcoding it
- rebalance the landing actions to a clean 2x2 / single row; rename
"Scope" to "Add to scope"
- collapse the mobile PDF toolbar to one compact row; tighten the page
indicator; move the verbose helper to a tooltip; relabel the viewer
mode toggle around the benefit ("Sharper zoom" / "Standard view")
- fit the canvas holder to the rendered page (reserve height only while
loading)
- de-emphasise the empty pinned-evidence state
- when a structured table exists, let the table lead and collapse the
raw table image behind a "Show original" toggle
- let the right rail flow with the page (no nested scroll), make the
header sticky with consistent offsets, and keep the floating composer
within the main column so it clears the right rail
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRailway production and staging deployment configuration, shared health-check handling, and document viewer presentation behavior were updated. The viewer changes cover summaries, evidence, tables, PDF controls, overview navigation, and responsive layout. ChangesRailway deployment
Health-check consolidation
Document viewer presentation
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Railway
participant ReadinessRoute
participant HealthResponse
participant Supabase
Railway->>ReadinessRoute: GET /api/health/ready
ReadinessRoute->>HealthResponse: request deep readiness response
HealthResponse->>Supabase: run deep probe
Supabase-->>HealthResponse: return probe result
HealthResponse-->>ReadinessRoute: return health JSON
ReadinessRoute-->>Railway: respond 200 or 503
✅ Pre-merge checks override appliedThe pre-merge checks have been overridden successfully. You can now proceed with the merge. Overridden by ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (8 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c60bf69c76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Non-dense tables used table-fixed plus overflow-wrap:anywhere with a forced-equal colgroup, so a word wider than its column (e.g. "Constipation" or "glucose/HbA1c" in a narrow right-rail column) was chopped mid-character. Switch non-dense tables to auto layout and drop the :anywhere override (keeping break-words), so columns size to their content (min-content = longest word), cells wrap at word boundaries, and a table too wide for its container scrolls via the existing overflow-x-auto wrapper. The dense preview keeps its fixed single-line-ellipsis layout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c9a02bc37
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/staging-setup.md (1)
72-79: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire a staging-only OpenAI key.
This runbook permits a shared key, but
docs/deployment-architecture.mdrequires separate production, staging, and CI keys to preserve rotation boundaries and limit blast radius. Remove “or shared” and explicitly reject production-key reuse in staging.🤖 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 `@docs/staging-setup.md` around lines 72 - 79, Update the Runtime secrets table in the staging setup documentation so OPENAI_API_KEY requires a staging-only OpenAI key. Remove the “or shared” wording and explicitly state that the production key must not be reused in staging, consistent with the deployment architecture requirements.src/components/DocumentViewer.tsx (1)
1423-1433: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWiden the fullscreen exit button.
iconButtonresolves totoolbarButton, which is a fixedh-tap w-tapsquare, so the visibleExitlabel will clip or overflow atsm+. Use the wider Fit-button sizing here instead.🤖 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 `@src/components/DocumentViewer.tsx` around lines 1423 - 1433, Update the fullscreen exit button in DocumentViewer’s fullscreenActive branch to use the wider Fit-button sizing instead of iconButton/toolbarButton, preserving its existing click handler, label, icon, and accessibility attributes.Source: Coding guidelines
🤖 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 `@docs/staging-setup.md`:
- Line 67: Add the appropriate language identifier to the fenced code block in
the staging setup documentation, changing the opening fence to use dotenv syntax
while leaving the block contents unchanged.
In `@railway.app.json`:
- Around line 7-8: Update the Railway deploy healthcheck configuration to target
a readiness endpoint or supported deep health probe that Railway can call
without query parameters or a secret token. Ensure the selected path actually
validates application readiness and does not pass solely because environment
variables are present, while preserving the existing healthcheck configuration
structure.
In `@src/components/DocumentViewer.tsx`:
- Around line 1793-1819: Update DocumentPagePreview so activating the “Jump to
p.X” chip changes the PDF viewer’s selected page by updating the page query or
invoking the existing page-change handler, rather than only scrolling to
`#pdf-preview-section`. Ensure the target page is passed through the existing
initialPage flow consumed by PdfCanvasViewer and NativePdfEmbed, while
preserving the current navigation label and fallback behavior.
- Around line 455-531: Update the hasStructuredTable logic used by the
image-collapse decision so it reflects whether AccessibleTable can actually
render content, requiring explicit rows or successfully parsable markdown rather
than tableColumns alone. Preserve inline image behavior for columns-only records
and unparseable pipe-filled snippets, while retaining the collapsed “Show
original” behavior when a structured table is renderable.
---
Outside diff comments:
In `@docs/staging-setup.md`:
- Around line 72-79: Update the Runtime secrets table in the staging setup
documentation so OPENAI_API_KEY requires a staging-only OpenAI key. Remove the
“or shared” wording and explicitly state that the production key must not be
reused in staging, consistent with the deployment architecture requirements.
In `@src/components/DocumentViewer.tsx`:
- Around line 1423-1433: Update the fullscreen exit button in DocumentViewer’s
fullscreenActive branch to use the wider Fit-button sizing instead of
iconButton/toolbarButton, preserving its existing click handler, label, icon,
and accessibility attributes.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6bc27a79-ff8a-41dd-8c9a-1000dc9a442b
📒 Files selected for processing (7)
.github/workflows/docker-image.ymldocs/deployment-architecture.mddocs/staging-setup.mdrailway.app.jsonrailway.worker.jsonsrc/app/globals.csssrc/components/DocumentViewer.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Cannot run autofix: This PR has merge conflicts. Please resolve the conflicts with the base branch and try again. Alternatively, use |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/components/AccessibleTable.tsx`:
- Around line 159-163: Update the header class logic in AccessibleTable so the
dense-preview Actions header retains fixed-layout, single-line truncation with
ellipsis instead of wrapping. Apply the existing dense-preview truncation
classes conditionally for actionsHeader while preserving the current wrapping
behavior for regular headers.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e6042e17-2691-4664-9e45-f70dd85a06f4
📒 Files selected for processing (1)
src/components/AccessibleTable.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Cannot run autofix: This PR has merge conflicts. Please resolve the conflicts with the base branch and try again. Alternatively, use |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Cannot run autofix: This PR has merge conflicts. Please resolve the conflicts with the base branch and try again. Alternatively, use |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Cannot run autofix: This PR has merge conflicts. Please resolve the conflicts with the base branch and try again. Alternatively, use |
|
@copilot please resolve merge conflicts |
|
@cursoragent resolve merge conflict |
|
You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor |
|
@anthropic-code-agent[agent] Resolve merge conflicts |
|
@copilot resolve the merge conflicts in this pull request |
|
@copilot resolve the merge conflicts in this pull request |
Head branch was pushed to by a user without write access
Resolved and pushed in |
|
@copilot resolve the merge conflicts on this branch. |
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. ❌ Cannot run autofix: This PR has merge conflicts. Please resolve the conflicts with the base branch and try again. Alternatively, use |
…roduction-deployment-setup-d83ef8 # Conflicts: # docs/deployment-architecture.md # docs/staging-setup.md # src/components/DocumentViewer.tsx
Resolved and pushed in |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/lib/health-response.ts (1)
21-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor:
isDemoMode()invoked twice per request.
isDemoMode()is called once inside the deep branch (line 21) and again for the response body (line 43). Not a correctness bug (deterministic within a request), but capturing it once avoids redundant recomputation.🤖 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 `@src/lib/health-response.ts` around lines 21 - 43, Capture the result of isDemoMode() once at the start of the health-response flow, then reuse that value in both the Supabase branch condition and the response body's demoMode field. Remove the duplicate invocation while preserving the existing branch behavior.tests/health-route.test.ts (1)
66-78: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLGTM on the assertions for the demo-mode branch — they match
healthResponse's logic correctly.Gap: the actual unauthenticated-live-probe branch (
demoMode: false,configured: true) is untested. That's the path where the auth bypass actually results in a real Supabase probe attempt, and it's the security-relevant one for this new route. Consider adding a case that mocks@/lib/supabase/admin/@/lib/supabase/healthdynamic imports to assert both theokanderroroutcomes are surfaced correctly without requiring the deep-probe header.🤖 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 `@tests/health-route.test.ts` around lines 66 - 78, Extend the GET /api/health/ready tests with configured, non-demo-mode cases that omit the diagnostic probe token and mock the dynamic `@/lib/supabase/admin` and `@/lib/supabase/health` imports. Assert that healthResponse surfaces both successful and failed Supabase probe outcomes with the expected response status and checks, confirming the unauthenticated live-probe branch works.
🤖 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 `@src/app/api/health/ready/route.ts`:
- Around line 6-10: Correct the misleading comment above GET so it accurately
states that unauthenticated callers receive coarse readiness and
configuration/reachability check states, including Supabase and OpenAI results,
while avoiding diagnostic detail claims. Keep the existing healthResponse
behavior and allowUnauthenticatedDeep setting unchanged.
In `@src/app/globals.css`:
- Around line 1435-1437: Update the left positioning rule for
.document-viewer-composer.floating-composer-edge to include the same
centered-shell offset used by right, while preserving the existing minimum 2rem
and safe-area-left constraints. Ensure wide viewports position the composer
within the centered document column rather than the outer gutter.
In `@src/lib/health-response.ts`:
- Around line 10-34: Update the deep Supabase probe flow in healthResponse to
enforce a short timeout around probeSupabaseHealth and cache the resulting
status for a few seconds, reusing the cached result for subsequent requests.
Ensure timeout or probe failures produce the existing "error" status and
preserve the unauthorized, skipped, and normal fresh-probe behavior.
---
Nitpick comments:
In `@src/lib/health-response.ts`:
- Around line 21-43: Capture the result of isDemoMode() once at the start of the
health-response flow, then reuse that value in both the Supabase branch
condition and the response body's demoMode field. Remove the duplicate
invocation while preserving the existing branch behavior.
In `@tests/health-route.test.ts`:
- Around line 66-78: Extend the GET /api/health/ready tests with configured,
non-demo-mode cases that omit the diagnostic probe token and mock the dynamic
`@/lib/supabase/admin` and `@/lib/supabase/health` imports. Assert that
healthResponse surfaces both successful and failed Supabase probe outcomes with
the expected response status and checks, confirming the unauthenticated
live-probe branch works.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: da563dc5-3883-4e6b-bc7c-f285ecff749a
📒 Files selected for processing (9)
docs/staging-setup.mdrailway.app.jsonsrc/app/api/health/ready/route.tssrc/app/api/health/route.tssrc/app/globals.csssrc/components/AccessibleTable.tsxsrc/components/DocumentViewer.tsxsrc/lib/health-response.tstests/health-route.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- railway.app.json
- src/components/AccessibleTable.tsx
- docs/staging-setup.md
# Conflicts: # src/app/api/health/route.ts
|
@copilot resolve the merge conflicts on this branch. |
|
@codex resolve all comments |
- Restore compact sticky-header h1 for document title and error states so DOM integrity and Playwright smoke tests match the viewer contract - Add aria-hidden to Lucide icons flagged by require-lucide-icon-aria - Regenerate docs/site-map.md for the new /api/health/ready route Co-authored-by: BigSimmo <BigSimmo@users.noreply.github.com>
…loyment-setup-d83ef8 # Conflicts: # docs/deployment-architecture.md
Head branch was pushed to by a user without write access
Resolved and pushed in |
Summary
Testing
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96af452baf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57c2e58f0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7cca301849
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| supabase link --project-ref <staging-ref> | ||
| supabase db push # applies supabase/migrations/* → matches schema.sql | ||
| ``` | ||
| ```dotenv |
There was a problem hiding this comment.
Close the schema command fence
In the staging setup runbook, this starts a literal ```dotenv marker inside the four-backtick bash block opened above, so the following health-check prose is rendered as part of the shell snippet. When an operator copies the “Apply the schema” commands, they can paste the stray fence/prose into a shell before running the health check; close the bash fence after supabase db push and keep the health note as normal text.
Useful? React with 👍 / 👎.
Summary
Two independent changes on this branch (one commit each).
1. Railway is the sole deployment platform of record (
ca37957)docs/deployment-architecture.mdRailway-first (decided + live), adding a measured Singapore↔Sydney connection section (physical RTT ~94 ms; authed PostgREST round-trip ~145–340 ms; ~+0.6–2 s per novel answer) with in-place optimisations and Railway-native mitigations (Supabase Singapore read replica; reduce sequential DB depth).railway.app.json,railway.worker.json) mirroring the liveapp+workerservices.2. Document-viewer UX cleanup — 20 fixes (
c60bf69)Navigation chrome, overview cards, actions, PDF toolbar, evidence/tables, and desktop layout. Highlights: single back affordance (no hamburger, no duplicate mode toggle), real "Jump to p.N" chips, de-duplicated overview, derived document-type eyebrow, rebalanced 2×2 actions, one-row mobile PDF toolbar, benefit-labelled zoom mode, page-fit canvas height, de-emphasised empty evidence, table-leads / raw image behind a "Show original" toggle, flowing right rail, sticky header, and a floating composer that clears the right rail.
Clinical governance preflight
Presentational and infrastructural only. The viewer changes are layout / labels / visibility — no change to retrieval, answer generation, ranking, document access or permissions, privacy, or clinical output. No ingestion, answer-generation, or source-governance logic is touched.
Verification
npm run typecheck,eslint, andprettier --checkpass on the changed files.🤖 Generated with Claude Code