Skip to content

fix(hook): claim no similarity when raw_score is null - #174

Merged
Paul-Kyle merged 1 commit into
phasespace-labs:mainfrom
chiruu12:fix/150-hook-similarity
Sep 1, 2026
Merged

fix(hook): claim no similarity when raw_score is null#174
Paul-Kyle merged 1 commit into
phasespace-labs:mainfrom
chiruu12:fix/150-hook-similarity

Conversation

@chiruu12

@chiruu12 chiruu12 commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

The Claude Code hook and the copy palinode init writes out, the last two surfaces from #150.

(.raw_score // .score // 0) falls through on null, so a BM25-only hit rendered the fused rank as a percentage. ranker.py:301 sets raw_score=None for those explicitly, so the top hit of any query, including an irrelevant one, rendered as a near-certain match.

The jq now runs the same three cases as describe_match:

raw_score before after
0.421 (42%) (42% match)
present, null (100%) (keyword match, rank 1.00)
absent (100%) (rank 1.00)

Absent and null stay separate. A pre-0.12 server never sent the field, so the arm is unknown and the rank is all there is to say. A present null is a claim the ranker made and has no similarity behind it.

Both files carry the same jq, since palinode init writes the hook out and test_embedded_copy_matches_canonical_example holds them byte-identical.

Three tests, all failing on the unfixed hook. The last one renders five hits through the real script and asserts each line matches what describe_match returns for the same dict, so the jq and the Python cannot drift apart quietly.

Not touching plugin/index.ts or plugins/core/src/index.ts.

Copilot AI lite review requested due to automatic review settings September 1, 2026 19:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The change is narrowly scoped, matches existing describe_match semantics, and is backed by targeted regression tests.

Pull request overview

This PR fixes score presentation in the Claude Code UserPromptSubmit recall hook (and the embedded copy written by palinode init) so that a fused rank (score) is no longer rendered as similarity when raw_score is explicitly present but null (BM25-only hit).

Changes:

  • Update the hook jq to distinguish three cases: raw_score absent → “rank …”, raw_score: null → “keyword match, rank …”, raw_score: number → “N% match”.
  • Apply the same jq change to both examples/hooks/palinode-user-prompt-submit.sh and the embedded USER_PROMPT_SUBMIT_HOOK_SCRIPT in palinode/cli/init.py.
  • Add/extend tests to cover null-vs-absent behavior and to pin hook rendering to palinode.core.scoring.describe_match.
File summaries
File Description
tests/test_user_prompt_submit_hook.py Adds regression tests for raw_score null vs absent and enforces hook output parity with describe_match.
palinode/cli/init.py Updates the embedded recall hook jq to avoid treating fused rank as similarity in the BM25-only case.
examples/hooks/palinode-user-prompt-submit.sh Updates the canonical recall hook jq with the same three-case score description logic.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/test_user_prompt_submit_hook.py
@Paul-Kyle

Copy link
Copy Markdown
Member

This is the one I'd point at if someone asked what a good contribution looks like here. The equivalence test is the reason — importing describe_match and asserting the shell script produces what the Python function does, for five different hit shapes, means the jq copy can't drift without a test going red. A jq transcription of Python logic is exactly the kind of thing that rots quietly, and you closed that off in the same PR that created it.

0.07 in the fixtures is the detail I enjoyed. That's the value a naive two-decimal formatter renders as 0.7, and your ($c % 100) + 100 padding handles it. You clearly tested fmt2 rather than assuming it.

One thing worth knowing, not a change request. The two implementations round differently at exact boundaries: jq's round goes half-away-from-zero, Python's round() is half-to-even, and :.2f differs again on values like 0.995 (jq 1.00, Python 0.99). So the equivalence test enforces agreement for the shapes it covers rather than for all inputs, and your PR's "cannot drift apart quietly" is very slightly stronger than what it buys. None of this touches what this PR fixes — the null-versus-absent distinction is exact and unaffected by boundary-case display rounding. Flagging it because you've been precise about the limits of your own work every time, and I'd rather hand it back than have you find it later.

Copilot left a note on the keyword-only test — that assert "%" not in ... is broader than the property you're checking, and "% match" would be the precise form. It's a fair point about future fragility rather than a defect, since the fixture's snippet is body; I've resolved it rather than sending you back for a one-liner. Worth folding in whenever you're next in that file.

Merging. That's six, and every one of them has left a guard behind.

@Paul-Kyle
Paul-Kyle merged commit 909b6e2 into phasespace-labs:main Sep 1, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants