fix: 정책 질의가 고정한 인용 상수로 결과 행을 거른다

tests/unit/test_policy_query_filter.py

@@ -0,0 +1,167 @@
+# SPDX-License-Identifier: Apache-2.0
+"""Regression tests: a policy query's quoted constants must FILTER rows (#326).
+
+``policy_result_line`` printed the whole extent of the policy predicate no
+matter which entity the query pinned, so ``needs_review("Bob", R)?`` reported
+three rows for a Bob the engine inferred NOTHING about — other subjects' reasons
+rendered as Bob's. The report is the artifact SKILL.md tells the reader to show
+verbatim before stating a conclusion, so a fabricated positive there is not a
+cosmetic defect.
+
+The filter must apply at EVERY argument position, not just the first: fixing
+only ``args[0]`` would leave ``pred(E, "stale")?`` mis-attributing rows while
+making the first-argument form accurate — which removes the only signal a reader
+had that the second line is untrustworthy.
+
+``ask_router.evaluate`` (the ``ask`` path) had the same defect at positions
+other than the first. The parity class below pins the two paths together.
+
+FACTLOG_ROOT is bound to a throwaway dir by ``tests/unit/conftest.py`` BEFORE
+any tool module is imported, which matters here: ``ask_router`` re-exports
+``FACTLOG_ROOT`` at import time from argv/env, so without that pin the parity
+test would read the developer's real knowledge base.
+"""
+from __future__ import annotations
+
+import re
+import unicodedata
+
+import pytest
+
+import ask_router
+import run_logic_check as rlc
+
+PREDICATE = "needs_review"
+INFERRED = {
+    PREDICATE: {
+        ("Alice", "low_conf"),
+        ("Carol", "stale"),
+        ("Dave", "no_source"),
+    }
+}
+
+
+def _row_count(report_line: str) -> int:
+    """Extract N from '<pred> results ...: N rows; ...'."""
+    match = re.search(r"(\d+) rows", report_line)
+    assert match, f"unparseable result line: {report_line!r}"
+    return int(match.group(1))
+
+
+def _report_rows(draft: str, inferred=None) -> int:
+    return _row_count(rlc.policy_result_line(PREDICATE, draft, inferred or INFERRED))
+
+
+def _router_rows(monkeypatch, draft: str, inferred=None) -> int:
+    monkeypatch.setattr(ask_router, "policy_predicates", lambda program: {PREDICATE})
+    monkeypatch.setattr(ask_router, "run_wirelog", lambda: inferred or INFERRED)
+    return ask_router.evaluate(draft, [])["count"]
+
+
+class TestPolicyResultLineFiltersFixedEntity:
+    def test_named_entity_reports_only_its_own_rows(self):
+        line = rlc.policy_result_line(PREDICATE, f'{PREDICATE}("Alice", R)?', INFERRED)
+        assert _row_count(line) == 1
+        assert "low_conf" in line
+        assert "stale" not in line and "no_source" not in line
+
+    def test_entity_with_no_inferred_rows_reports_zero(self):
+        line = rlc.policy_result_line(PREDICATE, f'{PREDICATE}("Bob", R)?', INFERRED)
+        assert _row_count(line) == 0
+        assert "low_conf" not in line and "stale" not in line and "no_source" not in line
+
+    def test_second_argument_constant_also_filters(self):
+        # The defect is not first-argument-specific: a reason-pinned query must
+        # not report the other reasons' rows either.
+        line = rlc.policy_result_line(PREDICATE, f'{PREDICATE}(E, "stale")?', INFERRED)
+        assert _row_count(line) == 1
+        assert "Carol" in line
+        assert "Alice" not in line and "Dave" not in line
+
+    def test_second_argument_constant_with_no_match_reports_zero(self):
+        line = rlc.policy_result_line(PREDICATE, f'{PREDICATE}(E, "missing_reason")?', INFERRED)
+        assert _row_count(line) == 0
+
+    def test_both_arguments_constant_matching_row(self):
+        line = rlc.policy_result_line(PREDICATE, f'{PREDICATE}("Carol", "stale")?', INFERRED)
+        assert _row_count(line) == 1
+
+    def test_both_arguments_constant_crossed_pair_reports_zero(self):
+        # Carol and low_conf each exist, but not together — a per-column filter
+        # that ORed the positions would wrongly report a row here.
+        line = rlc.policy_result_line(PREDICATE, f'{PREDICATE}("Carol", "low_conf")?', INFERRED)
+        assert _row_count(line) == 0
+
+    def test_all_variable_query_still_reports_full_extent(self):
+        line = rlc.policy_result_line(PREDICATE, f"{PREDICATE}(E, R)?", INFERRED)
+        assert _row_count(line) == 3
+        assert "E=Alice, R=low_conf" in line
+
+    # The 0-arity row case lives in TestReportRouterParity: asserting it on the
+    # report alone let a router that dropped its short-row guard survive.
+
+
+class TestReportRouterParity:
+    """The report and the ``ask`` router must answer a policy query identically.
+
+    Parity is NOT correctness: two paths can agree and both be wrong, which is
+    exactly the state before this fix at non-first argument positions. So every
+    case asserts the ABSOLUTE row count as well as the agreement, and the router
+    side calls ``ask_router.evaluate`` directly rather than re-implementing the
+    filter in the test (a re-implementation would prove nothing about either
+    production path).
+    """
+
+    @pytest.mark.parametrize(
+        ("draft", "expected"),
+        [
+            (f'{PREDICATE}("Alice", R)?', 1),
+            (f'{PREDICATE}("Bob", R)?', 0),
+            (f'{PREDICATE}(E, "stale")?', 1),
+            (f'{PREDICATE}(E, "missing_reason")?', 0),
+            (f'{PREDICATE}("Carol", "stale")?', 1),
+            (f'{PREDICATE}("Carol", "low_conf")?', 0),
+            (f"{PREDICATE}(E, R)?", 3),
+        ],
+    )
+    def test_report_and_router_agree_on_the_verified_row_count(self, monkeypatch, draft, expected):
+        report_rows = _report_rows(draft)
+        router_rows = _router_rows(monkeypatch, draft)
+        assert report_rows == expected
+        assert router_rows == expected
+        assert report_rows == router_rows
+
+    def test_zero_arity_row_is_dropped_by_both_paths(self, monkeypatch):
+        # The cases above are all 2-column rows, so neither path's short-row
+        # guard is exercised by them: a router that dropped the guard kept
+        # passing. A 0-arity row cannot satisfy a pinned constant, on EITHER path.
+        inferred = {PREDICATE: {(), ("Alice", "low_conf")}}
+        pinned = f'{PREDICATE}("Alice", R)?'
+        assert _report_rows(pinned, inferred) == 1
+        assert _router_rows(monkeypatch, pinned, inferred) == 1
+        # Non-vacuous: the empty row IS in the extent both paths start from, so
+        # the 1 above is a filter result, not an absent row.
+        variables = f"{PREDICATE}(E, R)?"
+        assert _report_rows(variables, inferred) == 2
+        assert _router_rows(monkeypatch, variables, inferred) == 2
+
+    def test_nfd_stored_entity_does_not_meet_an_nfc_query_on_either_path(self, monkeypatch):
+        # Pins CURRENT behaviour, not desired behaviour: both paths compare raw
+        # values, so an NFD-stored entity is invisible to an NFC-typed constant
+        # and the query reports 0 rows — which reads as a verified negative. That
+        # is a known gap (see #213 and policy_row_matches' docstring); what this
+        # test forbids is FIXING IT ON ONE PATH ONLY, which would put the report
+        # and ask back into disagreement. The cases above are ASCII-only, so a
+        # router that folded to NFC on its own kept passing them.
+        nfd = unicodedata.normalize("NFD", "박수영")
+        nfc = unicodedata.normalize("NFC", "박수영")
+        assert nfd != nfc
+        inferred = {PREDICATE: {(nfd, "stale")}}
+        nfc_query = f'{PREDICATE}("{nfc}", R)?'
+        assert _report_rows(nfc_query, inferred) == 0
+        assert _router_rows(monkeypatch, nfc_query, inferred) == 0
+        # Non-vacuous: the row is reachable — an NFD-typed constant finds it on
+        # both paths, so the 0 above is the folding gap and not an empty extent.
+        nfd_query = f'{PREDICATE}("{nfd}", R)?'
+        assert _report_rows(nfd_query, inferred) == 1
+        assert _router_rows(monkeypatch, nfd_query, inferred) == 1

