From a51ca712a05eb36fb33d89f91efe9b9898b52733 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Wed, 29 Apr 2026 08:33:03 -0400 Subject: [PATCH] qa: question_hash strips trailing punctuation so '?' and 'X?' dedupe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Today `question_hash` canonicalizes (NFC, ws-collapse, lowercase) but preserves trailing punctuation. So "who is X?" and "who is X" produce different hashes, different cache_keys, different providence_cache records — the cache misses on what's semantically the same question. Add `rstrip(".?!,;:")` as the third canonicalization step inside question_hash specifically. Local to question hashing — chunk leaf hashes go through `canonicalize()` directly and are unaffected. Equivalence class after this: "who is X" \ "who is X?" | "who is X." | -> all same question_hash, all same cache_key "who is X!" | "Who Is X" / Internal punctuation is preserved on purpose: "X, then Y" carries meaning that "X then Y" doesn't, even though both have the same content tokens. One-time impact: prior cached records whose canonical question ended in punctuation become orphans on lookup (re-derive on next ask). History stays on disk; nothing burned automatically. New test file tests/test_keys.py with 13 tests pinning the question_hash equivalence class plus light coverage of the other dim hashes (model_profile, conversation, governance, cache_key 8-dim invariant). 286 passed, 1 skipped. --- aborist/qa/keys.py | 21 +++++++- tests/test_keys.py | 118 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 2 deletions(-) create mode 100644 tests/test_keys.py diff --git a/aborist/qa/keys.py b/aborist/qa/keys.py index 20335de..3bd0db0 100644 --- a/aborist/qa/keys.py +++ b/aborist/qa/keys.py @@ -36,8 +36,25 @@ def _canonical_json(obj) -> str: def question_hash(question: str) -> str: - """SHA-256 of canonicalized + lowercased question text.""" - return _sha256(canonicalize(question).lower()) + """SHA-256 of canonicalized + lowercased + punctuation-stripped question text. + + Three-step canonicalization, in order: + + 1. ``canonicalize()`` — NFC, whitespace collapse, strip ends. + 2. ``.lower()`` — case-fold so "Who is X" == "who is X". + 3. ``rstrip(".?!,;:")`` — trailing punctuation carries no semantic + difference for retrieval. ``"who is X?"`` and ``"who is X"`` map + to the same cache_key, so the providence_cache hit returns the + same answer for either form. + + Step 3 is question-specific — chunk leaf hashes stay verbatim because + they go through ``canonicalize()`` directly, not through this + function. Bumping this rule orphans prior cache records whose + canonical question ended in punctuation; they live as history but + won't be re-hit on lookup. + """ + canon = canonicalize(question).lower().rstrip(".?!,;:") + return _sha256(canon) def model_profile_hash( diff --git a/tests/test_keys.py b/tests/test_keys.py new file mode 100644 index 0000000..06b03f4 --- /dev/null +++ b/tests/test_keys.py @@ -0,0 +1,118 @@ +"""Cache-key dimension hashes — pure functions, deterministic. + +The 8-dim cache_key invariants are spread across `aborist/qa/keys.py`. +Most behavior is covered indirectly by `test_qa.py` (cache hits, etc.); +this file pins the hash-input canonicalization rules so a future tweak +to question normalization can't silently change the equivalence class +of "what counts as the same question." +""" + +from __future__ import annotations + +from aborist.qa.keys import ( + cache_key, + conversation_hash, + governance_policy_hash, + model_profile_hash, + question_hash, +) + + +# --------------------------------------------------------------------------- +# question_hash equivalence classes +# --------------------------------------------------------------------------- + + +def test_question_hash_strips_trailing_question_mark(): + """Fox's catch on 2026-04-29: `who is X?` and `who is X` should hit + the same providence_cache record. Trailing punctuation carries no + semantic difference for retrieval.""" + assert question_hash("who is X") == question_hash("who is X?") + + +def test_question_hash_strips_trailing_period(): + assert question_hash("explain X") == question_hash("explain X.") + + +def test_question_hash_strips_trailing_exclamation(): + assert question_hash("explain X") == question_hash("explain X!") + + +def test_question_hash_strips_multiple_trailing_punct(): + """Multiple trailing chars all stripped — `?!` is a single class.""" + assert question_hash("explain X") == question_hash("explain X?!") + assert question_hash("explain X") == question_hash("explain X.?") + + +def test_question_hash_lowercases(): + assert question_hash("Who Is X") == question_hash("who is x") + + +def test_question_hash_collapses_internal_spaces(): + """canonicalize() collapses runs of spaces to a single space. Tabs/ + newlines map to newline (a separate equivalence class — see + aborist.document.canonicalize), so tab vs space is intentionally + NOT in the same bucket.""" + assert question_hash("who is X") == question_hash("who is X") + + +def test_question_hash_does_not_strip_internal_punctuation(): + """Internal `?` is a different question — ``what's up?`` mid-sentence + can carry meaning. Only trailing punctuation gets stripped.""" + assert question_hash("X, then Y") != question_hash("X then Y") + + +def test_question_hash_distinct_for_different_text(): + """Sanity: two genuinely different questions hash to different values.""" + assert question_hash("who is X") != question_hash("who is Y") + + +def test_question_hash_is_deterministic(): + """Same input → same output, every time. No randomness.""" + a = question_hash("who is russell ballestrini?") + b = question_hash("who is russell ballestrini?") + assert a == b + assert len(a) == 64 # sha256 hex + + +# --------------------------------------------------------------------------- +# Other dim hashes — light coverage so future refactors don't drift +# --------------------------------------------------------------------------- + + +def test_model_profile_hash_includes_revision_and_quantization(): + a = model_profile_hash("m", revision="r1", quantization="q1") + b = model_profile_hash("m", revision="r2", quantization="q1") + c = model_profile_hash("m", revision="r1", quantization="q2") + assert a != b + assert a != c + assert b != c + + +def test_conversation_hash_canonicalizes_message_order(): + """Message ordering matters — a different order is a different + conversation. Pin so anyone tempted to sort messages knows it would + invalidate cache.""" + msgs1 = [{"role": "system", "content": "a"}, {"role": "user", "content": "b"}] + msgs2 = [{"role": "user", "content": "b"}, {"role": "system", "content": "a"}] + assert conversation_hash(msgs1) != conversation_hash(msgs2) + + +def test_governance_policy_hash_sensitive_to_keys(): + a = governance_policy_hash({"k": 1}) + b = governance_policy_hash({"k": 2}) + c = governance_policy_hash({"j": 1}) + assert a != b + assert a != c + + +def test_cache_key_combines_all_eight_dims(): + """If any of the 8 dims changes, cache_key changes. v9.8 invariant.""" + base = ("src", "qh", "mh", "ch", "gh", "v9.8.0", "norm-v1", "tok-512-v1") + base_key = cache_key(*base) + for i in range(8): + mutated = list(base) + mutated[i] = mutated[i] + "_modified" + assert cache_key(*mutated) != base_key, ( + f"dim {i} change did not bump cache_key — 8-dim invariant broken" + )