PDF Processor - #46
Conversation
Separate OpenAI-compatible LLM access behind an interface so the active provider can be switched via LLM_PROVIDER while defaulting to Letsur.
Extract resource selection, resource content fetching, and SSE transport so ChatOrchestrationService only coordinates the turn flow.
Add unit tests for provider selection, resource selection/content, SSE transport, and LLM_PROVIDER env validation.
refactor: split LLM providers and chat orchestration
Introduce ready-state document catalog and processing_token fields so async PDF jobs can be claimed, cancelled, and recovered safely. Co-authored-by: Cursor <cursoragent@cursor.com>
Support base64 service-account credentials and processor tuning knobs for Lightsail deployments outside GCP ADC. Co-authored-by: Cursor <cursoragent@cursor.com>
Pass timeoutMs through OpenAI-compatible clients so page conversion and chunking can exceed the default chat timeout. Co-authored-by: Cursor <cursoragent@cursor.com>
Port processor prompts with escaped markdown so Nest can format Pass 1/2 LLM requests safely. Co-authored-by: Cursor <cursoragent@cursor.com>
Add text-only Pass1/2 pipeline, GCS storage, DB-backed queue with concurrency 1, and atomic processing ownership for cancel/reprocess. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace resource-center proxy with GCS reservation, status APIs, and reprocess while keeping the existing admin upload endpoint. Co-authored-by: Cursor <cursoragent@cursor.com>
…zation and overview preservation
… add ApiConsumes for file uploads
…ling and enhanced error handling
…r for improved validation and defaults
…xt extraction process
…upload and delete operations
…s and related logic
PDF 문서 처리 파이프라인 및 DB 기반 Retrieval 도입
📝 WalkthroughWalkthroughPDF 문서의 조직 기반 관리, GCS 저장, 비동기 처리, 청크 검색, LLM provider 추상화와 SSE 채팅 전송을 추가했습니다. 기존 MCP·OpenRouter 직접 의존성은 retrieval 및 공통 LLM 서비스로 전환했습니다. Changes문서 플랫폼
채팅 플랫폼
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/upload/upload.controller.ts (1)
180-197: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win사용하지 않는 파일 파트를 드레인하거나 400으로 거부하세요.
parts()에서fieldname !== 'file'인 파일 파트를 건너뛰면 미소비 파일 스트림 뒤의 필드를 읽히지 못하고 업로드 처리가 멈출 수 있습니다. 파일 파트는toBuffer()로 소비하거나, 허용되지 않는 이름일 경우 요청을 400으로 중단하세요.🤖 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/upload/upload.controller.ts` around lines 180 - 197, Update the multipart loop around parts() so every file part is either consumed via toBuffer() or streamToBuffer(), or immediately rejected with a 400 response when its fieldname is not "file". Do not silently skip unsupported file parts; preserve the existing handling for the accepted "file" part and fields.
🟡 Minor comments (14)
src/config/env.validation.ts-113-136 (1)
113-136: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win조건부 필드의 타입을 실제 반환값과 맞춰주세요.
ENVIRONMENT_VARIABLES에서LLM_PROVIDER=openrouter일 때LETSUR_AI_GATEWAY_BASE_URL,LETSUR_AI_GATEWAY_API_KEY검증은 건너뜁니다. 이때ConfigService.get<string>('LETSUR_AI_GATEWAY_...')은undefined를 반환할 수 있으므로,type과apiKey를 모두string이라고 가정하는BaseOpenAiCompatibleLlm설정 타입은 보정해야 합니다.🤖 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/config/env.validation.ts` around lines 113 - 136, Update the conditional LETSUR_AI_GATEWAY_BASE_URL and LETSUR_AI_GATEWAY_API_KEY declarations in EnvironmentVariables to reflect that validation skips them for the OpenRouter provider and ConfigService.get may return undefined. Propagate this optionality into the BaseOpenAiCompatibleLlm configuration type for type and apiKey, while preserving required validation when LLM_PROVIDER is letsur.src/upload/upload.controller.ts-114-116 (1)
114-116: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win경로 파라미터에
ParseUUIDPipe를 적용하세요.
documents.id는 uuid 컬럼입니다. 잘못된 형식의id가 들어오면 Postgres가invalid input syntax for type uuid(22P02)를 반환하고 API는 500을 응답합니다. Swagger에는 404만 명시되어 있습니다. 파이프를 적용하면 400으로 조기 거부됩니다.@Delete(':id')에도 동일하게 적용하세요.🐛 제안 수정
- async getOne(`@CurrentAdmin`() admin: AdminContext, `@Param`('id') id: string) { + async getOne( + `@CurrentAdmin`() admin: AdminContext, + `@Param`('id', ParseUUIDPipe) id: string, + ) { return this.uploadService.getById(id, admin.uuid); }- async reprocess(`@Param`('id') id: string) { + async reprocess(`@Param`('id', ParseUUIDPipe) id: string) { return this.uploadService.reprocess(id); }Also applies to: 235-241, 274-276
🤖 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/upload/upload.controller.ts` around lines 114 - 116, Apply NestJS ParseUUIDPipe to the id route parameter in getOne and the corresponding `@Delete`(':id') handler, so malformed UUIDs are rejected with 400 before reaching uploadService. Preserve the existing service calls and valid-UUID behavior.src/pdf-processor/pdf-processor.worker.spec.ts-80-100 (1)
80-100: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winESLint
prefer-promise-reject-errors오류를 수정하세요.
options.processPdfError의 타입은Error | undefined입니다. 그래서 82행에서 린트 오류가 발생합니다. 오류 값을 지역 상수로 좁힌 뒤 사용하세요.💚 제안 수정
+ const processPdfError = options.processPdfError; const pipeline = { - processPdf: options.processPdfError - ? jest.fn(() => Promise.reject(options.processPdfError)) + processPdf: processPdfError + ? jest.fn(() => Promise.reject(processPdfError)) : jest.fn(() =>🤖 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/pdf-processor/pdf-processor.worker.spec.ts` around lines 80 - 100, Update the processPdf mock construction to narrow options.processPdfError into a local constant before passing it to Promise.reject, then use that narrowed Error value in the rejection branch while preserving the existing resolved pipeline behavior.Source: Linters/SAST tools
src/pdf-processor/parse-finite-number.ts-10-11 (1)
10-11: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win빈 문자열과
null을 fallback으로 처리하십시오.
Number('')과Number(null)은0입니다. 환경 변수를PDF_PROCESSOR_CONTEXT_LENGTH=처럼 빈 값으로 두면 fallback 500이 아니라 0이 사용되고,{ min: 1 }클램프로 1이 됩니다. 이 값은src/pdf-pipeline.service.ts의 컨텍스트 길이와 워커 폴링 주기를 직접 결정합니다.🐛 제안 수정
- const n = typeof value === 'number' ? value : Number(value); + const n = + typeof value === 'number' + ? value + : typeof value === 'string' && value.trim() + ? Number(value) + : Number.NaN; const base = Number.isFinite(n) ? n : fallback;
parse-finite-number.spec.ts에''와null케이스를 추가하십시오.🤖 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/pdf-processor/parse-finite-number.ts` around lines 10 - 11, Update the value normalization in parseFiniteNumber so empty strings and null are treated as invalid inputs and use fallback before numeric conversion, while preserving existing finite-number handling. Add coverage for both '' and null in parse-finite-number.spec.ts.src/pdf-processor/pdf-pipeline.service.ts-176-186 (1)
176-186: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win빈 LLM 응답과 빈 페이지 텍스트를 실패로 집계하십시오.
markdown이 빈 문자열이고pageText도 빈 경우 이 코드는usedFallback: false를 반환합니다. 그러면 해당 페이지는failedPages에 들어가지 않습니다. 스캔 PDF처럼 텍스트 추출이 모두 실패하면assertPass1FailureWithinLimit가 통과하고, 이후Markdown section split produced 0 sections로 원인이 불명확한 오류가 발생합니다. 빈 결과를 실패로 표시하십시오.🐛 제안 수정
const markdown = response.choices?.[0]?.message?.content ?? ''; - if (!markdown.trim() && pageText.trim()) { + if (!markdown.trim()) { this.logger.warn( `Empty LLM markdown for page ${currentPage}; using raw extracted text`, ); return { markdown: `## Page ${currentPage}\n\n${pageText}`, usedFallback: true, }; }🤖 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/pdf-processor/pdf-pipeline.service.ts` around lines 176 - 186, Update the empty-result handling in the PDF pipeline method around the markdown response: when both the trimmed LLM markdown and trimmed pageText are empty, return the result with usedFallback set to true so the page is counted in failedPages. Preserve the existing raw-text fallback for non-empty pageText and the normal successful return for non-empty markdown.src/pdf-processor/pdf-chunk-parser.ts-20-27 (1)
20-27: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
baseName이 빈 문자열이면 무한 루프가 발생합니다.
baseName이''이면p === baseName조건이p === ''에서 항상 참입니다. 본문은p를 다시''로 할당하므로while루프가 종료되지 않습니다. 이 함수는src/pdf-processor/pdf-pipeline.service.ts의 LLM 경로 정규화에도 사용되므로 가드를 추가하십시오.🐛 제안 수정
export function toRelativeChunkPath(raw: string, baseName: string): string { let p = raw.trim().replace(/^\/+|\/+$/g, ''); + if (!baseName) return p; const prefix = `${baseName}/`;🤖 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/pdf-processor/pdf-chunk-parser.ts` around lines 20 - 27, Update toRelativeChunkPath so it immediately returns the trimmed path when baseName is empty, before entering the while loop. Preserve the existing prefix-stripping behavior for non-empty baseName values.src/pdf-processor/pdf-chunk-parser.ts-97-99 (1)
97-99: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win속성 값에 들어가는
description을 이스케이프하십시오.
chunk.description은 LLM 출력입니다. 값에"가 포함되면 생성된<document path="…" description="…">태그가 깨집니다. 이 루트 문서는 이후 동일한 정규식으로 다시 파싱될 수 있습니다.src/pdf-processor/pdf-pipeline.service.ts는 같은 위치에서escapeAttr를 사용하므로 동작이 서로 다릅니다. 동일한 이스케이프를 적용하십시오.🐛 제안 수정
+function escapeAttr(value: string): string { + return value.replace(/"/g, "'"); +} +stubLinks.push( - `<document path="${fullPath}" description="${chunk.description}"></document>`, + `<document path="${fullPath}" description="${escapeAttr(chunk.description)}"></document>`, );🤖 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/pdf-processor/pdf-chunk-parser.ts` around lines 97 - 99, Update the document stub construction in the chunk parser to escape chunk.description with the existing escapeAttr utility before interpolating it into the description attribute. Reuse the same escapeAttr behavior used by pdf-pipeline.service.ts so generated <document> tags remain valid and re-parseable.src/pdf-processor/mojibake.ts-33-49 (1)
33-49: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win복구한 문자열의 U+FFFD와 한글 비율을 검증하십시오.
iconv.encode는 목적 코드페이지 없는 문자를?로 silently 치환하고, 잘못된 바이트열은Buffer.toString('utf8')가 U+FFFD로 대체합니다. 둘 다 예외를 던지지 않으므로 한글 일부만 복구되고 나머지가 손상된 경우에도koreanCount > 0조건이 통과됩니다. 이 값을 페이지 텍스트로 저장하기 전에recovered.includes('\uFFFD')와 한글 비율을 함께 검사해야 합니다.🤖 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/pdf-processor/mojibake.ts` around lines 33 - 49, Update the recovery loop around the encoding and recovered text checks to reject candidates containing U+FFFD and require an acceptable Korean-character ratio, not merely koreanCount > 0, before returning recovered. Preserve trying the next encoding and returning null when no candidate passes validation.src/chat/services/chat-stream.transport.spec.ts-22-22 (1)
22-22: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win불필요한
as never타입 단언을 제거하십시오.ESLint가 22, 41, 80, 83번째 줄의
as never단언이 불필요하다고 보고합니다(@typescript-eslint/no-unnecessary-type-assertion). 대상 표현식이 이미 원래 타입을 받아들이므로 단언이 필요하지 않습니다. 이 오류는 lint 단계를 실패시킬 수 있습니다.🧹 제안: 불필요한 단언 제거
- transport.prepareSse(reply as never, req as never); + transport.prepareSse(reply, req);- const consumePromise = transport.consumeAndForward(stream, reply as never); + const consumePromise = transport.consumeAndForward(stream, reply);- transport.writeResources(reply as never, [ + transport.writeResources(reply, [ { path: 'a.pdf', formats: ['pdf'], url: 'a.pdf' }, ]); - transport.writeDone(reply as never); + transport.writeDone(reply);Also applies to: 41-41, 80-83
🤖 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/chat/services/chat-stream.transport.spec.ts` at line 22, Remove the unnecessary `as never` type assertions from the calls at lines 22, 41, and 80–83 in the transport tests, including the `transport.prepareSse` invocation. Pass the existing `reply` and `req` values directly while preserving the current test behavior.Source: Linters/SAST tools
src/chat/prompts/final-response.prompt.ts-38-42 (1)
38-42: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win41번째 줄의 오타를 수정하십시오.
"사용하새요"는 오타입니다. "사용하세요"로 수정하십시오.
✏️ 오타 수정
- - 답변은 명확하고 이해하기 쉽게 구성하세요. 소제목(## ###)과 문단으로 구분하고 필요시 번호나 불릿 포인트를 사용하새요. + - 답변은 명확하고 이해하기 쉽게 구성하세요. 소제목(## ###)과 문단으로 구분하고 필요시 번호나 불릿 포인트를 사용하세요.🤖 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/chat/prompts/final-response.prompt.ts` around lines 38 - 42, Update the typo in the answer-quality guidance within the final response prompt, changing “사용하새요” to “사용하세요” while leaving the surrounding instructions unchanged.src/chat/prompts/pdf-processor.ts-11-14 (1)
11-14: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win파일명을 LLM 프롬프트에 넣기 전에 안전 데이터 방식으로 처리하십시오.
src/pdf-processor/pdf-pipeline.service.ts#L156과#L226에서{filename}을 검증/이스케이프 없이PDF_PROCESSOR_PROMPT와PDF_CHUNKING_PROMPT에 직접 치환하고 있습니다. 파일명은 안전한 데이터 영역으로 분리된 뒤 프롬프트에 전달해야 합니다.🤖 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/chat/prompts/pdf-processor.ts` around lines 11 - 14, PDF_PROCESSOR_PROMPT와 PDF_CHUNKING_PROMPT에 삽입하는 filename을 검증·이스케이프하고 안전한 데이터 영역으로 분리해 전달하도록 수정하십시오. src/chat/prompts/pdf-processor.ts 11-14와 src/chat/prompts/pdf-chunking-prompt.ts 15-16의 템플릿을 조정하고, src/pdf-processor/pdf-pipeline.service.ts의 해당 PDF_PROCESSOR_PROMPT 및 PDF_CHUNKING_PROMPT 치환 지점(156, 226)에서 동일한 안전 처리 경로를 사용하십시오.src/chat/services/chat-orchestration.service.spec.ts-95-115 (1)
95-115: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win불필요한
as never단정이 lint 오류를 발생시킵니다.ESLint가 Line 96, 110, 132, 133에서
no-unnecessary-type-assertion오류를 보고합니다. 해당 파라미터는 단정 없이도 전달한 값의 타입을 받습니다. 단정을 제거하거나, 모킹 객체에 명시적 타입을 선언하십시오. lint 오류는 CI를 실패시킵니다.Also applies to: 129-134
🤖 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/chat/services/chat-orchestration.service.spec.ts` around lines 95 - 115, Remove the unnecessary `as never` assertions from the `ChatOrchestrationService` setup and its related constructor arguments around `ResourceContentService` and `ChatOrchestrationService`; pass the existing mocks directly, or give them explicit compatible mock types where required, so the test passes `no-unnecessary-type-assertion` without changing behavior.Source: Linters/SAST tools
src/chat/types/llm.types.ts-31-34 (1)
31-34: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPrettier 포맷 오류로 lint가 실패합니다.
ESLint가
prettier/prettier오류를 보고합니다.'auto' | 'none'을 한 줄로 합치십시오.🎨 제안 diff
- tool_choice?: - | 'auto' - | 'none' - | { type: 'function'; function: { name: string } }; + tool_choice?: 'auto' | 'none' | { type: 'function'; function: { name: string } };
npx prettier --write src/chat/types/llm.types.ts로 정확한 결과를 적용하십시오.🤖 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/chat/types/llm.types.ts` around lines 31 - 34, Update the tool_choice type declaration in llm.types.ts to place the 'auto' and 'none' union members on a single line, matching Prettier formatting while preserving the existing function-choice object type.Source: Linters/SAST tools
src/chat/llm/base-openai-compatible.llm.ts-204-233 (1)
204-233: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
safeStringify가undefined에서 예외를 던질 수 있습니다.
JSON.stringify는undefined나 함수를 받으면undefined를 반환합니다. 그 다음 라인의str.length접근이TypeError를 발생시킵니다. 현재 호출 경로는responseData != null가드가 있어 안전합니다. 그러나 이 메서드는protected이므로 하위 클래스가 임의 값으로 호출할 수 있습니다. 로깅 유틸이 로깅 중에 실패하면 원인 오류가 가려집니다.🛡️ 제안 diff
- const str = JSON.stringify(value, null, 2); + const str = JSON.stringify(value, null, 2) ?? String(value); return str.length > maxLen🤖 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/chat/llm/base-openai-compatible.llm.ts` around lines 204 - 233, Update safeStringify to handle JSON.stringify returning undefined, including undefined and function inputs, before accessing str.length. Return a safe string representation while preserving the existing truncation behavior and fallback inspection for values that cause JSON.stringify to throw.
🧹 Nitpick comments (31)
.env.example (1)
41-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
PDF_PROCESSOR_LLM_TIMEOUT단위를 주석으로 명시하세요.같은 블록의
PDF_PROCESSOR_POLL_INTERVAL_MS는_MS접미사로 단위를 표시합니다.PDF_PROCESSOR_LLM_TIMEOUT은 접미사가 없으므로 초 단위인지 밀리초 단위인지 알 수 없습니다. 운영자가 값을 1000배 잘못 설정할 수 있습니다.♻️ 제안 변경
PDF_PROCESSOR_CONTEXT_LENGTH=500 +# LLM 호출 타임아웃 (초) PDF_PROCESSOR_LLM_TIMEOUT=120🤖 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 @.env.example around lines 41 - 48, Update the PDF Processor configuration comments around PDF_PROCESSOR_LLM_TIMEOUT to explicitly document its unit, clarifying whether the timeout value is measured in seconds or milliseconds. Keep the existing environment variable name and value unchanged.drizzle/0012_brainy_dagger.sql (1)
2-2: 🚀 Performance & Scalability | 🔵 Trivial부분 인덱스를 검토하세요.
expires_at은 nullable입니다. 대부분의 문서가 만료일 없이 저장되면 인덱스가 NULL 항목을 다수 포함합니다. 만료 문서 정리 쿼리가expires_at IS NOT NULL인 행만 조회한다면,src/db/schema.ts에서 부분 인덱스로 정의하여 인덱스 크기를 줄일 수 있습니다.만료 정리 쿼리가
is_active도 필터링한다면(expires_at, is_active)복합 인덱스를 고려하세요.🤖 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 `@drizzle/0012_brainy_dagger.sql` at line 2, Update the documents expiration index definition in src/db/schema.ts to use a partial index that includes only rows where expires_at is not null, matching the cleanup query’s filter. If the cleanup query also filters is_active, define the index as the composite (expires_at, is_active) partial index instead.src/db/schema.ts (1)
222-235: 🚀 Performance & Scalability | 🔵 Trivial워커 폴링 쿼리에 맞는 복합 인덱스를 검토하세요.
documents_status_idx와documents_is_active_idx는 카디널리티가 낮습니다. 백그라운드 워커가status와updated_at(stale 처리 판정)을 함께 조건으로 조회하면 단일 컬럼 인덱스는 선택도가 낮습니다. 실제 폴링 쿼리를 확인한 뒤(status, updated_at)복합 인덱스 또는status IN ('queued','processing')부분 인덱스를 고려하세요.#!/bin/bash # documents 폴링/클레임 쿼리 패턴 확인 rg -nP --type=ts -C6 'documents\b' -g 'src/pdf-processor/**' -g 'src/retrieval/**'🤖 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/db/schema.ts` around lines 222 - 235, Review the worker polling and claim queries associated with the documents table, then update the schema callback near statusIdx and isActiveIdx to add the appropriate composite or partial index for their actual status and updatedAt/stale-filter conditions. Reuse the existing table columns and preserve unrelated indexes.src/config/env.validation.ts (1)
172-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
PDF_PROCESSOR_CONCURRENCY의 상한이 1이라 설정값이 사실상 고정됩니다.
@Min(1)과@Max(1)을 함께 적용하면 1 이외의 값은 부팅 시 검증 실패를 유발합니다. 운영자가PDF_PROCESSOR_CONCURRENCY=2를 설정하면 애플리케이션이 시작하지 않습니다. 단일 워커가 의도라면 상한 이유를 주석으로 남기세요. 향후 확장을 허용하려면 상한을 올리세요.♻️ 제안 변경
+ /** 현재 워커는 순차 처리만 지원합니다. 값은 1로 고정됩니다. */ `@IsOptional`() `@IsNumber`() `@Min`(1) `@Max`(1) PDF_PROCESSOR_CONCURRENCY?: number;🤖 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/config/env.validation.ts` around lines 172 - 176, Resolve the contradictory bounds on PDF_PROCESSOR_CONCURRENCY in the validation decorators. If concurrency is intentionally limited to one worker, add a comment documenting that constraint; otherwise raise `@Max`(1) to the supported upper limit so values such as 2 pass startup validation, while preserving the optional numeric and minimum-value checks.package.json (1)
71-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
@types/iconv-lite의존성을 제거하세요.
iconv-lite는 자체 타입 정의를 제공하며,@types/iconv-lite는 오래된 스텁 정의 패키지입니다. 별도 의존성으로 두지 마세요.🤖 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 `@package.json` at line 71, Remove the `@types/iconv-lite` entry from the package dependencies in package.json, leaving iconv-lite itself unchanged because it provides its own type definitions.src/upload/dto/update-expires-at.dto.ts (1)
5-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueApiProperty에
type: String을 명시하세요.필드 타입이
string | null유니온입니다. 그래서 리플렉션 메타데이터가Object로 추론되고 OpenAPI 스키마가 부정확해질 수 있습니다.type: String과format: 'date-time'을 명시하면 문서가 정확해집니다. 검증 데코레이터 조합 자체는 의도대로 동작합니다.null은 허용되고undefined는@IsDefined()로 거부됩니다.♻️ 제안 변경
`@ApiProperty`({ description: '문서 유효기간 (ISO-8601). null이면 무기한. 과거 시각은 허용하지 않습니다.', + type: String, + format: 'date-time', nullable: true, example: '2026-12-31T23:59:59.000Z', })🤖 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/upload/dto/update-expires-at.dto.ts` around lines 5 - 14, Update the ApiProperty configuration for expiresAt to explicitly declare the OpenAPI type as String and format as date-time, while preserving its nullable behavior, example, and existing validation decorators.src/upload/dto/document-list-item.dto.ts (1)
4-10: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win상태 목록을 DB enum에서 파생시키세요.
DOCUMENT_STATUSES는 값을 수동으로 나열합니다.src/db/schema.ts의documentStatusEnum이 변경되면 OpenAPI 문서가 실제 값과 어긋납니다.documentStatusEnum.enumValues를 그대로 사용하면 드리프트를 막을 수 있습니다.🤖 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/upload/dto/document-list-item.dto.ts` around lines 4 - 10, Update DOCUMENT_STATUSES in the document list item DTO to derive its values directly from documentStatusEnum.enumValues in the database schema instead of maintaining a manual list, keeping the OpenAPI status values synchronized with the DB enum.src/pdf-processor/documents.repository.ts (1)
317-323: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win청크 정렬에 타이브레이커를 추가하세요.
sortOrder가 중복되면 반환 순서가 비결정적입니다. 동일한 문서를 두 번 조회할 때 청크 순서가 달라질 수 있습니다.createdAt또는id를 2차 정렬 키로 추가하세요.♻️ 제안 변경
- .orderBy(asc(documentChunks.sortOrder)); + .orderBy(asc(documentChunks.sortOrder), asc(documentChunks.id));🤖 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/pdf-processor/documents.repository.ts` around lines 317 - 323, Update listChunks to add a deterministic secondary sort key after documentChunks.sortOrder, using documentChunks.createdAt or documentChunks.id, while preserving the existing documentId filter and primary sort order.src/pdf-processor/pdf-processor.worker.ts (1)
136-146: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win파이프라인 단계에 타임아웃을 두는 것을 검토하세요.
downloadPdf와processPdf에는 자체 타임아웃이 없습니다. LLM 또는 GCS 응답이 지연되면 슬롯이 무기한 점유됩니다. stale requeue는 30분 뒤에 동작하지만 원래 attempt는 계속 실행됩니다. 각 단계에 명시적 타임아웃을 적용하면 슬롯 회수 시점이 예측 가능해집니다.🤖 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/pdf-processor/pdf-processor.worker.ts` around lines 136 - 146, Update the PDF processing flow around gcs.downloadPdf and pipeline.processPdf to enforce explicit, bounded timeouts for each stage. Ensure slow GCS or LLM operations are interrupted and propagated as failures so worker slots are released predictably, while preserving the existing zero-chunk validation.src/upload/upload.service.ts (1)
183-197: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win재처리 시 이전 GCS 산출물 정리를 검토하세요.
enqueueReprocess는 DB 청크만 삭제합니다. 이전 실행이 만든 마크다운 객체는 GCS에 남습니다. 새 실행이 다른 청크 경로를 생성하면 사용되지 않는 객체가 누적되고,deleteResourceArtifacts시점까지 남아 있습니다. 재처리 큐 등록 후deleteProcessedArtifacts(row.resourceName)를 호출하면 원본 PDF는 보존하면서 산출물만 초기화할 수 있습니다.🤖 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/upload/upload.service.ts` around lines 183 - 197, After a successful enqueueReprocess call in the reprocessing flow, invoke deleteProcessedArtifacts with the document’s resourceName to remove prior GCS-derived markdown artifacts while preserving the original PDF. Keep the existing concurrency, not-found, and conflict handling unchanged, and do not perform cleanup when enqueueReprocess returns false.src/pdf-processor/pdf-text.service.ts (1)
38-40: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win텍스트 결합 방식이 줄 구조를 없앱니다.
textContent.items를 공백으로 연결하면 줄바꿈 정보가 사라집니다. 다운스트림 마크다운 청킹은 제목과 단락 구분에 의존합니다.hasEOL항목을 줄바꿈으로 변환하면 구조 보존에 도움이 됩니다.♻️ 제안 변경
- const raw = textContent.items - .map((item) => ('str' in item ? String(item.str) : '')) - .join(' '); + const raw = textContent.items + .map((item) => + 'str' in item + ? String(item.str) + (item.hasEOL ? '\n' : ' ') + : '', + ) + .join('');🤖 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/pdf-processor/pdf-text.service.ts` around lines 38 - 40, Update the text assembly around textContent.items to preserve line structure by using each item's hasEOL marker to insert newline separators instead of joining every item only with spaces. Keep extracting string content from str items, and ensure the resulting raw text retains paragraph and heading boundaries for downstream Markdown chunking.src/pdf-processor/gcs-storage.service.ts (1)
102-107: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value업로드를 제한 병렬로 처리하고 부분 실패를 명시하세요.
uploadDocuments는 모든 마크다운 파일을 직렬로 업로드합니다. 청크 수가 많은 문서에서 처리 시간이 파일 수에 비례해 늘어납니다. 또한 중간 실패 시 이미 업로드된 파일이 남습니다. worker가 실패 후deleteProcessedArtifacts로 정리하므로 정합성은 유지되지만, 제한 병렬(예: 5개) 적용으로 처리 시간을 줄일 수 있습니다.🤖 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/pdf-processor/gcs-storage.service.ts` around lines 102 - 107, Update uploadDocuments to upload files with bounded concurrency (for example, five active uploads) instead of awaiting each upload serially, while preserving the existing Uploaded debug logging. Ensure individual upload failures are surfaced to the caller so the worker can invoke deleteProcessedArtifacts and clean up already-uploaded artifacts.src/scripts/smoke-pdf-processor.ts (2)
21-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePDF 문자열 리터럴 특수문자를 이스케이프하십시오.
text는 PDF 문자열 리터럴(...)안에 그대로 삽입됩니다.(,),\가 포함되면 콘텐츠 스트림 구문이 깨집니다. 현재 호출은'Hello Ziggle'뿐이지만 스크립트를 재사용할 때 문제가 됩니다.♻️ 제안 수정
function buildMinimalPdf(text: string): Buffer { - const content = `BT /F1 12 Tf 50 700 Td (${text}) Tj ET`; + const escaped = text.replace(/[\\()]/g, (c) => `\\${c}`); + const content = `BT /F1 12 Tf 50 700 Td (${escaped}) Tj ET`;🤖 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/scripts/smoke-pdf-processor.ts` around lines 21 - 22, Update buildMinimalPdf so text is escaped before interpolation into the PDF string literal, including parentheses and backslashes, while preserving ordinary text such as “Hello Ziggle” unchanged.
111-122: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value임시 파일 왕복을 제거하십시오.
pdfBytes를 바로extractFirstPageText에 전달할 수 있습니다. 현재 코드는 파일 쓰기와 읽기를 추가로 수행합니다. 이 단계는 검증 대상이 아닙니다. 파일 시스템 의존을 제거하면 스크립트가 단순해집니다.🤖 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/scripts/smoke-pdf-processor.ts` around lines 111 - 122, Update the smoke test around extractFirstPageText to pass pdfBytes directly, removing the temporary path creation, writeFileSync/readFileSync round trip, and associated cleanup. Preserve the existing extracted-text validation and success logging while eliminating the unnecessary filesystem dependency.src/pdf-processor/markdown-section-splitter.ts (1)
184-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value병합 시 title 길이를 제한하십시오.
작은 섹션이 연속되면
title이A · B · C · …형태로 계속 누적됩니다. 이 값은slugifyTitle과 목차 출력에 사용되므로 매우 긴 문자열이 저장될 수 있습니다. 병합 제목을 상위 2개까지만 유지하는 방식을 권장합니다.♻️ 제안 변경
if (shouldMerge && combinedLength <= CHUNK_MAX_CHARS) { + const parts = current.title.split(' · '); current = { - title: `${current.title} · ${next.title}`, + title: [parts[0], next.title].join(' · '), content: `${current.content}\n\n${next.content}`, }; continue; }🤖 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/pdf-processor/markdown-section-splitter.ts` around lines 184 - 190, Update the merge logic in the section splitter to cap the combined title at the first two section titles when repeatedly merging small sections. Preserve the existing content concatenation and length checks, and ensure the resulting title remains suitable for slugifyTitle and table-of-contents output.src/pdf-processor/pdf-pipeline.service.spec.ts (1)
41-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value불필요한 타입 어서션을 제거하십시오.
ESLint가 44행을
@typescript-eslint/no-unnecessary-type-assertion으로 보고합니다.llmmock은 이미LlmClient와 호환됩니다. 어서션을 정리하거나 mock 변수에 명시적 타입을 지정하십시오.♻️ 제안 수정
- const llm = { + const llm: LlmClient = { getModel: jest.fn(() => 'normal-model'), callLLM: options.callLLM, generateFinalResponseStream: jest.fn(), - }; + } as unknown as LlmClient; return new PdfPipelineService( pdfTextService as unknown as PdfTextService, config as unknown as ConfigService, - llm as unknown as LlmClient, + llm, );🤖 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/pdf-processor/pdf-pipeline.service.spec.ts` around lines 41 - 45, Remove the unnecessary type assertion from the llm argument in the PdfPipelineService construction helper; the llm mock is already compatible with LlmClient. If needed, give the mock variable an explicit LlmClient type while preserving the existing PdfTextService and ConfigService conversions.Source: Linters/SAST tools
src/pdf-processor/pdf-chunk-parser.ts (1)
29-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
resourceStem과toResourceName의 중복을 정리하십시오.두 함수 모두 확장자 제거를 수행합니다.
resourceStem은 디렉터리 제거와 NFC 정규화를 하지 않습니다. 같은 입력에 대해 서로 다른 결과가 나올 수 있습니다.parseChunksFromMarkdown내부에서toResourceName을 재사용하는 방식을 권장합니다.🤖 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/pdf-processor/pdf-chunk-parser.ts` around lines 29 - 40, 중복된 확장자 제거 로직을 정리하십시오. `parseChunksFromMarkdown`에서 `resourceStem`을 사용하지 말고 기존 `toResourceName`을 재사용하여 디렉터리 제거와 NFC 정규화까지 동일하게 적용되도록 변경하십시오. 더 이상 필요하지 않은 `resourceStem` 함수와 관련 호출을 제거하십시오.src/pdf-processor/pdf-pipeline.service.ts (1)
93-109: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftLLM 호출의 소요 시간과 실패 처리를 개선하십시오.
Pass 1은 페이지를 순차로 처리합니다. 페이지당 최대
llmTimeoutMs(기본 120초)가 걸리므로 200페이지 문서는 최대 수 시간이 소요될 수 있습니다. Pass 2도 배치 실패 시 재시도 없이 전체 작업을 중단합니다.다음을 검토하십시오.
- Pass 2 배치는
previousContext의존이 없으므로 제한된 동시 실행을 적용하십시오.- 일시적 오류(타임아웃)에 대해 배치 단위 재시도와 백오프를 추가하십시오.
- 페이지 수 상한을 설정하고 초과 시 업로드를 명확한 오류로 거부하십시오.
Also applies to: 231-269
🤖 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/pdf-processor/pdf-pipeline.service.ts` around lines 93 - 109, Update the PDF pipeline’s page-processing flow around convertPageToMarkdown to avoid sequential Pass 1 latency where context permits, while preserving previousContext ordering where it is required. Add bounded concurrency for Pass 2 batches, with batch-level retries and backoff for transient timeout failures before aborting. Enforce a maximum page count before processing and reject oversized uploads with a clear error.src/chat/services/chat-orchestration.service.ts (1)
141-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win"관련 자료 없음" 스트림 생성 로직이 중복됩니다.
141-154줄과 171-184줄은
NO_RELEVANT_MATERIALS_SYSTEM_PROMPT,historyMessages,userQuestion, 모델'normal',temperature: 0을 사용해 완전히 동일한 스트림을 생성합니다. 두 곳을 헬퍼 메서드로 추출하십시오. 이렇게 하면 프롬프트나 모델을 변경할 때 한 곳만 수정하면 됩니다.♻️ 제안: 헬퍼 메서드로 추출
+ private buildNoRelevantMaterialsStream( + historyMessages: LlmMessage[], + userQuestion: string, + ): Promise<Readable> { + return this.llmClient.generateFinalResponseStream( + [ + { role: 'system', content: NO_RELEVANT_MATERIALS_SYSTEM_PROMPT }, + ...historyMessages, + { role: 'user', content: userQuestion }, + ], + [], + this.llmClient.getModel('normal'), + { temperature: 0 }, + ); + }그 다음 141-154줄과 171-184줄을 다음과 같이 대체하십시오.
- const stream = await this.llmClient.generateFinalResponseStream( - [ - { role: 'system', content: NO_RELEVANT_MATERIALS_SYSTEM_PROMPT }, - ...historyMessages, - { role: 'user', content: userQuestion }, - ], - [], - this.llmClient.getModel('normal'), - { temperature: 0 }, - ); + const stream = await this.buildNoRelevantMaterialsStream( + historyMessages, + userQuestion, + );Also applies to: 171-184
🤖 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/chat/services/chat-orchestration.service.ts` around lines 141 - 154, Extract the duplicated no-relevant-materials stream construction from the orchestration flow into a private helper method, preserving the existing NO_RELEVANT_MATERIALS_SYSTEM_PROMPT, historyMessages, userQuestion, normal model, and zero-temperature settings. Replace both branches around the hasResources check and the later no-resources path with calls to this helper, while keeping their existing return values unchanged.src/chat/llm/base-openai-compatible.llm.ts (3)
116-160: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win스트리밍 호출의 타임아웃과 토큰 한도가 고정되어 있습니다.
generateFinalResponseStream은timeout: 15000과max_tokens: 2000을 하드코딩합니다.callLLM은LlmCallOptions로 두 값을 모두 조정할 수 있습니다. 최종 답변 생성은 heavy 모델을 쓰므로 첫 응답 헤더까지 15초를 넘길 수 있습니다. 그 경우 스트림이 시작되지 못하고 500으로 실패합니다.
generateFinalResponseStream의options타입을LlmCallOptions로 확장하고,llm-client.interface.ts의 시그니처도 같이 맞추십시오.♻️ 제안 diff
async generateFinalResponseStream( messages: LlmMessage[], toolResults: LlmToolResult[], model?: string, - options?: { temperature?: number }, + options?: LlmCallOptions, ): Promise<Readable> { @@ temperature: options?.temperature ?? 0.7, - max_tokens: 2000, + max_tokens: options?.max_tokens ?? 2000, stream: true, @@ responseType: 'stream', - timeout: 15000, + timeout: options?.timeoutMs ?? 15000,🤖 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/chat/llm/base-openai-compatible.llm.ts` around lines 116 - 160, Update generateFinalResponseStream and its declaration in llm-client.interface.ts to accept LlmCallOptions, then use the supplied timeout and maxTokens values when constructing the streaming request instead of hardcoded 15000 and 2000 defaults. Preserve existing defaults when those options are omitted.
212-212: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win불필요한 타입 단정을 제거하십시오.
Line 212:
Buffer.isBuffer는unknown을 인자로 받는 타입 가드입니다.value as Buffer단정이 필요하지 않습니다. ESLint가no-unnecessary-type-assertion오류를 보고합니다.Line 250:
LlmMessage는 이미name?: string을 선언합니다.m as unknown as { name?: unknown }이중 단정 없이m.name을 직접 읽을 수 있습니다.♻️ 제안 diff
- if (typeof Buffer !== 'undefined' && Buffer.isBuffer(value as Buffer)) { + if (typeof Buffer !== 'undefined' && Buffer.isBuffer(value)) { const str = (value as Buffer).toString('utf8');- if ((m as unknown as { name?: unknown }).name != null) { + if (m.name != null) { toolRoleHasNameField += 1; }Also applies to: 250-250
🤖 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/chat/llm/base-openai-compatible.llm.ts` at line 212, Remove the unnecessary type assertions in the message-processing logic: update the Buffer check near the existing Buffer.isBuffer call to pass value directly, and update the LlmMessage handling to read m.name directly instead of using the double assertion to { name?: unknown }. Preserve the current behavior and surrounding validation.Source: Linters/SAST tools
92-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value동일 오류를 두 번 기록합니다.
catchError에서 이미logApiError가 상세 로그를 남깁니다. 그 후throw new InternalServerErrorException(...)이 아래catch블록으로 전달되어logger.error가 다시 실행됩니다. 로그가 중복됩니다.generateFinalResponseStream에도 같은 구조가 있습니다.
try/catch래퍼를 제거하거나,catchError안에서 요약 로깅만 남기십시오.♻️ 제안 diff
- try { - const response = await firstValueFrom( - this.httpService - .post<LlmResponse>(`${this.baseUrl}/chat/completions`, request, { - headers: this.buildHeaders(), - timeout: options?.timeoutMs ?? 15000, - }) - .pipe( - catchError((error: AxiosError) => { - this.logApiError(error, requestLogSummary); - throw new InternalServerErrorException( - `Failed to call ${this.providerLabel} API: ${error.message}`, - ); - }), - ), - ); - - return response.data; - } catch (error) { - this.logger.error(`Error calling ${this.providerLabel}: ${error}`); - throw error; - } + const response = await firstValueFrom( + this.httpService + .post<LlmResponse>(`${this.baseUrl}/chat/completions`, request, { + headers: this.buildHeaders(), + timeout: options?.timeoutMs ?? 15000, + }) + .pipe( + catchError((error: AxiosError) => { + this.logApiError(error, requestLogSummary); + throw new InternalServerErrorException( + `Failed to call ${this.providerLabel} API: ${error.message}`, + ); + }), + ), + ); + + return response.data;🤖 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/chat/llm/base-openai-compatible.llm.ts` around lines 92 - 113, Remove the redundant try/catch logging around the HTTP request in the method containing this flow, since catchError already calls logApiError before rethrowing. Apply the same change to generateFinalResponseStream, preserving the existing exception propagation and detailed error logging while eliminating the duplicate logger.error call.src/chat/services/resource-selection.service.spec.ts (1)
6-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value테스트 헬퍼가 두 spec 파일에 중복됩니다.
createLlmResponse와CallLLM타입은src/chat/services/chat-orchestration.service.spec.ts(Line 12-32, Line 70)에도 거의 동일하게 존재합니다. 토큰 분배 비율만 0.6과 0.7로 다릅니다. 공용 테스트 헬퍼 모듈로 추출하면LlmResponse형식이 변경될 때 한 곳만 수정하면 됩니다.🤖 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/chat/services/resource-selection.service.spec.ts` around lines 6 - 38, Extract the duplicated createLlmResponse helper and CallLLM type from ResourceSelectionService and chat-orchestration service specs into a shared test helper module, then import and reuse them in both specs. Preserve each spec’s existing token allocation behavior by allowing the shared helper to receive or configure the differing ratio.src/config/env.validation.spec.ts (1)
64-98: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win디코딩 후 GCS 자격증명 구조를 검증하는 테스트를 추가하십시오.
env.validation.ts는GCS_SERVICE_ACCOUNT_KEY_BASE64를@IsBase64()까지만 검사합니다. 실제 파싱은GcsStorageService생성자에서Buffer.from(encodedKey, 'base64')후JSON.parse하고client_email,private_key가 있는지만 확인합니다. 따라서not-json같은 유효한 base64 값과{ "type": "service_account" }같은 필수 필드 누락 케이스가 부팅 시점에 정상으로 통과합니다.env.validation.spec.ts에도GCS_SERVICE_ACCOUNT_KEY_BASE64케이스를 추가해 시작 전에 잡히게 하십시오.🤖 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/config/env.validation.spec.ts` around lines 64 - 98, Extend the GCS credential tests in the env validation suite to reject values that are valid base64 but decode to invalid JSON or JSON missing client_email/private_key. Update the validation path used by validate so these decoded-structure checks occur before startup, while preserving acceptance of a properly encoded service-account object.src/chat/services/chat-orchestration.service.spec.ts (1)
129-138: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value스트림 모킹을 pull 기반으로 교체하세요.
현재 테스트는
handleStreamingResponse를 await 없이 시작한 뒤setImmediate한 번으로consumeAndForward의 구독이 설정되기를 기다립니다.Readable.from의 별도finalStream.write()를 제거하면 테스트의 구독 타이밍 의존성이 줄어듭니다.🤖 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/chat/services/chat-orchestration.service.spec.ts` around lines 129 - 138, handleStreamingResponse 테스트의 스트림 모킹을 push 방식의 별도 finalStream.write 호출에서 pull 기반 Readable.from 방식으로 변경하세요. consumeAndForward 구독 타이밍을 기다리기 위한 setImmediate 의존성도 제거하고, 테스트 데이터가 스트림 생성 시점에 제공되도록 구성하면서 기존 응답 검증은 유지하세요.src/chat/prompts/resource-path-selection.prompt.ts (1)
138-148: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win후보 목록 크기에 상한을 두는 것을 검토하세요.
formatChunkCandidatesForSelection은 모든 후보를 문서 설명과 chunk 설명까지 포함해 한 문자열로 만듭니다. 문서 수가 늘어나면 프롬프트 길이가 선형으로 커지고light모델의 컨텍스트 한도와 비용을 압박합니다. 후보 수 또는 설명 길이 상한을 추가하는 방법을 검토하세요.🤖 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/chat/prompts/resource-path-selection.prompt.ts` around lines 138 - 148, Update formatChunkCandidatesForSelection to cap the number of formatted candidates and/or truncate resourceDescription and description before building the prompt string. Preserve the existing output format for entries within the limits and define bounded behavior that prevents large candidate lists or descriptions from exceeding light-model context constraints.src/chat/services/resource-content.service.ts (3)
31-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win경로 정규화 로직 중복을 제거하세요.
normalizeResourcePath는RetrievalService.stripKnownExtension(src/retrieval/retrieval.service.tsLine 77-85)과 동일한 규칙을 다시 구현합니다. Line 313과 Line 362에서 조회 결과 Map의 키와 맞추려고 이 로직을 사용하므로, 두 구현이 갈라지면 본문 조회가 조용히 실패합니다. 정규화 함수를 retrieval 레이어에 하나만 두고 공유하세요.🤖 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/chat/services/resource-content.service.ts` around lines 31 - 42, Remove the duplicated extension-stripping logic from ResourceContentService.normalizeResourcePath and reuse RetrievalService.stripKnownExtension as the single normalization implementation. Update the normalization calls used for lookup keys around the existing retrieval-result Map accesses so they share the retrieval-layer rules and remain consistent with stored keys.
673-676: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
generateResourceUrl이름과 반환값이 일치하지 않습니다.이 메서드는 URL을 만들지 않고 인코딩된 경로만 반환합니다. 또한
encodeURIComponent는/를%2F로 바꿉니다. 경로 구분자가 포함된 값을 전달하면 FE가 해석하지 못할 수 있습니다.encodeResourcePath처럼 동작을 나타내는 이름으로 변경하고, 경로 세그먼트 단위 인코딩이 필요한지 확인하세요.🤖 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/chat/services/resource-content.service.ts` around lines 673 - 676, Rename generateResourceUrl to reflect that it returns an encoded path rather than a complete URL, such as encodeResourcePath, and update all references to the method. Adjust encoding to preserve path separators by encoding individual segments when resourcePath contains `/`, ensuring the frontend can interpret the resulting path correctly.
453-671: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win불사용 구 형식 분기를 제거하세요.
fetchRelevantResourceContents()와 내부 도우미/LLM 프롬프트가 현재 호출 경로에 사용되지 않습니다.RetrievalService.listCatalog()도filteredResources: []를 반환하므로, 이 분기는 현재 데이터 경로와 연결되지 않습니다. 제거하거나 향후 API contract가 복구될 수 있도록 분리해 두세요.🤖 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/chat/services/resource-content.service.ts` around lines 453 - 671, Remove the unused legacy branch containing the filteredResources-based selection flow, including its calls to selectRelevantResourcePaths, getContentsByPaths, selectMostRelevantDocuments, and related helper/prompt dependencies. Ensure fetchRelevantResourceContents and any internal legacy-only helpers are no longer part of the active service path, while preserving the current listCatalog-based retrieval contract rather than maintaining a disconnected filteredResources: [] flow.src/retrieval/retrieval.service.ts (1)
18-51: 🚀 Performance & Scalability | 🔵 Trivial카탈로그 캐싱을 검토하세요.
listCatalog는 호출마다 ready 문서와 모든 chunk 행을 조인해 전량 로드합니다. 이전 MCP 경로에는 5분 TTL 캐시가 있었습니다(src/mcp/mcp-client.service.tsLine 49). 문서 수가 늘어나면 채팅 요청마다 DB 부하와 지연이 커집니다. TTL 캐시를 추가하고 업로드·삭제·재처리 시 무효화하는 방식을 검토하세요.🤖 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/retrieval/retrieval.service.ts` around lines 18 - 51, listCatalog currently reloads the full catalog on every call; add a 5-minute TTL cache around the retrieval result, reusing the existing MCP cache convention where available. Invalidate the cached catalog whenever upload, deletion, or reprocessing operations change ready documents or chunks, while preserving the current ListResourcesResult payload and logging behavior.src/chat/services/resource-selection.service.ts (1)
87-100: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win숫자 문자열 응답도 허용하세요.
현재 필터는
typeof value === 'number'만 통과시킵니다. LLM이["1", "3"]처럼 문자열 숫자를 반환하면 모든 항목이 탈락하고 선택 결과가 비어 답변이 근거 문서 없이 생성됩니다. 같은 파일의selectRelevantResourcePaths(Line 165-169)와selectMostRelevantDocuments(Line 250-254)는 정규식으로 숫자를 추출해 이 경우를 허용합니다. 처리 방식을 통일하세요.♻️ 제안 리팩터
const parsed = JSON.parse(selectedText) as unknown; - const numbers = Array.isArray(parsed) - ? parsed.filter( - (value): value is number => - typeof value === 'number' && - Number.isInteger(value) && - value >= 1 && - value <= candidates.length, - ) - : []; + const numbers = Array.isArray(parsed) + ? parsed + .map((value) => + typeof value === 'number' ? value : Number(value), + ) + .filter( + (value) => + Number.isInteger(value) && + value >= 1 && + value <= candidates.length, + ) + : [];🤖 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/chat/services/resource-selection.service.ts` around lines 87 - 100, Update the numeric filtering in the resource-selection flow around parsed and numbers to accept valid integer strings as well as numbers, matching the regex-based handling in selectRelevantResourcePaths and selectMostRelevantDocuments. Normalize accepted values to numeric indices before deduplication, slicing, and mapping, while preserving the existing range and maxResults constraints.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 71777cee-e1f7-4aa1-b76d-c6ee6e4d4ecd
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (72)
.env.exampledocker-compose.ymldrizzle/0009_goofy_starjammers.sqldrizzle/0010_gray_the_order.sqldrizzle/0011_odd_chimera.sqldrizzle/0012_brainy_dagger.sqldrizzle/meta/0009_snapshot.jsondrizzle/meta/0010_snapshot.jsondrizzle/meta/0011_snapshot.jsondrizzle/meta/0012_snapshot.jsondrizzle/meta/_journal.jsonpackage.jsonsrc/app.module.tssrc/chat/chat.module.tssrc/chat/llm/base-openai-compatible.llm.tssrc/chat/llm/letsur-llm.service.tssrc/chat/llm/llm-client.interface.tssrc/chat/llm/llm-client.provider.spec.tssrc/chat/llm/llm-client.provider.tssrc/chat/llm/open-router-llm.service.tssrc/chat/prompts/final-response.prompt.spec.tssrc/chat/prompts/final-response.prompt.tssrc/chat/prompts/index.tssrc/chat/prompts/pdf-chunking-prompt.tssrc/chat/prompts/pdf-processor.tssrc/chat/prompts/resource-path-selection.prompt.tssrc/chat/prompts/tool-selection.prompt.tssrc/chat/services/chat-orchestration.service.spec.tssrc/chat/services/chat-orchestration.service.tssrc/chat/services/chat-stream.transport.spec.tssrc/chat/services/chat-stream.transport.tssrc/chat/services/open-router.service.tssrc/chat/services/resource-content.service.spec.tssrc/chat/services/resource-content.service.tssrc/chat/services/resource-selection.service.spec.tssrc/chat/services/resource-selection.service.tssrc/chat/types/llm.types.tssrc/chat/types/open-router.types.tssrc/config/env.validation.spec.tssrc/config/env.validation.tssrc/db/schema.tssrc/mcp/mcp-client.service.tssrc/pdf-processor/documents.repository.tssrc/pdf-processor/gcs-storage.service.spec.tssrc/pdf-processor/gcs-storage.service.tssrc/pdf-processor/markdown-section-splitter.spec.tssrc/pdf-processor/markdown-section-splitter.tssrc/pdf-processor/mojibake.spec.tssrc/pdf-processor/mojibake.tssrc/pdf-processor/parse-finite-number.spec.tssrc/pdf-processor/parse-finite-number.tssrc/pdf-processor/pdf-chunk-parser.spec.tssrc/pdf-processor/pdf-chunk-parser.tssrc/pdf-processor/pdf-pipeline.service.spec.tssrc/pdf-processor/pdf-pipeline.service.tssrc/pdf-processor/pdf-processor.module.tssrc/pdf-processor/pdf-processor.worker.spec.tssrc/pdf-processor/pdf-processor.worker.tssrc/pdf-processor/pdf-text.service.tssrc/retrieval/retrieval.module.tssrc/retrieval/retrieval.repository.spec.tssrc/retrieval/retrieval.repository.tssrc/retrieval/retrieval.service.spec.tssrc/retrieval/retrieval.service.tssrc/retrieval/retrieval.types.tssrc/scripts/smoke-pdf-processor.tssrc/upload/dto/document-list-item.dto.tssrc/upload/dto/update-expires-at.dto.tssrc/upload/upload.controller.tssrc/upload/upload.module.tssrc/upload/upload.service.spec.tssrc/upload/upload.service.ts
💤 Files with no reviewable changes (3)
- src/chat/prompts/tool-selection.prompt.ts
- src/chat/types/open-router.types.ts
- src/chat/services/open-router.service.ts
…s to docker-compose
…in ChatStreamTransport
…d add tests for path normalization
…failures and improve error handling
…eakpoints enabled
…nts and enhance extension handling
feat: add organization-based document management
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (17)
src/db/migration-lock.spec.ts (2)
47-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win성공 경로에서 unlock 실패가 전파되는지 검증하는 테스트를 추가하세요.
이 테스트는 마이그레이션 실패 시 unlock 오류가 원래 오류를 덮지 않는 것을 확인합니다.
src/db/index.ts의 81행은 마이그레이션이 성공한 경우에는 unlock 오류를 전파합니다. 이 분기는 현재 검증되지 않습니다.💚 추가 테스트 제안
it('propagates an unlock failure when migration succeeds', async () => { const client: MigrationAdvisoryLockClient = { unsafe: jest.fn(async (query: string) => { if (query.includes('pg_advisory_unlock')) { throw new Error('unlock failed'); } }), }; await expect( withMigrationAdvisoryLock(client, async () => undefined), ).rejects.toThrow('unlock failed'); });🤖 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/db/migration-lock.spec.ts` around lines 47 - 62, Add a test alongside the existing migration-failure case that verifies withMigrationAdvisoryLock propagates an unlock failure when the migration callback succeeds. Configure the MigrationAdvisoryLockClient unsafe mock to fail only for pg_advisory_unlock, invoke withMigrationAdvisoryLock with a successful callback, and assert rejection with the unlock error.
64-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
describe블록 이름과 검증 대상이 일치하지 않습니다.이 두 테스트는
withReservedMigrationConnection을 검증합니다. 그러나 상위describe이름은withMigrationAdvisoryLock입니다. 테스트 실패 출력에서 대상 함수를 오해할 수 있습니다. 별도describe블록으로 분리하세요.🤖 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/db/migration-lock.spec.ts` around lines 64 - 101, Separate the tests for withReservedMigrationConnection into their own describe block instead of keeping them under withMigrationAdvisoryLock. Move both the operation-failure and reservation-failure cases together under the correctly named block so test output identifies the function being verified.src/upload/upload.controller.ts (1)
127-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win페이지네이션 검증 로직을 공용 헬퍼로 추출하세요.
동일한
limit/offset파싱과 검증 코드가listMyUploads(84-94행),listManageableDocuments(132-142행),OrganizationDocumentsController.list(447-457행)에 세 번 반복됩니다. 헬퍼 함수 또는 커스텀 파이프로 추출하면 검증 규칙이 한 곳에서 유지됩니다.♻️ 제안 리팩터
function parsePaging(limit?: string, offset?: string) { const limitNum = limit != null ? parseInt(limit, 10) : undefined; const offsetNum = offset != null ? parseInt(offset, 10) : undefined; if (limitNum != null && (Number.isNaN(limitNum) || limitNum < 1)) { throw new BadRequestException('limit must be a positive number'); } if (offsetNum != null && (Number.isNaN(offsetNum) || offsetNum < 0)) { throw new BadRequestException('offset must be a non-negative number'); } return { limit: limitNum, offset: offsetNum }; }🤖 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/upload/upload.controller.ts` around lines 127 - 147, Extract the duplicated limit/offset parsing and validation from listMyUploads, listManageableDocuments, and OrganizationDocumentsController.list into one shared helper or custom pipe. Reuse it in all three methods, preserving the existing positive-limit and non-negative-offset errors and returning the parsed pagination values.src/organizations/organization-access.service.spec.ts (1)
174-234: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMANAGER 역할 강제 테스트를 추가하세요.
OrganizationAccessService.requireOrganizationManager는 MEMBER 역할을ForbiddenException으로 거부합니다. 현재 spec은 이 분기를 검증하지 않습니다. 기본 mock의findAcceptedMembership이 MEMBER를 반환하므로 테스트 추가 비용은 낮습니다.💚 제안 테스트
+ it('rejects a MEMBER for manager-only organization actions', async () => { + const { service } = setup(); + await expect( + service.requireOrganizationManager(ORG_ID, principal()), + ).rejects.toBeInstanceOf(ForbiddenException); + });🤖 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/organizations/organization-access.service.spec.ts` around lines 174 - 234, Add a test covering the MEMBER-role branch of requireOrganizationManager: use the default findAcceptedMembership mock returning MEMBER, call requireOrganizationManager with a regular principal, and assert it rejects with ForbiddenException.src/upload/upload.service.ts (2)
97-112: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
getById에서 문서 접근 상태를 두 번 조회합니다.
getById는 L101에서requireDocumentView를 호출합니다. 그리고reauthorizeKnownDecisions=true로 인해toListItems가 L396에서 같은 문서에 대해requireDocumentView를 다시 호출합니다. 이는 단건 조회마다 문서 접근 상태 쿼리를 중복 실행합니다. 단건 조회 경로에서는 재인가를 생략하거나,getById가 초기 조회 없이toListItems의 재인가 결과만 사용하도록 정리하세요.Also applies to: 393-399
🤖 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/upload/upload.service.ts` around lines 97 - 112, Update getById and the toListItems invocation so the document access decision is fetched only once per single-document lookup: either disable reauthorization for this path while preserving the existing decision, or remove the initial requireDocumentView call and rely on toListItems’ authorization result. Keep the returned DocumentListItemDto behavior unchanged.
319-338: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win이전 작업은
canTransfer로 검사하세요.
transferDocument는requireDocumentShare를 호출하므로canShare만 확인합니다. 정책은canShare와canTransfer를 별도 필드로 정의하고, DTO도 두 값을 별도로 노출합니다. 현재 두 값이 동일하므로 지금은 권한 우회가 없습니다. 두 권한이 분기되면 이전 경로가 잘못된 권한으로 통과합니다. 전용 검사 메서드를 추가하세요.♻️ 제안 변경
src/organizations/organization-access.service.ts에 전용 검사를 추가합니다.async requireDocumentTransfer(documentId: string, principal: AdminPrincipal) { const access = await this.getDocumentAccess(documentId, principal); if (!access.canView) { throw new NotFoundException(`Document not found: ${documentId}`); } if (!access.canTransfer) { throw new ForbiddenException('Document transfer permission required'); } return access; }이후 이전 경로에서 이 메서드를 사용합니다.
- const decision = await this.access.requireDocumentShare(id, principal); + const decision = await this.access.requireDocumentTransfer(id, principal);🤖 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/upload/upload.service.ts` around lines 319 - 338, 전용 이전 권한 검사를 사용하도록 수정하세요. OrganizationAccessService의 getDocumentAccess를 기반으로 canView와 canTransfer를 검증하고 적절한 NotFoundException 또는 ForbiddenException을 발생시키는 requireDocumentTransfer 메서드를 추가한 뒤, transferDocument의 requireDocumentShare 호출을 이 메서드로 교체하세요.src/organizations/dto/organization.dto.ts (1)
17-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winslug 입력도 정규화하세요.
name은 L7에서 trim을 적용합니다.slug는 정규화가 없습니다. 사용자가Student-Support를 보내면 정규식 검증이 실패하고 400이 반환됩니다. 입력을 먼저 정규화하면 이 실패를 방지할 수 있습니다.♻️ 제안 변경
+ `@Transform`(({ value }) => + typeof value === 'string' ? value.trim().toLowerCase() : value, + ) `@IsString`() `@Matches`(/^[a-z0-9]+(?:-[a-z0-9]+)*$/) `@MaxLength`(255) slug: string;🤖 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/organizations/dto/organization.dto.ts` around lines 17 - 20, Normalize the slug input before validation in the organization DTO, matching the existing trim behavior used for name. Update the slug property’s transformation/validation flow so values such as “ Student-Support ” are trimmed before the existing IsString, Matches, and MaxLength checks run.src/organizations/organizations.service.ts (1)
308-325: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
isUniqueViolation헬퍼를 공통 모듈로 옮겨 중복을 제거하세요.
src/organizations/organizations.service.ts와src/upload/upload.service.ts가 PostgreSQL23505cause 체인 검사 로직을 각각 정의합니다. 이 헬퍼를src/organizations/organizations.repository.ts나 별도 유틸 모듈로 옮겨 두 서비스에서 공유하면 409 매핑 변경으로 인한 회귀를 방지할 수 있습니다.🤖 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/organizations/organizations.service.ts` around lines 308 - 325, Move the duplicated PostgreSQL 23505 cause-chain helper from organizations.service.ts into a shared utility module or organizations.repository.ts, then update both organizations.service.ts and upload.service.ts to import and reuse the shared isUniqueViolation implementation. Preserve its current bounded cause traversal and boolean behavior so existing 409 mappings remain unchanged.src/organizations/dto/membership.dto.ts (1)
16-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value허용 값 목록에는
@IsIn을 사용하세요.
ORGANIZATION_ROLES가 문자열 배열이므로@IsIn(ORGANIZATION_ROLES)로 변경하세요.@IsEnum도 동작하지만 공식 의도는 TypeScript enum 객체 대상 검증입니다.🤖 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/organizations/dto/membership.dto.ts` around lines 16 - 24, Update the role validation decorators in the DTOs shown, including the role property on UpdateOrganizationMemberDto and the corresponding membership DTO property, to use `@IsIn`(ORGANIZATION_ROLES) instead of `@IsEnum`. Keep the existing API metadata, defaults, and OrganizationRole types unchanged.src/organizations/organizations.repository.spec.ts (2)
42-47: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value슬라이스 범위가 메서드 선언 순서에 의존합니다.
43행은
async setShare위치부터 파일 끝까지를 잘라냅니다. 이 구간에는setShare,removeShare,transferDocument와 모든 private 헬퍼가 들어갑니다. 현재documentChunks와processingToken은enqueueDocumentReprocess와cancelAndSoftDeleteDocument에 있고, 두 메서드는setShare보다 위에 있습니다. 그래서 테스트가 통과합니다.
enqueueDocumentReprocess를 파일 아래쪽으로 옮기기만 해도 동작 변경 없이 이 테스트가 실패합니다. 변수명transferSection도 실제 범위와 맞지 않습니다. 검사 대상 메서드 본문만 추출하거나, 위 코멘트처럼 동작 검사로 전환하세요.🤖 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/organizations/organizations.repository.spec.ts` around lines 42 - 47, Update the test around transferSection so it inspects only the bodies of setShare, removeShare, and transferDocument (and their relevant private helpers), rather than slicing from async setShare to the file end. Ensure the assertions remain independent of method declaration order and continue verifying that transfer/share code does not reference documentChunks, queued status, or processingToken.
10-40: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win소스 문자열 검사 대신 동작 검사로 바꾸는 것을 권장합니다.
이 테스트는
organizations.repository.ts의 소스 텍스트를 읽고toContain으로 코드 문자열을 검사합니다. 특히 25-27행은 줄바꿈과 들여쓰기까지 포함한 문자열을 요구합니다. Prettier 재포맷, 변수명 변경, 인자 순서 변경만으로도 동작이 동일한데 테스트가 깨집니다. 반대로 잠금 순서가 잘못되어도 문자열만 남아 있으면 통과합니다.가짜 트랜잭션 객체(
tx) 또는 데이터베이스 e2e 테스트로 실제 잠금·재인증 순서를 검증하세요.test/organization-database.e2e-spec.ts의serializes concurrent mutations of different MANAGER rows테스트가 좋은 예입니다.🤖 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/organizations/organizations.repository.spec.ts` around lines 10 - 40, Replace the source-text assertions in the tests around the organization repository behaviors with runtime tests using a fake transaction object or database-backed setup. Exercise the final-manager flow, ownership transfer, and document-management reauthorization through their public repository methods, asserting actual lock ordering, conditional updates, share deletion, and authorization outcomes; use the concurrency test in organization-database.e2e-spec.ts as the model instead of matching formatted source strings.src/upload/dto/document-list-item.dto.ts (1)
168-169: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
accessRelationenum을 상수로 추출하는 것을 권장합니다.
status필드는DOCUMENT_STATUSES상수를 재사용합니다.accessRelation은 배열 리터럴과 유니온 타입을 각각 따로 작성합니다. 값이 두 곳에 중복되므로 한쪽만 수정하면 Swagger 문서와 타입이 어긋납니다.♻️ 제안 변경
+export const DOCUMENT_ACCESS_RELATIONS = ['OWNER', 'SHARED'] as const; +export type DocumentAccessRelation = + (typeof DOCUMENT_ACCESS_RELATIONS)[number];그다음 필드 선언을 상수와 연결합니다.
- `@ApiProperty`({ enum: ['OWNER', 'SHARED'] }) - accessRelation: 'OWNER' | 'SHARED'; + `@ApiProperty`({ enum: DOCUMENT_ACCESS_RELATIONS }) + accessRelation: DocumentAccessRelation;🤖 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/upload/dto/document-list-item.dto.ts` around lines 168 - 169, Extract the OWNER and SHARED values used by accessRelation into a shared constant, then reuse that constant for both the ApiProperty enum metadata and the accessRelation union/type declaration, following the existing DOCUMENT_STATUSES pattern so the Swagger schema and TypeScript type remain synchronized.test/organization-database.e2e-spec.ts (1)
405-416: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win테스트가 선언 순서에 의존합니다.
415행은
managerUuid의 ACCEPTED 멤버십이 정확히 2개라고 단언합니다. 그러나 418행의 다음 테스트가repo.createOrganization을 호출해 세 번째 MANAGER 멤버십을 만듭니다. 340행 테스트도 그 시점까지 다른 문서가 없다고 가정합니다.Jest는 기본적으로 선언 순서로 실행하므로 현재는 통과합니다.
--randomize옵션을 켜거나 테스트를 재배치하면 실패합니다. 각 테스트가 자기 데이터만 검증하도록 범위를 좁히세요. 예를 들어 415행은 조직 ID를 명시해 필터하세요.♻️ 제안 변경
.where( and( eq(organizationMemberships.memberIdpUuid, managerUuid), eq(organizationMemberships.status, 'ACCEPTED'), + inArray(organizationMemberships.organizationId, [ + sourceOrganizationId, + targetOrganizationId, + ]), ), ); expect(memberships).toHaveLength(2);🤖 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 `@test/organization-database.e2e-spec.ts` around lines 405 - 416, Update the test “supports one user as accepted MANAGER in multiple organizations” to scope its membership query to the specific organization IDs created by that test, rather than asserting all ACCEPTED memberships for managerUuid. Preserve the two-membership assertion while ensuring it remains independent of data created by neighboring tests such as repo.createOrganization.src/organizations/organizations.controller.ts (1)
96-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
@ApiParam선언을 일관되게 추가하세요.
listMembers와inviteMember는organizationId에@ApiParam({ format: 'uuid' })를 선언합니다.updateMember,removeMember,acceptInvitation,rejectInvitation은 선언하지 않습니다. 런타임 검증은ParseUUIDPipe가 수행하므로 동작은 정상입니다. Swagger 문서에서만uuid형식이 빠집니다.♻️ 제안 변경
`@Patch`('organizations/:organizationId/members/:membershipId') `@ApiOperation`({ summary: '조직 멤버 역할 변경' }) + `@ApiParam`({ name: 'organizationId', format: 'uuid' }) + `@ApiParam`({ name: 'membershipId', format: 'uuid' }) `@ApiResponse`({ status: 200, type: OrganizationMembershipDto })🤖 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/organizations/organizations.controller.ts` around lines 96 - 103, Add consistent Swagger `@ApiParam`({ format: 'uuid' }) metadata for the organizationId parameters in updateMember, removeMember, acceptInvitation, and rejectInvitation, matching listMembers and inviteMember while retaining the existing ParseUUIDPipe validation.test/organizations.e2e-spec.ts (2)
98-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win테스트가 프로덕션
ValidationPipe설정을 복제합니다.223행 테스트 이름은 "global validation contract"를 검증한다고 말합니다. 그러나 98-112행은 파이프 설정을 이 파일에 다시 작성합니다.
src/main.ts의 설정이 바뀌면 이 테스트는 계속 통과하지만 실제 계약은 달라집니다.
ValidationPipe설정을 공용 팩토리로 추출하고,main.ts와 이 테스트가 같은 팩토리를 사용하게 하세요. 팩토리 추출을 도와드릴까요?🤖 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 `@test/organizations.e2e-spec.ts` around lines 98 - 112, Extract the shared ValidationPipe configuration from the inline setup in the test into a reusable factory, then update both the production bootstrap in main.ts and the test setup to use that factory. Preserve the existing transform, whitelist, forbidNonWhitelisted, and exceptionFactory behavior so the “global validation contract” test exercises the actual production configuration.
120-248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win204 응답 계약을 검증하는 테스트가 없습니다.
이 스위트는
POST organizations,POST members,POST invitations/:id/accept만 호출합니다.organizations.controller.ts의 나머지 엔드포인트는 HTTP 수준 검증이 없습니다.
PATCH organizations/:organizationId/members/:membershipIdDELETE organizations/:organizationId/members/:membershipId—@HttpCode(204)DELETE organization-invitations/:membershipId—@HttpCode(204)GET organizations,GET organizations/:organizationId/members,GET organization-invitations두 DELETE 경로의
@HttpCode(HttpStatus.NO_CONTENT)는 이번 PR에서 새로 추가된 계약입니다. 컨트롤러 메서드가Promise<void>를 반환하므로 본문이 비어야 합니다. 최소한 이 두 경로의 상태 코드와 빈 본문을 검증하는 테스트를 추가하세요. 테스트 작성을 도와드릴까요?🤖 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 `@test/organizations.e2e-spec.ts` around lines 120 - 248, The organization E2E suite lacks HTTP contract coverage for the two new no-content deletion endpoints. Add tests for DELETE /api/v1/admin/organizations/:organizationId/members/:membershipId and DELETE /api/v1/admin/organization-invitations/:membershipId, asserting a 204 status, an empty response body, and the corresponding repository methods are called with the expected identifiers.src/organizations/organizations.repository.ts (1)
888-918: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
setShare는 대상 조직을 검증하지 않습니다.
setShare는input.targetOrganizationId를 외래 키 제약만 통과시킨 뒤 삽입합니다.transferDocument(957-990행)와 비교하면 두 가지가 빠집니다.
- 대상 조직 행을 잠그지 않습니다.
transferDocument는lockOrganizations로 소유 조직과 대상 조직을 함께 잠급니다.- 대상 조직이 소유 조직과 같은지 확인하지 않습니다. 소유 조직 자신에게 공유하면 의미 없는 공유 행이 생기고, 문서 목록의
sharedOrganizations에 소유 조직이 중복 표시됩니다.권한 모델상 소유 조직의 MANAGER가 외부로 공유를 결정하는 것은 정당합니다. 따라서 권한 문제는 아닙니다. 자기 조직 공유만 차단하세요.
♻️ 제안 변경
if (state.kind !== 'ok') return state; + if (input.targetOrganizationId === input.expectedOwnerOrganizationId) { + return state; + } await tx .insert(documentOrganizationShares)🤖 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/organizations/organizations.repository.ts` around lines 888 - 918, Update setShare to lock both expectedOwnerOrganizationId and targetOrganizationId via lockOrganizations before authorization, matching transferDocument’s locking behavior. Reject the operation when targetOrganizationId equals expectedOwnerOrganizationId, while preserving valid external sharing and existing authorization handling.
🤖 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 `@drizzle/0013_aberrant_alex_wilder.sql`:
- Line 2: Update migration 0013 so duplicate document_chunks rows sharing the
same document_id and path are removed before creating
document_chunks_document_id_path_unique; retain one row per pair, then create
the unique index.
In `@drizzle/0014_unique_dracula.sql`:
- Line 94: document_ownership_transfers의 document_id 외래 키 제약 조건에서 삭제 정책을 ON
DELETE restrict에서 ON DELETE cascade로 변경하세요. document_organization_shares의 기존
cascade 정책과 일관되게 설정하고, DocumentsRepository.hardDelete()가 문서와 관련 ownership
transfer를 함께 삭제하도록 유지하세요.
In `@src/chat/services/chat-stream.transport.spec.ts`:
- Line 81: Remove the unnecessary `as never` assertions from the
`consumeAndForward` calls in the test, including the calls assigned to
`consumePromise` and the corresponding invocation at the second reported
location. Pass the existing reply object directly, preserving the object-literal
parameter type expected by `consumeAndForward`.
In `@src/db/index.ts`:
- Around line 128-132: Update the migration flow around
withReservedMigrationConnection to pass a dedicated postgres migration client to
migrate instead of drizzle(connection). Create the migration-specific database
instance from migrationClient, preserving the existing schema configuration,
advisory-lock handling, and migrationsFolder options.
In `@src/organizations/organizations.repository.ts`:
- Around line 1049-1063: Replace the table-level SHARE lock in
isSoleNormalizedAdminIdentity with a row-scoped shared lock, and add the
proposed unique functional index on lower(trim(email)) through the appropriate
migration. Resolve or update the existing normalized-email collision setup in
organization-database.e2e-spec.ts so the migration succeeds while preserving the
invariant that each normalized email maps to one admin identity.
In `@src/pdf-processor/pdf-pipeline.service.spec.ts`:
- Line 160: pdf-pipeline.service.spec.ts의 해당 테스트에서 llmResponse를 설정하는
mockResolvedValueOnce 호출과 180-182행의 포맷을 Prettier 규칙에 맞게 정리하세요. 기존 테스트 동작은 유지하고
prettier --write 또는 동일한 자동 포맷을 적용해 prettier/prettier 린트 오류를 제거하세요.
In `@src/upload/upload.controller.ts`:
- Around line 137-142: Remove the unnecessary `as number` assertions from both
`offsetNum` usages in the validation logic, including the matching check near
the other `offset` handling. Keep the existing `Number.isNaN` and negative-value
validation behavior unchanged.
In `@test/organization-database.e2e-spec.ts`:
- Around line 93-95: Remove the unnecessary `as unknown as Database` assertions
from the `OrganizationsRepository`, `OrganizationAccessService`, and
`RetrievalRepository` initialization in the test setup, passing `db` directly
since `drizzle(client, { schema })` already returns a compatible type. Apply the
same change at the additional occurrence around line 246, and remove the
`Database` import if it becomes unused.
---
Nitpick comments:
In `@src/db/migration-lock.spec.ts`:
- Around line 47-62: Add a test alongside the existing migration-failure case
that verifies withMigrationAdvisoryLock propagates an unlock failure when the
migration callback succeeds. Configure the MigrationAdvisoryLockClient unsafe
mock to fail only for pg_advisory_unlock, invoke withMigrationAdvisoryLock with
a successful callback, and assert rejection with the unlock error.
- Around line 64-101: Separate the tests for withReservedMigrationConnection
into their own describe block instead of keeping them under
withMigrationAdvisoryLock. Move both the operation-failure and
reservation-failure cases together under the correctly named block so test
output identifies the function being verified.
In `@src/organizations/dto/membership.dto.ts`:
- Around line 16-24: Update the role validation decorators in the DTOs shown,
including the role property on UpdateOrganizationMemberDto and the corresponding
membership DTO property, to use `@IsIn`(ORGANIZATION_ROLES) instead of `@IsEnum`.
Keep the existing API metadata, defaults, and OrganizationRole types unchanged.
In `@src/organizations/dto/organization.dto.ts`:
- Around line 17-20: Normalize the slug input before validation in the
organization DTO, matching the existing trim behavior used for name. Update the
slug property’s transformation/validation flow so values such as “
Student-Support ” are trimmed before the existing IsString, Matches, and
MaxLength checks run.
In `@src/organizations/organization-access.service.spec.ts`:
- Around line 174-234: Add a test covering the MEMBER-role branch of
requireOrganizationManager: use the default findAcceptedMembership mock
returning MEMBER, call requireOrganizationManager with a regular principal, and
assert it rejects with ForbiddenException.
In `@src/organizations/organizations.controller.ts`:
- Around line 96-103: Add consistent Swagger `@ApiParam`({ format: 'uuid' })
metadata for the organizationId parameters in updateMember, removeMember,
acceptInvitation, and rejectInvitation, matching listMembers and inviteMember
while retaining the existing ParseUUIDPipe validation.
In `@src/organizations/organizations.repository.spec.ts`:
- Around line 42-47: Update the test around transferSection so it inspects only
the bodies of setShare, removeShare, and transferDocument (and their relevant
private helpers), rather than slicing from async setShare to the file end.
Ensure the assertions remain independent of method declaration order and
continue verifying that transfer/share code does not reference documentChunks,
queued status, or processingToken.
- Around line 10-40: Replace the source-text assertions in the tests around the
organization repository behaviors with runtime tests using a fake transaction
object or database-backed setup. Exercise the final-manager flow, ownership
transfer, and document-management reauthorization through their public
repository methods, asserting actual lock ordering, conditional updates, share
deletion, and authorization outcomes; use the concurrency test in
organization-database.e2e-spec.ts as the model instead of matching formatted
source strings.
In `@src/organizations/organizations.repository.ts`:
- Around line 888-918: Update setShare to lock both expectedOwnerOrganizationId
and targetOrganizationId via lockOrganizations before authorization, matching
transferDocument’s locking behavior. Reject the operation when
targetOrganizationId equals expectedOwnerOrganizationId, while preserving valid
external sharing and existing authorization handling.
In `@src/organizations/organizations.service.ts`:
- Around line 308-325: Move the duplicated PostgreSQL 23505 cause-chain helper
from organizations.service.ts into a shared utility module or
organizations.repository.ts, then update both organizations.service.ts and
upload.service.ts to import and reuse the shared isUniqueViolation
implementation. Preserve its current bounded cause traversal and boolean
behavior so existing 409 mappings remain unchanged.
In `@src/upload/dto/document-list-item.dto.ts`:
- Around line 168-169: Extract the OWNER and SHARED values used by
accessRelation into a shared constant, then reuse that constant for both the
ApiProperty enum metadata and the accessRelation union/type declaration,
following the existing DOCUMENT_STATUSES pattern so the Swagger schema and
TypeScript type remain synchronized.
In `@src/upload/upload.controller.ts`:
- Around line 127-147: Extract the duplicated limit/offset parsing and
validation from listMyUploads, listManageableDocuments, and
OrganizationDocumentsController.list into one shared helper or custom pipe.
Reuse it in all three methods, preserving the existing positive-limit and
non-negative-offset errors and returning the parsed pagination values.
In `@src/upload/upload.service.ts`:
- Around line 97-112: Update getById and the toListItems invocation so the
document access decision is fetched only once per single-document lookup: either
disable reauthorization for this path while preserving the existing decision, or
remove the initial requireDocumentView call and rely on toListItems’
authorization result. Keep the returned DocumentListItemDto behavior unchanged.
- Around line 319-338: 전용 이전 권한 검사를 사용하도록 수정하세요. OrganizationAccessService의
getDocumentAccess를 기반으로 canView와 canTransfer를 검증하고 적절한 NotFoundException 또는
ForbiddenException을 발생시키는 requireDocumentTransfer 메서드를 추가한 뒤, transferDocument의
requireDocumentShare 호출을 이 메서드로 교체하세요.
In `@test/organization-database.e2e-spec.ts`:
- Around line 405-416: Update the test “supports one user as accepted MANAGER in
multiple organizations” to scope its membership query to the specific
organization IDs created by that test, rather than asserting all ACCEPTED
memberships for managerUuid. Preserve the two-membership assertion while
ensuring it remains independent of data created by neighboring tests such as
repo.createOrganization.
In `@test/organizations.e2e-spec.ts`:
- Around line 98-112: Extract the shared ValidationPipe configuration from the
inline setup in the test into a reusable factory, then update both the
production bootstrap in main.ts and the test setup to use that factory. Preserve
the existing transform, whitelist, forbidNonWhitelisted, and exceptionFactory
behavior so the “global validation contract” test exercises the actual
production configuration.
- Around line 120-248: The organization E2E suite lacks HTTP contract coverage
for the two new no-content deletion endpoints. Add tests for DELETE
/api/v1/admin/organizations/:organizationId/members/:membershipId and DELETE
/api/v1/admin/organization-invitations/:membershipId, asserting a 204 status, an
empty response body, and the corresponding repository methods are called with
the expected identifiers.
🪄 Autofix
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: 5e6c7d32-cd70-4904-b0ec-8c2181e9198f
📒 Files selected for processing (47)
docker-compose.ymldrizzle/0013_aberrant_alex_wilder.sqldrizzle/0014_unique_dracula.sqldrizzle/meta/0013_snapshot.jsondrizzle/meta/0014_snapshot.jsondrizzle/meta/_journal.jsonsrc/app.module.tssrc/chat/services/chat-stream.transport.spec.tssrc/chat/services/chat-stream.transport.tssrc/db/index.tssrc/db/migration-lock.spec.tssrc/db/organizations.schema.spec.tssrc/db/schema.tssrc/main.tssrc/organizations/dto/membership.dto.tssrc/organizations/dto/organization.dto.tssrc/organizations/organization-access.policy.tssrc/organizations/organization-access.service.spec.tssrc/organizations/organization-access.service.tssrc/organizations/organization.types.tssrc/organizations/organizations.controller.tssrc/organizations/organizations.module.tssrc/organizations/organizations.repository.spec.tssrc/organizations/organizations.repository.tssrc/organizations/organizations.service.spec.tssrc/organizations/organizations.service.tssrc/pdf-processor/documents.repository.tssrc/pdf-processor/gcs-storage.service.spec.tssrc/pdf-processor/gcs-storage.service.tssrc/pdf-processor/pdf-chunk-parser.spec.tssrc/pdf-processor/pdf-chunk-parser.tssrc/pdf-processor/pdf-pipeline.service.spec.tssrc/pdf-processor/pdf-pipeline.service.tssrc/pdf-processor/pdf-processor.worker.spec.tssrc/pdf-processor/pdf-processor.worker.tssrc/retrieval/retrieval.repository.spec.tssrc/retrieval/retrieval.service.spec.tssrc/retrieval/retrieval.service.tssrc/upload/dto/document-list-item.dto.spec.tssrc/upload/dto/document-list-item.dto.tssrc/upload/dto/transfer-document.dto.tssrc/upload/upload.controller.tssrc/upload/upload.module.tssrc/upload/upload.service.spec.tssrc/upload/upload.service.tstest/organization-database.e2e-spec.tstest/organizations.e2e-spec.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- src/app.module.ts
- src/retrieval/retrieval.service.spec.ts
- src/retrieval/retrieval.repository.spec.ts
- src/upload/upload.module.ts
- src/pdf-processor/pdf-chunk-parser.spec.ts
- src/retrieval/retrieval.service.ts
- src/pdf-processor/pdf-chunk-parser.ts
- src/pdf-processor/pdf-pipeline.service.ts
- src/chat/services/chat-stream.transport.ts
| @@ -0,0 +1,2 @@ | |||
| CREATE INDEX "document_chunks_path_idx" ON "document_chunks" USING btree ("path");--> statement-breakpoint | |||
| CREATE UNIQUE INDEX "document_chunks_document_id_path_unique" ON "document_chunks" USING btree ("document_id","path"); No newline at end of file | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# document_chunks 삽입 경로와 중복 처리 방식을 확인한다.
set -euo pipefail
rg -n --type=ts -C 6 'documentChunks' src | head -200Repository: gsainfoteam/chatbot-be
Length of output: 12619
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== migration =="
cat -n drizzle/0013_aberrant_alex_wilder.sql
echo
echo "== schema relevant =="
sed -n '324,348p' src/db/schema.ts | cat -n
echo
echo "== document chunk insert/reprocess paths =="
sed -n '170,235p' src/pdf-processor/documents.repository.ts | cat -n
echo
echo "== organizations delete chunks path =="
sed -n '860,888p' src/organizations/organizations.repository.ts | cat -n
echo
echo "== all documentChunks insert/delete/upsert occurrences =="
rg -n --type=ts \
"insert\\(documentChunks\\)|delete\\(documentChunks\\)|sql`|onConflict|onConflictDoUpdate|replaceChunks|setShare|setDocumentStatus" srcRepository: gsainfoteam/chatbot-be
Length of output: 4968
중복 document_id + path를 제거한 후 유니크 인덱스를 추가하세요.
현재 completeProcessing()은 문서별 청크를 먼저 삭제한 뒤 배치 삽입하므로 재처리 충돌보다는 기존 데이터의 중복 (document_id, path)가 CREATE UNIQUE INDEX 실패 원인이 됩니다. 마이그레이션 전에 중복이 없으면 배포해도 되고, 중복이 있을 수 있으면 인덱스 생성 전 정리 문장을 추가하세요.
🤖 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 `@drizzle/0013_aberrant_alex_wilder.sql` at line 2, Update migration 0013 so
duplicate document_chunks rows sharing the same document_id and path are removed
before creating document_chunks_document_id_path_unique; retain one row per
pair, then create the unique index.
| ALTER TABLE "documents" ALTER COLUMN "owner_organization_id" SET NOT NULL;--> statement-breakpoint | ||
| ALTER TABLE "document_organization_shares" ADD CONSTRAINT "document_organization_shares_document_id_documents_id_fk" FOREIGN KEY ("document_id") REFERENCES "public"."documents"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "document_organization_shares" ADD CONSTRAINT "document_organization_shares_organization_id_organizations_id_fk" FOREIGN KEY ("organization_id") REFERENCES "public"."organizations"("id") ON DELETE restrict ON UPDATE no action;--> statement-breakpoint | ||
| ALTER TABLE "document_ownership_transfers" ADD CONSTRAINT "document_ownership_transfers_document_id_documents_id_fk" FOREIGN KEY ("document_id") REFERENCES "public"."documents"("id") ON DELETE restrict ON UPDATE no action;--> statement-breakpoint |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# documents 테이블의 하드 삭제 경로를 확인한다.
set -euo pipefail
rg -n --type=ts -C 6 'delete\(documents\)|from\(documents\)\s*\)|\.delete\(' src | head -200Repository: gsainfoteam/chatbot-be
Length of output: 13940
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== migration constraints around document_ownership_transfers =="
sed -n '1,140p' drizzle/0014_unique_dracula.sql | nl -ba
echo
echo "== upload/delete controller and service outlines =="
fd -a 'upload\.(controller|service)\.ts$' src | sed 's#^\./##' | while read -r f; do
echo "--- $f"
ast-grep outline "$f" --match delete --view compact || true
done
echo
echo "== relevant source snippets =="
sed -n '340,410p' src/upload/upload.controller.ts | nl -ba
sed -n '1,220p' src/upload/upload.service.ts | nl -ba
echo
echo "== repository hardDelete usages =="
rg -n -C 4 'hardDelete\(' src | sed -n '1,220p'
echo
echo "== all delete(documents) occurrences =="
rg -n -C 4 'delete\(documents\)' src drizzle | sed -n '1,220p'Repository: gsainfoteam/chatbot-be
Length of output: 264
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== migration constraints around document_ownership_transfers =="
sed -n '1,140p' drizzle/0014_unique_dracula.sql
echo
echo "== upload/delete controller snippets =="
sed -n '340,410p' src/upload/upload.controller.ts
echo
echo "== upload service snippets =="
sed -n '1,220p' src/upload/upload.service.ts
echo
echo "== hardDelete usages =="
rg -n -C 4 'hardDelete\(' src
echo
echo "== delete(documents) occurrences =="
rg -n -C 4 'delete\(documents\)' src drizzleRepository: gsainfoteam/chatbot-be
Length of output: 19218
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== rollbackUpload references =="
rg -n -C 6 'rollbackUpload|rollback' src/upload/upload.service.ts
echo
echo "== DocumentsRepository source around chunk ownership and soft delete =="
sed -n '1,260p' src/pdf-processor/documents.repository.ts
echo
echo "== OrganizationsRepository around cancelAndSoftDeleteDocument =="
sed -n '820,920p' src/organizations/organizations.repository.ts
echo
echo "== ownership transfer insert context =="
rg -n -C 8 'documentOwnershipTransfers|ownershipTransfers|ownership transfer|transfer' src | sed -n '1,260p'Repository: gsainfoteam/chatbot-be
Length of output: 29483
document_ownership_transfers 삭제 정책을 보존으로 설정하세요.
이 마이그레이션은 document_organization_shares.document_id를 ON DELETE cascade로 설정하지만, document_ownership_transfers.document_id는 ON DELETE restrict로 남깁니다. 파일 삭제 시 DocumentsRepository.hardDelete()가 실행되므로, 해당 문서의 ownership transfer가 있으면 외래 키 위반으로 로그 롤백도 실패합니다. 감사 기록만 남길 수 없다면 cascade를 사용하세요.
🤖 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 `@drizzle/0014_unique_dracula.sql` at line 94, document_ownership_transfers의
document_id 외래 키 제약 조건에서 삭제 정책을 ON DELETE restrict에서 ON DELETE cascade로 변경하세요.
document_organization_shares의 기존 cascade 정책과 일관되게 설정하고,
DocumentsRepository.hardDelete()가 문서와 관련 ownership transfer를 함께 삭제하도록 유지하세요.
| raw: { write: jest.fn(), end: jest.fn(), writableEnded: false }, | ||
| }; | ||
| const stream = new PassThrough(); | ||
| const consumePromise = transport.consumeAndForward(stream, reply as never); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
불필요한 as never 단언을 제거하세요.
ESLint가 81행과 104행을 @typescript-eslint/no-unnecessary-type-assertion 오류로 보고합니다. consumeAndForward의 두 번째 매개변수가 이 객체 리터럴 타입을 그대로 받습니다.
Also applies to: 104-104
🧰 Tools
🪛 ESLint
[error] 81-81: This assertion is unnecessary since the receiver accepts the original type of the expression.
(@typescript-eslint/no-unnecessary-type-assertion)
🤖 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/chat/services/chat-stream.transport.spec.ts` at line 81, Remove the
unnecessary `as never` assertions from the `consumeAndForward` calls in the
test, including the calls assigned to `consumePromise` and the corresponding
invocation at the second reported location. Pass the existing reply object
directly, preserving the object-literal parameter type expected by
`consumeAndForward`.
Source: Linters/SAST tools
| it('falls back to the section title when metadata path traverses upward', async () => { | ||
| const callLLM = jest | ||
| .fn<(...args: unknown[]) => Promise<LlmResponse>>() | ||
| .mockResolvedValueOnce(llmResponse(`## 안전한 제목\n\n${'본문 '.repeat(700)}`)) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Prettier 오류를 수정해 주세요.
ESLint는 Line 160 및 Line 180-182에서 prettier/prettier 오류를 보고합니다. 이 오류를 수정하지 않으면 린트 단계가 실패할 수 있습니다. prettier --write를 실행하거나 아래 형식을 적용해 주세요.
수정 예시
- .mockResolvedValueOnce(llmResponse(`## 안전한 제목\n\n${'본문 '.repeat(700)}`))
+ .mockResolvedValueOnce(
+ llmResponse(`## 안전한 제목\n\n${'본문 '.repeat(700)}`),
+ )
- expect(Object.keys(result.documents).some((path) => path.includes('..'))).toBe(
- false,
- );
+ expect(
+ Object.keys(result.documents).some((path) => path.includes('..')),
+ ).toBe(false);Also applies to: 180-182
🧰 Tools
🪛 ESLint
[error] 160-160: Replace llmResponse(##·안전한·제목\n\n${'본문·'.repeat(700)}) with ⏎········llmResponse(##·안전한·제목\n\n${'본문·'.repeat(700)}),⏎······
(prettier/prettier)
🤖 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/pdf-processor/pdf-pipeline.service.spec.ts` at line 160,
pdf-pipeline.service.spec.ts의 해당 테스트에서 llmResponse를 설정하는 mockResolvedValueOnce
호출과 180-182행의 포맷을 Prettier 규칙에 맞게 정리하세요. 기존 테스트 동작은 유지하고 prettier --write 또는 동일한
자동 포맷을 적용해 prettier/prettier 린트 오류를 제거하세요.
Source: Linters/SAST tools
| if ( | ||
| offset != null && | ||
| (Number.isNaN(offsetNum as number) || (offsetNum as number) < 0) | ||
| ) { | ||
| throw new BadRequestException('offset must be a non-negative number'); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
불필요한 타입 어설션을 제거하세요.
ESLint가 139행과 454행의 offsetNum as number를 @typescript-eslint/no-unnecessary-type-assertion 오류로 보고합니다. Number.isNaN은 원래 타입을 그대로 받습니다. 오류 규칙이므로 CI가 실패합니다. 454행의 동일한 코드에도 같은 수정을 적용하세요.
🔧 제안 수정
if (
offset != null &&
- (Number.isNaN(offsetNum as number) || (offsetNum as number) < 0)
+ (Number.isNaN(offsetNum) || (offsetNum as number) < 0)
) {
throw new BadRequestException('offset must be a non-negative number');
}🧰 Tools
🪛 ESLint
[error] 139-139: This assertion is unnecessary since the receiver accepts the original type of the expression.
(@typescript-eslint/no-unnecessary-type-assertion)
🤖 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/upload/upload.controller.ts` around lines 137 - 142, Remove the
unnecessary `as number` assertions from both `offsetNum` usages in the
validation logic, including the matching check near the other `offset` handling.
Keep the existing `Number.isNaN` and negative-value validation behavior
unchanged.
Source: Linters/SAST tools
| repo = new OrganizationsRepository(db as unknown as Database); | ||
| access = new OrganizationAccessService(repo); | ||
| retrievalRepo = new RetrievalRepository(db as unknown as Database); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
불필요한 타입 단언을 제거하세요.
ESLint가 93, 95, 246행의 as unknown as Database 단언을 @typescript-eslint/no-unnecessary-type-assertion 오류로 보고합니다. drizzle(client, { schema })의 반환 타입은 이미 Database와 호환됩니다. 린트 게이트가 있으면 CI가 실패합니다.
♻️ 제안 변경
- repo = new OrganizationsRepository(db as unknown as Database);
+ repo = new OrganizationsRepository(db);
access = new OrganizationAccessService(repo);
- retrievalRepo = new RetrievalRepository(db as unknown as Database);
+ retrievalRepo = new RetrievalRepository(db);246행도 같이 변경합니다.
- const documentsRepo = new DocumentsRepository(db as unknown as Database);
+ const documentsRepo = new DocumentsRepository(db);Database 타입 임포트가 더 이상 쓰이지 않으면 22행 임포트 목록에서 제거하세요.
🧰 Tools
🪛 ESLint
[error] 93-93: This assertion is unnecessary since it does not change the type of the expression.
(@typescript-eslint/no-unnecessary-type-assertion)
[error] 95-95: This assertion is unnecessary since it does not change the type of the expression.
(@typescript-eslint/no-unnecessary-type-assertion)
🤖 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 `@test/organization-database.e2e-spec.ts` around lines 93 - 95, Remove the
unnecessary `as unknown as Database` assertions from the
`OrganizationsRepository`, `OrganizationAccessService`, and
`RetrievalRepository` initialization in the test setup, passing `db` directly
since `drizzle(client, { schema })` already returns a compatible type. Apply the
same change at the additional occurrence around line 246, and remove the
`Database` import if it becomes unused.
Source: Linters/SAST tools
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/db/index.ts (1)
133-139: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueadvisory lock 세션 유실 가능성을 확인하세요.
pg_advisory_lock은 세션 범위 잠금입니다.max: 1풀은 잠금과migrate()를 같은 세션에서 실행하지만, postgres.js가 연결 손실 후 자동 재연결하면 잠금이 조용히 해제됩니다. 이때 두 인스턴스가 동시에 마이그레이션을 실행할 수 있습니다. 운영 환경에서 이 시나리오를 허용할지 확인하세요. 필요하면migrate()직전에pg_advisory_lock보유 여부를pg_locks로 재확인하는 방어 로직을 추가하세요.🤖 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/db/index.ts` around lines 133 - 139, Review the migration flow around migrate() and its single-connection migrationClient pool to handle session loss during automatic reconnects. Immediately before migrate(), verify via pg_locks that the current session still holds the pg_advisory_lock; if it does not, reacquire the lock or abort migration according to the existing failure policy, preventing concurrent migrations after connection loss.
🤖 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.
Nitpick comments:
In `@src/db/index.ts`:
- Around line 133-139: Review the migration flow around migrate() and its
single-connection migrationClient pool to handle session loss during automatic
reconnects. Immediately before migrate(), verify via pg_locks that the current
session still holds the pg_advisory_lock; if it does not, reacquire the lock or
abort migration according to the existing failure policy, preventing concurrent
migrations after connection loss.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c52ded55-f442-42c7-89c9-70fccd719eca
📒 Files selected for processing (7)
drizzle/0015_marvelous_lady_mastermind.sqldrizzle/meta/0015_snapshot.jsondrizzle/meta/_journal.jsonsrc/db/index.tssrc/db/schema.tssrc/organizations/organizations.repository.tstest/organization-database.e2e-spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- drizzle/meta/_journal.json
- src/db/schema.ts
- src/organizations/organizations.repository.ts
- test/organization-database.e2e-spec.ts
2 participants
Summary by CodeRabbit
새 기능
버그 수정
테스트