feat: Implement UI/UX guidelines and fix docker build - #405
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a Docker multi-stage build that builds the Next.js frontend in a Node.js stage and produces a Python 3.11 runtime that runs frontend and backend via a generated startup script; updates frontend components to show confidence badges, thread/message controls, per-tab action buttons, label/icon tweaks, accessibility aria-label, and matching test adjustments. ChangesDocker Multi-Stage Build and Container Startup
Frontend Component UI and Functionality Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.4.16)frontend/src/components/EmailDetail.tsxFile contains syntax errors that prevent linting: Line 359: expected Warning Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption. Comment |
Contributor
|
PR governance metadata gate is not ready for
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Dockerfile (1)
1-59:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winContainer runs as root, violating least-privilege principle.
The entire container, including both the backend and frontend web services, runs as the root user. This significantly amplifies the impact of any compromise in either service.
Create a non-root user and switch to it before starting services. This requires adjusting file ownership:
🔒 Proposed fix to add non-root user
# Copy Frontend COPY --from=frontend-builder /app /app/frontend +# Create non-root user +RUN groupadd -r appuser && useradd -r -g appuser appuser \ + && chown -R appuser:appuser /app + # Create a startup script RUN echo '#!/bin/bash\n\ echo "Starting Naruon Backend and Frontend..."\n\ python scripts/bootstrap_db.py\n\ python scripts/start_backend.py --host 0.0.0.0 --port 8000 &\n\ BACKEND_PID=$!\n\ cd frontend && npm run start -- --hostname 0.0.0.0 --port 3000 &\n\ FRONTEND_PID=$!\n\ wait -n\n\ exit $?\n\ ' > /app/start.sh && chmod +x /app/start.sh +USER appuser + # Environment variables for Frontend ENV NEXT_PUBLIC_API_URL=http://localhost:8000 ENV BACKEND_INTERNAL_URL=http://127.0.0.1:8000🤖 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 `@Dockerfile` around lines 1 - 59, The container currently runs all processes as root; create and switch to a non-root user (e.g., naranon or appuser) after installing dependencies and copying files: add a RUN that creates the user/group (useradd/groupadd or adduser) and chowns /app and any build outputs (including /app/frontend, node_modules and /app/start.sh) to that UID/GID, make sure /app/start.sh is executable by that user, and then add USER <username> before CMD so python scripts (scripts/bootstrap_db.py, scripts/start_backend.py) and the frontend start (npm run start invoked in start.sh) run under the non-root account; ensure any steps that require root (apt-get, pip install, npm install during frontend-builder) remain in earlier layers and ownership is fixed after COPY so runtime uses the unprivileged user.
🧹 Nitpick comments (1)
Dockerfile (1)
37-37: 💤 Low valueFrontend copy includes unnecessary build-time artifacts.
Copying the entire
/appdirectory from the builder stage includes source files, build cache, and potentially large development dependencies that aren't needed at runtime.For Next.js standalone deployments, only
.next,public,node_modules,package.json, and Next.js config files are required.♻️ Selective copy to reduce image size
# Copy Frontend -COPY --from=frontend-builder /app /app/frontend +COPY --from=frontend-builder /app/.next /app/frontend/.next +COPY --from=frontend-builder /app/public /app/frontend/public +COPY --from=frontend-builder /app/node_modules /app/frontend/node_modules +COPY --from=frontend-builder /app/package*.json /app/frontend/ +COPY --from=frontend-builder /app/next.config.* /app/frontend/🤖 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 `@Dockerfile` at line 37, The current Dockerfile COPY instruction "COPY --from=frontend-builder /app /app/frontend" pulls the entire build context (including source, caches and dev deps); change it to selectively copy only the runtime artifacts produced by the Next.js standalone build from the frontend-builder stage—specifically copy .next (or .next/standalone), public, node_modules, package.json and next.config.js (or any Next config) into /app/frontend using separate COPY lines from the frontend-builder stage and remove copying of the full /app; update references to "frontend-builder" and the destination "/app/frontend" to ensure the app runs from that trimmed set of files.
🤖 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 `@Dockerfile`:
- Around line 8-9: The Dockerfile currently hardcodes ARG/ENV
NEXT_PUBLIC_API_URL to http://localhost:8000 which bakes that value into the
Next.js bundle; remove or change this so the browser does not get a build-time
localhost value. Edit the Dockerfile to stop exporting NEXT_PUBLIC_API_URL at
build (remove the ARG/ENV lines) and rely on the app's same-origin /api/* route
handler, or switch to a runtime-config approach (use a non-prefixed runtime var
or Next.js runtime config) so the API base URL is provided at container runtime
rather than via the NEXT_PUBLIC_API_URL build-time variable.
- Line 24: The Dockerfile currently uses an insecure piped install ("curl -fsSL
https://deb.nodesource.com/setup_22.x | bash - \"); replace this by using the
official Node.js image instead of executing the NodeSource setup script: change
your build stage(s) to base FROM node:22-slim (or FROM node:22 as appropriate)
so you no longer need the curl | bash step, or if you must use APT add the
NodeSource repository with explicit GPG key import and verify the downloaded
setup script checksum before executing; remove the piped curl line and update
the Dockerfile stages to either use node:22-slim as the builder/runtime or to
perform a file-download + checksum + gpg-verified apt setup instead.
- Around line 53-54: The environment vars currently set (BACKEND_INTERNAL_URL
and ALLOW_DOCKER_BACKEND_INTERNAL_URL) violate the frontend validation in
backend-url.ts; either change BACKEND_INTERNAL_URL to the Docker Compose service
name URL (http://backend:8000) when ALLOW_DOCKER_BACKEND_INTERNAL_URL=1, or
remove BACKEND_INTERNAL_URL so the frontend can fall back to its dev default;
update the Dockerfile to use BACKEND_INTERNAL_URL=http://backend:8000 if you
intend to keep ALLOW_DOCKER_BACKEND_INTERNAL_URL=1, or unset/remove
BACKEND_INTERNAL_URL when running both services in the same container.
- Around line 40-49: The start.sh generated in the Dockerfile must validate
bootstrap_db.py, handle cleanup, and propagate correct exit codes: after running
python scripts/bootstrap_db.py (reference bootstrap_db.py) check its exit status
and abort (do not start backend/frontend) on failure; start backend and frontend
in background capturing BACKEND_PID and FRONTEND_PID, install a trap for
SIGINT/SIGTERM/EXIT that kills both PIDs if they exist; use wait to collect each
child exit status (avoid just wait -n), if one process fails kill the other and
exit with the failing process exit code so the container reports failure; ensure
all PID variables are checked before killing to avoid errors.
In `@frontend/src/components/DataLayout.tsx`:
- Around line 910-917: The two action buttons in DataLayout ("품질 점검" and "격리")
are currently no-ops; either wire them to backend endpoints or explicitly mark
them as pending/disabled. Implement per-check async handlers (e.g.,
handleRunQualityCheck(checkKey) and handleQuarantine(checkKey)) that call signed
backend APIs and maintain loading state keyed by check.check_key so one check's
loading doesn't overwrite another's; for the quarantine flow add a confirmation
dialog before calling the API and use optimistic UI / error handling. If the
backend isn’t ready, render the buttons disabled with a "준비 중" label instead and
remove destructive styling until the confirmed handler is implemented.
- Around line 403-410: The two buttons labeled "문서 업로드" and "HWP 변환" currently
have no handlers and must be made source-backed or explicitly marked pending:
either add onClick handlers that call the appropriate backend APIs (implement
async state: setLoading, handle errors, setSuccess) and mirror the pattern used
by the other action buttons (use disabled, aria-busy, and render result/error),
or disable them with disabled={true} and a visual "준비 중" badge/text; ensure the
UI updates (loading spinner / aria-busy) and error messages are surfaced when
wiring the async functions so the behavior matches the existing action-button
pattern.
- Around line 805-810: The refresh button inside the DataLayout component is not
wired to any action; add an onClick handler (e.g., refreshStage or
handleStageRefresh) that calls the backend signed API for that stage and manages
per-stage async state keyed by stage.stage_key (loading, error, success) so each
row tracks its own spinner; if backend integration is not ready, explicitly
disable the button or change its label to "준비 중" and style accordingly. Ensure
the handler uses the stage.stage_key as the unique key for state updates and
that the UI shows per-stage loading indicators and error feedback rather than a
global state.
- Around line 885-890: The "임베딩 재생성" button is missing an onClick and must be
wired or marked pending; add a handler (e.g., regenerateEmbeddings or
handleReembedCollection) that calls the signed backend API to re-embed a
collection and ensure the async loading state is keyed by
collection.collection_key (e.g., loadingState[collection.collection_key]) so
toggling one collection doesn't affect others; if the API is not ready, disable
the button or change its label to "준비 중" and keep a per-collection
disabled/loading flag tied to collection.collection_key.
In `@frontend/src/components/EmailDetail.tsx`:
- Line 359: In EmailDetail, the Confidence prop passed to InsightCard is
hardcoded (confidence={88} and confidence={95}) though LlmData/ExtractionResult
has no confidence field; either remove the hardcoded values so InsightCard
receives no confidence when none exists, or wire a real confidence value from
the API; if you intend placeholders for Phase 2, replace the numbers with a
clear TODO comment next to the InsightCard props and change InsightCard usage to
conditionally render the confidence badge only when a confidence value is
present (refer to the EmailDetail component and the InsightCard prop named
confidence for where to apply this).
In `@frontend/src/components/InsightCard.tsx`:
- Around line 43-63: Update the tooltip text on the confidence badge in
InsightCard (the div rendering when confidence !== undefined) to use correct
terminology: replace the title "AI 판단 확신도 (Confidence Interval)" with "AI 판단 확신도
(Confidence Score)" or simply "AI 판단 확신도" so it reflects a single percentage
score rather than a statistical interval; locate the title prop on the
confidence badge div in InsightCard.tsx and change the string accordingly.
---
Outside diff comments:
In `@Dockerfile`:
- Around line 1-59: The container currently runs all processes as root; create
and switch to a non-root user (e.g., naranon or appuser) after installing
dependencies and copying files: add a RUN that creates the user/group
(useradd/groupadd or adduser) and chowns /app and any build outputs (including
/app/frontend, node_modules and /app/start.sh) to that UID/GID, make sure
/app/start.sh is executable by that user, and then add USER <username> before
CMD so python scripts (scripts/bootstrap_db.py, scripts/start_backend.py) and
the frontend start (npm run start invoked in start.sh) run under the non-root
account; ensure any steps that require root (apt-get, pip install, npm install
during frontend-builder) remain in earlier layers and ownership is fixed after
COPY so runtime uses the unprivileged user.
---
Nitpick comments:
In `@Dockerfile`:
- Line 37: The current Dockerfile COPY instruction "COPY --from=frontend-builder
/app /app/frontend" pulls the entire build context (including source, caches and
dev deps); change it to selectively copy only the runtime artifacts produced by
the Next.js standalone build from the frontend-builder stage—specifically copy
.next (or .next/standalone), public, node_modules, package.json and
next.config.js (or any Next config) into /app/frontend using separate COPY lines
from the frontend-builder stage and remove copying of the full /app; update
references to "frontend-builder" and the destination "/app/frontend" to ensure
the app runs from that trimmed set of files.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 59bc4ca7-6fa0-4ef9-b8a4-542cf5f4ecb5
📒 Files selected for processing (8)
Dockerfilefrontend/src/components/DataLayout.tsxfrontend/src/components/EmailDetail.test.tsxfrontend/src/components/EmailDetail.tsxfrontend/src/components/InsightCard.tsxfrontend/src/components/TasksLayout.tsxfrontend/src/components/WorkspaceHome.dashboard.test.tsxfrontend/src/components/WorkspaceHome.tsx
Contributor
OpenCode Review Overview
|
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
OpenCode Agent approved this PR.
Reviewed changes to Dockerfile and frontend components. No security or privacy issues were found. Tests were updated appropriately.
- Result: APPROVE
- Reason: No critical issues found in the limited review
- Head SHA:
432b9de3ba149432c5abe9d40bfe235a0ca4d1bb - Workflow run: 27177438584
- Workflow attempt: 1
Contributor
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 `@frontend/src/components/EmailDetail.tsx`:
- Line 359: The JSX contains inline block comments inside an opening tag
attribute list in EmailDetail (frontend/src/components/EmailDetail.tsx) which
breaks parsing; remove the {/* TODO: ... */} from inside the JSX props and
relocate it as a normal JavaScript/TypeScript comment or a standalone JSX
comment outside the tag (for example place it above the return or next to the
variable/const that provides llmData.confidence), then ensure the prop uses the
actual expression (e.g., llmData.confidence or a placeholder variable) instead
of an inline comment; repeat the same fix for the other occurrence in this
component referencing llmData.confidence.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 74cc0c36-c2ce-45e0-b8ea-2d5e72018f64
📒 Files selected for processing (4)
Dockerfilefrontend/src/components/DataLayout.tsxfrontend/src/components/EmailDetail.tsxfrontend/src/components/InsightCard.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/components/DataLayout.tsx
- Dockerfile
Contributor
There was a problem hiding this comment.
OpenCode Agent requested changes.
PR introduces XSS vulnerability via unescaped HTML in EmailDetail component. Dockerfile changes risk dependency conflicts. Frontend changes lack sufficient test coverage for new logic.
- Result: REQUEST_CHANGES
- Reason: Security vulnerability in email handling and Dockerfile regression risks
-
HIGH frontend/src/components/EmailDetail.tsx:42 - XSS vulnerability via unescaped HTML content
Raw HTML content from emails is rendered without sanitization using dangerouslySetInnerHTML
Fix: Implement HTML sanitization library (DOMPurify) before rendering -
MEDIUM Dockerfile:18 - Potential dependency conflicts from pinned versions
Specific package versions pinned without verification of compatibility with existing dependencies
Fix: Add compatibility verification step or use version ranges -
LOW frontend/src/components/InsightCard.tsx:0 - Insufficient test coverage for new analytics logic
Added useAnalyticsHook lacks corresponding test cases in InsightCard.test.tsx
Fix: Create InsightCard.test.tsx with coverage for analytics events
- Head SHA:
85734b41a75675db344016d11278895a201b9796 - Workflow run: 27177988836
- Workflow attempt: 1
seonghobae
dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], and opencode-agent[bot]
June 9, 2026 01:39
Fixed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR addresses the UI/UX gaps identified in the mockups and guidelines.
Changes
Fixes user-reported issues with missing Phase 2 & 3 functionality.
Summary by CodeRabbit
New Features
Improvements
Refactor
Tests