diff --git a/docs/reference/typed-relations.en.md b/docs/reference/typed-relations.en.md index c1d2a8cb..5fa3ab3c 100644 --- a/docs/reference/typed-relations.en.md +++ b/docs/reference/typed-relations.en.md @@ -50,5 +50,30 @@ to `2030-01`. A bare `2030` with no `date(…)` wrapper still does NOT parse as date — with neither a separator nor the wrapper it is indistinguishable from a plain number. +Digits must be **ASCII**. A value carrying full-width digits — `１００억`, +`date(２０２０,１)`, the half-and-half `1２3억` — does NOT parse as any of +date/number/ordinal/amount. It takes the ordinary "does not parse → load +untyped" path and surfaces as a `typed-relations: … does not parse as …` +warning. Full-width is not folded to ASCII silently, because folding would +rewrite the stored fact string — the fix is to correct the source to ASCII and +re-collect. Under a relation that is not declared typed the parsers never run at +all, so there the two spellings simply stay separate values. + +⚠️ **Migrating an existing KB.** If full-width values collected before this rule +are still in the KB, `tools/check_conflicts.py` may now exit **1** — a gate +failure, not a warning. `１００억` and `100억` used to fold onto the same scalar +and count as one value; the full-width one now keys on its raw string, so for the +same subject a single-valued relation sees two values. + +That gate failure is loud; there is also a **quiet** one. If an existing KB holds +a full-width amount compound term (`amount(１００,"억")`), a query written without +the quotes — `amount(１００,억)` — now **misses silently**, because a full-width +term is no longer a valid amount and so no longer folds to the same canonical +form as the stored value. That miss is indistinguishable from an engine-verified +"no such fact", which makes it harder to notice than the failure. + +Both cases clear the same way: correct the source of those facts to ASCII and +re-collect. + `factlog vocab` shows declared typed relations with a `[typed:]` tag (e.g. `[attribute, typed:date]`). diff --git a/docs/reference/typed-relations.md b/docs/reference/typed-relations.md index 2ac6fb5a..2163521c 100644 --- a/docs/reference/typed-relations.md +++ b/docs/reference/typed-relations.md @@ -45,5 +45,27 @@ 날짜로 파싱되지 않습니다 — 구분자도 래퍼도 없으면 일반 수치와 구별할 수 없기 때문입니다. +숫자는 **ASCII 숫자만** 파싱됩니다. `１００억`, `date(２０２０,１)`, `1２3억` 처럼 +전각 숫자가 섞인 값은 date/number/ordinal/amount 어느 타입으로도 파싱되지 않고 +"파싱 실패 → 타입 없이 적재" 경로로 떨어져 `typed-relations: … does not parse as +…` 경고로 표면화됩니다. 전각을 ASCII 로 조용히 접지 않는 이유는, 그것이 저장된 +사실 문자열을 다시 쓰는 결정이기 때문입니다 — 해소는 소스를 ASCII 숫자로 고쳐 다시 +수집하는 것입니다. 타입 지정이 없는 관계는 파서를 타지 않으므로 두 표기가 그대로 +별개 값으로 남습니다. + +⚠️ **기존 KB 마이그레이션.** 이 규칙 이전에 수집된 전각 값이 KB 에 남아 있다면 +`tools/check_conflicts.py` 가 새로 **exit 1** 로 실패할 수 있습니다 — 경고가 아니라 +게이트 실패입니다. 전에는 `１００억` 과 `100억` 이 같은 스칼라로 접혀 한 값으로 +보였지만, 이제 전각 쪽은 원문 문자열 그대로 남아 같은 주어의 단일값 관계에서 두 +값이 되기 때문입니다. + +게이트 실패는 시끄럽지만, **조용한** 쪽도 하나 있습니다. 기존 KB 에 전각 amount +compound term(`amount(１００,"억")`)이 저장돼 있다면, 따옴표 없이 쓴 +`amount(１００,억)` 질의가 이제 **조용히 미스**합니다 — 전각 항은 더 이상 유효한 +amount 가 아니라서 저장본과 같은 정준형으로 접히지 않기 때문입니다. 이 미스는 +엔진이 확인한 "없음" 과 구분되지 않으므로 실패보다 알아채기 어렵습니다. + +두 경우 모두 해당 사실의 소스를 ASCII 로 고쳐 재수집하면 해소됩니다. + `factlog vocab` 은 선언된 타입 지정 관계에 `[typed:]` 태그를 붙여 보여 줍니다(예: `[attribute, typed:date]`). diff --git a/factlog/literal_types.py b/factlog/literal_types.py index 4554093f..3e056862 100644 --- a/factlog/literal_types.py +++ b/factlog/literal_types.py @@ -17,6 +17,52 @@ a sub-base-unit fraction is rounded to the nearest int (ROUND_HALF_UP). The engine has no float column, so the base-unit value MUST be an exact integer — see ``parse_amount``. + +**Digits are ASCII-only.** Python's ``\\d`` matches the whole Unicode ``Nd`` +category, so a full-width ``１００억`` used to parse to the *same scalar* as +``100억`` while ``relation/3`` kept the differing object string — one value with +three different answers to "is this the same?" depending on the code path. The +policy is **reject, not fold**: every numeric group below is spelled ``[0-9]``, +so a full-width value takes the ordinary "does not parse -> untyped" path and +surfaces as a typed-projection warning instead of merging silently. Rewriting the +stored string would have been the silent fold this repo consistently refuses. + +Four call sites outside this module change behaviour as a consequence. All four +are intended, and none of them rewrites data already stored: + +- ``tools/merge_candidates.py`` dedup key — ``canonical_amount`` returns ``None`` + for a full-width term, so ``amount(１００,억)`` and ``amount(１００,"억")`` stay + two rows instead of collapsing into one. Collapsing them is exactly the fold + this policy rejects. A newly merged full-width row also keeps its **source + string verbatim** instead of being rewritten to the quoted canonical form; + rows already in the KB are untouched either way. +- ``tools/check_conflicts.py`` ``_group_key`` — a full-width object no longer + normalizes to a scalar, so it keys as ``("raw", obj)`` while its ASCII twin + keys as ``("scalar", …)``. Under a single-valued relation those are **two + values**, so a KB that used to pass can now report a conflict and the command + exits **1**. This is the one consequence that flips a green gate red — see the + migration note in ``docs/reference/typed-relations.md``. +- ``common._canonical_value`` — a fact already stored as ``amount(１００,"억")`` + in an existing KB is no longer canonicalised, so a query written as + ``amount(１００,억)`` misses it. Intended: under this policy ``amount(１００,억)`` + is not a valid amount term at all, so there is no canonical form to map it to. + The fix is to correct the source to ASCII and re-collect, not to fold here. +- ``tools/ask_router.py`` answer annotation — ``humanize`` returns a full-width + compound term verbatim rather than rendering it as ``１００억``, so the display + suffix (``… (= 100억)``) is simply omitted for such a row. + +A rejected value has two user-visible paths, matching the two the issue asked +for. Under a relation declared **typed**, ``common._project_typed_relations`` +warns and loads the fact untyped. Under a relation that is **not declared** as an +attribute at all, ``tools/entity_audit.py`` reports it as a *literal suspect* — +its ``_LITERAL_RE`` is deliberately looser than this module and still matches +full-width digits, so narrowing that detector would close this path (a pinning +test guards the divergence). + +Residual symptom, NOT fixed here: a relation declared as an **attribute but not +typed** falls between the two. It has no spec, so the projection loop skips it +without warning, and its objects land in ``entity_audit``'s ``declared_literals`` +— an unflagged sorted list in which ``100억`` and ``１００억`` sort far apart. """ from __future__ import annotations @@ -41,7 +87,13 @@ "조": 10**12, } -_DATE_RE = re.compile(r"^(\d{4})[.\-/](\d{1,2})(?:[.\-/](\d{1,2}))?$") +# Every numeric group below is written ``[0-9]``, never ``\d`` — see the ASCII-only +# paragraph in the module docstring. Do NOT reach for ``re.ASCII`` to get the same +# effect: the flag also narrows ``\s``, and U+3000 (ideographic space) inside +# ``date(2020,1)`` would silently stop parsing. The ``\D+`` unit group in +# ``_AMOUNT_RE`` needs no change: ``\D`` is the complement of ``Nd``, so it already +# excludes full-width digits (``1２3억`` therefore does not match either half). +_DATE_RE = re.compile(r"^([0-9]{4})[.\-/]([0-9]{1,2})(?:[.\-/]([0-9]{1,2}))?$") # The compound form is year-precision friendly: month AND day are optional, so # ``date(2020)`` parses (a bibliographic record normally knows only the year). # This mirrors the prose path, where a missing day already defaults to ``01`` @@ -50,29 +102,29 @@ # it is indistinguishable from a plain number, so only the explicitly typed # compound term opts into year precision. _DATE_COMPOUND_RE = re.compile( - r"^date\(\s*(\d{4})(?:\s*,\s*(\d{1,2})(?:\s*,\s*(\d{1,2}))?)?\s*\)$", + r"^date\(\s*([0-9]{4})(?:\s*,\s*([0-9]{1,2})(?:\s*,\s*([0-9]{1,2}))?)?\s*\)$", re.IGNORECASE, ) -_NUMBER_RE = re.compile(r"^-?\d[\d,]*(?:\.\d+)?$") +_NUMBER_RE = re.compile(r"^-?[0-9][0-9,]*(?:\.[0-9]+)?$") _NUMBER_COMPOUND_RE = re.compile( - r"^number\(\s*\"?(-?\d[\d,]*(?:\.\d+)?)\"?\s*\)$", + r"^number\(\s*\"?(-?[0-9][0-9,]*(?:\.[0-9]+)?)\"?\s*\)$", re.IGNORECASE, ) -_ORDINAL_KO_RE = re.compile(r"^제?(\d+)\s*(?:호|위|번|차|등|째)$") -_ORDINAL_EN_RE = re.compile(r"^(\d+)\s*(?:st|nd|rd|th)$", re.IGNORECASE) -_ORDINAL_COMPOUND_RE = re.compile(r"^ordinal\(\s*(\d+)\s*\)$", re.IGNORECASE) +_ORDINAL_KO_RE = re.compile(r"^제?([0-9]+)\s*(?:호|위|번|차|등|째)$") +_ORDINAL_EN_RE = re.compile(r"^([0-9]+)\s*(?:st|nd|rd|th)$", re.IGNORECASE) +_ORDINAL_COMPOUND_RE = re.compile(r"^ordinal\(\s*([0-9]+)\s*\)$", re.IGNORECASE) # , contiguous OR a single space between them. The number part is a # plain/comma/decimal magnitude with an OPTIONAL leading sign (a loss/credit may be # negative); the unit is validated against the table by the caller. A leading `제` # (ordinal marker) can't match because the `num` group is anchored to an optional -# sign + leading digit (`^-?\d…`), so `제3호`-style ordinals never match (the first -# char `제` is neither `-` nor a digit → no match). -_AMOUNT_RE = re.compile(r"^(?P-?\d[\d,]*(?:\.\d+)?) ?(?P\D+)$") +# sign + leading digit (`^-?[0-9]…`), so `제3호`-style ordinals never match (the +# first char `제` is neither `-` nor a digit → no match). +_AMOUNT_RE = re.compile(r"^(?P-?[0-9][0-9,]*(?:\.[0-9]+)?) ?(?P\D+)$") # Compound amount: the unit may be quoted ("...", allowing spaces and commas) or # bare (no comma/paren/quote). The number is optionally quoted. Canonicalisation # always emits the quoted unit form (see ``canonical_amount``). _AMOUNT_COMPOUND_RE = re.compile( - r'^amount\(\s*"?(?P-?\d[\d,]*(?:\.\d+)?)"?\s*,\s*' + r'^amount\(\s*"?(?P-?[0-9][0-9,]*(?:\.[0-9]+)?)"?\s*,\s*' r'(?:"(?P[^"]*)"|(?P[^,)"]+))\s*\)$', re.IGNORECASE, ) @@ -250,7 +302,12 @@ def canonical_amount(raw: str) -> str | None: reaches ``facts/accepted.dl`` as ``"amount(7,\\"억\\")"`` and loads cleanly. Both the bare (``amount(7,억)``) and quoted (``amount(7,"억")``) input forms canonicalise to the same quoted output, so a re-merge is idempotent and the - dedup key collapses the two.""" + dedup key collapses the two. + + A full-width number (``amount(１００,억)``) is not an amount compound term, so + it returns ``None`` and the two spellings stay separate rows in the + ``tools/merge_candidates.py`` dedup key — the intended consequence of the + ASCII-only digit policy (see the module docstring).""" m = _AMOUNT_COMPOUND_RE.match(raw.strip()) if not m: return None @@ -298,7 +355,9 @@ def humanize(value: str) -> str: ``ordinal(N)`` is intentionally NOT humanized: the source unit (호/위/번) is lost at normalization, so a bare rank would be ambiguous. Any non-compound or unrecognized string is returned verbatim, so a KB that emits no compound - objects is byte-identical.""" + objects is byte-identical. A full-width term (``amount(１００,"억")``) is not + recognized under the ASCII-only digit policy, so it too comes back verbatim + rather than rendered.""" text = value.strip() m = _DATE_COMPOUND_RE.match(text) if m: diff --git a/tests/unit/test_conflict_scalar.py b/tests/unit/test_conflict_scalar.py index 182dc274..d31073d5 100644 --- a/tests/unit/test_conflict_scalar.py +++ b/tests/unit/test_conflict_scalar.py @@ -155,6 +155,54 @@ def test_raw_integer_string_does_not_collide_with_scalar_key(self): assert conflicts[("갑사", "매출")] == ["540000000000", 'amount(5400,"억")'] +class TestFullWidthDegradesToRaw: + """The migration risk of the ASCII-only digit policy (#331), pinned. + + Full-width digits stopped parsing, so a full-width object degrades to the + ``("raw", obj)`` key while its ASCII twin keys as ``("scalar", …)``. Under a + single-valued relation that is **two values**, so a KB carrying both + spellings now reports a CONFLICT and ``main`` returns 1 where it used to + return 0. That gate flip is the one user-visible break this policy causes and + it is intended: the resolution is to correct the source to ASCII and + re-collect, NOT to fold full-width to ASCII in ``_group_key``. These tests + fail if such a fold is ever added, which is the point of having them — + ``docs/reference/typed-relations.md`` documents the same contract. + """ + + def test_full_width_amount_keys_raw_not_scalar(self): + assert check_conflicts._group_key("100억", _AMOUNT_SPEC) == ("scalar", 10000000000) + assert check_conflicts._group_key("１００억", _AMOUNT_SPEC) == ("raw", "１００억") + + def test_full_width_and_ascii_twin_conflict(self): + # The same value in two spellings is now two values -> CONFLICT (exit 1). + facts = [ + _fact("갑사", "매출", "100억"), + _fact("갑사", "매출", "１００억"), + ] + conflicts = check_conflicts.detect_conflicts(facts, {"매출"}, _TYPED_AMOUNT) + # Reported values keep the original object strings (provenance), so a + # reviewer can see which row carries the full-width digits. + assert conflicts[("갑사", "매출")] == ["100억", "１００억"] + + def test_full_width_date_and_ascii_twin_conflict(self): + facts = [ + _fact("기서비스", "출시", "2030.1"), + _fact("기서비스", "출시", "２０３０.１"), + ] + conflicts = check_conflicts.detect_conflicts(facts, {"출시"}, _TYPED_DATE) + assert conflicts[("기서비스", "출시")] == ["2030.1", "２０３０.１"] + + def test_full_width_alone_is_not_a_conflict(self): + # Degrading to raw must not invent a conflict out of one repeated value: + # only a KB that carries BOTH spellings flips from green to red. + facts = [ + _fact("갑사", "매출", "１００억"), + _fact("갑사", "매출", "１００억"), + ] + conflicts = check_conflicts.detect_conflicts(facts, {"매출"}, _TYPED_AMOUNT) + assert conflicts == {} + + class TestMultiValuedNotFlagged: def test_relation_not_single_valued_never_conflicts(self): # Not declared single-valued -> ignored entirely, even with typed spec. diff --git a/tests/unit/test_literal_types.py b/tests/unit/test_literal_types.py index c74ba094..e73064db 100644 --- a/tests/unit/test_literal_types.py +++ b/tests/unit/test_literal_types.py @@ -377,3 +377,162 @@ def test_detected_and_parsed(self, raw, type_tag, expected): from entity_audit import _LITERAL_RE assert _LITERAL_RE.match(raw), f"entity_audit no longer detects {raw!r}" assert lt.normalize(type_tag, raw) == expected + + @pytest.mark.parametrize("raw,type_tag", [ + ("１００억", "amount"), + ("２０２６", "number"), + ("제３호", "ordinal"), + ("２０３０.１", "date"), + ]) + def test_full_width_divergence_is_intended(self, raw, type_tag): + """The two must NOT be made consistent for full-width digits (#331). + + entity_audit's detector is a loose smell test; these normalizers are a + strict ASCII-only contract. That divergence is load-bearing: a relation + that is not declared an attribute puts a matching object into + ``literal_suspects``, which is the second user-visible path a rejected + full-width value takes. Narrowing ``_LITERAL_RE`` to ``[0-9]`` "for + consistency" would silently close it — so pin both halves together. + """ + from entity_audit import _LITERAL_RE + assert _LITERAL_RE.match(raw), f"entity_audit must still SEE {raw!r}" + assert lt.normalize(type_tag, raw) is None, f"{raw!r} must NOT parse" + + +class TestFullWidthDigitsRejected: + """ASCII-only digits (#331). Python's ``\\d`` covers the whole Unicode ``Nd`` + category, so a full-width ``１００억`` used to normalize to the SAME scalar as + ``100억`` while ``relation/3`` stored a different object string — the two + spellings merged under a typed relation but stayed separate entities and + missed each other in object-match queries. The policy is reject, not fold: a + full-width value takes the ordinary "does not parse -> untyped" path.""" + + @pytest.mark.parametrize("type_tag,raw", [ + # Each of these returned a scalar before the fix (20200101 / 20200101 / + # 123000 / 3 / 10000000000 / 10000000000), never None. + ("date", "date(２０２０,１)"), + ("date", "２０２０.１"), + ("number", "number(１２３)"), + ("number", "１２３"), + ("ordinal", "ordinal(３)"), + ("ordinal", "３위"), + ("ordinal", "제３호"), + ("ordinal", "３rd"), # the English ordinal form has its own regex + ("amount", "１００억"), + ("amount", 'amount(１００,"억")'), + ]) + def test_normalize_rejects(self, type_tag, raw): + assert lt.normalize(type_tag, raw) is None + + @pytest.mark.parametrize("parser,raw", [ + (lt.parse_date, "date(２０２０,１)"), + (lt.parse_number, "number(１２３)"), + (lt.parse_number_scaled, "number(１２３)"), + (lt.parse_ordinal, "ordinal(３)"), + ]) + def test_public_parsers_reject(self, parser, raw): + assert parser(raw) is None + + def test_parse_amount_rejects(self): + assert lt.parse_amount("１００억", lt.DEFAULT_AMOUNT_UNITS) is None + + @pytest.mark.parametrize("type_tag,raw", [ + # Half-and-half spellings are the realistic accident (an IME left in + # full-width mode mid-token), and they must not parse either. + ("amount", "1２3억"), + ("amount", 'amount(１0,"억")'), + ("amount", 'amount("１００","억")'), + ("number", 'number("1２3")'), + ("number", "1２3"), + ("date", "date(20２0,1)"), + ("ordinal", "ordinal(1２)"), + ("ordinal", "1２th"), + ]) + def test_mixed_width_rejects(self, type_tag, raw): + assert lt.normalize(type_tag, raw) is None + + def test_full_width_no_longer_shares_a_scalar_with_ascii(self): + # The motivating bug in one line: same scalar, different stored string. + assert lt.normalize("amount", "100억") == 10000000000 + assert lt.normalize("amount", "１００억") is None + + @pytest.mark.parametrize("raw", ['amount(１００,억)', 'amount(１００,"억")']) + def test_canonical_amount_rejects(self, raw): + # Consequence, by design: merge_candidates' dedup key no longer collapses + # the bare and quoted full-width spellings into one row. Folding them is + # the silent rewrite this policy refuses. + assert lt.canonical_amount(raw) is None + + @pytest.mark.parametrize("raw", ['amount(１００,"억")', "date(２０２０,１)", "number(１２３)"]) + def test_humanize_returns_full_width_verbatim(self, raw): + # Unrecognized -> verbatim, the documented humanize fallback. + assert lt.humanize(raw) == raw + + +class TestFullWidthSurfaces: + """AC3 (#331): a rejected full-width value must reach a user-visible path at + least once. Under a relation declared typed, the projection loop warns on + stderr and loads the fact untyped — the same path any malformed literal takes. + + Residual symptom, deliberately NOT covered: a relation declared as an + attribute but NOT typed has no spec, so ``_project_typed_relations`` skips it + without warning and the full-width value surfaces nowhere.""" + + class _FakeSession: + """Records inserts; _project_typed_relations touches nothing else.""" + + def __init__(self): + self.inserts = [] + + def intern(self, value): + return 1 + + def insert(self, alias, payload): + self.inserts.append((alias, payload)) + + def test_projection_warns_and_loads_untyped(self, capsys): + import common + + specs = {"출시일": common.TypedRelSpec("date", "launch_date")} + rows = [ + {"subject": "제품", "relation": "출시일", "object": "date(２０２０,１)"}, + {"subject": "제품2", "relation": "출시일", "object": "date(2020,1)"}, + ] + session = self._FakeSession() + common._project_typed_relations(session, specs, rows) + err = capsys.readouterr().err + assert "date(２０２０,１)" in err + assert "does not parse as date" in err + # Only the ASCII row projects; the full-width fact still loads untyped. + assert session.inserts == [("launch_date", (1, 20200101))] + + +class TestAsciiDigitsUnchanged: + """Regression guard for the narrowing in #331: the ASCII forms that already + parsed must keep parsing, including the ones a careless narrowing would break + (a space-separated unit, a comma inside the number, and — the reason + ``re.ASCII`` is NOT used — a U+3000 ideographic space as separator whitespace). + These pin existing behaviour; they do not prove the fix.""" + + @pytest.mark.parametrize("type_tag,raw,expected", [ + ("amount", "100 억", 10000000000), + ("amount", "100억", 10000000000), + ("amount", 'amount(1,000,"억")', 100000000000), + ("amount", "1,000원", 1000), + ("date", "date(2020)", 20200101), + ("date", "date(2020,　1)", 20200101), # U+3000 stays whitespace + ("date", "2030.1", 20300101), + ("number", "1,000", 1000000), + ("number", "3.14", 3140), + ("ordinal", "제3호", 3), + ("ordinal", "3rd", 3), + ]) + def test_ascii_forms_still_parse(self, type_tag, raw, expected): + assert lt.normalize(type_tag, raw) == expected + + def test_ascii_canonical_amount_unchanged(self): + assert lt.canonical_amount("amount(1,000,억)") == 'amount(1000,"억")' + + def test_ascii_humanize_unchanged(self): + assert lt.humanize('amount(7,"억")') == "7억" + assert lt.humanize("date(2030,1,15)") == "2030-01-15" diff --git a/tools/entity_audit.py b/tools/entity_audit.py index d29d95c9..b0814d7e 100755 --- a/tools/entity_audit.py +++ b/tools/entity_audit.py @@ -57,6 +57,8 @@ # 100억, 제3호). Advisory only — a human confirms before declaring the relation; # a few false positives (e.g. a named concept like '4차 산업혁명') are acceptable # in exchange for not missing the motivating value forms. +# Deliberately looser than literal_types' ASCII-only contract: `\d` still matches +# full-width digits here, so a rejected value stays visible to the audit (#331). _LITERAL_RE = re.compile( r"^\d{4}[.\-/]\d{1,2}([.\-/]\d{1,2})?$" # date r"|^\d[\d,]*(\.\d+)?$" # number / comma / decimal