Skip to content

🎨 Palette: [동적 파일 크기 μ œν•œ μ—λŸ¬ λ©”μ‹œμ§€ 제곡] - #365

Closed
seonghobae wants to merge 1 commit into
mainfrom
ux-dynamic-filesize-message-10272059860397431265
Closed

🎨 Palette: [동적 파일 크기 μ œν•œ μ—λŸ¬ λ©”μ‹œμ§€ 제곡]#365
seonghobae wants to merge 1 commit into
mainfrom
ux-dynamic-filesize-message-10272059860397431265

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

πŸ’‘ What: ν΄λΌμ΄μ–ΈνŠΈ μΈ‘ 파일 μ—…λ‘œλ“œ 크기 검증 λ‘œμ§μ—μ„œ ν•˜λ“œμ½”λ”©λœ μš©λŸ‰ μ œν•œ ν…μŠ€νŠΈ('5 GiB')λ₯Ό 동적 ν¬λ§·νŒ…(formatBinaryBytes(MAX_UPLOAD_BYTES))으둜 λŒ€μ²΄ν–ˆμŠ΅λ‹ˆλ‹€.
🎯 Why: ν•˜λ“œμ½”λ”©λœ λ¬Έμžμ—΄μ€ ν–₯ν›„ MAX_UPLOAD_BYTES μƒμˆ˜κ°€ 변경될 경우 μ‹€μ œ μ œν•œκ³Ό 였λ₯˜ λ©”μ‹œμ§€κ°€ λΆˆμΌμΉ˜ν•˜μ—¬ μ‚¬μš©μžμ—κ²Œ ν˜Όλž€μ„ 쀄 수 μžˆμŠ΅λ‹ˆλ‹€. 이λ₯Ό λ™μ μœΌλ‘œ κ³„μ‚°ν•˜μ—¬ 항상 μ •ν™•ν•œ μ œν•œ μš©λŸ‰μ„ μ•ˆλ‚΄ν•˜λ„λ‘ κ°œμ„ ν–ˆμŠ΅λ‹ˆλ‹€.
πŸ“Έ Before/After: Before: "File exceeds 5 GiB limit." -> After: "File exceeds 5.00 GiB limit." (μƒμˆ˜μ— 따라 μžλ™ 변경됨)
β™Ώ Accessibility: 슀크린 리더(ARIA)에도 항상 μ •ν™•ν•œ μš©λŸ‰ μ œν•œ 정보가 μ „λ‹¬λ˜μ–΄ 폼 검증 ν”Όλ“œλ°±μ˜ 신뒰성이 ν–₯μƒλ©λ‹ˆλ‹€.


PR created automatically by Jules for task 10272059860397431265 started by @seonghobae

Summary by CodeRabbit

  • 버그 μˆ˜μ •

    • 파일 μ—…λ‘œλ“œ μš©λŸ‰ 초과 μ‹œ 였λ₯˜ λ©”μ‹œμ§€μ™€ 미리보기에 μ‹€μ œ 적용 쀑인 μ—…λ‘œλ“œ μ œν•œκ°’μ΄ λ™μ μœΌλ‘œ ν‘œμ‹œλ©λ‹ˆλ‹€.
    • ν•˜λ“œμ½”λ”©λœ μš©λŸ‰ λŒ€μ‹  ν˜„μž¬ 섀정에 λ§žλŠ” ν˜•μ‹μœΌλ‘œ μ œν•œ μš©λŸ‰μ„ μ•ˆλ‚΄ν•©λ‹ˆλ‹€.
  • λ¬Έμ„œ

    • μ—…λ‘œλ“œ μ œν•œκ°’μ„ λ™μ μœΌλ‘œ ν‘œμ‹œν•΄μ•Ό ν•˜λŠ” κΈ°μ€€κ³Ό κ΄€λ ¨ 지침을 λ³΄μ™„ν–ˆμŠ΅λ‹ˆλ‹€.

β€¦μŠ€νŠΈλ₯Ό μ œκ±°ν•˜κ³ , MAX_UPLOAD_BYTES μƒμˆ˜λ₯Ό 기반으둜 μ—λŸ¬ λ©”μ‹œμ§€λ₯Ό λ™μ μœΌλ‘œ μƒμ„±ν•˜λ„λ‘ κ°œμ„ ν–ˆμŠ΅λ‹ˆλ‹€.
@google-labs-jules

Copy link
Copy Markdown

πŸ‘‹ Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a πŸ‘€ emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

πŸ“ Walkthrough

Walkthrough

파일 μ—…λ‘œλ“œ μ œν•œ 초과 λ©”μ‹œμ§€μ™€ 미리보기 문ꡬ가 ν•˜λ“œμ½”λ”©λœ 5 GiB λŒ€μ‹  MAX_UPLOAD_BYTESμ—μ„œ λ™μ μœΌλ‘œ μƒμ„±λœ μ œν•œκ°’μ„ ν‘œμ‹œν•˜λ„λ‘ λ³€κ²½λ˜μ—ˆμŠ΅λ‹ˆλ‹€. κ΄€λ ¨ ν…ŒμŠ€νŠΈμ™€ λ¬Έμ„œλ„ κ°±μ‹ λ˜μ—ˆμŠ΅λ‹ˆλ‹€.

