Conversation
apps/frontend/lib/utils/download.ts
Outdated
| } else if (cleanName) { | ||
| raw = `${cleanName} - ${typeLabel}`; | ||
| } else { | ||
| return sanitizeFilename(cleanCompany, fallbackId, type); |
There was a problem hiding this comment.
WARNING: Inconsistent fallback behavior when name is missing but company exists
When cleanName is falsy but cleanCompany has a value, this returns a filename like Acme Corp.pdf without indicating the document type (Resume/Cover Letter). This differs from the documented fallback chain in the PR description.
The current behavior:
- Name + Company →
John Smith - Resume - Acme Corp.pdf - Name only →
John Smith - Resume.pdf - Company only →
Acme Corp.pdf(no type indicator) - Neither →
resume_<uuid>.pdf
Consider whether the "company only" case should include the type label for consistency:
| return sanitizeFilename(cleanCompany, fallbackId, type); | |
| return sanitizeFilename(cleanCompany ? `${typeLabel} - ${cleanCompany}` : null, fallbackId, type); |
|
|
||
| const getCompanyFromTitle = (title: string | null | undefined): string | null => { | ||
| if (!title) return null; | ||
| const atIdx = title.indexOf(' @ '); |
There was a problem hiding this comment.
SUGGESTION: Consider handling multiple @ occurrences in title
If a title contains multiple @ patterns (e.g., "Senior Dev @ Startup @ Subsidiary"), this will return "Startup @ Subsidiary" as the company name. While edge case, you may want to use lastIndexOf instead to capture only the company name:
| const atIdx = title.indexOf(' @ '); | |
| const atIdx = title.lastIndexOf(' @ '); |
Alternatively, if the current behavior is intentional (taking everything after the first @), consider adding a comment documenting this design decision.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Previous Issues - Now Resolved ✅Both issues from the previous review have been addressed in commit
Positive Observations
Files Reviewed (3 files)
Reviewed by claude-4.5-opus-20251124 · 197,614 tokens |
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/frontend/lib/utils/download.ts">
<violation number="1" location="apps/frontend/lib/utils/download.ts:84">
P2: When name is missing but company is present, the filename omits the document type, so resume and cover letter downloads both become `<company>.pdf` and collide.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Include document type label (Resume/Cover Letter) in filename when
only company is present, preventing ambiguity between downloads
- Use lastIndexOf(' @ ') instead of indexOf to correctly extract
company from titles with multiple @ occurrences
- Fix Prettier formatting in diff-preview-modal.tsx
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pull Request Title
personalize PDF download filenames with applicant name and company
Related Issue
Description
Currently, both the resume and cover letter PDFs are downloaded using the AI-generated title as the filename (e.g.
Business Systems Analyst @ Torch Professional Services Pty Ltd.pdf). This has a few problems:@character is problematic on some filesystemsThis PR changes the download filename to a personal, professional format:
John Smith - Resume - Torch Professional Services.pdfJohn Smith - Cover Letter - Torch Professional Services.pdfThe applicant's name is read from
resumeData.personalInfo.nameand the company is parsed from the existing AI-generated title (splitting on@). Fallbacks are handled gracefully — if the company is unavailable the format becomesJohn Smith - Resume.pdf, and if neither name nor company is available it falls back to the existing UUID-based naming.Type
personalize PDF download filenames with applicant name and company
Proposed Changes
buildResumeFilename()utility function inapps/frontend/lib/utils/download.tsthat constructs{Name} - {Type} - {Company}.pdfwith graceful fallbacksgetCompanyFromTitle()helper inresume-builder.tsxto extract the company name from the AI-generated"Role @ Company"title stringhandleDownloadandhandleDownloadCoverLetterinresume-builder.tsxto use the new naming function, so resume and cover letter downloads now have distinct, personalized filenamesScreenshots / Code Snippets (if applicable)
Before:
Business Systems Analyst @ Torch Professional Services Pty Ltd.pdf ← resume
Business Systems Analyst @ Torch Professional Services Pty Ltd.pdf ← cover letter (identical)
After:
John Smith - Resume - Torch Professional Services Pty Ltd.pdf
John Smith - Cover Letter - Torch Professional Services Pty Ltd.pdf
Fallback chain:
Both name + company → John Smith - Resume - Acme Corp.pdf
Name only → John Smith - Resume.pdf
Neither → resume_.pdf (existing behaviour)
How to Test
/builder?id=...){Your Name} - Resume - {Company}.pdf{Your Name} - Cover Letter - {Company}.pdf{Your Name} - Resume.pdfpersonalInfo.namein the editor and download — verify it falls back to the previous title-based or UUID-based filenameChecklist
Additional Information
The
buildResumeFilename()function reuses the existingsanitizeFilename()utility for Unicode normalization and invalid-character stripping, so all existing safety guarantees (handling of/,\,*,?, etc. and multi-byte character truncation) apply to the new format as well.Summary by cubic
Personalized PDF download filenames for resumes and cover letters using the applicant’s name and company. Files are now clear, distinct, and free of problematic characters.
resume_<uuid>.pdf.buildResumeFilename()and switched download handlers; company parsing now useslastIndexOf(' @ ')to handle titles with multiple@.Written for commit 3063bf6. Summary will update on new commits.