[FEAT#237] LLM selector 검증 실패 시 raw content 재큐잉

[juhy0987]

연관 이슈

Closes #237

구현 내용

문제

LLM 자동 룰 생성 과정에서 validateSelectors() 실패 시 해당 raw content는 TTL(1h) 만료 후 cleanup에 의해 소멸됩니다. LLM이 다음 요청에서 룰 생성에 성공해도 이미 버려진 raw는 재파싱 기회를 얻지 못합니다.

해결

validate 실패 시 RawContentRef를 issuetracker.fetched에 재발행 → 다음 파싱 시도 시 룰이 생성되어 있으면 정상 처리됩니다.

변경 파일 (4 commits)

1. core.RawContentRef — LLMRetryCount int 추가

• omitempty JSON 태그 → zero-value 메시지 하위 호환
• 기존 메시지 파싱에 영향 없음

2. llmgen/generator.go

• selectorValidationError sentinel 타입 추가 — LLM API 실패 등 다른 에러와 구분
• SetValidateFailureHandler(fn func(ctx, raw, retryCount)) 메서드 추가
• Enqueue() 시그니처에 llmRetryCount int 파라미터 추가
• validate 실패 시 goroutine 내에서 handler 호출

3. parser_worker/parser_worker.go

• processMessage → processCategoryPage / processArticlePage → handleRuleError → llmGen.Enqueue까지 llmRetryCount 전파
• RequeueForLLMRetry(ctx, raw, llmRetryCount) 공개 메서드 추가
  • nextCount > maxLLMRetries(3) 시 재큐잉 중단 (무한루프 방지)
  • RawContentRef{LLMRetryCount: nextCount}를 TopicFetched에 발행

4. cmd/issuetracker/main.go

• llmGen.SetValidateFailureHandler(pw.RequeueForLLMRetry) wiring

테스트

test/internal/processor/parser/worker/requeue_test.go 신규:

• TestRequeueForLLMRetry_PublishesWithIncrementedCount — 정상 재큐 + LLMRetryCount +1 검증
• TestRequeueForLLMRetry_RespectsMaxRetries — maxLLMRetries 초과 시 publish 없음
• TestRequeueForLLMRetry_SecondRetry — 두 번째 재큐 시 LLMRetryCount=2 검증

CI / 머지 게이트

• 빌드: make build 통과
• 테스트: go test -race ./... 전체 통과
• 기존 llmgen 테스트: Enqueue 시그니처 변경에 맞게 갱신

변경 영향 범위

• Enqueue() 시그니처 변경 → 기존 호출처 1곳(handleRuleError) + 테스트 갱신 완료
• RawContentRef 필드 추가 → omitempty로 기존 Kafka 메시지 하위 호환
• validate 실패 시 재큐 최대 3회 → Kafka 메시지 증가 폭 미미

롤백 계획

llmGen.SetValidateFailureHandler 호출 제거 시 즉시 이전 동작으로 복귀합니다.

Summary by CodeRabbit

• New Features
  • Improved reliability: LLM content parsing now automatically retries failed validations up to 3 times, enhancing robustness when using AI-powered parsing rules.

[coderabbitai]

Warning

Rate limit exceeded

@juhy0987 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 25 minutes and 18 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 58abd7bb-39d5-4e22-95be-732ae3f008ee

📥 Commits

Reviewing files that changed from the base of the PR and between 329f5fd and 0bf80f5.

📒 Files selected for processing (4)

• internal/processor/parser/rule/llmgen/generator.go
• internal/processor/parser/worker/parser_worker.go
• test/internal/processor/parser/rule/llmgen/generator_test.go
• test/internal/processor/parser/worker/requeue_test.go

📝 Walkthrough

Walkthrough

The PR implements an LLM selector validation failure retry mechanism. RawContentRef gains an LLMRetryCount field. Generator.SetValidateFailureHandler registers a callback invoked on selector-validation errors. The parser worker threads retry counts through message processing and exports RequeueForLLMRetry to republish failed content to the queue with an incremented count, capped at 3 retries. Main wires the handler to connect validation failures to requeue logic.

Changes

LLM Selector Validation Retry

Layer / File(s)	Summary
Data Model internal/processor/fetcher/core/models.go	RawContentRef adds LLMRetryCount field (JSON llm_retry_count,omitempty) to track LLM requeue attempts.
Core Validation Handler internal/processor/parser/rule/llmgen/generator.go	New sentinel error type selectorValidationError wraps validation failures. Generator.SetValidateFailureHandler registers a callback. Enqueue signature extended to accept llmRetryCount; async handler invokes callback on validation failures.
Worker Integration internal/processor/parser/worker/parser_worker.go	Thread ref.LLMRetryCount through per-message processing into category/article handlers. New maxLLMRetries = 3 constant and exported method RequeueForLLMRetry republish raw content to queue.TopicFetched with incremented LLMRetryCount, aborting with warn log once limit exceeded. handleRuleError passes retry count to LLM enqueue call.
Application Wiring cmd/issuetracker/main.go	Conditionally wire llmGen.SetValidateFailureHandler(pw.RequeueForLLMRetry) when LLM is enabled, connecting generator validation failures to parser worker requeue logic.
Test Helpers test/internal/processor/parser/worker/helpers_test.go	New newMinimalWorker helper constructs a minimal ParserWorker from a queue.Producer and *logger.Logger for RequeueForLLMRetry testing.
Tests & Test Coverage test/internal/processor/parser/rule/llmgen/generator_test.go, test/internal/processor/parser/worker/requeue_test.go	Updated all g.Enqueue(...) call sites to pass llmRetryCount argument (0). New test file validates RequeueForLLMRetry: incrementing count, respecting max retries, and multi-retry scenarios.

Sequence Diagram

sequenceDiagram
    participant Parser as ParserWorker
    participant Gen as Generator
    participant Handler as ValidateFailureHandler
    participant Queue as Queue (TopicFetched)

    rect rgba(200, 150, 100, 0.5)
    Note over Parser,Queue: Selector Validation Failure Path

    Parser->>Gen: Enqueue(raw, llmRetryCount=0)
    Gen->>Gen: validateSelectors(html)
    Gen->>Gen: Validation fails → wrap selectorValidationError
    
    Note over Gen: Async error handling
    Gen->>Handler: SetValidateFailureHandler callback invoked
    Handler->>Handler: RequeueForLLMRetry(ctx, raw, llmRetryCount=0)
    
    alt Retry count < maxLLMRetries (3)
        Handler->>Handler: Increment count: LLMRetryCount=1
        Handler->>Queue: Publish RawContentRef with LLMRetryCount=1
        Note over Queue: Raw re-queued for reprocessing
    else Retry count >= maxLLMRetries
        Handler->>Handler: Warn log, abort (no requeue)
    end
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• [FEAT#190] 점진적 정밀화 트리거 + hybrid 합성 (이슈 #173 단계 4-2 완성) #191: Modifies LLM wiring in cmd/issuetracker/main.go to refactor and build the shared LLM provider and refiner components, overlapping with this PR's main.go handler wiring.
• [FEAT#149] LLM 자동 parsing rule 생성 wiring (1차: cmd + llmgen + parser_worker fallback) #168: Extends the earlier LLM auto-rule wiring by building on the same llmgen.Generator and parser worker codepaths, directly related to the retry count field and handler integration patterns.
• [REFAC#206] processor.Stage 인터페이스 도입 + ContentProcessor 제거 + main.go []Stage 패턴 #207: Refactors main.go and introduces a parser Stage that centralizes ParserWorker and llmGen wiring, overlapping on the same parser/LLM handler responsibilities.

Poem

🐰 A selector stumbles, validation falls,
No fear—the queue recalls!
Three chances to make it right,
Retry by night, then fade from sight.
Hop, requeue, and try once more! 🔄

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title is in Korean and references FEAT#237, directly corresponding to linked issue #237, clearly indicating the main feature: LLM selector validation failure triggering raw content requeuing.
Linked Issues check	✅ Passed	The PR successfully implements all coding requirements from issue #237: adds LLMRetryCount to RawContentRef [#237], implements validate-failure sentinel error and callback in Generator [#237], propagates retry count through parser_worker and provides RequeueForLLMRetry with maxLLMRetries limit [#237], and wires callback in main.go [#237].
Out of Scope Changes check	✅ Passed	All changes are tightly scoped to the LLM selector validation requeue feature: RawContentRef field addition, Generator callback mechanism, parser_worker requeue logic, test additions, and main.go integration represent cohesive implementation of issue #237 with no extraneous modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feature/#237/llm-validate-fail-requeue

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 0/1 reviews remaining, refill in 25 minutes and 18 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[juhy0987]

자동 코드 리뷰 결과

Minor — validateFailureHandler 필드 동기화 부재

SetValidateFailureHandler 는 g.validateFailureHandler 에 직접 쓰고, 해당 필드는 Enqueue 가 spawn 한 goroutine 에서 읽힙니다.
현재 계약(Stop 전 초기화 1회)이 지켜지는 한 실제 race 는 발생하지 않으나, atomic.Pointer[func(...)]] 나 초기화 이후 불변으로 명시하는 방식으로 보강하면 data race detector 에 안전하게 통과됩니다.

그 외 버그·보안·아키텍처·성능 항목 특이 사항 없음.

[gemini-code-assist]

Code Review

This pull request implements a retry mechanism for LLM-generated selectors that fail validation (Issue #237). It introduces a LLMRetryCount field to RawContentRef, a sentinel error type for validation failures, and a callback mechanism in the Generator to trigger requeueing of raw content back to the fetched topic. The feedback highlights a critical issue where metadata such as crawlerName and targetType is lost during the requeue process, which would cause category pages to be incorrectly processed as articles. It also recommends using context.WithoutCancel for asynchronous requeue paths to preserve observability metadata and ensuring that requeue operations return errors to support at-least-once processing.

[Copilot]

Pull request overview

이 PR은 LLM 자동 룰 생성 과정에서 validateSelectors()가 실패할 경우 raw content가 TTL 만료로 소멸되어 재처리 기회를 잃는 문제를 해결하기 위해, selector 검증 실패 시 raw reference를 issuetracker.fetched로 재발행(재큐잉) 하도록 파이프라인을 확장합니다.

Changes:

• core.RawContentRef에 LLMRetryCount를 추가해 재큐잉 횟수를 메시지로 전파
• llmgen.Generator에 selector 검증 실패 전용 sentinel error 및 실패 콜백(wiring) 추가
• parser_worker에서 llmRetryCount를 전파하고, validate 실패 시 TopicFetched로 재발행하는 API 및 테스트 추가

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
internal/processor/fetcher/core/models.go	RawContentRef에 LLMRetryCount 필드 추가(하위 호환용 omitempty)
internal/processor/parser/rule/llmgen/generator.go	validate 실패를 구분하는 sentinel error + 실패 콜백 등록/호출 + Enqueue 시그니처 확장
internal/processor/parser/worker/parser_worker.go	llmRetryCount 전파 및 validate 실패 시 raw requeue 메서드 추가
cmd/issuetracker/main.go	llmGen.SetValidateFailureHandler(pw.RequeueForLLMRetry) wiring 추가
test/internal/processor/parser/worker/requeue_test.go	재큐잉 동작(카운트 증가/최대 재시도 제한) 테스트 신규 추가
test/internal/processor/parser/worker/helpers_test.go	ParserWorker 최소 구성 생성 헬퍼 추가
test/internal/processor/parser/rule/llmgen/generator_test.go	Enqueue(..., llmRetryCount) 시그니처 변경에 맞춘 테스트 갱신

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

test/internal/processor/parser/rule/llmgen/generator_test.go (1)

213-232: ⚡ Quick win

Add a direct test for the new validate-failure callback path.

These cases now compile against the new Enqueue(..., llmRetryCount) signature, but nothing here proves that SetValidateFailureHandler fires on selector-validation failures or stays silent for provider/JSON failures. That leaves the main behavior added in this PR unprotected. A small test that registers a handler, uses a non-zero retry count, and asserts the callback receives the original raw and retry count would lock this down.

As per coding guidelines, "Go 핵심 패키지는 최소 70% 테스트 커버리지, 크롤러/처리 로직은 90%, 에러 핸들링은 100%".

Also applies to: 234-250

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/internal/processor/parser/rule/llmgen/generator_test.go` around lines
213 - 232, The test currently verifies no INSERT on selector-validation failure
but doesn't assert the new validate-failure callback path; add a small subtest
(or modify TestGenerator_Enqueue_ValidationFailure_NoInsert and the similar
234-250 case) that registers a SetValidateFailureHandler handler, calls
g.Enqueue with a non-zero llmRetryCount, and asserts the handler was invoked
exactly once with the original *core.RawContent and the same retry count (and
still that repo.inserts() is empty); use the existing fakeProvider,
recordingRepo, and samplePageHTML to trigger the selector validation failure and
capture the callback invocation for verification.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@internal/processor/parser/worker/parser_worker.go`:
- Around line 546-595: RequeueForLLMRetry drops routing headers so requeued
messages lose target_type; update the queue.Message before calling
w.producer.Publish to include the original routing metadata (at minimum set
msg.Headers["target_type"] = raw.SourceInfo.TargetType or the equivalent field
used by the fetcher), preserving any existing headers if present, so
TopicFetched consumers dispatch using the same target_type as the original
message.

---

Nitpick comments:
In `@test/internal/processor/parser/rule/llmgen/generator_test.go`:
- Around line 213-232: The test currently verifies no INSERT on
selector-validation failure but doesn't assert the new validate-failure callback
path; add a small subtest (or modify
TestGenerator_Enqueue_ValidationFailure_NoInsert and the similar 234-250 case)
that registers a SetValidateFailureHandler handler, calls g.Enqueue with a
non-zero llmRetryCount, and asserts the handler was invoked exactly once with
the original *core.RawContent and the same retry count (and still that
repo.inserts() is empty); use the existing fakeProvider, recordingRepo, and
samplePageHTML to trigger the selector validation failure and capture the
callback invocation for verification.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cb04d4b5-54b2-4a6a-afc9-e56802034c2f

📥 Commits

Reviewing files that changed from the base of the PR and between 504610a and 329f5fd.

📒 Files selected for processing (7)

• cmd/issuetracker/main.go
• internal/processor/fetcher/core/models.go
• internal/processor/parser/rule/llmgen/generator.go
• internal/processor/parser/worker/parser_worker.go
• test/internal/processor/parser/rule/llmgen/generator_test.go
• test/internal/processor/parser/worker/helpers_test.go
• test/internal/processor/parser/worker/requeue_test.go