Changes

μ—…λ‘œλ“œ μ œν•œ λ©”μ‹œμ§€

Layer / File(s) Summary
동적 μ œν•œκ°’ ν‘œμ‹œ
saas_web.py, tests/test_saas_web.py, CHANGELOG.md, .jules/palette.md
μ—…λ‘œλ“œ 였λ₯˜ λ©”μ‹œμ§€μ™€ 미리보기 문ꡬ가 MAX_UPLOAD_BYTESλ₯Ό ν¬λ§·ν•œ 값을 μ‚¬μš©ν•©λ‹ˆλ‹€. ν…ŒμŠ€νŠΈλŠ” λ™μ μœΌλ‘œ μƒμ„±λœ μ œν•œκ°’μ„ ν™•μΈν•©λ‹ˆλ‹€. λ³€κ²½ λ‘œκ·Έμ™€ νŒ”λ ˆνŠΈ 지침도 ν•΄λ‹Ή λ™μž‘μ„ λ°˜μ˜ν•©λ‹ˆλ‹€.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • ContextualWisdomLab/codec-carver#323: saas_web.py, κ΄€λ ¨ ν…ŒμŠ€νŠΈ, λ³€κ²½ 둜그, νŒ”λ ˆνŠΈ μ§€μΉ¨μ—μ„œ MAX_UPLOAD_BYTESλ₯Ό λ™μ μœΌλ‘œ ν¬λ§·ν•˜λŠ” λ³€κ²½κ³Ό 직접 μ—°κ²°λ©λ‹ˆλ‹€.
πŸš₯ Pre-merge checks | βœ… 5
βœ… Passed checks (5 passed)
Check name Status Explanation
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check βœ… Passed 제λͺ©μ€ ν•˜λ“œμ½”λ”©λœ 파일 크기 μ œν•œμ„ λ™μ μœΌλ‘œ ν‘œμ‹œν•˜λŠ” μ£Όμš” λ³€κ²½ 사항을 λͺ…ν™•ν•˜κ²Œ μ„€λͺ…ν•©λ‹ˆλ‹€.
Docstring Coverage βœ… Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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
πŸ“ Generate docstrings
  • Create stacked PR
  • Commit on current branch
πŸ§ͺ Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ux-dynamic-filesize-message-10272059860397431265

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

@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: 1

🧹 Nitpick comments (1)
tests/test_saas_web.py (1)

55-55: 🎯 Functional Correctness | πŸ”΅ Trivial | ⚑ Quick win

μ‹€μ œ 였λ₯˜ 문ꡬ의 연결을 κ²€μ¦ν•˜μ‹­μ‹œμ˜€.

ν˜„μž¬ ν…ŒμŠ€νŠΈλŠ” limitText μ„ μ–Έλ§Œ ν™•μΈν•©λ‹ˆλ‹€. limitTextκ°€ setCustomValidity λ˜λŠ” preview.innerText에 μ‚¬μš©λ˜μ§€ μ•Šμ•„λ„ ν…ŒμŠ€νŠΈκ°€ ν†΅κ³Όν•©λ‹ˆλ‹€. 두 μ‚¬μš©μž ν‘œμ‹œ 문ꡬ가 limitTextλ₯Ό μ‚¬μš©ν•˜λŠ”μ§€ ν™•μΈν•˜κ³ , κ°€λŠ₯ν•˜λ©΄ λ³€κ²½λœ MAX_UPLOAD_BYTES κ°’μœΌλ‘œ μƒμ„±λœ HTML도 κ²€μ¦ν•˜μ‹­μ‹œμ˜€.

