[REFAC#148] LinkDiscovery ArticleURLPattern optional + 사이트 rule all-pass 전환

[juhy0987]

연관 이슈

• Closes [REFACTOR] LinkDiscovery 의 ArticleURLPattern 을 optional 로 — 기사만이 아닌 페이지 내 모든 글 수집 #148

이슈 #100 / #139 후속. 사이트별 "기사 URL" regex 강제로 인해 누락되던 비-article 컨텐츠
(공지/이벤트/시리즈/오피니언/멀티미디어 등) 를 모두 수집하도록 LinkDiscovery 의 pattern 을 optional 로 강등.

구현 내용

1. PageLinkDiscovery 컴포넌트 — pattern optional

• internal/crawler/parser/rule/discovery.go
• cfg.ArticleURLPattern 빈 문자열 시 regex compile/매칭 단계 skip
• pkg/links.Extractor 의 SameOriginOnly / PathPrefixes / ExcludePatterns / MaxLinksPerPage 만으로 필터
• 0건 매칭 메시지를 모드별 분기 (pattern stale vs page-empty)

2. rule.Parser ParseLinks 분기 완화

• internal/crawler/parser/rule/parser.go
• 기존: cfg != nil && cfg.ArticleURLPattern != \"\" → discovery
• 신규: cfg != nil → discovery (빈 pattern 도 all-pass 모드)
• ItemContainer fallback 은 LinkDiscovery 객체 자체가 nil 일 때만

3. Migration 008 — 운영 사이트 rules all-pass 전환

• naver/daum/yonhap/cnn × list rule = 4 rows UPDATE
• article_url_pattern 빈 문자열로 갱신
• exclude_patterns 강화 — 사이트별 광고/공유/네비/미디어/about/login 등 노이즈 컷
• max_links_per_page=200, same_origin_only site profile 유지

5. MaxLinksPerPage 우선순위 정책 (추가 commit)

사용자 피드백 반영 — same-origin 우선 + cross-origin random sample 로 변경:

1. same-origin (raw.URL host 와 동일) 링크는 maxOut 무시하고 모두 통과
2. cross-origin 은 잔여 슬롯 (maxOut - len(same)) 만큼 math/rand/v2.Shuffle 로 무작위 sample
3. maxOut == 0 (무제한) 이면 cross 도 모두 통과

이유:

• 사이트 자체 컨텐츠 (same) 는 noise 적고 가치 높음 — 모두 발행
• 외부 링크 (cross) 는 광고/제휴 noise 많음 — cap 통제 + 무작위 sample 로 특정 영역 (페이지 상단 sponsored slot 등) 편향 회피

4. 테스트 갱신 (3건)

• TestPageLinkDiscovery_EmptyPattern_AllPass — 빈 pattern 으로 모든 same-origin 링크 통과 + 기본 제외 (login 등) 차단 검증
• TestPageLinkDiscovery_EmptyPattern_ExcludeStillApplies — all-pass 모드에서 ExcludePatterns 동작
• TestParser_ParseLinks_LinkDiscoveryWithEmptyPattern_AllPassDiscovery — 객체만 있고 pattern 비었을 때 all-pass discovery 진입 (ItemContainer fallback X)
• 기존 TestPageLinkDiscovery_EmptyPattern_ReturnsEmptySelector 는 의미 변경에 따라 위 케이스로 대체

CI / 머지 게이트 점검

변경 영향 범위

• 영향 패키지/모듈:
  • 변경: internal/crawler/parser/rule/{discovery.go, parser.go}
  • 신규: migrations/{up,down}/008_*.sql
  • 테스트 변경: test/internal/parser/rule/discovery_test.go
• 위험도: Medium
  • 코드 변경은 작지만 라이브 동작이 "매칭 가능 article 만" → "모든 의미 있는 링크" 로 확장됨
  • publish 수가 사이트별 2~5배 증가 가능 → backlog throttle ([FEATURE] Scheduler 가 Kafka backlog 임계값 초과 시 URL 발행 차단 #124) / URL cache ([FEAT#126] Publisher 에 URL dedup 적용 — Kafka 큐잉 단계에서 중복 URL 사전 필터링 #127) / JobLocker ([FEATURE] Publisher 에 URL dedup 적용 — Kafka 큐잉 단계에서 중복 URL 사전 필터링 #126) 가 흡수
  • validator reject 비율 증가 가능 — 라이브 모니터링 후 ExcludePatterns 점진 정밀화

Required Status Checks

• Commit Lint
• PR Title Lint
• Linked Issue Check
• Format Check
• Build
• Test
• Lint

배포 순서

1. migration 008 적용 (production DB 의 4개 list rule UPDATE)
2. 코드 deploy
3. 라이브 모니터링:
   • 카테고리 페이지의 chained article jobs published 에서 url_count 증가 추이
   • validator reject 비율 — noise 흡수 여부
   • Kafka backlog — issuetracker.crawl.normal 토픽 lag

롤백 계획

1. 코드 revert: git revert (4 commits, 역순)
2. migration 008 down 적용 — 007 의 좁은 ArticleURLPattern 으로 복원
3. 즉시 이전 동작 회귀 (rule cache TTL 5분 이내 자동 적용)

후속 작업 (별도 PR)

• 라이브 측정 후 사이트별 ExcludePatterns 추가 정밀화
• LLM ([FEATURE] rule.Parser ErrNoRule fallback — LLM 자동 규칙 생성 wiring (이슈 #100 follow-up) #149) 도입 시 noise 분류기 정확도 측정

Summary by CodeRabbit

• Bug Fixes

  • Improved link discovery mechanism to support all-pass filtering mode, allowing exclusion patterns to work independently of URL pattern matching.

• Chores

  • Updated parsing rules for Naver, Daum, Yonhap, and CNN to use relaxed link discovery patterns with enhanced exclusion-based filtering.

[coderabbitai]

Warning

Rate limit exceeded

@juhy0987 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 46 minutes and 36 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: bb4e3ef2-f37b-42be-a88e-e60e23074639

📥 Commits

Reviewing files that changed from the base of the PR and between 4cfc9a4 and fc95aec.

📒 Files selected for processing (4)

• internal/crawler/parser/rule/discovery.go
• internal/storage/parsing_rule.go
• migrations/up/008_relax_link_discovery_patterns.sql
• test/internal/parser/rule/discovery_test.go

📝 Walkthrough

Walkthrough

Make ArticleURLPattern optional in LinkDiscovery by allowing empty string patterns, enabling collection of all links on a page with filtering via exclude patterns. Code paths updated to treat LinkDiscovery presence as the sole condition for discovery routing, removing the pattern-emptiness check.

Changes

Cohort / File(s)	Summary
Rule Discovery Logic internal/crawler/parser/rule/discovery.go, internal/crawler/parser/rule/parser.go	Modified to conditionally compile and apply regex filtering only when ArticleURLPattern is non-empty; empty pattern triggers "all-pass" discovery mode. Parser now routes to discovery based solely on LinkDiscovery presence, not pattern value.
Database Migrations migrations/up/008_relax_link_discovery_patterns.sql, migrations/down/008_relax_link_discovery_patterns.sql	Migration 008 sets article_url_pattern to empty string for four news hosts (naver, daum, yonhap, cnn) and configures site-specific exclude_patterns, max_links_per_page (200), and same_origin_only flags; down migration reverts changes.
Test Updates test/internal/parser/rule/discovery_test.go	Updated discovery tests to verify "all-pass" mode behavior when ArticleURLPattern is empty; added assertions for ExcludePatterns, SameOriginOnly, and MaxLinksPerPage enforcement; modified parser routing expectations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• [REFAC#100] 파싱 규칙 DB 일원화 — 단일 page parser engine (모든 웹페이지 도메인 중립) #145: Directly modifies ParseLinks and LinkDiscovery routing logic in the same discovery vs. container fallback decision path.

Suggested labels

enhancement

Poem

🐰 A rabbit hops through patterns wild and free,
No rigid URLs to constrain what we see,
All links now welcome on every page,
Exclude the noise—let truth engage! ✨

🚥 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
Title check	✅ Passed	The title references REFAC#148 and describes the main change: making ArticleURLPattern optional and migrating site rules to all-pass mode.
Linked Issues check	✅ Passed	All code requirements from #148 are met: ArticleURLPattern is optional, discovery.go skips regex when empty, parser.go calls discovery when LinkDiscovery exists, migration 008 updates four site rules with empty patterns and enhanced exclude_patterns, and tests validate all-pass behavior.
Out of Scope Changes check	✅ Passed	All changes directly support issue #148 objectives: code modifications enable optional ArticleURLPattern, migrations implement all-pass mode for four sites, and tests verify new behavior. No unrelated changes detected.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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/#148/optional-article-url-pattern

Warning

Review ran into problems

🔥 Problems

Timed out fetching pipeline failures after 30000ms

---

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 46 minutes and 36 seconds.}

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

[gemini-code-assist]

Code Review

This pull request implements an 'all-pass' mode for link discovery, enabling the crawler to capture all links when ArticleURLPattern is empty. It includes logic updates to skip regex filtering, refined exclusion patterns in database migrations for major news sites, and updated unit tests. Feedback indicates that related documentation in LinkDiscoveryConfig should be updated to reflect these changes and that a comment in the migration script incorrectly describes the same_origin_only setting.

[coderabbitai]

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)

internal/crawler/parser/rule/parser.go (1)

163-170: ⚠️ Potential issue | 🟡 Minor

Update the fallback error text to match the new routing.

This branch now runs only when LinkDiscovery is nil, so pointing operators at LinkDiscovery.ArticleURLPattern is misleading. If they hit this error, they need the whole link_discovery block, not just the pattern field.

Proposed text tweak

-			Message:    "list rule missing required ItemContainer or ItemLink selector (or set LinkDiscovery.ArticleURLPattern)",
+			Message:    "list rule missing required ItemContainer or ItemLink selector (or set LinkDiscovery)",

🤖 Prompt for AI Agents

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

In `@internal/crawler/parser/rule/parser.go` around lines 163 - 170, The error
message returned when both rule.Selectors.ItemContainer and
rule.Selectors.ItemLink are missing (the Error with Code ErrEmptySelector)
incorrectly points users to LinkDiscovery.ArticleURLPattern even though this
branch only runs when LinkDiscovery is nil; update the Error.Message text to
instruct operators to provide the full link_discovery block (not just
ArticleURLPattern) so it accurately reflects the required configuration when
LinkDiscovery is absent.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@test/internal/parser/rule/discovery_test.go`:
- Around line 308-324: The test must prove ParseLinks used LinkDiscovery rather
than falling back to ItemContainer: modify
TestParser_ParseLinks_LinkDiscoveryWithEmptyPattern_AllPassDiscovery so the
fakeRepo or repo spy records/guards ItemContainer calls (e.g., have fakeRepo's
ItemContainer method panic or set a flag if invoked) and assert it was not
called after calling rule.NewParser(...).ParseLinks; alternatively make
r.Selectors.ItemContainer an impossible selector that cannot match listHTML so
only LinkDiscovery can produce the three links. Ensure you reference ParseLinks,
TestParser_ParseLinks_LinkDiscoveryWithEmptyPattern_AllPassDiscovery,
LinkDiscoveryConfig, fakeRepo and ItemContainer in the change.
- Around line 186-229: Extend the empty-pattern tests to assert that
PathPrefixes still filter links when ArticleURLPattern is disabled: in
TestPageLinkDiscovery_EmptyPattern_ExcludeStillApplies (or add a new similar
test) set cfg.PathPrefixes (e.g., []string{"/article/", "/news/"}) along with
ArticleURLPattern:"" and SameOriginOnly:true, call d.Discover(makeRaw(...), cfg)
and then assert that no returned item.URL has any of the configured
PathPrefixes; reference NewPageLinkDiscovery, Discover and
storage.LinkDiscoveryConfig to locate where to add this assertion.

---

Outside diff comments:
In `@internal/crawler/parser/rule/parser.go`:
- Around line 163-170: The error message returned when both
rule.Selectors.ItemContainer and rule.Selectors.ItemLink are missing (the Error
with Code ErrEmptySelector) incorrectly points users to
LinkDiscovery.ArticleURLPattern even though this branch only runs when
LinkDiscovery is nil; update the Error.Message text to instruct operators to
provide the full link_discovery block (not just ArticleURLPattern) so it
accurately reflects the required configuration when LinkDiscovery is absent.

🪄 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: 041050c3-adab-4a1c-b7af-2fb1ad4c76e1

📥 Commits

Reviewing files that changed from the base of the PR and between 0833e0c and 4cfc9a4.

📒 Files selected for processing (5)

• internal/crawler/parser/rule/discovery.go
• internal/crawler/parser/rule/parser.go
• migrations/down/008_relax_link_discovery_patterns.sql
• migrations/up/008_relax_link_discovery_patterns.sql
• test/internal/parser/rule/discovery_test.go