Skip to content

fix(upload): align client precheck with optional public max MB - #1069

Merged
BigSimmo merged 18 commits into
mainfrom
claude/upload-size-precheck-123366
Jul 22, 2026
Merged

fix(upload): align client precheck with optional public max MB#1069
BigSimmo merged 18 commits into
mainfrom
claude/upload-size-precheck-123366

Conversation

@BigSimmo

@BigSimmoBigSimmo commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

  • Follow-up to merged feat(upload): reject over-limit files in the browser before transferring (FV-04) #1064: client precheck no longer always uses the 150 MB ceiling.
  • Adds getClientMaxUploadMb() / NEXT_PUBLIC_MAX_UPLOAD_MB (clamped to ceiling) for DocumentManagerPanel hint + precheck; server env.MAX_UPLOAD_MB stays authoritative.
  • Wires NEXT_PUBLIC_MAX_UPLOAD_MB through Docker ARG/ENV and docker-image CI build-args so Railway/Docker builds can inline a lowered limit.
  • Pins empty NEXT_PUBLIC_MAX_UPLOAD_MB in DOM test beforeEach so inherited shell/CI env cannot flake default-limit assertions.
  • Updates .env.example and deployment docs for build-time mirroring.

Verification

  • node scripts/run-vitest.mjs run tests/upload-size-precheck.dom.test.tsx (prior Codex pass; re-run in CI)
  • UI verification not run: upload-limit wiring only; Production UI already green on prior head
  • Verification not run locally in this sweep worktree (no node_modules); relying on CI after push

Risk and rollout

  • Risk: Client precheck could diverge from server if NEXT_PUBLIC_MAX_UPLOAD_MB is not rebuilt when MAX_UPLOAD_MB is lowered — mitigated by Docker build-arg wiring and docs.
  • Rollback: Revert this PR; upload path falls back to server 413 only.
  • Provider or production effects: None

Clinical Governance Preflight