tests/unit/test_policy_result_line_rendering.py

@@ -0,0 +1,126 @@
+# SPDX-License-Identifier: Apache-2.0
+"""Regression tests: what a policy query result line may claim (#326).
+
+Two rendering defects that survive the quoted-constant filter:
+
+1. A MALFORMED query still got an answer. No trailing '?' makes ``query_args``
+   return [] and ``pred()?`` returns one empty arg — either way no constant is
+   pinned, the filter passes every row, and the line reports the predicate's
+   whole extent. Wrong arity (``pred("Alice")?``, ``pred(E, R, "zzz")?``) filters
+   on whatever constants happen to line up with a column and reports a plausible
+   but meaningless count. All four shapes are rejected in the report's own Errors
+   section at the same time; an error plus a fabricated answer is worse than the
+   error alone. The guard here is the arity test ``validate_query`` already uses,
+   and ``ask_router.evaluate`` raises on the same shapes so neither path answers.
+
+2. The filtered result line and the "Policy evaluation:" extent line sit a few
+   lines apart in the same report and now legitimately disagree (3 rows there,
+   0 rows here). Echoing the query is what makes that pair readable as scope
+   rather than contradiction. The echo is limited to constant-pinned queries so a
+   variable-only query keeps its original text; the extent line is deliberately
+   left alone (it is pinned by tests/golden/logic_report.txt).
+"""
+from __future__ import annotations
+
+import pytest
+
+import ask_router
+import run_logic_check as rlc
+
+PREDICATE = "needs_review"
+INFERRED = {
+    PREDICATE: {
+        ("Alice", "low_conf"),
+        ("Carol", "stale"),
+        ("Dave", "no_source"),
+    }
+}
+
+
+MALFORMED = [
+    f'{PREDICATE}("Bob", R)',  # no trailing '?' — query_args returns []
+    PREDICATE,  # bare predicate, not an atom at all
+    f"{PREDICATE}()?",  # one empty arg, not zero args
+    f'{PREDICATE}("Alice")?',  # too few
+    f'{PREDICATE}("Alice", R, "zzz")?',  # too many
+]
+
+
+class TestMalformedQueryEmitsNoResultLine:
+    def _evaluate(self, monkeypatch, line):
+        monkeypatch.setattr(rlc, "query_lines", lambda: [line])
+        return rlc.evaluate_queries([], INFERRED, {PREDICATE})
+
+    @pytest.mark.parametrize("line", MALFORMED)
+    def test_malformed_query_produces_no_result_line(self, monkeypatch, line):
+        assert self._evaluate(monkeypatch, line) == []
+
+    @pytest.mark.parametrize("line", MALFORMED)
+    def test_malformed_query_returns_none_not_a_row_count_string(self, line):
+        assert rlc.policy_result_line(PREDICATE, line, INFERRED) is None
+
+    def test_wellformed_query_still_produces_a_result(self, monkeypatch):
+        results = self._evaluate(monkeypatch, f'{PREDICATE}("Alice", R)?')
+        assert len(results) == 1
+        assert "1 rows" in results[0]
+
+
+class TestMalformedQueryParity:
+    """A malformed policy query must get NO answer from either path.
+
+    Before the guard the two paths disagreed outright on the same input: for
+    ``pred("Bob", R)`` (no '?') the router returned count=3 while the report
+    returned nothing, and for ``pred(E, R, "zzz")?`` both rendered '0 rows',
+    which reads as a verified negative for a query classify_query rejects as
+    BAD_ARITY. The router refuses by raising (cmd_evaluate turns that into error
+    JSON); the report refuses by emitting no line. Neither invents a count.
+    """
+
+    @pytest.mark.parametrize("draft", MALFORMED)
+    def test_router_refuses_and_report_stays_silent(self, monkeypatch, draft):
+        monkeypatch.setattr(ask_router, "policy_predicates", lambda program: {PREDICATE})
+        monkeypatch.setattr(ask_router, "run_wirelog", lambda: INFERRED)
+        with pytest.raises(NotImplementedError):
+            ask_router.evaluate(draft, [])
+        assert rlc.policy_result_line(PREDICATE, draft, INFERRED) is None
+
+    def test_wellformed_query_is_still_answered_by_both(self, monkeypatch):
+        monkeypatch.setattr(ask_router, "policy_predicates", lambda program: {PREDICATE})
+        monkeypatch.setattr(ask_router, "run_wirelog", lambda: INFERRED)
+        draft = f'{PREDICATE}("Alice", R)?'
+        assert ask_router.evaluate(draft, [])["count"] == 1
+        assert "1 rows" in rlc.policy_result_line(PREDICATE, draft, INFERRED)
+
+
+class TestResultLineNamesTheQueryItAnswers:
+    def test_variable_only_query_text_is_byte_identical_to_before_the_fix(self):
+        # The fix promised not to change the output of a variable-only query.
+        # This is the literal upstream/main (c6d359d) rendering of this input,
+        # so the echo must not reach it: a variable-only query reports the extent,
+        # which is exactly what the 'Policy evaluation:' line says, so it cannot
+        # produce the mismatch the echo exists to explain.
+        line = rlc.policy_result_line(PREDICATE, f"{PREDICATE}(E, R)?", INFERRED)
+        assert line == (
+            "needs_review results: 3 rows; "
+            "E=Alice, R=low_conf; E=Carol, R=stale; E=Dave, R=no_source"
+        )
+        assert "query:" not in line
+
+    def test_result_line_echoes_the_query(self):
+        draft = f'{PREDICATE}("Bob", R)?'
+        line = rlc.policy_result_line(PREDICATE, draft, INFERRED)
+        # Without the echo, '0 rows' here reads as a contradiction of the
+        # 'needs_review: 3 rows' extent line printed just above it.
+        assert draft in line
+        assert "0 rows" in line
+
+    def test_echo_distinguishes_two_queries_on_the_same_predicate(self, monkeypatch):
+        monkeypatch.setattr(
+            rlc,
+            "query_lines",
+            lambda: [f'{PREDICATE}("Alice", R)?', f'{PREDICATE}("Bob", R)?'],
+        )
+        first, second = rlc.evaluate_queries([], INFERRED, {PREDICATE})
+        assert "Alice" in first and "1 rows" in first
+        assert "Bob" in second and "0 rows" in second
+        assert first != second

