feat: add AI ATS suggestion to LaTeX resume workflow#335
feat: add AI ATS suggestion to LaTeX resume workflow#335AishwaryJhunjhunwala wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR implements the "Apply ATS Suggestions" feature, enabling students to select AI-generated resume improvements and apply them directly. The flow spans server-side suggestion application via AI (with premium gating and usage tracking), client-side multi-select UI for individual suggestions, and seamless navigation to a prefilled LaTeX editor with a visual confirmation banner. ChangesATS Apply Suggestions Workflow
Sequence Diagram(s)sequenceDiagram
participant Client
participant AtsController
participant AtsService
participant ATS_AI
participant UsageLog
Client->>AtsController: POST /apply-suggestions
Note over AtsController: Auth & Premium check
AtsController->>AtsService: applySuggestions(studentId, input)
AtsService->>ATS_AI: LaTeX rewrite prompt<br/>with suggestions
ATS_AI-->>AtsService: reply & latex tags
AtsService->>UsageLog: Log GENERATE_RESUME
AtsService-->>AtsController: {reply, updatedLatex}
AtsController-->>Client: 200 JSON response
sequenceDiagram
participant UI as Selection UI
participant Mutation as applyMutation
participant API as /apply-suggestions
participant Editor as LaTeX Editor
UI->>Mutation: Click "Apply suggestions"
Mutation->>API: POST selectedIndices
alt Success
API-->>Mutation: {updatedLatex, reply}
Mutation->>Editor: Navigate with initialLatex state
Editor-->>UI: Show banner & prefilled editor
else 429 Rate Limit
Mutation->>UI: Toast "AI usage limit reached"
else Other Error
Mutation->>UI: Toast backend error message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@client/src/module/student/ats/LatexResumeEditor.tsx`:
- Around line 155-157: In LatexResumeEditor, avoid calling
window.history.replaceState when location.state?.initialLatex is present;
instead use React Router's navigation API (e.g. the navigate function from
useNavigate) to replace the entry so Router keeps its metadata (key, idx, usr).
Replace the direct call in the if (location.state?.initialLatex) block with a
call that navigates to the same path with replace: true and an updated state
object that removes only the initialLatex flag while preserving other
location.state properties (merge existing state minus initialLatex) so Router's
internal state remains consistent.
In `@server/src/module/ats/ats.controller.ts`:
- Around line 93-109: The catch block in ats.controller.ts currently only maps
"429", "Could not extract", and "ENOENT" to 4xx; extend that block to map other
service-domain/user errors to 4xx by inspecting err.message and err.name and
returning the appropriate status and message via res (e.g., if err.message or
err.name includes "ValidationError", "Invalid", "BadRequest" =>
res.status(400).json({ message: err.message }); if it includes "Unauthorized" or
"401" => res.status(401)...; if it includes "Forbidden", "Ownership", or "403"
=> res.status(403)...; if it includes "NotFound" or "404" =>
res.status(404)...). Keep existing checks for "429", "Could not extract", and
"ENOENT", and ensure all 4xx responses include the original err.message for
client diagnostics; fall back to res.status(500) only for unexpected errors.
In `@server/src/module/ats/ats.service.ts`:
- Around line 102-104: The current isOwned check wrongly treats any parseable S3
URL as owned; instead parse the S3 key with getS3KeyFromUrl(input.resumeUrl) and
verify that that exact key (or the canonical resume URL) is associated with the
current user (e.g., compare against user.resumes or user.resumeKeys), only
granting ownership if user.resumes.includes(parsedKey) or an equivalent
user-owned identifier matches; update the isOwned logic to use the parsed key
and explicit membership check rather than just nullity.
In `@server/src/module/ats/ats.validation.ts`:
- Around line 15-17: The applySuggestionsSchema currently allows an unbounded
suggestions array which can be abused; update the schema (applySuggestionsSchema
which extends scoreResumeSchema and defines suggestions) to enforce a reasonable
maximum number of items (e.g., .max(...)) and a maximum string length per
suggestion (use z.string().max(...)) and keep the existing .min(1). Also
consider trimming/normalizing inputs (e.g., .trim() or a transform) to avoid
whitespace abuse and include clear error messages for both the array and item
limits.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2f21d7d4-98f4-4116-a890-1bbaf4ed83cb
📒 Files selected for processing (7)
client/src/module/student/ats/AtsScorePage.tsxclient/src/module/student/ats/LatexResumeEditor.tsxserver/src/module/ats/ats.controller.tsserver/src/module/ats/ats.routes.tsserver/src/module/ats/ats.service.tsserver/src/module/ats/ats.types.tsserver/src/module/ats/ats.validation.ts
| if (location.state?.initialLatex) { | ||
| window.history.replaceState({}, document.title); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In React Router v7, what is the recommended way to clear location.stateafter navigation? Is directly callingwindow.history.replaceState({}, document.title) safe, or can it break router-managed history metadata (such as key/idx/usr)?
💡 Result:
In React Router v7, the recommended way to clear location.state is to use the navigate function returned by the useNavigate hook to perform a navigation to the current location with the replace: true option and an empty or modified state object [1][2][3]. The idiomatic approach is: const location = useLocation; const navigate = useNavigate; // To clear the state entirely navigate(location.pathname, { replace: true, state: {} }); // Or, to preserve the current URL (including search/hash) while clearing state navigate(location, { replace: true, state: undefined }); Directly calling window.history.replaceState({}, document.title) is generally discouraged and potentially unsafe when using React Router [2]. React Router manages its own internal history stack, which includes metadata such as key, idx, and usr (user-provided state) [4][5]. By bypassing the router and manipulating the browser's history API directly, you risk desynchronizing the router's internal state from the actual browser history [4]. This can lead to bugs in features that rely on this metadata, such as: 1. Navigation blocking and history stack tracking [4]. 2. Correct identification of "back" vs "forward" navigation [4]. 3. Proper handling of location.key for caching or state restoration [5]. If you need to clear state, always use the router's provided navigation methods to ensure the internal history stack remains consistent [1][2].
Citations:
- 1: https://stackoverflow.com/questions/72121228/how-to-update-location-state-in-react-router-v6
- 2: https://stackoverflow.com/questions/40099431/how-do-i-clear-location-state-in-react-router-on-page-reload
- 3: https://stackoverflow.com/questions/40099431/how-do-i-clear-location-state-in-react-router-on-page-reload/66359848
- 4: Fall back to push/replace behavior when `navigate(-1)` would leave the website remix-run/react-router#9922
- 5: https://api.reactrouter.com/v7/interfaces/react-router.Location.html
Replace window.history.replaceState with router navigation to avoid desynchronizing React Router's internal state.
Calling window.history.replaceState({}, document.title) directly bypasses React Router's state management and strips critical metadata (key, idx, usr) that the router tracks. This can break navigation features like history blocking and location.key caching.
Suggested fix
export default function LatexResumeEditor() {
const location = useLocation();
+ const navigate = useNavigate();
const [searchParams, setSearchParams] = useSearchParams();
const templateId = searchParams.get("template");
@@
useEffect(() => {
if (templateId) setSearchParams({}, { replace: true });
if (location.state?.initialLatex) {
- window.history.replaceState({}, document.title);
+ navigate(".", { replace: true, state: null });
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
@@
- const navigate = useNavigate();🤖 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 `@client/src/module/student/ats/LatexResumeEditor.tsx` around lines 155 - 157,
In LatexResumeEditor, avoid calling window.history.replaceState when
location.state?.initialLatex is present; instead use React Router's navigation
API (e.g. the navigate function from useNavigate) to replace the entry so Router
keeps its metadata (key, idx, usr). Replace the direct call in the if
(location.state?.initialLatex) block with a call that navigates to the same path
with replace: true and an updated state object that removes only the
initialLatex flag while preserving other location.state properties (merge
existing state minus initialLatex) so Router's internal state remains
consistent.
| if (err instanceof Error) { | ||
| if (err.message.includes("429")) { | ||
| res.status(429).json({ message: "AI usage limit reached. Please try again later." }); | ||
| return; | ||
| } | ||
| if (err.message.includes("Could not extract")) { | ||
| res.status(400).json({ message: err.message }); | ||
| return; | ||
| } | ||
| if (err.message.includes("ENOENT")) { | ||
| res.status(404).json({ message: "Resume file not found. Please upload again." }); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| res.status(500).json({ message: "Failed to improve resume. Please try again." }); | ||
| } |
There was a problem hiding this comment.
Map known service-domain errors to 4xx instead of generic 500.
Known user/input/ownership failures from the service currently become 500, which is misleading for clients and monitoring.
🧭 Suggested error mapping additions
if (err instanceof Error) {
+ if (err.message.includes("Resume does not belong")) {
+ res.status(403).json({ message: err.message });
+ return;
+ }
+ if (err.message.includes("User not found")) {
+ res.status(401).json({ message: err.message });
+ return;
+ }
+ if (err.message.includes("Invalid resume URL format")) {
+ res.status(400).json({ message: err.message });
+ return;
+ }
if (err.message.includes("429")) {
res.status(429).json({ message: "AI usage limit reached. Please try again later." });
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (err instanceof Error) { | |
| if (err.message.includes("429")) { | |
| res.status(429).json({ message: "AI usage limit reached. Please try again later." }); | |
| return; | |
| } | |
| if (err.message.includes("Could not extract")) { | |
| res.status(400).json({ message: err.message }); | |
| return; | |
| } | |
| if (err.message.includes("ENOENT")) { | |
| res.status(404).json({ message: "Resume file not found. Please upload again." }); | |
| return; | |
| } | |
| } | |
| res.status(500).json({ message: "Failed to improve resume. Please try again." }); | |
| } | |
| if (err instanceof Error) { | |
| if (err.message.includes("Resume does not belong")) { | |
| res.status(403).json({ message: err.message }); | |
| return; | |
| } | |
| if (err.message.includes("User not found")) { | |
| res.status(401).json({ message: err.message }); | |
| return; | |
| } | |
| if (err.message.includes("Invalid resume URL format")) { | |
| res.status(400).json({ message: err.message }); | |
| return; | |
| } | |
| if (err.message.includes("429")) { | |
| res.status(429).json({ message: "AI usage limit reached. Please try again later." }); | |
| return; | |
| } | |
| if (err.message.includes("Could not extract")) { | |
| res.status(400).json({ message: err.message }); | |
| return; | |
| } | |
| if (err.message.includes("ENOENT")) { | |
| res.status(404).json({ message: "Resume file not found. Please upload again." }); | |
| return; | |
| } | |
| } | |
| res.status(500).json({ message: "Failed to improve resume. Please try again." }); | |
| } |
🤖 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 `@server/src/module/ats/ats.controller.ts` around lines 93 - 109, The catch
block in ats.controller.ts currently only maps "429", "Could not extract", and
"ENOENT" to 4xx; extend that block to map other service-domain/user errors to
4xx by inspecting err.message and err.name and returning the appropriate status
and message via res (e.g., if err.message or err.name includes
"ValidationError", "Invalid", "BadRequest" => res.status(400).json({ message:
err.message }); if it includes "Unauthorized" or "401" => res.status(401)...; if
it includes "Forbidden", "Ownership", or "403" => res.status(403)...; if it
includes "NotFound" or "404" => res.status(404)...). Keep existing checks for
"429", "Could not extract", and "ENOENT", and ensure all 4xx responses include
the original err.message for client diagnostics; fall back to res.status(500)
only for unexpected errors.
| const isOwned = | ||
| user.resumes.includes(input.resumeUrl) || | ||
| getS3KeyFromUrl(input.resumeUrl) !== null; |
There was a problem hiding this comment.
Tighten resume ownership check to prevent cross-user resume access.
This check currently authorizes any parseable S3 URL, not just URLs owned by the current student.
🔒 Suggested ownership-safe check
- const isOwned =
- user.resumes.includes(input.resumeUrl) ||
- getS3KeyFromUrl(input.resumeUrl) !== null;
+ const requestedKey = getS3KeyFromUrl(input.resumeUrl);
+ const ownedS3Keys = new Set(
+ user.resumes
+ .map((resumeUrl) => getS3KeyFromUrl(resumeUrl))
+ .filter((key): key is string => key !== null),
+ );
+ const isOwned =
+ user.resumes.includes(input.resumeUrl) ||
+ (requestedKey !== null && ownedS3Keys.has(requestedKey));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isOwned = | |
| user.resumes.includes(input.resumeUrl) || | |
| getS3KeyFromUrl(input.resumeUrl) !== null; | |
| const requestedKey = getS3KeyFromUrl(input.resumeUrl); | |
| const ownedS3Keys = new Set( | |
| user.resumes | |
| .map((resumeUrl) => getS3KeyFromUrl(resumeUrl)) | |
| .filter((key): key is string => key !== null), | |
| ); | |
| const isOwned = | |
| user.resumes.includes(input.resumeUrl) || | |
| (requestedKey !== null && ownedS3Keys.has(requestedKey)); |
🤖 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 `@server/src/module/ats/ats.service.ts` around lines 102 - 104, The current
isOwned check wrongly treats any parseable S3 URL as owned; instead parse the S3
key with getS3KeyFromUrl(input.resumeUrl) and verify that that exact key (or the
canonical resume URL) is associated with the current user (e.g., compare against
user.resumes or user.resumeKeys), only granting ownership if
user.resumes.includes(parsedKey) or an equivalent user-owned identifier matches;
update the isOwned logic to use the parsed key and explicit membership check
rather than just nullity.
| export const applySuggestionsSchema = scoreResumeSchema.extend({ | ||
| suggestions: z.array(z.string()).min(1, "At least one suggestion is required"), | ||
| }); |
There was a problem hiding this comment.
Bound suggestions input to avoid prompt-size abuse.
suggestions is currently unbounded (count and item length). A crafted payload can create very large prompts and spike AI latency/cost.
🔧 Suggested schema hardening
export const applySuggestionsSchema = scoreResumeSchema.extend({
- suggestions: z.array(z.string()).min(1, "At least one suggestion is required"),
+ suggestions: z
+ .array(
+ z
+ .string()
+ .trim()
+ .min(1, "Suggestion cannot be empty")
+ .max(300, "Suggestion is too long"),
+ )
+ .min(1, "At least one suggestion is required")
+ .max(10, "Too many suggestions selected"),
});🤖 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 `@server/src/module/ats/ats.validation.ts` around lines 15 - 17, The
applySuggestionsSchema currently allows an unbounded suggestions array which can
be abused; update the schema (applySuggestionsSchema which extends
scoreResumeSchema and defines suggestions) to enforce a reasonable maximum
number of items (e.g., .max(...)) and a maximum string length per suggestion
(use z.string().max(...)) and keep the existing .min(1). Also consider
trimming/normalizing inputs (e.g., .trim() or a transform) to avoid whitespace
abuse and include clear error messages for both the array and item limits.
Sachinchaurasiya360
added
enhancement
|
Can you check for the conflict |
2 participants
Fixes #57
Summary
This PR bridges the gap between the ATS analysis and the resume editing workflow by allowing students to automatically apply AI-generated ATS suggestions to their resume. Previously, students had to manually switch tabs and rewrite their resume based on the ATS results. Now, they can select specific suggestions and have Gemini rewrite their LaTeX resume directly, prefilling the editor with the improved draft.
Changes
POST /api/ats/apply-suggestionsendpoint inats.routes.ts.<reply>/<latex>parsing logic inats.service.tsto instruct Gemini to apply specific suggestions to the user's uploaded LaTeX resume.isPremiumUsergating andusageLimit("GENERATE_RESUME")tracking inats.controller.ts.AtsScorePage.tsxto replace the static suggestions list with a selectable checklist (including a "Select All" toggle) and added a primary CTA button to apply the selected suggestions.LatexResumeEditor.tsxto consume the initial AI-improved LaTeX draft viareact-router'slocation.stateand display a subtle notification banner reminding the user to review the changes.How to test
MONTHLY/ACTIVEin the database).aarav@example.com) and verify that clicking the button returns a Premium required error.Screenshots
Summary by CodeRabbit