Document-access / upload path touch (private guideline PDF uploads), not answer generation or retrieval.

  • Source-backed claims still require linked source verification before clinical use
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval
  • Supabase target remains Clinical KB Database (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only
  • Demo/synthetic content remains clearly separated from real clinical sources
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed — N/A (upload size precheck only; no clinical decision-support change)

Test plan

  • node scripts/run-vitest.mjs run tests/upload-size-precheck.dom.test.tsx
  • Confirm hint/precheck still default to 150 when public env unset
  • With NEXT_PUBLIC_MAX_UPLOAD_MB=50, confirm 51 MB file is rejected locally

RAG impact: no retrieval behaviour change — upload UI limit wiring only.

BigSimmoand others added 8 commits July 22, 2026 11:40
…ing (FV-04)
An over-limit guideline PDF was transferred in full before the server
answered 413 — on a large file over a clinic connection that is a long
wait for a guaranteed rejection, with the limit itself never stated
anywhere in the UI.
- New dependency-free `src/lib/upload-limits.ts` holds the ceiling so a
client component can import it; `src/lib/http.ts` (next/server) and
`src/lib/env.ts` (server-only secrets) both can't cross into the bundle.
- `env.MAX_UPLOAD_MB` now derives its cap and default from that constant,
so the configured limit can only ever be *lower* than what the client
rejects up front — the pre-check can never refuse a file the server
would have accepted, and the server stays the authority.
- `UploadPanel` skips an over-ceiling file with the server's own 413
wording (shared message builder, so the two can't diverge) and still
uploads the rest of the batch, matching per-file outcome semantics.
- The file field states "PDF only, up to 150 MB per file", wired via
aria-describedby so it is announced, not just visible.
The ingress pin in tests/upload-ingress-limits.test.ts now asserts the
constant's value plus env's derivation from it, keeping the 151mb proxy
envelope tied to the same number.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Expose NEXT_PUBLIC_MAX_UPLOAD_MB via getClientMaxUploadMb() so DocumentManagerPanel rejects/hints at a lowered operator limit instead of always using the 150 MB ceiling; server env.MAX_UPLOAD_MB remains authoritative.
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@supabase

supabaseBot commented Jul 22, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@coderabbitai

coderabbitaiBot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds shared client upload-limit helpers, rejects oversized files before upload, exposes an accessible limit hint, adds DOM coverage, and wires NEXT_PUBLIC_MAX_UPLOAD_MB through Docker and deployment configuration.

Changes

Client upload pre-check

Layer / File(s)Summary
Shared client limit contract
src/lib/upload-limits.ts
Defines the 150 MB ceiling, client-limit parsing and clamping, size predicates, and standardized error messaging.
UploadPanel validation and accessibility
src/components/clinical-dashboard/DocumentManagerPanel.tsx, tests/upload-size-precheck.dom.test.tsx
Displays the effective limit through an accessible hint, records oversized files as local 413 failures, continues valid uploads, and tests default, boundary, mixed-batch, and lowered-limit behavior.
Build-time client limit wiring
.env.example, Dockerfile, .github/workflows/docker-image.yml, docs/deployment-architecture.md
Documents and passes NEXT_PUBLIC_MAX_UPLOAD_MB as a Docker build-time value for client bundle configuration.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
participant User
participant UploadPanel
participant upload_limits
participant UploadAPI
User->>UploadPanel: select files
UploadPanel->>upload_limits: validate each file size
alt oversized file
upload_limits-->>UploadPanel: limit exceeded
UploadPanel-->>User: show failed outcome
else valid file
UploadPanel->>UploadAPI: upload file
UploadAPI-->>UploadPanel: return upload result
end
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring 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 nameStatusExplanation
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
Title check✅ PassedThe title clearly matches the main change: adding an optional public max MB so client prechecks align with upload limits.
Description check✅ PassedThe description follows the template with Summary, Verification, Risk and rollout, Clinical Governance Preflight, and Notes filled in.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/upload-size-precheck-123366

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

@BigSimmo
BigSimmo enabled auto-merge July 22, 2026 06:12
#1064 was squash-merged, so this branch's own FV-04 commits are not ancestors
of main even though their content is. That made every FV-04 file an add/add or
content conflict against main's squashed copy.
Verified before resolving that main's version of all six FV-04 files is
byte-identical to the state merged as #1064 (git diff d51e2fb origin/main
was empty for each), so main contributed no new content to them and the branch
side is the correct resolution. The three resolved files are byte-identical to
the pre-merge branch tip c023b92, leaving only the NEXT_PUBLIC_MAX_UPLOAD_MB
work as this PR's delta.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@BigSimmo
BigSimmo disabled auto-merge July 22, 2026 06:27

@chatgpt-codex-connectorchatgpt-codex-connectorBot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:ab53ff0925

ℹ️ 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".

Comment threadsrc/lib/upload-limits.ts
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@codex resolve actionable Codex review findings for this pull request and current head using the repository instructions. This is the pull request's single automatic repair pass: do not perform a fresh review, create new standalone findings, or request another review. Work only the existing unresolved Codex threads on the current head. Always fix P0 and P1 findings. For P2 and lower findings, fix only clear, scoped, low-risk issues; otherwise disposition them with a concise reason. After fixing or dispositioning a thread, reply in that thread with as the first line, followed by a concise summary; that marker authorizes the workflow to close that exact thread. If human input or new authorization is required, do not use the marker and leave the thread open with the blocker. Finish only after every actionable thread is fixed or dispositioned and closed, or explicitly left open for a human decision. Do not update the branch from main, address unrelated reviews, broaden scope, or create more than one scoped fix commit. Do not use external APIs, paid services, credentials, dependency changes, or broad refactors unless explicitly authorized. Add targeted tests where behavior changes and run the narrowest relevant validation.

@BigSimmo
BigSimmo enabled auto-merge (squash) July 22, 2026 06:34
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit:ab53ff0925

ℹ️ 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".

@chatgpt-codex-connectorchatgpt-codex-connectorBot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:0900a1e416

ℹ️ 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".

Comment threadsrc/lib/upload-limits.ts

@chatgpt-codex-connectorchatgpt-codex-connectorBot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:ee6d038374

ℹ️ 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".

Comment threadtests/upload-size-precheck.dom.test.tsx
@BigSimmo

Copy link
Copy Markdown
OwnerAuthor

@codex please resolve the merge conflict for me and commit and push it

@chatgpt-codex-connector

Copy link
Copy Markdown

Summary

  • No merge conflicts are present in the working tree.
  • The branch is clean at c04b8592a17dc15a611f6fffb8c856b769eac38f (Merge branch 'main' into claude/upload-size-precheck-123366).
  • I did not create a new commit because there were no unresolved conflicts or file changes to commit.
  • I did not push because this checkout has no Git remote configured (git remote -v returned no remotes).
  • I did not call make_pr because no new codebase changes were made and no new commit was created.

Testing

  • git status --short --branch
  • git status --porcelain=v1 -uall
  • git diff --check
  • rg -n "^(<<<<<<<|=======|>>>>>>>)" . --glob '!node_modules/**' --glob '!package-lock.json'
  • ⚠️git remote -v returned no configured remote, so I could not push from this checkout.

View task →

BigSimmoand others added 4 commits July 22, 2026 16:47
Add NEXT_PUBLIC_MAX_UPLOAD_MB ARG/ENV + CI build-arg so lowered upload
limits inline at image build time, and stub the env empty in default
DOM cases so inherited shell values cannot flake the 150 MB assertions.
Co-authored-by: Cursor <cursoragent@cursor.com>
@BigSimmo
BigSimmo merged commit 7ff2942 into mainJul 22, 2026
35 of 37 checks passed
@BigSimmo
BigSimmo deleted the claude/upload-size-precheck-123366 branch July 22, 2026 09:05
Sign up for freeto 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

@BigSimmo