tools/ask_router.py

@@ -420,18 +420,59 @@ def _reachable_pairs(facts: list[dict[str, str]]) -> set[tuple[str, str]]:
     return pairs
 
 
+def policy_row_matches(args: list[str], row: tuple[str, ...] | list[str]) -> bool:
+    """True when *row* satisfies every quoted constant *args* pins, by position.
+
+    A quoted constant is a FILTER, at whatever position it appears. This branch
+    used to test args[0] only, so `pred(E, "stale")?` returned the whole extent
+    and `pred("Carol", "low_conf")?` returned Carol's row under a reason that is
+    not hers — a fabricated positive for the exact pair the user asked about.
+
+    A row shorter than the pinned position cannot satisfy the constant, so the
+    0-arity row an engine may emit is dropped from a constant-pinned query (an
+    all-variable query still returns it).
+
+    Comparison is RAW (`arg_value` only), deliberately not `canonical_value`
+    which the relation branch uses: it mirrors run_logic_check's report path
+    exactly so `ask` and the report cannot diverge, which is the property
+    tests/unit/test_policy_query_filter.py pins. This means the policy path does
+    NOT go through the "#213 single query-value comparison chokepoint"
+    (common.py `_canonical_value`), contrary to what that docstring claims for
+    every query-match path. Measured consequence: an NFD-stored entity queried
+    with an NFC-typed constant now yields 0 rows, which reads as a verified
+    negative. Folding both sides belongs in one place for BOTH paths and is out
+    of scope here; see #213.
+
+    The matching rule is kept identical to run_logic_check's `policy_row_matches`
+    (same body, module-specific docstring). The natural home is common.py
+    alongside the other query-parsing helpers, but hoisting it there is a wider
+    change than this fix needs; the report/router parity test fails if the two
+    copies ever drift. Two of its cases carry that load — the 0-arity row and the
+    NFD-stored/NFC-queried entity. Every other case is a 2-column ASCII row,
+    which a copy that lost the short-row guard, or that folded to NFC on its own,
+    still gets right; do not delete those two.
+    """
+    for index, arg in enumerate(args):
+        if not is_quoted_string(arg):
+            continue
+        if index >= len(row) or arg_value(arg) != row[index]:
+            return False
+    return True
+
+
 def evaluate(draft: str, facts: list[dict[str, str]]) -> dict[str, object]:
     """Evaluate a validated engine query: relation, path, or a policy predicate.
 
     - relation: match against accepted facts.
     - path: a fully-quoted query returns the dependency path (or none); a query
       with a variable returns the reachable (start, target) pairs.
     - policy predicate: the inferred (entity, reason) rows from the engine,
-      optionally filtered by a quoted entity argument.
+      filtered by every quoted constant the query pins, at whatever argument
+      position it appears (see policy_row_matches).
 
     A truly unknown predicate raises NotImplementedError rather than returning 0
     rows, so a caller never mistakes an unsupported predicate for a verified
-    negative.
+    negative. A malformed count or policy query raises for the same reason.
     """
     predicate = _predicate_of(draft)
     args = query_args(draft)
