Skip to content

[REFAC#417] fetcher/parser/validate stage 패키지 구조 정합화 - #418

Merged
juhy0987 merged 5 commits into
mainfrom
refactor/#417/stage-structure-alignment
May 13, 2026
Merged

juhy0987 merged 5 commits into
mainfrom
refactor/#417/stage-structure-alignment

Conversation

@juhy0987

@juhy0987 juhy0987 commented May 13, 2026

Copy link
Copy Markdown
Member

연관 이슈

Closes #417

구현 내용

세 stage (fetcher / parser / validate) 의 비일관 패키지 구조를 canonical 패턴으로 정합. 라이브 동작 영향 0 — 순수 패키지 위치 / import 경로 변경.

Canonical 구조

<stage>/
  stage.go              # Stage adapter (top-level, processor.Stage 인터페이스 구현)
  worker/               # worker pool 구현 + 부속 helpers
  types/                # 도메인 인터페이스 + 결과 타입 (cyclic-free leaf)
  <domain>|<rule>/      # 도메인 / rule engine

Phase A — validate worker 서브디렉토리화

Before After
validate/worker.go (621라인, top-level) validate/worker/worker.go (package worker)
validate/validator.go (NewValidator dispatcher / RunValidation) validate/worker/validator.go
validate/reparse.go (IsReparseEligible / readReparseCount) validate/worker/reparse.go
stageName 내부 상수 worker.StageName (exported) — stage.go / worker.go 공유

validate 부모 패키지에는 stage.go 만 잔존 — Stage 가 *worker.Worker 보유.

Phase B — parser/stage 평탄화

  • parser/stage/stage.goparser/stage.go (top-level, package parser)
  • 외부 import: parser/stage.NewStageparser.NewStage

Phase C — parser 인터페이스 진입점 → types/

  • parser/parser.go (interfaces) → parser/types/types.go (package types)
  • 외부 호출자 4 파일 갱신 — parser.Page / LinkItem / ContentParser / LinkListParsertypes.X

cyclic 회피 효과: parser 부모 (이제 stage.go) 가 rule/* 를 import 해도 사이클 없음 (rule/* 가 parser/types 만 import).

cmd / test 갱신

  • cmd/issuetracker/main.go: parserStage import 제거 + parser.NewStage 직접 / validateWorkerPkg alias 추가
  • cmd/processor/main.go: validateWorker alias 도입 → worker.NewWorker 호출
  • 테스트: validate/worker_test.go / reparse_test.go / processor_test.govalidate/worker/ 서브디렉토리 (package worker_test)
  • 신규 helpers_test.gonewNewsContent / newCommunityContent fixture 복사 (sub-package 분리로 cross-package 접근 불가)

fetcher 예외

fetcher/core/ 는 http_client / retry / errors 등 helpers 다중 책임 — 단순 types/ 로 축소 불가. 의도된 foundation 패키지로 잔존. canonical 패턴의 stage.go / worker/ 부분은 일치.

도달 상태

stage stage.go worker/ types/
fetcher ✓ top core/ (확장 foundation)
parser ✓ top (flatten) ✓ types/ (interfaces)
validate ✓ top ✓ (new) ✓ types/ (interfaces)

CI / 머지 게이트 점검

  • gofmt -l — clean
  • go build ./internal/... ./cmd/... ./pkg/... ./test/... ./examples/... — pass
  • go test -race -count=1 -timeout=180s ./test/... — 전 패키지 통과 (회귀 0)
  • PR 타이틀 [REFAC#417]
  • commit [REFAC]: prefix + 한국어

변경 영향 범위 + 위험도

  • 영향: 16 파일 (8 rename + 8 modify)
  • 위험도 Medium → Low (동작 동등성 확보):
    • 순수 위치 / import 경로 변경 — 비즈니스 로직 미수정
    • 모든 기존 테스트 새 위치 / 새 import 로 통과
    • cmd / test wiring 동기 갱신

롤백 계획

PR revert 시 16 파일 동시 원복 — 디렉토리 / 패키지 / import 경로 모두 동시에 이전 상태 복귀.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor

    • Reorganized parser and validation internals to a clearer package layout and consistent stage/worker naming.
    • Standardized parsing outputs and content DTOs across the pipeline.
    • Streamlined worker construction and stage wiring without changing runtime behavior.
  • Tests

    • Updated and added deterministic test helpers and test packages to match the new layout; test logic and assertions preserved.

Review Change Stack

세 stage 의 비일관 구조 → canonical 패턴 정합:

  <stage>/
    stage.go              # Stage adapter (top-level)
    worker/               # worker pool 구현 + 부속 helpers
    types/                # 도메인 인터페이스 + 결과 타입 (cyclic-free leaf)
    <domain>|<rule>/      # 도메인 / rule engine

## Phase A — validate worker 서브디렉토리화

이동 (모두 package validate → worker):
- validate/worker.go         → validate/worker/worker.go
- validate/validator.go      → validate/worker/validator.go
- validate/reparse.go        → validate/worker/reparse.go

validate 부모 패키지에는 stage.go 만 잔존 — Stage 가 *worker.Worker 보유.

stageName 상수 → worker.StageName (exported) — stage.go 와 worker.go 가 공유.

## Phase B — parser/stage 평탄화

- parser/stage/stage.go → parser/stage.go (top-level, package parser)
- cyclic 회피: parser 부모가 더 이상 rule/* 를 import 해도 사이클 없음 (interfaces 가 types/ 로 이동 후)

## Phase C — parser 인터페이스 진입점

- parser/parser.go → parser/types/types.go (package types)
- 외부 호출자 4 파일 갱신 (parser/rule/parser.go / discovery.go, parser/worker/parser_worker.go, fetcher/domain/general/convert.go)
- parser.Page / LinkItem / ContentParser / LinkListParser → types.X 일괄 치환

## cmd / test 갱신

- cmd/issuetracker: parserStage import 제거 + parser.NewStage 직접 호출 / validateWorkerPkg alias 추가
- cmd/processor: validateWorker import alias 도입 + worker.NewWorker 호출
- test/internal/processor/validate/{worker_test, reparse_test, processor_test}.go → worker/ 서브디렉토리 이동, package worker_test
- 신규 helpers_test.go — newNewsContent / newCommunityContent fixture 복사 (sub-package 분리로 cross-package 접근 불가)

## fetcher 예외

fetcher/core/ 는 http_client / retry / errors 등 helpers 다중 책임 보유 — types/ 로 단순 축소
불가. 의도된 foundation 패키지로 잔존. canonical 패턴의 stage.go / worker/ 부분은 일치.

## 도달 상태

| stage | stage.go | worker/ | types/ |
|---|---|---|---|
| fetcher | ✓ top | ✓ | core/ (확장 foundation) |
| parser | ✓ top (flatten) | ✓ | ✓ types/ (interfaces) |
| validate | ✓ top | ✓ (new) | ✓ types/ (interfaces) |

## 검증

- go build ./internal/... ./cmd/... ./test/... ./pkg/... ./examples/... — pass
- go test -race -count=1 -timeout=180s ./test/... — 전 패키지 통과 (회귀 0)
- gofmt clean
- 라이브 동작 영향 0 — 순수 패키지 위치 / import 경로 변경

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 13, 2026 12:10
@juhy0987 juhy0987 added the refactor Code refactoring label May 13, 2026
@coderabbitai

coderabbitai Bot commented May 13, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

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

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ 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: 7e040258-2f9f-47b0-9280-6ed7213167c9

📥 Commits

Reviewing files that changed from the base of the PR and between 6e3c420 and 08a9da7.

📒 Files selected for processing (6)
  • internal/processor/validate/domain/community/validator.go
  • internal/processor/validate/domain/news/validator.go
  • internal/processor/validate/stage.go
  • internal/processor/validate/worker/validator.go
  • test/internal/processor/validate/domain/community/validator_test.go
  • test/internal/processor/validate/domain/news/validator_test.go
📝 Walkthrough

Walkthrough

Parser types move to parser/types, parser stage is flattened to parser, validate worker code moves into validate/worker (exporting StageName), and CLI code plus tests are updated to use the new packages and types.

Changes

Parser and Validate Package Restructuring

Layer / File(s) Summary
Parser types foundation and stage relocation
internal/processor/parser/types/types.go, internal/processor/parser/stage.go
Parser types package renamed from parser to types; stage file flattened from parser/stage/ to parser/ with package parser.
Parser rule implementation switching to new types
internal/processor/parser/rule/parser.go
ParsePage and ParseLinks signatures now use types models; internal allocations and compile-time assertions updated to types.ContentParser / types.LinkListParser.
Parser discovery and worker layer using types
internal/processor/parser/rule/discovery.go, internal/processor/parser/worker/parser_worker.go
PageLinkDiscovery and parser worker helper functions now use types.LinkItem and *types.Page in signatures and construction.
Fetcher domain adapting to parser types
internal/processor/fetcher/domain/general/convert.go
ConvertPage signature updated to accept *types.Page instead of *parser.Page.
Validate worker extraction to subdirectory
internal/processor/validate/worker/worker.go, internal/processor/validate/worker/validator.go, internal/processor/validate/worker/reparse.go
Validate implementation relocated into validate/worker package; package declarations updated and exported StageName constant introduced; Start uses StageName for logging/config.
Validate stage integration with restructured worker
internal/processor/validate/stage.go
Stage adapter now accepts *worker.Worker and returns worker.StageName from Stage.Name().
Main orchestration wiring and CLI updates
cmd/issuetracker/main.go, cmd/processor/main.go
CLI entrypoints updated imports and constructor calls to use parser.NewStage and validate/worker.NewWorker.
Validate test suite updates and fixtures
test/internal/processor/validate/worker/*
Tests switched packages to *_test, imports updated to validate/worker, test calls updated to worker.*, and deterministic helper fixtures added.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped through packages, names aligned,
Types found new burrows where they can hide,
Workers moved rooms and called their StageName,
Tests sip fixed tea — no fleeting time to blame,
A tidy forest of paths, neat and wide.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.90% 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 clearly references REFAC#417 and summarizes the main change: aligning package structure across fetcher/parser/validate stages.
Linked Issues check ✅ Passed All Phase A (validate worker subdir), Phase B (parser/stage flatten), and Phase C (parser/types interfaces) objectives from issue #417 are met; all files moved, packages updated, imports corrected, and canonical structure applied across all stages.
Out of Scope Changes check ✅ Passed All changes align with issue #417 objectives: package structure alignment, file relocations, import updates, test package reorganization, and helper fixture duplication—no unrelated or extraneous modifications detected.

✏️ 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 refactor/#417/stage-structure-alignment

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

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

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
internal/processor/validate/worker/validator.go (1)

1-6: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update package comment to match the actual package name.

The package comment on line 1 still says "Package validate" but the package declaration is worker. This creates documentation inconsistency in godoc.

📝 Proposed fix
-// Package validate 는 Content 검증 처리 단계를 구현합니다.
+// Package worker 는 Content 검증 처리 단계를 구현합니다.
 //
-// Package validate implements the content validation stage of the processing pipeline.
+// Package worker implements the content validation stage of the processing pipeline.
 // It dispatches to source-type-specific validators (news, community) via NewValidator.
 // Worker 가 Validator 결과를 직접 사용 — 별도 ContentProcessor 어댑터 없음.
 package worker
🤖 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 `@internal/processor/validate/worker/validator.go` around lines 1 - 6, Update
the top-of-file package comment in validator.go so it matches the declared
package name `worker` (e.g., begin the doc comment with "Package worker ..."
instead of "Package validate"), keeping the existing descriptive text
(Korean/English) intact and formatted as a proper package comment for godoc.
🧹 Nitpick comments (1)
test/internal/processor/validate/worker/helpers_test.go (1)

29-29: ⚡ Quick win

Consider using fixed timestamps in test fixtures for determinism.

Both newNewsContent() and newCommunityContent() use time.Now() to generate PublishedAt values. This introduces non-determinism that could lead to flaky tests, especially for time-sensitive validations.

For better test reproducibility, consider using a fixed timestamp like time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC).

🕒 Suggested improvement
+var testPublishedAt = time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC)
+
 func newNewsContent() *core.Content {
 	return &core.Content{
 		ID:          "content-001",
 		...
-		PublishedAt: time.Now(),
+		PublishedAt: testPublishedAt,
 		...
 	}
 }

 func newCommunityContent() *core.Content {
 	return &core.Content{
 		ID:          "content-002",
 		...
-		PublishedAt: time.Now(),
+		PublishedAt: testPublishedAt,
 		...
 	}
 }

Also applies to: 47-47

🤖 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/internal/processor/validate/worker/helpers_test.go` at line 29, Replace
non-deterministic time.Now() usage in the test fixtures by setting PublishedAt
to a fixed timestamp to avoid flaky tests: update the functions newNewsContent
and newCommunityContent to use a constant like time.Date(2024, 1, 1, 12, 0, 0,
0, time.UTC) for the PublishedAt field (instead of time.Now()) so tests are
deterministic and reproducible.
🤖 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.

Outside diff comments:
In `@internal/processor/validate/worker/validator.go`:
- Around line 1-6: Update the top-of-file package comment in validator.go so it
matches the declared package name `worker` (e.g., begin the doc comment with
"Package worker ..." instead of "Package validate"), keeping the existing
descriptive text (Korean/English) intact and formatted as a proper package
comment for godoc.

---

Nitpick comments:
In `@test/internal/processor/validate/worker/helpers_test.go`:
- Line 29: Replace non-deterministic time.Now() usage in the test fixtures by
setting PublishedAt to a fixed timestamp to avoid flaky tests: update the
functions newNewsContent and newCommunityContent to use a constant like
time.Date(2024, 1, 1, 12, 0, 0, 0, time.UTC) for the PublishedAt field (instead
of time.Now()) so tests are deterministic and reproducible.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 80915296-284e-4cd7-b200-9d1bf43583b2

📥 Commits

Reviewing files that changed from the base of the PR and between 91ea414 and 33cf892.

📒 Files selected for processing (16)
  • cmd/issuetracker/main.go
  • cmd/processor/main.go
  • internal/processor/fetcher/domain/general/convert.go
  • internal/processor/parser/rule/discovery.go
  • internal/processor/parser/rule/parser.go
  • internal/processor/parser/stage.go
  • internal/processor/parser/types/types.go
  • internal/processor/parser/worker/parser_worker.go
  • internal/processor/validate/stage.go
  • internal/processor/validate/worker/reparse.go
  • internal/processor/validate/worker/validator.go
  • internal/processor/validate/worker/worker.go
  • test/internal/processor/validate/worker/helpers_test.go
  • test/internal/processor/validate/worker/processor_test.go
  • test/internal/processor/validate/worker/reparse_test.go
  • test/internal/processor/validate/worker/worker_test.go

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the parser and validate packages to resolve circular dependencies and improve project structure. Key changes include moving core interfaces and models to new types sub-packages and relocating worker logic to worker sub-packages. Additionally, test fixtures were consolidated within the worker test package to maintain accessibility while avoiding code duplication. The reviewer suggested consolidating test helper functions within the same package to improve reusability, which is a sound recommendation for Go testing patterns.

Comment thread test/internal/processor/validate/worker/helpers_test.go
…성 (CodeRabbit)

CodeRabbit 피드백 2건:

1. validator.go 의 \"// Package validate\" 주석이 실제 \"package worker\" 선언과 불일치 →
   godoc 표시 일관성을 위해 \"// Package worker\" 로 정정.

2. helpers_test.go 의 newNewsContent / newCommunityContent 가 time.Now() 사용 — 시간-민감한
   validation 분기에서 flaky test 위험. 공유 fixed timestamp (2024-01-01T12:00:00Z) 로 변경.

gemini 피드백 (helpers_test 위치) 는 이미 same-package *_test.go 패턴 적용 중 — 별도
testutil 패키지 아님. validate_test (parent) 와의 cross-package 중복은 Go 패키지 경계상
testutil 없이는 불가피. 본 PR scope 외 — 추후 community/news 테스트 재배치 시 추가 정리 가능.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@juhy0987 juhy0987 self-assigned this May 13, 2026
juhy0987 and others added 3 commits May 13, 2026 22:14
테스트 디렉토리 구조 규칙 (.claude/rules/05-testing.md) 일관성 — 소스가 sub-package 면
테스트도 동일 sub-package 미러:

- test/internal/processor/validate/community_validator_test.go
  → test/internal/processor/validate/community/validator_test.go (package community_test)
- test/internal/processor/validate/news_validator_test.go
  → test/internal/processor/validate/news/validator_test.go (package news_test)

각 sub-validator 테스트가 자기 패키지의 leaf test 로 위치 — 부모 validate_test 패키지가
더 이상 sub-validator 직접 테스트 보유 안 함. 신규 sub-validator 추가 시 자연스럽게
\`<sub>/validator_test.go\` 패턴 적용.

각 테스트는 자체 helper (newCommunityContent / newNewsContent) 를 로컬 유지 — 다른
sub-package 와 cross-package 공유 불가능 (Go 패키지 boundary). worker/helpers_test.go
의 사본은 worker 통합 테스트 fixture 로 그대로 잔존.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… (PR #418 추가)

fetcher 가 domain/general / domain/search 패턴을 사용하는 것과 일관 — validate 의 source-type
별 검증기도 동일하게 domain/ 하위로 이동.

이동:
- internal/processor/validate/community → internal/processor/validate/domain/community
- internal/processor/validate/news      → internal/processor/validate/domain/news
- test 미러도 동일 경로 갱신

import path 갱신:
- internal/processor/validate/worker/validator.go (NewValidator dispatcher)
- test/.../validate/domain/{community,news}/validator_test.go

이전 평면 sub-package 보다 \"도메인 grouping\" 의도가 더 명확 — 신규 source type 추가 시도
domain/<new>/validator.go 로 자연스럽게 확장. fetcher 와 패턴 일치.

stage.go doc 도 신구조 반영.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[REFACTOR] fetcher/parser/validate stage 패키지 구조 정합화

2 participants