From b0aa4c5bcc324af1fea32c34349cde410f46cc2c Mon Sep 17 00:00:00 2001 From: changukyu Date: Fri, 18 Sep 2026 23:53:02 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20=EC=9C=84=ED=97=98=EB=8F=84=20=EC=A0=90?= =?UTF-8?q?=EC=88=98=20v3=20=E2=80=94=20=EC=A0=9C=EA=B3=B5=EC=9E=90?= =?UTF-8?q?=EB=B3=84=20=ED=92=80=EB=A7=81=C2=B7=EB=B2=84=ED=82=B7,=20?= =?UTF-8?q?=EC=A3=BD=EC=9D=80=20category=20=EB=B6=84=EA=B8=B0=20=EC=A0=9C?= =?UTF-8?q?=EA=B1=B0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /code-review 지적 3건을 처리했고, 그 과정에서 리뷰가 놓친 라이브 버그 1건이 나왔다. 1) 제공자 간 확증이 dedup 으로 통째로 사라지던 버그 (리뷰 미지적) `counted_web_buckets` 가 match 타입만으로 키를 만들었다. CSE 웹 결과는 google_custom_search.py:301 에서 match="page" 를 달고 오는데, Vision 이 먼저 match="page" 행을 보고하면 같은 "page" 버킷이라 CSE 히트가 점수도 사유도 없이 버려졌다 — 운영자는 두 번째 제공자가 이미지를 찾았다는 사실 자체를 볼 수 없었다. 버킷을 (provider, match) 로 쪼갰다. 2) WEB_PRESENCE 풀을 제공자별로 분리 풀이 교정하려던 상관은 한 응답 안에 있다. Vision 의 "full copy" 와 "hosting page" 는 한 사실의 두 시각이지만, CSE 가 독립적으로 찾은 게시 페이지는 다른 사실이다. 한 풀에 묶으니 주석 자신이 "high 에 도달해야 한다"고 말하는 교차 확증이 60/medium 에 갇혔다. Vision 자기 상관 뷰 4개: 125 -> 60/medium (의도대로 유지) Vision full + CSE page: 45+35 = 80/high (이전엔 45/medium — CSE 가 dedup) 3) category 탈출구는 되살리지 않고 제거 `category in {character, celebrity, work}` 분기는 writer 가 없다. src 전체에 그 키를 쓰는 곳이 없고(Vision 엔티티 행은 {entity, entity_id, match, provider}), MID -> 카테고리 해석은 폐쇄망에서 불가능한 지식그래프 조회다. 즉 "유명인은 상한 제외"라고 읽히는 주석이 실제로는 도달 불가 경로를 문서화하고 있었다. 분기를 지우고, 미등록 유명인이 라벨만으로 high 가 안 되는 것은 의도된 미탐 트레이드임을 명시했다. 실존하는 승격 경로는 지식DB/주의목록(FINGERPRINT, 풀링 대상 아님)이며 테스트로 고정했다. 4) match 없는 레거시 행의 +45 는 보존 현재 생산자는 모두 match 를 채우므로 url 폴백은 레거시 전용이다. 라운드 트립이 result_url -> url 별칭을 만들기 때문에 이 분기를 지우면 과거 행의 점수가 바뀐다 — 주석으로 레거시 전용임을 못박고 가중치는 유지했다. SCORER_VERSION v2 -> v3. v2 주석이 WEB_ENTITY_MAX_POINTS 만 적고 두 변경 중 영향이 더 큰 WEB_PRESENCE_MAX_POINTS 를 빠뜨렸던 것도 함께 보강했다. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BStHFrjQj2VypecSG9ex1V --- src/rights_filter/analysis/risk_scoring.py | 66 +++++-- .../analysis/test_risk_scoring.py | 186 +++++++++++++----- 2 files changed, 191 insertions(+), 61 deletions(-) diff --git a/src/rights_filter/analysis/risk_scoring.py b/src/rights_filter/analysis/risk_scoring.py index aa2da32..e1be02d 100644 --- a/src/rights_filter/analysis/risk_scoring.py +++ b/src/rights_filter/analysis/risk_scoring.py @@ -16,9 +16,20 @@ FACE_PERSON_BACKGROUND_POINTS = 10 # band was unreachable. Capped below the medium threshold (30), labels can # colour a case but never band it on their own. # -# Entities carrying a category (character / celebrity / work) are deliberately -# NOT capped: that classification is a rights judgement rather than a raw label, -# and it keeps its own +45 per distinct entity. +# The cap applies to EVERY entity, with no escape hatch, because the adapter +# hands us nothing that separates "Dog" from a celebrity's name: Vision's +# webEntities carry a description and an opaque MID, and resolving a MID to a +# category would take the knowledge-graph lookup this air-gapped build cannot +# make. A rights subject we actually know about escalates through the knowledge +# base / watchlist path (FINGERPRINT evidence), which is not pooled here; an +# unknown celebrity cannot be banded high on a raw label alone. That is a +# deliberate false-negative trade, not an oversight. +# +# Do NOT re-add a `category` branch without a writer for it. The previous one +# read a key nothing in src/ ever set - every producer in +# cloud_vision_web_detection.py builds entity rows as +# {entity, entity_id, match: "entity", provider} - so it documented an escape +# hatch no real image could reach while reading as though celebrities were safe. WEB_ENTITY_MAX_POINTS = 20 # Ceiling on the combined weight of web-presence match types. One Vision @@ -31,14 +42,26 @@ WEB_ENTITY_MAX_POINTS = 20 # many ways it was seen, but it takes a SECOND kind of signal — a portrait, a # knowledge-base fingerprint match, a promoted search hit — to reach high. # Existence on the web is not itself infringement; that is the operator's call. +# +# The pool is PER PROVIDER, because the correlation it corrects for lives inside +# one response. Vision saying "full copy" and "hosting page" is one finding read +# two ways; Google custom search independently returning a published page is a +# different finding, and pooling the two together capped genuine cross-provider +# corroboration - the very second signal the paragraph above says should reach +# high - at 60/medium. WEB_PRESENCE_MAX_POINTS = 60 # Stamped onto every submission at creation so a later weight change is # distinguishable from a scoring bug. Bump this whenever the weights, the bucket # rules, or the band thresholds below change — the stamp is what lets # initialRiskScore be compared against a rescore and mean something. -# v2: uncategorized web entities capped in aggregate (WEB_ENTITY_MAX_POINTS). -SCORER_VERSION = "v2" +# v2: uncategorized web entities capped in aggregate (WEB_ENTITY_MAX_POINTS) +# and web-presence match types pooled under WEB_PRESENCE_MAX_POINTS. +# v3: the presence pool and the match-type dedup bucket are both keyed by +# provider. Before v3 a Google custom-search page hit was dropped whole - +# no points, no reason line - whenever Vision had already reported a +# match="page" row, because both rows landed in one global "page" bucket. +SCORER_VERSION = "v3" class RiskScorer: @@ -49,7 +72,7 @@ class RiskScorer: counted_naver_buckets: set[str] = set() counted_face_person = False web_entity_points = 0 - web_presence_points = 0 + web_presence_points: dict[str, int] = {} for item in evidence: if item.data.get("operator_status") in {"irrelevant", "false_positive"}: @@ -92,7 +115,8 @@ class RiskScorer: if kind == _WEB_LABEL: web_entity_points += points elif kind == _WEB_PRESENCE: - web_presence_points += points + pool = _web_presence_pool(item) + web_presence_points[pool] = web_presence_points.get(pool, 0) + points else: score += points if points: @@ -145,7 +169,8 @@ class RiskScorer: # Every distinct row still surfaces as a reason; only the combined weight # of each pool is bounded. - score += min(web_presence_points, WEB_PRESENCE_MAX_POINTS) + for pooled_presence in web_presence_points.values(): + score += min(pooled_presence, WEB_PRESENCE_MAX_POINTS) score += min(web_entity_points, WEB_ENTITY_MAX_POINTS) score = min(100, score) @@ -206,19 +231,34 @@ def _web_detection_points(item: Evidence) -> tuple[int, str]: return 35, _WEB_PRESENCE if item.data.get("match") == "visual": return 10, _WEB_PRESENCE - if item.data.get("url"): + # Legacy rows only: every current producer sets `match`, so a web-detection + # row that has a URL and no match type was written by an older build. It + # keeps the original weight rather than being rescored against a rule it + # predates - and it must stay below the `match` checks above, because the + # persistence round-trip aliases result_url -> url on every row. + if not item.data.get("match") and item.data.get("url"): return 45, _WEB_PRESENCE - if item.data.get("category") in {"character", "celebrity", "work"}: - return 45, _WEB_STANDALONE if item.data.get("entity"): return round(40 * item.confidence), _WEB_LABEL return 0, _WEB_STANDALONE +def _web_presence_pool(item: Evidence) -> str: + # Rows restored from persistence lose `provider` but keep `domain`, which + # held the provider name; an unattributable row pools on its own rather than + # joining someone else's response. + provider = str(item.data.get("provider", "")).strip() + if not provider: + provider = str(item.data.get("domain", "")).strip() + return provider or "unattributed" + + def _web_detection_bucket(item: Evidence) -> str: match = str(item.data.get("match", "")) if match in {"full", "partial", "page", "visual"}: - return match + # Scoped by provider: two providers each reporting a hosting page are two + # findings. Unscoped, the second one deduped away silently. + return f"{_web_presence_pool(item)}:{match}" # Distinct named entities are independent risk signals (e.g. two different # celebrities/characters in one image), so they must not share one "entity" # bucket and dedup each other away. Key the bucket by the entity name; only @@ -227,7 +267,7 @@ def _web_detection_bucket(item: Evidence) -> str: if match == "entity" or entity: return f"entity:{entity.casefold()}" if entity else "entity" if item.data.get("url"): - return "url" + return f"{_web_presence_pool(item)}:url" return "" diff --git a/tests/rights_filter/analysis/test_risk_scoring.py b/tests/rights_filter/analysis/test_risk_scoring.py index 3dcd2e6..0caafda 100644 --- a/tests/rights_filter/analysis/test_risk_scoring.py +++ b/tests/rights_filter/analysis/test_risk_scoring.py @@ -45,28 +45,45 @@ def test_operator_false_positive_evidence_is_not_scored(): assert result.reasons == [] -def test_character_web_evidence_can_create_high_risk_for_ai_or_fanart(): - scorer = RiskScorer() - result = scorer.score( +def test_known_rights_subject_reaches_high_through_the_knowledge_base_not_labels(): + # A "character" or "celebrity" Vision entity cannot band a case by itself, + # because the adapter gives us nothing that separates it from "Dog" - see + # WEB_ENTITY_MAX_POINTS. The route to high for a subject we actually know + # about is a knowledge-base fingerprint match, which is not pooled. Both + # halves are asserted together so a later cap change cannot quietly remove + # the only remaining route out of the label pool. + labels_and_a_hosted_copy = [ + Evidence( + source=EvidenceSource.WEB_DETECTION, + reason="Web entity matched character", + confidence=0.94, + data={"entity": "유명 웹툰 캐릭터", "match": "entity"}, + ), + Evidence( + source=EvidenceSource.WEB_DETECTION, + reason="Matching image URL found", + confidence=0.9, + data={"url": "https://example.com/official-character"}, + ), + ] + + without_knowledge = RiskScorer().score(labels_and_a_hosted_copy) + assert without_knowledge.band != "high" + assert "Web entity matched character" in without_knowledge.reasons + + with_knowledge = RiskScorer().score( [ + *labels_and_a_hosted_copy, Evidence( - source=EvidenceSource.WEB_DETECTION, - reason="Web entity matched character", - confidence=0.94, - data={"entity": "유명 웹툰 캐릭터", "category": "character"}, - ), - Evidence( - source=EvidenceSource.WEB_DETECTION, - reason="Matching image URL found", - confidence=0.9, - data={"url": "https://example.com/official-character"}, + source=EvidenceSource.FINGERPRINT, + reason="지식DB 등록 이미지와 일치", + confidence=1.0, + data={"similarity": 0.95}, ), ] ) - - assert result.score >= 70 - assert result.band == "high" - assert "Web entity matched character" in result.reasons + assert with_knowledge.band == "high" + assert "지식DB 등록 이미지와 일치" in with_knowledge.reasons def test_failure_reason_does_not_lower_existing_high_risk_score(): @@ -234,31 +251,17 @@ def test_score_is_clamped_to_100(): assert result.band == "high" -def test_two_distinct_web_entities_stack_into_high_risk(): +def test_two_distinct_web_entities_are_two_signals_even_though_pooled(): # Two different named entities in one image are independent signals and must # not dedup each other away. Regression: both collapsed into one "entity" - # bucket, dropping the second +45 and under-scoring a high image to medium. - result = RiskScorer().score( - [ - Evidence( - source=EvidenceSource.WEB_DETECTION, - reason="Web entity matched character A", - confidence=0.94, - data={"entity": "캐릭터 A", "category": "character"}, - ), - Evidence( - source=EvidenceSource.WEB_DETECTION, - reason="Web entity matched celebrity B", - confidence=0.94, - data={"entity": "유명인 B", "category": "celebrity"}, - ), - ] - ) + # bucket, dropping the second entirely. The pool caps their combined weight + # (they are still bare labels), but each has to surface as its own reason - + # that is the only place the operator sees there were two. + result = RiskScorer().score([_label("캐릭터 A", 0.94), _label("유명인 B", 0.94)]) - assert result.score >= 70 - assert result.band == "high" - assert "Web entity matched character A" in result.reasons - assert "Web entity matched celebrity B" in result.reasons + assert result.score == 20 + assert "Web entity matched 캐릭터 A" in result.reasons + assert "Web entity matched 유명인 B" in result.reasons def test_same_web_entity_repeated_does_not_double_count(): @@ -270,19 +273,19 @@ def test_same_web_entity_repeated_does_not_double_count(): source=EvidenceSource.WEB_DETECTION, reason="Web entity matched character A", confidence=0.94, - data={"entity": "캐릭터 A", "category": "character"}, + data={"entity": "캐릭터 A", "match": "entity"}, ), Evidence( source=EvidenceSource.WEB_DETECTION, reason="Web entity matched character A (again)", confidence=0.94, - data={"entity": "캐릭터 A", "category": "character"}, + data={"entity": "캐릭터 A", "match": "entity"}, ), ] ) - assert result.score == 45 - assert result.band == "medium" + assert result.score == 20 + assert result.reasons == ["Web entity matched character A"] def _label(name: str, confidence: float) -> Evidence: @@ -330,10 +333,14 @@ def test_original_photo_found_nowhere_on_the_web_stays_low(): assert result.band == "low" -def test_classified_entities_are_not_capped_like_bare_labels(): - # character / celebrity / work is a rights classification, not a raw label, - # so it keeps its own +45 per distinct entity and is excluded from the pool. - result = RiskScorer().score( +def test_a_category_key_does_not_buy_an_entity_out_of_the_pool(): + # Guards the removal of an escape hatch that shipped dead: the old branch + # read a `category` key nothing in src/ ever wrote (Vision entity rows are + # {entity, entity_id, match, provider}), so it read as though celebrities + # were safe from the cap while no real image could reach it. If a writer is + # ever added it needs its own scoring decision, and this test to change with + # it - not a branch that silently does nothing. + categorized = RiskScorer().score( [ Evidence( source=EvidenceSource.WEB_DETECTION, @@ -344,8 +351,9 @@ def test_classified_entities_are_not_capped_like_bare_labels(): _label("Portrait", 0.9), ] ) + bare = RiskScorer().score([_label("유명인 B", 0.94), _label("Portrait", 0.9)]) - assert result.score == 45 + 20 + assert categorized.score == bare.score == 20 def _presence_set() -> list[Evidence]: @@ -400,3 +408,85 @@ def test_search_skipped_reason_is_not_listed_as_a_risk_reason(): assert result.score == 0 assert result.reasons == [] + + +def _vision(reason: str, match: str, confidence: float) -> Evidence: + return Evidence( + source=EvidenceSource.WEB_DETECTION, + reason=reason, + confidence=confidence, + data={"match": match, "provider": "google", "url": "https://vision.example.test/a.jpg"}, + ) + + +def _custom_search_page(reason: str) -> Evidence: + # What integrations/google_custom_search.py:_map_web_response actually emits. + return Evidence( + source=EvidenceSource.WEB_DETECTION, + reason=reason, + confidence=0.35, + data={ + "match": "page", + "provider": "google_custom_search", + "search_type": "web", + "result_url": "https://blog.example.test/post/1", + }, + ) + + +def test_one_providers_correlated_match_types_stay_pooled(): + # The pool exists for exactly this: one Vision response reporting one fact + # ("this image is on the web") through four correlated views. + result = RiskScorer().score( + [ + _vision("Full match", "full", 0.9), + _vision("Partial match", "partial", 0.8), + _vision("Page hosts image", "page", 0.8), + _vision("Visually similar", "visual", 0.6), + ] + ) + + assert result.score == 60 + assert result.band == "medium" + + +def test_two_providers_each_finding_a_published_copy_reaches_high(): + # Two independent providers each locating a published copy is the corroborated + # web presence the pool's own rationale says should reach high. Pooled + # together with Vision's correlated views it capped at 60/medium instead. + result = RiskScorer().score( + [_vision("Full match", "full", 0.9), _custom_search_page("Google custom web search result found")] + ) + + assert result.score == 45 + 35 + assert result.band == "high" + + +def test_a_custom_search_page_hit_is_not_deduped_by_a_vision_page_row(): + # Regression: the dedup bucket was the bare match type, so a Google custom + # search page hit landed in the same "page" bucket Vision had already filled + # and was dropped whole - no points, and no reason line telling the operator + # a second provider had found the image. + result = RiskScorer().score( + [_vision("Page hosts image", "page", 0.8), _custom_search_page("Google custom web search result found")] + ) + + assert "Google custom web search result found" in result.reasons + assert result.score == 35 + 35 + + +def test_legacy_web_row_without_a_match_type_keeps_its_original_weight(): + # Rows written before every producer set `match`. Rescoring them against the + # current rules would rewrite history; the round-trip has to reproduce the + # score they were created with. + legacy = Evidence( + source=EvidenceSource.WEB_DETECTION, + reason="Matching image URL found", + confidence=0.5, + data={"url": "https://legacy.example.test/a.jpg"}, + ) + + result = RiskScorer().score([legacy]) + + assert result.score == 45 + assert result.band == "medium"