@@ -496,15 +537,27 @@ def evaluate(draft: str, facts: list[dict[str, str]]) -> dict[str, object]:
         # verified negative). Catch broad Exception — never BaseException, so
         # KeyboardInterrupt/SystemExit still propagate — because the engine may
         # raise non-FactlogError types.
+        # Guard arity BEFORE evaluating, like the count branch above (#257) and
+        # for the same reason: with a malformed query no constant lines up with a
+        # column, so the filter passes rows it cannot have checked — `pred("X")?`
+        # returned a filtered-looking count and `pred(E, R, "zzz")?` returned 0
+        # rows, which renders as a verified negative for a query classify_query
+        # already rejects as BAD_ARITY. classify_query rejects these too, so the
+        # render path never reaches here; the `evaluate` subcommand does, and
+        # cmd_evaluate turns NotImplementedError into a clean error JSON.
+        # run_logic_check drops the result line on the same shapes, so the report
+        # and ask agree that a malformed policy query has no answer.
+        if len(args) != 2:
+            raise NotImplementedError("policy query must have entity and reason arguments")
         try:
             inferred = run_wirelog()
         except Exception as exc:  # noqa: BLE001 — engine/loader raise non-FactlogError too
             return {"rows": [], "count": 0, "policy_unevaluable": str(exc)}
-        rows = []
-        for row in sorted(inferred.get(predicate, set())):
-            if args and is_quoted_string(args[0]) and (not row or arg_value(args[0]) != row[0]):
-                continue
-            rows.append(list(row))
+        rows = [
+            list(row)
+            for row in sorted(inferred.get(predicate, set()))
+            if policy_row_matches(args, row)
+        ]
         return {"rows": rows, "count": len(rows)}
     raise NotImplementedError(f"engine evaluation of predicate '{predicate}' is not supported")