Phase 1: SOLID refactor of AI layer#7
Merged
Conversation
added 2 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 latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR centralizes four AI API routes around shared server-side provider and handler abstractions, adds feature-specific prompts and validation, introduces JSON parsing and timeout support, adds unit tests, removes the browser AI module, and aligns project documentation with the PostgreSQL and Field Manual setup. ChangesAI request handling
Documentation alignment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant AIRoute
participant createAIHandler
participant ZAIProvider
participant FeatureConfig
Client->>AIRoute: POST AI request
AIRoute->>createAIHandler: invoke with FeatureConfig
createAIHandler->>FeatureConfig: validate body and build prompt
createAIHandler->>ZAIProvider: request timed completion
ZAIProvider-->>createAIHandler: model text
createAIHandler-->>Client: parsed result or standardized 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 (4)
__tests__/unit/ai-handlers.test.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this test to the mirrored source path.
Place it under
__tests__/unit/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; place 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` at line 1, Move the test file from the top-level __tests__/unit location to __tests__/unit/lib/ai/handlers.test.ts, mirroring the source module path src/lib/ai/handlers.ts. Preserve the existing Vitest test content and behavior without other changes.Source: Coding guidelines
src/lib/ai/client.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the required
@/alias consistently across the AI layer.
src/lib/ai/client.ts#L1-L1: importjsonthrough@/lib/ai/json.src/lib/ai/handlers.ts#L3-L3: importclientthrough@/lib/ai/client.src/lib/ai/assessment.ts#L1-L1: importhandlersthrough@/lib/ai/handlers.src/lib/ai/coach.ts#L1-L1: importhandlersthrough@/lib/ai/handlers.src/lib/ai/cover-letter.ts#L1-L1: importhandlersthrough@/lib/ai/handlers.As per coding guidelines, “Use the
@/path alias for imports mapped tosrc/.”🤖 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/client.ts` at line 1, Replace the relative imports with the required `@/` alias throughout the AI layer: update src/lib/ai/client.ts lines 1-1 to import json from `@/lib/ai/json`, src/lib/ai/handlers.ts lines 3-3 to import client from `@/lib/ai/client`, src/lib/ai/assessment.ts lines 1-1 and src/lib/ai/coach.ts lines 1-1 to import handlers from `@/lib/ai/handlers`, and src/lib/ai/cover-letter.ts lines 1-1 to import handlers from `@/lib/ai/handlers`.Source: Coding guidelines
__tests__/unit/ai-json.test.ts (1)
1-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove this test to mirror the source structure.
This should live at
__tests__/lib/ai/json.test.tsso its path mirrorssrc/lib/ai/json.ts.As per coding guidelines, “Use Vitest for unit, API, component, and stress tests; place 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-json.test.ts` around lines 1 - 40, Move the extractJson unit test suite from the top-level __tests__/unit location to __tests__/lib/ai/json.test.ts so it mirrors src/lib/ai/json.ts. Preserve the existing Vitest tests and assertions unchanged.Source: Coding guidelines
src/lib/ai/resume.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the configured path alias.
-import { validateShape, type AIHandlerConfig } from './handlers'; +import { validateShape, type AIHandlerConfig } from '`@/lib/ai/handlers`';As per coding guidelines, “Use the
@/path alias for imports mapped tosrc/.”🤖 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` at line 1, Update the import in resume.ts to use the configured `@/` path alias for the handlers module instead of the relative path, while preserving the existing validateShape and AIHandlerConfig imports.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
malformed brace text before valid JSON, such as a template placeholder followed
by an object, and expect the valid object. Update extractJson’s object-candidate
scanning to try independently valid JSON candidates from successive opening
braces rather than using one greedy span, while preserving existing prose,
code-fence, and array extraction behavior.
In `@FIX-PLAN.md`:
- Line 3: Update the audit date in FIX-PLAN.md to July 18, 2026, or explicitly
mark the document as a future plan instead of claiming it was audited on July
19, 2026.
- Around line 18-26: Update FIX-PLAN.md to reflect the shipped architecture:
mark Phase 0 and the Phase 1 AI refactor as completed, referencing
src/lib/ai/client.ts, ZAIProvider, createAIHandler, feature configuration
modules, and __tests__/unit/. Remove or replace guidance for
src/lib/server-ai.ts, OpenAI/Anthropic, outdated test paths, and presenting
Phase 0 as a future first PR; ensure the remaining plan does not duplicate
implemented work or conflict with the current architecture.
In `@README.md`:
- Line 40: Update the Runtime entry in the README to state that CI uses Bun,
keeping it consistent with the repository policy in AGENTS.md and its
prohibition on npm lockfiles.
- Line 93: Remove the subscription/ entry entirely from the project tree
documentation in README.md, including its “Removed (product is free)”
annotation, while leaving the surrounding directory structure unchanged.
- Line 153: Synchronize the README test-count references and coverage table:
update the API suite count in the section around the test list to 187, remove
the obsolete subscription.test.ts entry, and ensure the total row remains
consistent at 187.
In `@REMEDIATION_PLAN.md`:
- Line 115: Update the P2.6 status paragraph in REMEDIATION_PLAN.md to use a
non-future-dated, accurate README correction date or status, and remove the
obsolete instruction to add standalone output since next.config.ts already
configures it. Ensure the surrounding P2.6 wording is internally consistent.
- Around line 90-91: The download route is incorrectly documented as 53 lines in
both plans. Update REMEDIATION_PLAN.md lines 90-91 and FIX-PLAN.md line 15 to
reference src/app/api/downloads/[id]/route.ts as 879 lines, preserving the
surrounding plan content.
In `@src/lib/ai/assessment.ts`:
- Around line 39-52: Strengthen validation at all three sites: in
src/lib/ai/assessment.ts lines 39-52, require assessmentTitle to be a non-empty
string and userAnswers to match its declared supported type before applying the
existing size limit; in src/lib/ai/coach.ts lines 98-107, require bounded,
non-empty question and answer strings; and in src/lib/ai/cover-letter.ts lines
47-56, require a non-empty string before checking its length. Preserve the
existing 400 validation-error responses and downstream typed request handling.
- Around line 45-55: Update buildUserPrompt to serialize structured userAnswers
with JSON.stringify before interpolation, while preserving readable handling for
string answers. Ensure objects and arrays retain their actual answer data
instead of becoming “[object Object]”.
In `@src/lib/ai/client.ts`:
- Around line 27-48: Wire request cancellation through the AI completion flow:
update completeJson usage in src/lib/ai/handlers.ts:75-79 to pass
request.signal, and update the wrapper around withTimeout in
src/lib/ai/client.ts:27-48 to settle when the AbortController is aborted as well
as when timeoutMs expires, ensuring the SDK call stops for disconnected
requests.
In `@src/lib/ai/coach.ts`:
- Around line 19-34: Update the coaching response contract to include a
truthfulnessWarning field in the model schema, the CoachResult type, and both
fallback objects. Ensure every success and failure path returns this field with
an appropriate warning value, including the logic covering the additional
referenced range.
In `@src/lib/ai/handlers.ts`:
- Around line 81-90: Update the handler’s result-processing flow around
completeJson and config.normalize to require and invoke a runtime validateResult
contract after parsing and before normalization. Reject invalid model output
through the existing parse-failure/error response path, and only call
config.normalize or return HTTP 200 when validation succeeds.
In `@src/lib/ai/json.ts`:
- Around line 16-28: Replace the greedy objectMatch and arrayMatch extraction in
the JSON parsing utility with a string- and escape-aware balanced scanner that
returns the first complete JSON object or array encountered in input order.
Ensure braces and brackets inside quoted strings do not affect nesting, and
preserve parsing/fallback behavior for malformed candidates. Add tests covering
multiple JSON values and delimiters inside strings.
In `@src/lib/ai/resume.ts`:
- Around line 40-56: Add a normalize validator to resumeReviewConfig that
runtime-validates ResumeReviewResult output before returning it: require every
contract string field and string-array field, and ensure the score is within its
defined range. Throw an error for malformed payloads such as empty objects,
while preserving valid parsed results.
- Around line 7-16: Remove the trailing comma after the truthWarnings field in
the JSON format defined by the resume response instructions, while preserving
all fields and their existing order.
- Around line 42-52: Strengthen the validate callback in the resume review flow
to require a non-empty string resumeText and, when provided, a string targetRole
before returning ResumeReviewBody. Reject null, numbers, objects, and empty or
whitespace-only values with the existing 400 validation response, while
preserving the length limit and successful return for valid input.
---
Nitpick comments:
In `@__tests__/unit/ai-handlers.test.ts`:
- Line 1: Move the test file from the top-level __tests__/unit location to
__tests__/unit/lib/ai/handlers.test.ts, mirroring the source module path
src/lib/ai/handlers.ts. Preserve the existing Vitest test content and behavior
without other changes.
In `@__tests__/unit/ai-json.test.ts`:
- Around line 1-40: Move the extractJson unit test suite from the top-level
__tests__/unit location to __tests__/lib/ai/json.test.ts so it mirrors
src/lib/ai/json.ts. Preserve the existing Vitest tests and assertions unchanged.
In `@src/lib/ai/client.ts`:
- Line 1: Replace the relative imports with the required `@/` alias throughout the
AI layer: update src/lib/ai/client.ts lines 1-1 to import json from
`@/lib/ai/json`, src/lib/ai/handlers.ts lines 3-3 to import client from
`@/lib/ai/client`, src/lib/ai/assessment.ts lines 1-1 and src/lib/ai/coach.ts
lines 1-1 to import handlers from `@/lib/ai/handlers`, and
src/lib/ai/cover-letter.ts lines 1-1 to import handlers from `@/lib/ai/handlers`.
In `@src/lib/ai/resume.ts`:
- Line 1: Update the import in resume.ts to use the configured `@/` path alias for
the handlers module instead of the relative path, while preserving the existing
validateShape and AIHandlerConfig imports.
🪄 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: 7611af80-e665-4f82-8962-a2099d5b580b
📒 Files selected for processing (19)
AGENTS.mdFIX-PLAN.mdREADME.mdREDESIGN-PLAN.mdREMEDIATION_PLAN.md__tests__/unit/ai-handlers.test.ts__tests__/unit/ai-json.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/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.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 malformed brace text preceding valid JSON.
The current greedy object match consumes from the first { through the final }, so a response such as Template: {field}\n{"score":8} incorrectly returns null. Add this regression test and update the parser to scan for the first independently valid JSON candidate.
+ it('skips malformed brace text before valid JSON', () => {
+ const text = 'Template: {field}\n{"score":8}';
+ expect(extractJson(text)).toEqual({ score: 8 });
+ });
+🤖 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 malformed brace text before valid JSON, such
as a template placeholder followed by an object, and expect the valid object.
Update extractJson’s object-candidate scanning to try independently valid JSON
candidates from successive opening braces rather than using one greedy span,
while preserving existing prose, code-fence, and array extraction behavior.
| @@ -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
Correct the audit date.
This file says it was audited on July 19, 2026, but the current date is July 18, 2026. Use the actual audit date or mark this as a future plan.
🤖 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, Update the audit date in FIX-PLAN.md to July 18,
2026, or explicitly mark the document as a future plan instead of claiming it
was audited on July 19, 2026.
Comment on lines
+18
to
+26
| ### 0.2 Delete dead code | ||
| | 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 | | ||
|
|
||
| **Verify:** `grep -r "browser-llm-integration\|UpgradeModal\|SubscriptionBanner" src` → no results; `bun run build` passes. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the fix plan to match the shipped architecture.
The PR objectives state that Phase 0 and the Phase 1 AI refactor are implemented using src/lib/ai/client.ts, ZAIProvider, createAIHandler, feature configuration modules, and tests under __tests__/unit/. This plan still treats those phases as pending, prescribes src/lib/server-ai.ts with OpenAI/Anthropic, points to different test paths, and calls Phase 0 the suggested first PR. Mark completed work as completed or rewrite the plan around the actual architecture to prevent duplicate or conflicting implementation.
Also applies to: 30-58, 132-133
🧰 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 18 - 26, Update FIX-PLAN.md to reflect the shipped
architecture: mark Phase 0 and the Phase 1 AI refactor as completed, referencing
src/lib/ai/client.ts, ZAIProvider, createAIHandler, feature configuration
modules, and __tests__/unit/. Remove or replace guidance for
src/lib/server-ai.ts, OpenAI/Anthropic, outdated test paths, and presenting
Phase 0 as a future first PR; ensure the remaining plan does not duplicate
implemented work or conflict with the current architecture.
| | **Framework** | Next.js 16.1.1 (App Router, standalone) | | ||
| | **Runtime** | Bun | | ||
| | **Database** | Prisma v6 + SQLite (dev) / PostgreSQL (prod) | | ||
| | **Runtime** | Bun (CI uses npm) | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep package-manager guidance consistent with the repository policy.
This says CI uses npm, while AGENTS.md Line 71 requires Bun exclusively and disallows npm lockfiles. Either document Bun in CI or explicitly revise the repository-wide policy.
🤖 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` at line 40, Update the Runtime entry in the README to state that
CI uses Bun, keeping it consistent with the repository policy in AGENTS.md and
its prohibition on npm lockfiles.
Source: Coding guidelines
| │ │ ├── profile/ # User profile (GET/PUT) | ||
| │ │ ├── dashboard/ # Dashboard aggregation | ||
| │ │ ├── subscription/ # Checkout, status, usage, webhook | ||
| │ │ ├── subscription/ # Removed (product is free) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the retired directory from the project tree.
The subscription/ entry is still listed with a “Removed” annotation. Omit it entirely so readers do not look for a route that no longer exists.
🤖 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` at line 93, Remove the subscription/ entry entirely from the
project tree documentation in README.md, including its “Removed (product is
free)” annotation, while leaving the surrounding directory structure unchanged.
Comment on lines
+81
to
+90
| if (!parsed) { | ||
| const failure = config.onParseFailure(body); | ||
| if (!failure.ok) { | ||
| return NextResponse.json({ error: failure.error }, { status: failure.status }); | ||
| } | ||
| return NextResponse.json(failure.value); | ||
| } | ||
|
|
||
| const result = config.normalize ? config.normalize(parsed, body) : parsed; | ||
| return NextResponse.json(result); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Validate model output before returning HTTP 200.
completeJson<TResult> only casts parsed JSON; it does not prove the configured result shape. Assessment can therefore return arbitrary JSON, while coach normalization can operate on malformed values. Add a required runtime validateResult contract before normalization.
🤖 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/handlers.ts` around lines 81 - 90, Update the handler’s
result-processing flow around completeJson and config.normalize to require and
invoke a runtime validateResult contract after parsing and before normalization.
Reject invalid model output through the existing parse-failure/error response
path, and only call config.normalize or return HTTP 200 when validation
succeeds.
Comment on lines
+16
to
+28
| 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; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Extract the first balanced JSON value instead of using greedy regexes.
For prefix {"a":1} suffix {"b":2}, the object regex captures both objects and parsing returns null. It also selects a later object before an earlier array. Use a string/escape-aware balanced scanner and add tests for multiple values and braces inside strings.
🤖 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 - 28, Replace the greedy objectMatch and
arrayMatch extraction in the JSON parsing utility with a string- and
escape-aware balanced scanner that returns the first complete JSON object or
array encountered in input order. Ensure braces and brackets inside quoted
strings do not affect nesting, and preserve parsing/fallback behavior for
malformed candidates. Add tests covering multiple JSON values and delimiters
inside strings.
Comment on lines
+7
to
+16
| Respond in the following JSON format only: | ||
| { | ||
| "score": <number 0-100>, | ||
| "missingKeywords": ["<keyword 1>", "<keyword 2>"], | ||
| "weakSections": ["<section 1>", "<section 2>"], | ||
| "improvedSummary": "<rewritten professional summary>", | ||
| "improvedBullets": ["<bullet 1>", "<bullet 2>"], | ||
| "skillsRecommendations": ["<skill 1>", "<skill 2>"], | ||
| "truthWarnings": ["<warning if user fabricated experience>"], | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the trailing comma from the requested JSON format.
Line 15 makes the example invalid JSON, increasing the likelihood of parse failures.
- "truthWarnings": ["<warning if user fabricated experience>"],
+ "truthWarnings": ["<warning if user fabricated experience>"]📝 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
| Respond in the following JSON format only: | |
| { | |
| "score": <number 0-100>, | |
| "missingKeywords": ["<keyword 1>", "<keyword 2>"], | |
| "weakSections": ["<section 1>", "<section 2>"], | |
| "improvedSummary": "<rewritten professional summary>", | |
| "improvedBullets": ["<bullet 1>", "<bullet 2>"], | |
| "skillsRecommendations": ["<skill 1>", "<skill 2>"], | |
| "truthWarnings": ["<warning if user fabricated experience>"], | |
| } | |
| Respond in the following JSON format only: | |
| { | |
| "score": <number 0-100>, | |
| "missingKeywords": ["<keyword 1>", "<keyword 2>"], | |
| "weakSections": ["<section 1>", "<section 2>"], | |
| "improvedSummary": "<rewritten professional summary>", | |
| "improvedBullets": ["<bullet 1>", "<bullet 2>"], | |
| "skillsRecommendations": ["<skill 1>", "<skill 2>"], | |
| "truthWarnings": ["<warning if user fabricated experience>"] | |
| } |
🤖 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 7 - 16, Remove the trailing comma after
the truthWarnings field in the JSON format defined by the resume response
instructions, while preserving all fields and their existing order.
Comment on lines
+40
to
+56
| export const resumeReviewConfig: AIHandlerConfig<ResumeReviewBody, ResumeReviewResult> = { | ||
| systemPrompt: RESUME_REVIEW_PROMPT, | ||
| 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 }; | ||
| }, | ||
| buildUserPrompt: (body) => | ||
| `Target Role: ${body.targetRole || 'Amazon VA'}\n\nResume Text:\n${body.resumeText}`, | ||
| onParseFailure: () => ({ ok: false, status: 500, error: 'Failed to parse resume review' }), | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Runtime-validate parsed model output.
The ResumeReviewResult generic is erased at runtime. Because extractJson accepts any valid JSON and this config has no normalization guard, malformed objects such as {} are returned with HTTP 200 despite violating the route contract.
Add a normalize validator for the score range and every required string/string-array field, throwing when the payload is invalid.
🤖 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 40 - 56, Add a normalize validator to
resumeReviewConfig that runtime-validates ResumeReviewResult output before
returning it: require every contract string field and string-array field, and
ensure the score is within its defined range. Throw an error for malformed
payloads such as empty objects, while preserving valid parsed results.
Comment on lines
+42
to
+52
| 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 | 🟠 Major | ⚡ Quick win
Validate actual field types before returning ResumeReviewBody.
validateShape only rejects undefined, so null, numbers, objects, and an empty resumeText currently reach the model. targetRole is also accepted with any runtime type.
Proposed validation
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) {
+ const resumeText = shape.data.resumeText;
+ const targetRole = shape.data.targetRole;
+ if (typeof resumeText !== 'string' || resumeText.trim() === '') {
+ return { ok: false, status: 400, error: 'Resume text is required' };
+ }
+ if (resumeText.length > 20000) {
return { ok: false, status: 400, error: 'Resume is too long' };
}
- return { ok: true, value: body as ResumeReviewBody };
+ if (targetRole !== undefined && typeof targetRole !== 'string') {
+ return { ok: false, status: 400, error: 'Target role must be a string' };
+ }
+ return { ok: true, value: { resumeText, targetRole } };
},📝 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
| 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 }; | |
| }, | |
| validate: (body) => { | |
| const shape = validateShape(body, ['resumeText']); | |
| if (!shape.ok) { | |
| return { ok: false, status: 400, error: 'Resume text is required' }; | |
| } | |
| const resumeText = shape.data.resumeText; | |
| const targetRole = shape.data.targetRole; | |
| if (typeof resumeText !== 'string' || resumeText.trim() === '') { | |
| return { ok: false, status: 400, error: 'Resume text is required' }; | |
| } | |
| if (resumeText.length > 20000) { | |
| return { ok: false, status: 400, error: 'Resume is too long' }; | |
| } | |
| if (targetRole !== undefined && typeof targetRole !== 'string') { | |
| return { ok: false, status: 400, error: 'Target role must be a string' }; | |
| } | |
| return { ok: true, value: { resumeText, targetRole } }; | |
| }, |
🤖 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 - 52, Strengthen the validate callback
in the resume review flow to require a non-empty string resumeText and, when
provided, a string targetRole before returning ResumeReviewBody. Reject null,
numbers, objects, and empty or whitespace-only values with the existing 400
validation response, while preserving the length limit and successful return for
valid input.
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 1 of the SOLID remediation roadmap (FIX-PLAN.md): refactor the AI layer. The 4 AI route handlers duplicated ~75 lines of auth + JSON-extract + error-handling each and called the SDK directly.
What changed
AIProviderinterface +ZAIProviderwith a 30s timeout and abort support. The SDK is now swappable behind an interface (DIP).createAIHandlerfactory centralizes auth, input validation, model call, parsing, and error handling (OCP: add a feature by adding a config, not copying a route). Includes a tinyvalidateShapehelper instead of adding zod.coach,resume-review,cover-letter,assessment-score) are now single-lineexport const POST = createAIHandler(config).Behavior preserved exactly
coachandcover-letterreturn graceful fallbacks (200) on parse failure;resume-reviewandassessment-scorereturn 500.Tests / verification
__tests__/unit/ai-json.test.ts,ai-handlers.test.ts) — all passing.tsc --noEmit: clean.eslint: clean.Note: the integration tests in CI for
/api/ai/cover-letterand/api/ai/resume-reviewstill 500 because CI has noZAI_API_KEY— this pre-existing failure also exists onmainand is unrelated to this refactor.🤖 Generated with opencode
Summary by CodeRabbit
Improvements
Documentation
Tests