Conversation
chore: nginx.dev.conf 수정
…두고, 각각다른 훅과 헬퍼함수를 통해 호출하여 하나의 버스로 모든 이벤트가 이동할 수 있도록 리팩토링
Summary of ChangesHello @alsgud8311, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces significant improvements to the application's event handling system, migrating from a custom Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🛠️ Build SummaryStatus: ✅ SUCCESS 📋 Build Output (마지막 45줄)🤖 Generated by GitHub Actions at Tue Dec 23 13:31:19 UTC 2025 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant refactoring of the event handling system to a more generic, centralized model, and adds a major new feature for asynchronous resume evaluation, including archiving and polling for results. These are excellent architectural improvements. However, the review identified several high and critical severity issues. The event emitter refactoring has introduced bugs related to stale closures in React hooks and has critically broken the communication with the native layer in the webview application. The new resume evaluation form also contains bugs in its validation logic and state management that affect usability. Additionally, a bug was found in the API retry logic. It's recommended to address these issues before merging.
| import { publishEvent, useSubscribeEvents } from "@/utils/events"; | ||
| import { InterviewEventType, InterviewEventPayloads } from "@kokomen/types"; | ||
| import { DependencyList } from "react"; | ||
|
|
||
| export default interviewEventHelpers; | ||
| export function useInterviewEvent<K extends InterviewEventType>( | ||
| event: K, | ||
| handler: InterviewEventPayloads[K] extends undefined | ||
| ? () => void | ||
| : // eslint-disable-next-line no-unused-vars | ||
| (payload: InterviewEventPayloads[K]) => void, | ||
| // eslint-disable-next-line no-unused-vars | ||
| deps: DependencyList = [] | ||
| ): void { | ||
| useSubscribeEvents<InterviewEventType>([{ event, handler }], []); | ||
| } | ||
|
|
||
| export const publishInterviewEvent = publishEvent< | ||
| InterviewEventType, | ||
| InterviewEventPayloads | ||
| >(); |
There was a problem hiding this comment.
This refactoring has critically broken the communication between the webview and the React Native host application. The previous implementation correctly used window.ReactNativeWebView.postMessage to send events to the native layer for speech recognition. The new implementation uses a GlobalEventBus which is confined to the webview's JavaScript context. As a result, events like interview:startVoiceRecognition will be published but never reach the native side, and speech recognition will not work. You should revert to using window.ReactNativeWebView.postMessage for this webview-specific implementation.
| const eventEmitter = useSubscribeEvents<InterviewEventType>( | ||
| [{ event, handler }], | ||
| [] | ||
| ); |
There was a problem hiding this comment.
The deps argument from useInterviewEvent is not being passed down to useSubscribeEvents. It's hardcoded as an empty array []. This will cause the event handler to have stale closures if it depends on any props or state that change over time, as the useEffect in useSubscribeEvents will not re-run to subscribe the new handler with the updated dependencies.
| const eventEmitter = useSubscribeEvents<InterviewEventType>( | |
| [{ event, handler }], | |
| [] | |
| ); | |
| const eventEmitter = useSubscribeEvents<InterviewEventType>( | |
| [{ event, handler }], | |
| deps | |
| ); |
| } | ||
|
|
||
| await exponentialDelay(retryCount); | ||
| return resumeServerInstance.request(error.config as AxiosRequestConfig); |
There was a problem hiding this comment.
In the onRejectedPolling interceptor for resumePollingServerInstance, the request is being retried using resumeServerInstance. These two instances have different baseURLs (/resumes/evaluations vs. /resumes). This will cause the retry request to be sent to the wrong endpoint. The retry should use resumePollingServerInstance to ensure it hits the correct URL.
| return resumeServerInstance.request(error.config as AxiosRequestConfig); | |
| return resumePollingServerInstance.request(error.config as AxiosRequestConfig); |
| .refine( | ||
| (data) => | ||
| data.portfolio_id || (data.portfolio && data.portfolio.length > 0), | ||
| { | ||
| message: "포트폴리오를 선택해주세요", | ||
| path: ["portfolio"] | ||
| } | ||
| ); |
There was a problem hiding this comment.
The Zod schema's .refine for the portfolio makes it a required field, which seems unintentional as portfolios are typically optional. If a user doesn't select an archived portfolio (portfolio_id is undefined), they are forced to upload a portfolio file. This contradicts the optional nature of the field. To fix this, this refinement check should be removed.
| useEffect(() => { | ||
| const resume = form.getValues("resume"); | ||
| const portfolio = form.getValues("portfolio"); | ||
| if (resume instanceof FileList && resume.length > 0) { | ||
| setDisplayName({ ...displayName, resume: "" }); | ||
| form.setValue("resume_id", ""); | ||
| } | ||
| if (portfolio instanceof FileList && portfolio.length > 0) { | ||
| setDisplayName({ ...displayName, portfolio: "" }); | ||
| form.setValue("portfolio_id", ""); | ||
| } | ||
| }, [form.watch("resume_id"), form.watch("portfolio_id")]); |
There was a problem hiding this comment.
The logic within this useEffect is flawed. When a user selects an archived resume (which sets resume_id), this effect is triggered. If a file was previously selected in the file input, the effect will immediately clear the resume_id that was just set, making it impossible to use archived resumes if a file has ever been selected.
The logic to clear one input type when the other is used should be handled in the respective event handlers. For example, in onclickArchiveButton, when setting a resume_id, you should also clear the file input using form.resetField("resume"). This useEffect should be removed.
| const eventEmitter = useSubscribeEvents<ReportEventType>( | ||
| [{ event, handler }], | ||
| [] | ||
| ); |
There was a problem hiding this comment.
There is a similar issue here as in interviewEventEmitter.ts. The deps argument from useReportevent is not being passed to useSubscribeEvents. It's hardcoded as an empty array [], which can lead to stale closures in the event handler.
| const eventEmitter = useSubscribeEvents<ReportEventType>( | |
| [{ event, handler }], | |
| [] | |
| ); | |
| const eventEmitter = useSubscribeEvents<ReportEventType>( | |
| [{ event, handler }], | |
| deps | |
| ); |
| robots.txt | ||
| sitemap.xml No newline at end of file |
There was a problem hiding this comment.
robots.txt and sitemap.xml are generally not included in .gitignore as they are crucial for search engine optimization (SEO). If these files are generated during the build process and are meant to be publicly accessible, they should be committed or be part of the build output that gets deployed. Ignoring them might prevent search engine crawlers from finding and indexing your site correctly. Please verify if ignoring these files is the intended behavior.
| const list = | ||
| type === "ALL" | ||
| ? data?.resumes | ||
| : type === "RESUME" | ||
| ? data?.resumes | ||
| : data?.portfolios; |
There was a problem hiding this comment.
The logic to determine the list to render is a bit complex and can be simplified for better readability and maintainability.
const list =
type === "PORTFOLIO" ? data?.portfolios : data?.resumes;
| onclickArchivedItem( | ||
| type === "ALL" | ||
| ? "RESUME" | ||
| : type === "RESUME" | ||
| ? "RESUME" | ||
| : "PORTFOLIO", | ||
| item | ||
| ); |
There was a problem hiding this comment.
The logic to determine the type argument for onclickArchivedItem is convoluted and can be simplified for better readability.
onclickArchivedItem(
type === "PORTFOLIO" ? "PORTFOLIO" : "RESUME",
item
);
| import { UseFormRegisterReturn } from "react-hook-form"; | ||
| import { cn } from "../../utils/index.ts"; | ||
|
|
||
| const MAX_FILE_SIZE = 5 * 1024 * 1024; // 10MB |
There was a problem hiding this comment.
The comment for MAX_FILE_SIZE states the limit is 10MB, but the value is set to 5MB (5 * 1024 * 1024). This discrepancy can be confusing for future developers. Please update either the value or the comment to be consistent.
| const MAX_FILE_SIZE = 5 * 1024 * 1024; // 10MB | |
| const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB |
🚀 Lighthouse Report for TEST1📅 Date: 12/23/2025
📊 Performance Details
🚀 Lighthouse Report for TEST2📅 Date: 12/23/2025
📊 Performance Details
🚀 Lighthouse Report for TEST3📅 Date: 12/23/2025
📊 Performance Details
🚀 Lighthouse Report for TEST4📅 Date: 12/23/2025
📊 Performance Details
🚀 Lighthouse Report for TEST5📅 Date: 12/23/2025
📊 Performance Details
|
2 participants
RELEASE-20251223