Phase 2: split export layer + fix PDF truncation#8
Merged
Conversation
added 3 commits
July 19, 2026 01:18
- Rewrite AGENTS.md/README design system to the live 'Field Manual' light theme (the 'Ethereal Glass' dark design was reverted) - Correct tech stack: PostgreSQL-only (no SQLite), Bun/CI-npm, pdfkit note - Fix REMEDIATION_PLAN false claims (download route is 53 lines, not 879; SQLite) - Mark REDESIGN-PLAN.md as superseded - Delete truly-dead code (zero references): browser-llm-integration.ts (338-line unused fake rule-based AI) - NOTE: UpgradeModal/SubscriptionBanner/pricing.ts are compile-time shims still imported by components; their removal is deferred to Phase 3/4 (entitlement + component refactor), not Phase 0. - Add FIX-PLAN.md tracking the full SOLID/UI remediation roadmap
Extract shared AI logic out of the 4 route handlers into a single tested
abstraction, fixing SRP/OCP/DIP violations:
- src/lib/ai/json.ts: central JSON extraction from free-text LLM responses
- src/lib/ai/client.ts: AIProvider interface + ZAIProvider with 30s timeout
and abort support (swap provider without touching routes — DIP)
- src/lib/ai/handlers.ts: createAIHandler factory centralizes auth, input
validation, model call, parsing, and error handling (OCP: add a feature
by adding a config, not copying a route)
- src/lib/ai/{coach,resume,cover-letter,assessment}.ts: per-feature prompt
+ validation + fallback configs (SRP)
Routes shrink to single-line exports. Behavior is preserved exactly:
coach/cover-letter return graceful fallbacks on parse failure; resume/
assessment return 500. No new dependencies (validateShape replaces zod).
Tests: add 17 unit tests for json.ts + handlers.ts (all passing).
tsc --noEmit and eslint clean.
The export route was a 200-line monolith that owned auth, DOCX generation, and a hand-rolled PDF builder that SILENTLY TRUNCATED content at the bottom of the page (if (y < 50) break). Changes: - src/lib/export/types.ts: shared ExportRequest + MAX_CONTENT_LENGTH (50k) + filename sanitizer - src/lib/export/docx.ts: DOCX builder extracted (SRP) - src/lib/export/pdf.ts: PDF builder extracted AND paginated. Long documents now flow onto new pages instead of being cut off — fixes the data-loss bug. - src/app/api/export/route.ts: shrunk to ~50 lines — auth, validation, type dispatch, and a content-length guard only. Kept the manual Courier PDF approach (no external fonts) to avoid the pdfkit font-resolution issues noted in the original code, rather than adding a new dependency. Tests: add 11 unit tests (pdf pagination/no-truncation, docx validity, helpers). tsc --noEmit and eslint clean.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR updates project documentation, removes the browser LLM implementation, centralizes AI route handling, adds AI configurations and JSON parsing, extracts DOCX/PDF generation, adds export limits, and introduces unit tests for AI and export behavior. ChangesAI and export refactor
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AI Route
participant createAIHandler
participant AI Provider
Client->>AI Route: POST request
AI Route->>createAIHandler: delegate request
createAIHandler->>AI Provider: submit prompts
AI Provider-->>createAIHandler: return model response
createAIHandler-->>Client: return JSON result or error
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 17
🧹 Nitpick comments (3)
__tests__/unit/export-docx.test.ts (1)
1-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMirror the source layout for export tests.
__tests__/unit/export-docx.test.ts#L1-L22: move to the test path corresponding tosrc/lib/export/docx.ts.__tests__/unit/export-pdf.test.ts#L1-L64: move to the test path corresponding tosrc/lib/export/pdf.ts.As per coding guidelines, tests must be “under
__tests__/mirroring thesrc/structure.”🤖 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__/unit/export-docx.test.ts` around lines 1 - 22, Move the tests from __tests__/unit/export-docx.test.ts (lines 1-22) to the __tests__/ path mirroring src/lib/export/docx.ts, preserving the generateDocx test suite unchanged. Also move __tests__/unit/export-pdf.test.ts (lines 1-64) to the corresponding mirrored path for src/lib/export/pdf.ts, without changing its test behavior.Source: Coding guidelines
__tests__/unit/ai-handlers.test.ts (1)
1-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMirror the source path under
__tests__/.Move this to
__tests__/lib/ai/handlers.test.tsso its location mirrorssrc/lib/ai/handlers.ts.As per coding guidelines, "Use Vitest for unit, API, component, and stress tests, with tests under
__tests__/mirroring thesrc/structure."🤖 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__/unit/ai-handlers.test.ts` around lines 1 - 115, Move the test file from the top-level __tests__ location into __tests__/lib/ai/handlers.test.ts so it mirrors the source module path src/lib/ai/handlers.ts. Preserve all existing test contents and behavior.Source: Coding guidelines
__tests__/unit/ai-json.test.ts (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this test to mirror the source structure.
Place it at
__tests__/lib/ai/json.test.tsrather than__tests__/unit/ai-json.test.ts.As per coding guidelines, tests under
__tests__/must mirror thesrc/structure. <coding_guidelines>🤖 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__/unit/ai-json.test.ts` around lines 1 - 2, Move the test file from the unit-level location to __tests__/lib/ai/json.test.ts so it mirrors the source module imported by extractJson. Preserve the existing test contents and behavior without other changes.Source: Coding guidelines
🤖 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 `@__tests__/unit/ai-json.test.ts`:
- Around line 13-26: Add a regression test in the extractJson test suite for
text containing multiple JSON objects, asserting that extractJson returns the
first JSON value rather than null or combining later segments. Use the existing
extraction test style and preserve current expectations for prose, fenced JSON,
and embedded arrays.
In `@AGENTS.md`:
- Line 27: Update AGENTS.md:27 and README.md:47 to document the shipped custom
paginated PDF builder in src/lib/export/pdf.ts instead of describing pdfkit as
the implementation; keep the PDF export descriptions consistent across both
files. Leave FIX-PLAN.md:66-68 unchanged as future-work planning for a possible
pdfkit migration.
In `@FIX-PLAN.md`:
- Around line 19-24: Insert one blank line between the preceding paragraph and
the table in FIX-PLAN.md, leaving the table content unchanged.
- Line 3: Remove or defer the future-dated July 19, 2026 claims: update the
audit date in FIX-PLAN.md line 3 and the README-correction date in
REMEDIATION_PLAN.md line 115 to a current or non-future date, preserving the
surrounding plan content.
In `@README.md`:
- Around line 168-177: Update the documentation index link label for
REDESIGN-PLAN.md to identify it as a superseded historical redesign plan, while
preserving the existing link target and surrounding documentation.
- Around line 152-153: Update the README test inventory to remove the stale
subscription.test.ts entry and change the project-tree API test count from 218
to 187, keeping the detailed table total and all other test entries consistent.
- Around line 40-47: Align the package-manager guidance in README.md and
AGENTS.md with the repository’s npm-based CI by making npm the documented source
of truth. Update any contributor commands that reference Bun and ensure the
committed lockfile and documentation consistently use npm without introducing a
competing package-manager lockfile.
In `@src/app/api/export/route.ts`:
- Around line 14-27: Update the request validation around the destructuring in
the export route to require that content is a string and, when provided, title
is also a string before checking length or calling sanitizeFilename. Return the
existing 400 validation response for invalid types, while preserving valid type
and content handling.
In `@src/lib/ai/assessment.ts`:
- Around line 39-52: Update the validate callback around validateShape to verify
assessmentTitle is a non-empty string and userAnswers is a valid non-empty value
of the expected runtime type before casting to AssessmentBody. Return the
existing 400 required-fields error for invalid or missing values, while
preserving the current length check and successful return for valid requests.
- Around line 3-20: Add a server-owned, user-visible truthfulness disclaimer
field to the JSON response contract in ASSESSMENT_PROMPT, alongside the existing
assessment feedback fields, and instruct the model to populate it consistently.
Keep the “Never guarantee” instruction, but do not rely on generated prose
alone; ensure the assessment response schema explicitly includes the stable
warning required for AI-generated coaching content.
- Around line 54-55: Update buildUserPrompt to serialize non-string
body.userAnswers with JSON.stringify before interpolation, while preserving
string answers unchanged and retaining the existing prompt formatting and length
behavior.
In `@src/lib/ai/coach.ts`:
- Around line 19-34: Update the coaching response JSON contract, fallback
responses, and normalization logic in coach.ts to include a truthWarnings field
containing actionable warnings for potentially unsupported claims. Ensure the
field is always present with a consistent collection shape, including fallback
and normalized responses, so clients can surface warnings for AI-generated
resume and coaching content.
In `@src/lib/ai/json.ts`:
- Around line 16-29: The JSON extraction logic in the parsing function should
replace greedy objectMatch and arrayMatch regexes with balanced-delimiter
scanning that respects quoted strings and escaped characters. Return the first
valid balanced object or array payload, including nested structures, and add
regression coverage for multiple adjacent/following payloads and nested
payloads.
In `@src/lib/ai/resume.ts`:
- Around line 42-51: Update the validators in src/lib/ai/resume.ts lines 42-51,
src/lib/ai/coach.ts lines 98-103, and src/lib/ai/cover-letter.ts lines 47-56 to
require each request field to be a trimmed, non-empty string before casting or
applying length checks: resumeText, question, userAnswer, and jobDescription
respectively. Return the existing 400 validation response for invalid values,
and preserve the current length-limit behavior for valid strings.
In `@src/lib/export/pdf.ts`:
- Around line 23-39: Update the line-processing logic in the PDF export routine
to wrap every styled text line to the available page width using Courier
character measurements before vertical pagination and Tj generation. Preserve
each line’s existing text styling, indentation, and blank-line handling across
wrapped segments, including the corresponding logic around the additional
pagination range.
- Around line 82-111: Correct object allocation in the PDF export flow around
addObject so every /Pages, /Page, /Contents, and font reference uses the actual
assigned object IDs rather than hard-coded values. Reserve IDs or reorder
emission so the catalog, pages tree, page objects, content streams, and fonts
remain consistent, then add a parser-based regression test covering at least a
one-page export and validating all cross-references.
- Around line 35-36: Update the PDF stream generation around allLines and the
final document serialization to encode the stream once, use the resulting byte
array’s exact length for /Length, and write those same bytes without a later
Latin-1 conversion. Replace the current font/encoding choice with one that
preserves the supported Unicode input, including the bullet character, while
keeping text rendering unchanged.
---
Nitpick comments:
In `@__tests__/unit/ai-handlers.test.ts`:
- Around line 1-115: Move the test file from the top-level __tests__ location
into __tests__/lib/ai/handlers.test.ts so it mirrors the source module path
src/lib/ai/handlers.ts. Preserve all existing test contents and behavior.
In `@__tests__/unit/ai-json.test.ts`:
- Around line 1-2: Move the test file from the unit-level location to
__tests__/lib/ai/json.test.ts so it mirrors the source module imported by
extractJson. Preserve the existing test contents and behavior without other
changes.
In `@__tests__/unit/export-docx.test.ts`:
- Around line 1-22: Move the tests from __tests__/unit/export-docx.test.ts
(lines 1-22) to the __tests__/ path mirroring src/lib/export/docx.ts, preserving
the generateDocx test suite unchanged. Also move
__tests__/unit/export-pdf.test.ts (lines 1-64) to the corresponding mirrored
path for src/lib/export/pdf.ts, without changing its test behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a4e5ad0-3564-4fd0-8126-b18ee568ad4a
📒 Files selected for processing (25)
AGENTS.mdFIX-PLAN.mdREADME.mdREDESIGN-PLAN.mdREMEDIATION_PLAN.md__tests__/unit/ai-handlers.test.ts__tests__/unit/ai-json.test.ts__tests__/unit/export-docx.test.ts__tests__/unit/export-pdf.test.tssrc/app/api/ai/assessment-score/route.tssrc/app/api/ai/coach/route.tssrc/app/api/ai/cover-letter/route.tssrc/app/api/ai/resume-review/route.tssrc/app/api/export/route.tssrc/lib/ai/assessment.tssrc/lib/ai/client.tssrc/lib/ai/coach.tssrc/lib/ai/cover-letter.tssrc/lib/ai/handlers.tssrc/lib/ai/json.tssrc/lib/ai/resume.tssrc/lib/browser-llm-integration.tssrc/lib/export/docx.tssrc/lib/export/pdf.tssrc/lib/export/types.ts
💤 Files with no reviewable changes (1)
- src/lib/browser-llm-integration.ts
Comment on lines
+13
to
+26
| it('extracts JSON from prose', () => { | ||
| const text = 'Here is your result:\n{"score": 8, "ok": true}\nHope that helps.'; | ||
| expect(extractJson(text)).toEqual({ score: 8, ok: true }); | ||
| }); | ||
|
|
||
| it('extracts JSON wrapped in code fences', () => { | ||
| const text = '```json\n{"a":1}\n```'; | ||
| expect(extractJson(text)).toEqual({ a: 1 }); | ||
| }); | ||
|
|
||
| it('extracts an array embedded in text', () => { | ||
| const text = 'words ["x","y"] more words'; | ||
| expect(extractJson(text)).toEqual(['x', 'y']); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Cover responses containing multiple JSON segments.
The greedy object matcher in src/lib/ai/json.ts captures from the first { through the last }, so multiple JSON blocks produce null. Add a regression test to establish first-value extraction.
Proposed test
+ it('extracts the first JSON object when later JSON follows', () => {
+ const text = 'Result: {"score":8} Metadata: {"cached":false}';
+ expect(extractJson(text)).toEqual({ score: 8 });
+ });📝 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.
Suggested change
| it('extracts JSON from prose', () => { | |
| const text = 'Here is your result:\n{"score": 8, "ok": true}\nHope that helps.'; | |
| expect(extractJson(text)).toEqual({ score: 8, ok: true }); | |
| }); | |
| it('extracts JSON wrapped in code fences', () => { | |
| const text = '```json\n{"a":1}\n```'; | |
| expect(extractJson(text)).toEqual({ a: 1 }); | |
| }); | |
| it('extracts an array embedded in text', () => { | |
| const text = 'words ["x","y"] more words'; | |
| expect(extractJson(text)).toEqual(['x', 'y']); | |
| }); | |
| it('extracts JSON from prose', () => { | |
| const text = 'Here is your result:\n{"score": 8, "ok": true}\nHope that helps.'; | |
| expect(extractJson(text)).toEqual({ score: 8, ok: true }); | |
| }); | |
| it('extracts JSON wrapped in code fences', () => { | |
| const text = ' |
🤖 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__/unit/ai-json.test.ts` around lines 13 - 26, Add a regression test
in the extractJson test suite for text containing multiple JSON objects,
asserting that extractJson returns the first JSON value rather than null or
combining later segments. Use the existing extraction test style and preserve
current expectations for prose, fenced JSON, and embedded arrays.
| | **Animation** | Framer Motion v12 | Scroll reveals, stagger, hover physics | | ||
| | **Fonts** | Space Grotesk (headings) + Plus Jakarta Sans (body) | No Inter/Roboto | | ||
| | **AI** | Z AI Web Dev SDK | Coaching, scoring, content generation | | ||
| | **Export** | docx, PDF (pdfkit), Excel (exceljs) | Resource downloads | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== File inventory ==\n'
git ls-files AGENTS.md README.md FIX-PLAN.md src/lib/export/pdf.ts
printf '\n== Relevant snippets ==\n'
for f in AGENTS.md README.md FIX-PLAN.md src/lib/export/pdf.ts; do
if [[ -f "$f" ]]; then
echo "--- $f ---"
wc -l "$f"
case "$f" in
AGENTS.md) sed -n '1,80p' "$f" | cat -n ;;
README.md) sed -n '35,60p' "$f" | cat -n ;;
FIX-PLAN.md) sed -n '55,80p' "$f" | cat -n ;;
src/lib/export/pdf.ts) sed -n '1,260p' "$f" | cat -n ;;
esac
echo
fi
done
printf '\n== Search for pdfkit usage ==\n'
rg -n --hidden --glob '!**/.git/**' '\bpdfkit\b|manual builder|paginate|silent truncation|content size guard|Export' .
printf '\n== Search for export PDF implementation references ==\n'
rg -n --hidden --glob '!**/.git/**' 'src/lib/export/pdf\.ts|export/pdf|pdf\.ts' .Repository: projectamazonph/Interview-lab
Length of output: 20065
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Hello"Repository: projectamazonph/Interview-lab
Length of output: 173
Align the PDF export docs with the shipped implementation. AGENTS.md and README.md should describe the custom paginated PDF builder in src/lib/export/pdf.ts consistently; leave FIX-PLAN.md as future work if you still want a pdfkit migration plan.
📍 Affects 3 files
AGENTS.md#L27-L27(this comment)FIX-PLAN.md#L66-L68README.md#L47-L47
🤖 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 `@AGENTS.md` at line 27, Update AGENTS.md:27 and README.md:47 to document the
shipped custom paginated PDF builder in src/lib/export/pdf.ts instead of
describing pdfkit as the implementation; keep the PDF export descriptions
consistent across both files. Leave FIX-PLAN.md:66-68 unchanged as future-work
planning for a possible pdfkit migration.
| @@ -0,0 +1,133 @@ | |||
| # Interview Lab — Thorough Fix Plan | |||
|
|
|||
| > Audited 2026-07-19. A sequenced, merge-ready plan. Each phase: **goal → files → changes → verification**. | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the future-dated documentation claims.
Both documents claim work occurred on July 19, 2026, but the current date is July 18, 2026.
FIX-PLAN.md#L3-L3: correct or defer the July 19 audit date.REMEDIATION_PLAN.md#L115-L115: correct or defer the July 19 README-correction date.
📍 Affects 2 files
FIX-PLAN.md#L3-L3(this comment)REMEDIATION_PLAN.md#L115-L115
🤖 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 `@FIX-PLAN.md` at line 3, Remove or defer the future-dated July 19, 2026
claims: update the audit date in FIX-PLAN.md line 3 and the README-correction
date in REMEDIATION_PLAN.md line 115 to a current or non-future date, preserving
the surrounding plan content.
Comment on lines
+19
to
+24
| | File | Reason | | ||
| |------|--------| | ||
| | `src/lib/browser-llm-integration.ts` (338 lines) | Not imported anywhere; fake rule-based AI | | ||
| | `src/components/interview-lab/UpgradeModal.tsx` | Stub, no-op | | ||
| | `src/components/interview-lab/SubscriptionBanner.tsx` | Stub, no-op | | ||
| | `src/lib/pricing.ts` (tier config) — *optional* | Only powers stubs; keep if PricingPage ever real | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required blank line before the table.
The table needs a blank line after the preceding paragraph to satisfy Markdown linting.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 19-19: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 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 `@FIX-PLAN.md` around lines 19 - 24, Insert one blank line between the
preceding paragraph and the table in FIX-PLAN.md, leaving the table content
unchanged.
Source: Linters/SAST tools
Comment on lines
+40
to
+47
| | **Runtime** | Bun (CI uses npm) | | ||
| | **Database** | Prisma v6 + PostgreSQL (PostgreSQL only) | | ||
| | **Auth** | Custom JWT sessions (jose) + HttpOnly cookies | | ||
| | **UI** | Tailwind CSS v4 + custom glass design system | | ||
| | **UI** | Tailwind CSS v4 + "Field Manual" light design system | | ||
| | **Icons** | Phosphor Icons (light weight) | | ||
| | **Fonts** | Space Grotesk (headings) + Plus Jakarta Sans (body) | | ||
| | **AI** | Z AI Web Dev SDK for coaching, scoring, content generation | | ||
| | **Export** | docx, PDF (pdfkit), Excel (exceljs) | | ||
| | **Export** | docx, PDF (manual builder / pdfkit), Excel (exceljs) | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files | rg '^(README\.md|AGENTS\.md|REMEDIATION_PLAN\.md|remediation_plan\.md|package\.json|bun\.lockb|bun\.lock|package-lock\.json|pnpm-lock\.yaml|yarn.lock|\.github/workflows/.*)$'
printf '\n== README snippets ==\n'
sed -n '1,220p' README.md
printf '\n== AGENTS.md ==\n'
if [ -f AGENTS.md ]; then sed -n '1,220p' AGENTS.md; fi
printf '\n== REMEDIATION_PLAN.md ==\n'
if [ -f REMEDIATION_PLAN.md ]; then sed -n '1,220p' REMEDIATION_PLAN.md; fi
printf '\n== package.json ==\n'
if [ -f package.json ]; then sed -n '1,240p' package.json; fi
printf '\n== ci/workflow refs to bun/npm ==\n'
rg -n '\b(bun|npm|pnpm|yarn)\b' .github/workflows package.json README.md AGENTS.md REMEDIATION_PLAN.md 2>/dev/null || trueRepository: projectamazonph/Interview-lab
Length of output: 25475
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== .github/workflows/ci.yml ==\n'
sed -n '1,220p' .github/workflows/ci.yml
printf '\n== package-lock root metadata ==\n'
python3 - <<'PY'
import json
from pathlib import Path
p = Path("package-lock.json")
data = json.loads(p.read_text())
print(data.get("name"))
print(data.get("lockfileVersion"))
print("packages-root:", data.get("packages", {}).get("", {}).keys())
PYRepository: projectamazonph/Interview-lab
Length of output: 2896
Align the package-manager guidance with CI
README.md (and AGENTS.md) still point contributors to Bun, but CI runs npm ci/npm run ... and the repo only has package-lock.json. Pick one package manager as the source of truth and update the docs and lockfile to match.
🤖 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 `@README.md` around lines 40 - 47, Align the package-manager guidance in
README.md and AGENTS.md with the repository’s npm-based CI by making npm the
documented source of truth. Update any contributor commands that reference Bun
and ensure the committed lockfile and documentation consistently use npm without
introducing a competing package-manager lockfile.
Comment on lines
+16
to
+29
| const objectMatch = trimmed.match(/\{[\s\S]*\}/); | ||
| if (objectMatch) { | ||
| try { | ||
| return JSON.parse(objectMatch[0]) as T; | ||
| } catch { | ||
| // malformed; fall through | ||
| } | ||
| } | ||
|
|
||
| const arrayMatch = trimmed.match(/\[[\s\S]*\]/); | ||
| if (arrayMatch) { | ||
| try { | ||
| return JSON.parse(arrayMatch[0]) as T; | ||
| } catch { |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Extract one balanced JSON value instead of greedy delimiters.
At Line 16 and Line 25, the regex consumes from the first opening delimiter through the last closing delimiter. A response containing {"a":1} followed later by another brace-delimited fragment becomes unparsable, despite containing a valid first payload. Scan balanced delimiters while respecting quoted strings, and add regression cases for multiple and nested payloads.
🤖 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 `@src/lib/ai/json.ts` around lines 16 - 29, The JSON extraction logic in the
parsing function should replace greedy objectMatch and arrayMatch regexes with
balanced-delimiter scanning that respects quoted strings and escaped characters.
Return the first valid balanced object or array payload, including nested
structures, and add regression coverage for multiple adjacent/following payloads
and nested payloads.
Comment on lines
+42
to
+51
| validate: (body) => { | ||
| const shape = validateShape(body, ['resumeText']); | ||
| if (!shape.ok) { | ||
| return { ok: false, status: 400, error: 'Resume text is required' }; | ||
| } | ||
| const resumeText = (body as ResumeReviewBody).resumeText; | ||
| if (typeof resumeText === 'string' && resumeText.length > 20000) { | ||
| return { ok: false, status: 400, error: 'Resume is too long' }; | ||
| } | ||
| return { ok: true, value: body as ResumeReviewBody }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Validate required request values as non-empty strings. validateShape checks only key presence, then each config casts unknown; JSON values such as { "resumeText": 7 } reach the model instead of receiving a 400.
src/lib/ai/resume.ts#L42-L51: require trimmed stringresumeTextbefore applying the length limit.src/lib/ai/coach.ts#L98-L103: require trimmed stringquestionanduserAnswer.src/lib/ai/cover-letter.ts#L47-L56: require trimmed stringjobDescriptionbefore applying the length limit.
📍 Affects 3 files
src/lib/ai/resume.ts#L42-L51(this comment)src/lib/ai/coach.ts#L98-L103src/lib/ai/cover-letter.ts#L47-L56
🤖 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 `@src/lib/ai/resume.ts` around lines 42 - 51, Update the validators in
src/lib/ai/resume.ts lines 42-51, src/lib/ai/coach.ts lines 98-103, and
src/lib/ai/cover-letter.ts lines 47-56 to require each request field to be a
trimmed, non-empty string before casting or applying length checks: resumeText,
question, userAnswer, and jobDescription respectively. Return the existing 400
validation response for invalid values, and preserve the current length-limit
behavior for valid strings.
Comment on lines
+23
to
+39
| for (const line of lines) { | ||
| const trimmed = line.trim(); | ||
| if (!trimmed) { | ||
| allLines.push({ text: '', size: 4, bold: false, indent: 0, color: '0 0 0' }); | ||
| continue; | ||
| } | ||
| if (trimmed.startsWith('### ')) { | ||
| allLines.push({ text: trimmed.replace('### ', ''), size: 12, bold: true, indent: 0, color: '0 0 0' }); | ||
| } else if (trimmed.startsWith('## ')) { | ||
| allLines.push({ text: trimmed.replace('## ', ''), size: 14, bold: true, indent: 0, color: '0 0 0' }); | ||
| } else if (trimmed.startsWith('# ')) { | ||
| allLines.push({ text: trimmed.replace('# ', ''), size: 16, bold: true, indent: 0, color: '0 0 0' }); | ||
| } else if (trimmed.startsWith('- ') || trimmed.startsWith('• ')) { | ||
| allLines.push({ text: `• ${trimmed.replace(/^[-•]\s*/, '')}`, size: 11, bold: false, indent: 20, color: '0 0 0' }); | ||
| } else { | ||
| allLines.push({ text: trimmed, size: 11, bold: false, indent: 0, color: '0 0 0' }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Wrap lines before paginating them.
Each input line becomes one Tj operation, so long lines extend beyond the right page boundary and remain visually truncated. Split each styled line according to Courier width and available horizontal space before applying vertical pagination.
Also applies to: 85-93
🤖 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 `@src/lib/export/pdf.ts` around lines 23 - 39, Update the line-processing logic
in the PDF export routine to wrap every styled text line to the available page
width using Courier character measurements before vertical pagination and Tj
generation. Preserve each line’s existing text styling, indentation, and
blank-line handling across wrapped segments, including the corresponding logic
around the additional pagination range.
Comment on lines
+35
to
+36
| } else if (trimmed.startsWith('- ') || trimmed.startsWith('• ')) { | ||
| allLines.push({ text: `• ${trimmed.replace(/^[-•]\s*/, '')}`, size: 11, bold: false, indent: 20, color: '0 0 0' }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Use consistent PDF text encoding and stream lengths.
Buffer.byteLength(stream) counts UTF-8 bytes, but the final document is serialized as Latin-1. The generated • alone is counted as three bytes but written as one, producing an incorrect /Length; other Unicode characters are also corrupted. Encode stream bytes once, use their exact length, and use a font/encoding capable of preserving supported input.
Also applies to: 91-94, 110-111, 129-129
🤖 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 `@src/lib/export/pdf.ts` around lines 35 - 36, Update the PDF stream generation
around allLines and the final document serialization to encode the stream once,
use the resulting byte array’s exact length for /Length, and write those same
bytes without a later Latin-1 conversion. Replace the current font/encoding
choice with one that preserves the supported Unicode input, including the bullet
character, while keeping text rendering unchanged.
Comment on lines
+82
to
+111
| addObject('<< /Type /Catalog /Pages 2 0 R >>'); | ||
| const contentObjIds: number[] = []; | ||
|
|
||
| for (const pageLines of pages) { | ||
| let stream = ''; | ||
| let py = MARGIN_TOP; | ||
| for (const item of pageLines) { | ||
| const fontName = item.bold ? '/F2' : '/F1'; | ||
| const x = MARGIN_LEFT + item.indent; | ||
| stream += `BT\n${fontName} ${item.size} Tf\n${item.color} rg\n${x} ${py} Td\n(${escapePdfText(item.text)}) Tj\nET\n`; | ||
| py -= item.size + 4; | ||
| } | ||
| const streamObjId = addObject(`<< /Length ${Buffer.byteLength(stream)} >>\nstream\n${stream}\nendstream`); | ||
| contentObjIds.push(streamObjId); | ||
| } | ||
|
|
||
| // Pages object (object 2) references all page objects. Page objects are laid out | ||
| // immediately after this one at positions 3, 5, 7, ... (each page gets one object), | ||
| // matching the content-stream object ids produced above. | ||
| const pagesKids = pages.map((_, i) => `${3 + i * 2} 0 R`).join(' '); | ||
| addObject(`<< /Type /Pages /Kids [${pagesKids}] /Count ${pages.length} >>`); | ||
|
|
||
| for (let i = 0; i < pages.length; i++) { | ||
| addObject( | ||
| `<< /Type /Page /Parent 2 0 R /MediaBox [0 0 ${PAGE_WIDTH} ${PAGE_HEIGHT}] /Contents ${contentObjIds[i]} 0 R /Resources << /Font << /F1 5 0 R /F2 6 0 R >> >> >>`, | ||
| ); | ||
| } | ||
|
|
||
| addObject('<< /Type /Font /Subtype /Type1 /BaseFont /Courier /Encoding /WinAnsiEncoding >>'); | ||
| addObject('<< /Type /Font /Subtype /Type1 /BaseFont /Courier-Bold /Encoding /WinAnsiEncoding >>'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | 🏗️ Heavy lift
Build object references from their actual IDs.
Content streams consume object IDs starting at 2, but the catalog still points /Pages to object 2, while page and font references are also hard-coded. Even a one-page export therefore has invalid cross-references.
Reserve IDs up front or emit objects in this order: catalog, pages tree, page objects, content streams, fonts. Add a parser-based regression test after correcting the layout.
🤖 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 `@src/lib/export/pdf.ts` around lines 82 - 111, Correct object allocation in
the PDF export flow around addObject so every /Pages, /Page, /Contents, and font
reference uses the actual assigned object IDs rather than hard-coded values.
Reserve IDs or reorder emission so the catalog, pages tree, page objects,
content streams, and fonts remain consistent, then add a parser-based regression
test covering at least a one-page export and validating all cross-references.
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.
Summary
Phase 2 of the SOLID remediation roadmap (FIX-PLAN.md): refactor the export layer. The
export/route.tswas a 200-line monolith that owned auth, DOCX generation, and a hand-rolled PDF builder that silently truncated content at the bottom of the page (if (y < 50) break— data loss on long resumes/letters).What changed
ExportRequest,MAX_CONTENT_LENGTH(50k), filename sanitizer.Kept the manual Courier PDF approach (no external font files) to avoid the
pdfkitfont-resolution issues the original code warned about — so no new dependency was added (per AGENTS.md guardrails). Thepdfkitdependency remains present but still unused; removing it is a separate cleanup.Tests / verification
export-pdf.test.ts: pagination + no-truncation + escaping;export-docx.test.ts: valid DOCX zip output;typeshelpers) — all passing.tsc --noEmit: clean.eslint: clean.🤖 Generated with opencode
Summary by CodeRabbit
New Features
Documentation
Bug Fixes