πŸ€– 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/test_saas_web.py` at line 55, Update the test around the HTML assertion
for limitText to also verify that both user-facing messages, setCustomValidity
and preview.innerText, use limitText. Render or assert HTML with the changed
MAX_UPLOAD_BYTES value so the generated error text is validated rather than only
the declaration.
πŸ€– 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 `@saas_web.py`:
- Around line 233-236: Update the HTML/JavaScript generation around the
client-side MAX_UPLOAD_BYTES definition so it receives the server-side Python
MAX_UPLOAD_BYTES as a safely serialized numeric value instead of a hardcoded
constant. Ensure the injected value drives both file validation and limitText
display, and add or update tests to verify the server setting is reflected
consistently in the UI and client rejection behavior.

---

Nitpick comments:
In `@tests/test_saas_web.py`:
- Line 55: Update the test around the HTML assertion for limitText to also
verify that both user-facing messages, setCustomValidity and preview.innerText,
use limitText. Render or assert HTML with the changed MAX_UPLOAD_BYTES value so
the generated error text is validated rather than only the declaration.
πŸͺ„ 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5966867e-6a63-466f-979b-8f9cad73fe1d

πŸ“₯ Commits

Reviewing files that changed from the base of the PR and between 963291f and 22c057d.

πŸ“’ Files selected for processing (4)
  • .jules/palette.md
  • CHANGELOG.md
  • saas_web.py
  • tests/test_saas_web.py

Comment thread saas_web.py
Comment on lines +233 to +236
const limitText = formatBinaryBytes(MAX_UPLOAD_BYTES);
input.setCustomValidity('File exceeds ' + limitText + ' limit.');
input.setAttribute('aria-invalid', 'true');
preview.innerText = 'Selected file size: ' + text + ' (exceeds 5 GiB limit)';
preview.innerText = 'Selected file size: ' + text + ' (exceeds ' + limitText + ' limit)';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

πŸ—„οΈ Data Integrity & Integration | 🟠 Major | ⚑ Quick win

μ„œλ²„ μ„€μ •μ˜ MAX_UPLOAD_BYTESλ₯Ό ν΄λΌμ΄μ–ΈνŠΈμ— μ£Όμž…ν•˜μ‹­μ‹œμ˜€.

limitText의 ν¬λ§·νŒ…μ€ λ™μ μ΄μ§€λ§Œ, JavaScript의 MAX_UPLOAD_BYTESλŠ” Line 194μ—μ„œ Python의 MAX_UPLOAD_BYTES와 λ³„λ„λ‘œ ν•˜λ“œμ½”λ”©λ˜μ–΄ μžˆμŠ΅λ‹ˆλ‹€. μ„œλ²„ μ œν•œλ§Œ λ³€κ²½ν•˜λ©΄ λΈŒλΌμš°μ €κ°€ 잘λͺ»λœ μ œν•œμ„ ν‘œμ‹œν•˜κ±°λ‚˜ νŒŒμΌμ„ 잘λͺ» κ±°λΆ€ν•  수 μžˆμŠ΅λ‹ˆλ‹€. HTML 생성 μ‹œ μ„œλ²„ 섀정값을 μ•ˆμ „ν•œ 숫자둜 μ£Όμž…ν•˜κ³ , λ³€κ²½λœ 섀정값이 UI와 μ„œλ²„μ— λ™μΌν•˜κ²Œ μ μš©λ˜λŠ”μ§€ ν…ŒμŠ€νŠΈν•˜μ‹­μ‹œμ˜€.

πŸ€– 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 `@saas_web.py` around lines 233 - 236, Update the HTML/JavaScript generation
around the client-side MAX_UPLOAD_BYTES definition so it receives the
server-side Python MAX_UPLOAD_BYTES as a safely serialized numeric value instead
of a hardcoded constant. Ensure the injected value drives both file validation
and limitText display, and add or update tests to verify the server setting is
reflected consistently in the UI and client rejection behavior.

Copy link
Copy Markdown
Contributor Author

@opencode-agent

Keep this as the single maintained dynamic single-file limit-message slice after closing overbroad duplicate #360, but repair the exact head before merge.

Required bounded changes:

  1. Remove .jules/palette.md; it is not authoritative doctoring.
  2. Preserve server/client parity: derive only the existing single-file validation message from MAX_UPLOAD_BYTES. Do not add an aggregate batch-byte limit unless the backend first owns and enforces that separate contract.
  3. Strengthen the regression beyond checking one declaration string. Prove the rendered UI contains one limitText = formatBinaryBytes(MAX_UPLOAD_BYTES) value and that both setCustomValidity and the visible/ARIA-live preview use that exact value; prove the obsolete literal File exceeds 5 GiB limit. and (exceeds 5 GiB limit) are absent. Retain the valid boundary behavior for exactly-at-limit and over-limit values in the existing executable UI/HTML contract available in the repository.
  4. Keep CHANGELOG.md concise and user-facing; add authoritative accessibility/operational doctoring only if a maintained document already owns this UI contract.
  5. Run the complete suite with zero unexpected skips, configured 100% production statement/branch/docstring gates, packaging/entry-point checks, Security Scan, Semgrep, and fresh exact-head review.

Do not alter the backend limit, batch semantics, dependencies, workflows, reviewer credentials, release state, or branch protection. Publish only if the live head still matches the expected head and the final diff is limited to the HTML implementation, direct tests, and authoritative changelog/documentation.

@seonghobae
seonghobae marked this pull request as draft August 6, 2026 23:59

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #289. Both PRs make the same production change in saas_web.py (formatBinaryBytes(MAX_UPLOAD_BYTES) for the client-side over-limit validity and preview text), while #289 also asserts both dynamic message sinks in the regression test rather than only the formatter assignment. Keeping #289 as the canonical branch avoids competing writes and duplicate review/check work; no unique production behavior from this PR is being discarded.

@seonghobae seonghobae closed this Aug 7, 2026
@google-labs-jules

Copy link
Copy Markdown

Closing as superseded by #289. Both PRs make the same production change in saas_web.py (formatBinaryBytes(MAX_UPLOAD_BYTES) for the client-side over-limit validity and preview text), while #289 also asserts both dynamic message sinks in the regression test rather than only the formatter assignment. Keeping #289 as the canonical branch avoids competing writes and duplicate review/check work; no unique production behavior from this PR is being discarded.

Understood. Acknowledging that this work is now superseded by #289 and stopping work on this task.

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