feat(retrieval): Calibrate strict-recall threshold, add filter_score config, pass through min_score via MCP/hook, and add full evaluation report - #63
Conversation
…param to MCP and Claude Code recall hooks
…gurable' into feat/issue-31-recall-floor-configurable
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds LoCoMo threshold calibration, composite-score strict-recall filtering, configurable request thresholds, recall-hook overrides, frontend validation, calibration reports, and regression tests. ChangesStrict-recall calibration
Production retrieval configuration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SearchRouter
participant Searcher
participant Reranker
Client->>SearchRouter: send filter_score or min_score
SearchRouter->>SearchRouter: apply configured defaults
SearchRouter->>Searcher: execute strict-recall search
Searcher->>Reranker: rerank candidate results
Searcher->>Searcher: filter by pre_rerank_score
Searcher-->>Client: return filtered results
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hebb/mcp/server.py (1)
91-107: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winComplete the
search_memoryAPI docstring.
search_memoryis public. Its docstring omitsReturnsandRaisessections. Document the returned formatted result and propagated request failures.As per coding guidelines, “Include docstring with Args, Returns, and Raises sections for all public APIs.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/mcp/server.py` around lines 91 - 107, Complete the public search_memory docstring by adding Returns and Raises sections. Document that it returns the formatted search results and that request failures propagated by the function are raised, while preserving the existing Args documentation.Source: Coding guidelines
🧹 Nitpick comments (3)
eval/threshold_probe.py (2)
455-475: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe composite distribution is empty in every rerank run.
Line 469 appends
r.scoreonly for non-reranked entries. Whentop_ncovers the whole result list,composite_tailhasn=0. Both rerank reports show this (full-B_rerank_sigmoid_floor.jsonandfull-C_rerank_composite_floor.json,composite_tail.n = 0). The recommended default filters on the composite score, so the probe should report the composite distribution in the same run that produces the recommendation. Collectr.composite_scorefor all entries.♻️ Proposed change
for rec in records: for r in rec.results: + comp_scores.append(r.composite_score) if r.is_reranked: sig_scores.append(r.score) if r.session_id in rec.relevant: rel_sig.append(r.score) else: irr_sig.append(r.score) - else: - comp_scores.append(r.score)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/threshold_probe.py` around lines 455 - 475, Update _score_distributions so composite_tail is populated with r.composite_score for every result, regardless of r.is_reranked. Keep sigmoid, relevant_sigmoid, and irrelevant_sigmoid collection scoped to reranked entries, while ensuring the composite distribution is available for recommendation generation even when all results were reranked.
115-176: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcode of the dataset path limits reuse.
Line 117 hardcodes
eval/data/locomo/locomo10.json. The probe fails when the caller runs it from another working directory, and it cannot point at a downloaded copy. Add a--data-pathoption and pass it through.♻️ Proposed change
-def _load_locomo() -> list[Unit]: +def _load_locomo(data_path: Path = Path("eval/data/locomo/locomo10.json")) -> list[Unit]: adapter = LoCoMoAdapter() - scenarios = adapter.load(Path("eval/data/locomo/locomo10.json")) + scenarios = adapter.load(data_path)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/threshold_probe.py` around lines 115 - 176, Update _load_locomo to accept a data-path argument and use it when constructing the Path passed to LoCoMoAdapter.load instead of hardcoding eval/data/locomo/locomo10.json. Add a --data-path CLI option, parse it with the existing argument handling, and pass the supplied path through to _load_locomo so callers can run from any working directory or use downloaded data.eval/retrieval_lab.py (1)
455-460: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueIn sweep mode,
empty_fracreports only the first config.Line 457 prints
empty_fracinside theif not header_done:block. Each sweep config computes its own_empty_frac, but the header shows the value of the first config only, and the reader sees it as a property of the whole sweep. Addempty_fracas a per-row column instead.♻️ Proposed change
if not header_done: - print(f" units={res['_units']} q={res['_total']} empty_frac={res['_empty_frac']:.3f}") - print(f" {'config':22s} " + " ".join(f"{k:>8s}" for k in kk)) + print(f" units={res['_units']} q={res['_total']}") + print(f" {'config':22s} " + " ".join(f"{k:>8s}" for k in kk) + f" {'empty':>8s}") header_done = True - print(f" {label:22s} " + " ".join(f"{res[k]:8.3f}" for k in kk)) + print(f" {label:22s} " + " ".join(f"{res[k]:8.3f}" for k in kk) + + f" {res['_empty_frac']:8.3f}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/retrieval_lab.py` around lines 455 - 460, Move empty_frac out of the one-time header in the sweep output and add it as a per-row column alongside the config metrics. Update the header and each row in the block following the result-key collection so every config prints its own res['_empty_frac'] value, while retaining the existing units and total metadata in the one-time header.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Around line 20-25: Update the strict-recall threshold calibration changelog
entry to state explicitly that the shipped recall_min_score default remains 0.8
in this release, while the 0.6 value is only a recommendation from the
calibration report.
In `@eval/reports/threshold_calibration/full-C_rerank_composite_floor.json`:
- Around line 2-10: The report configuration built in main_async must identify
composite floor runs and avoid recording an unused ratio sweep. Add a floor_mode
field set to "composite" for --composite-floor reports, and record
rerank_floor_ratio_range as "N/A" in that mode while preserving the existing
range for other modes. Ensure the recorded default ratio is not misleading for
composite runs, including the current ratio field near the report’s summary
configuration.
In `@eval/reports/threshold_calibration/threshold_calibration_report.md`:
- Line 34: Update the threshold explanation at
eval/reports/threshold_calibration/threshold_calibration_report.md:34-34 to
remove the p90/93.1% percentile claim and state that the 0.5 floor exceeds the
relevant-sample median of 0.166, eliminating more than half of correct hits.
Apply the equivalent wording change at
eval/reports/threshold_calibration/threshold_calibration_report_EN.md:34-34,
removing “93.1st percentile” while preserving the same numerical conclusion.
- Around line 46-52: Correct the Mode C values in both
eval/reports/threshold_calibration/threshold_calibration_report.md#L46-L52 and
eval/reports/threshold_calibration/threshold_calibration_report_EN.md#L46-L52 to
R@1 57.1%, R@10 64.9%, and average results 2.30; in both documents, update the
Mode C R@10 values at lines 91, 124, and 133 for min_score 0.60 from 91.5% to
91.7%, matching full-C_rerank_composite_floor.json.
In `@eval/retrieval_lab.py`:
- Around line 381-385: Update the emptiness check in the retrieval loop around
_ranked_keys so it uses keys, which is assigned by both the kw_channel and
regular search branches, instead of reading resp.results. Preserve the existing
empty_count increment when keys is empty.
- Around line 377-380: Update the argument handling around args.min_score and
args.rerank_floor_ratio so mq_kwargs["rerank_floor_ratio"] is set whenever
args.rerank_floor_ratio is not None, independently of the args.min_score > 0
check. Preserve conditional inclusion of mq_kwargs["min_score"] only for
positive min_score values.
In `@eval/threshold_probe.py`:
- Around line 541-548: Update the fallback selection logic around `non_empty` so
cells with `empty_frac == 0.0` are eligible and preferred when minimizing empty
results. Remove the strict positive filter, preserve the existing
minimum-within-tolerance and `(recall_at[k], min_score)` tie-breaking behavior,
and ensure `best` is selected whenever any cell is available.
- Around line 628-635: Update _parse_range to reject non-positive step values
and ranges where stop is less than start before calculating n. Raise a clear
ValueError for either invalid condition, while preserving the existing parsing
and generated values for valid ranges.
- Around line 671-690: Update the sweep setup in the probe to read the
production defaults from Settings.recall_min_score and
Settings.rerank_floor_ratio before building min_scores and ratios, replacing the
hardcoded current_ms and current_ratio values. Ensure both defaults are inserted
into their respective grids, while preserving the existing --rerank gating for
current_ratio insertion.
In `@src/hebb/integrations/claude_code/recall.py`:
- Around line 60-64: Complete the public docstrings for handle and handle_prompt
by adding Args, Returns, and Raises sections. Document each function’s
parameters, state that both return None, and describe that operational failures
are suppressed; do not alter their behavior.
In `@src/hebb/models/memory.py`:
- Line 121: Ensure every MemorySearchResult preserves its composite score before
reranking: in src/hebb/models/memory.py lines 121-121, remove the 0.0 default
for pre_rerank_score so it cannot represent a missing value; in
src/hebb/retrieval/searcher.py lines 268-268, initialize pre_rerank_score to
disp_score when constructing each MemorySearchResult before the rerank loop.
In `@src/hebb/retrieval/searcher.py`:
- Line 286: Add a PEP 257-style docstring to the public MemorySearcher.search
method, documenting its parameters in an Args section, return value in Returns,
and possible exceptions in Raises. Keep the implementation unchanged and
describe the method’s existing behavior accurately.
In `@src/hebb/server/routers/search.py`:
- Around line 41-45: Preserve strict-recall semantics across all three call
sites: in src/hebb/server/routers/search.py lines 41-45, inject
settings.rerank_floor_ratio whenever strict_recall is enabled and the request
lacks an explicit ratio, including when min_score is provided; in
src/hebb/mcp/server.py lines 112-116 and
src/hebb/integrations/claude_code/recall.py lines 155-159, include
strict_recall=True alongside the explicit min_score request.
In `@src/hebb/static/js/components/config-section.js`:
- Around line 176-185: Update the numeric validation in the configuration save
flow around rerank_floor_ratio and recall_min_score to parse trimmed input with
Number(raw.trim()) and validate it using Number.isFinite(num), while retaining
the existing 0–1 bounds and error handling. This must reject malformed values
such as “0.5invalid” before constructing newValue or calling the API.
In `@tests/integration/server/test_api.py`:
- Around line 218-224: Strengthen the min_score integration tests in
tests/integration/server/test_api.py at lines 218-224 and 231-241: in
test_explicit_min_score_no_strengthening, assert the search returns results
before validating access counts; in
test_explicit_min_score_not_overridden_by_router, verify the stricter threshold
filters results or assert both thresholds reach the searcher unchanged, while
preserving the existing loose-versus-strict comparison.
---
Outside diff comments:
In `@src/hebb/mcp/server.py`:
- Around line 91-107: Complete the public search_memory docstring by adding
Returns and Raises sections. Document that it returns the formatted search
results and that request failures propagated by the function are raised, while
preserving the existing Args documentation.
---
Nitpick comments:
In `@eval/retrieval_lab.py`:
- Around line 455-460: Move empty_frac out of the one-time header in the sweep
output and add it as a per-row column alongside the config metrics. Update the
header and each row in the block following the result-key collection so every
config prints its own res['_empty_frac'] value, while retaining the existing
units and total metadata in the one-time header.
In `@eval/threshold_probe.py`:
- Around line 455-475: Update _score_distributions so composite_tail is
populated with r.composite_score for every result, regardless of r.is_reranked.
Keep sigmoid, relevant_sigmoid, and irrelevant_sigmoid collection scoped to
reranked entries, while ensuring the composite distribution is available for
recommendation generation even when all results were reranked.
- Around line 115-176: Update _load_locomo to accept a data-path argument and
use it when constructing the Path passed to LoCoMoAdapter.load instead of
hardcoding eval/data/locomo/locomo10.json. Add a --data-path CLI option, parse
it with the existing argument handling, and pass the supplied path through to
_load_locomo so callers can run from any working directory or use downloaded
data.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ce9dfc0d-0879-4ff7-912d-93c61d98e29a
📒 Files selected for processing (21)
CHANGELOG.mdeval/reports/threshold_calibration/full-A_composite_baseline.jsoneval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.jsoneval/reports/threshold_calibration/full-C_rerank_composite_floor.jsoneval/reports/threshold_calibration/threshold_calibration_report.mdeval/reports/threshold_calibration/threshold_calibration_report_EN.mdeval/retrieval_lab.pyeval/threshold_probe.pysrc/hebb/config/settings.pysrc/hebb/integrations/claude_code/recall.pysrc/hebb/mcp/server.pysrc/hebb/models/memory.pysrc/hebb/retrieval/searcher.pysrc/hebb/server/routers/search.pysrc/hebb/static/js/components/activate.jssrc/hebb/static/js/components/config-section.jssrc/hebb/static/js/components/system.jssrc/hebb/static/js/i18n.jstests/integration/server/test_api.pytests/unit/integrations/test_claude_code_hooks.pytests/unit/test_audit_retrieval.py
| "config": { | ||
| "dataset": "locomo", | ||
| "reranker": "BAAI/bge-reranker-base", | ||
| "top_n": 30, | ||
| "top_k": 10, | ||
| "vector": false, | ||
| "min_score_range": "0.5,0.95,0.05", | ||
| "rerank_floor_ratio_range": "0.3,1.0,0.05" | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
The recorded config does not describe the run.
Line 9 records "rerank_floor_ratio_range": "0.3,1.0,0.05", but every sweep cell in this file has "ratio": 1.0, because --composite-floor routes to _sweep_composite and that function ignores the ratio grid. Line 176 also records "ratio": 1.0 as the current default, while the production default is 0.625. A reader of this artifact cannot tell which floor mode produced the numbers.
Record the filter mode in config and set the unused range to "N/A" in composite mode. Add a field such as "floor_mode": "composite" in main_async so each committed report is self-describing.
Also applies to: 174-182
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@eval/reports/threshold_calibration/full-C_rerank_composite_floor.json` around
lines 2 - 10, The report configuration built in main_async must identify
composite floor runs and avoid recording an unused ratio sweep. Add a floor_mode
field set to "composite" for --composite-floor reports, and record
rerank_floor_ratio_range as "N/A" in that mode while preserving the existing
range for other modes. Ensure the recorded default ratio is not misleading for
composite runs, including the current ratio field near the report’s summary
configuration.
| if args.min_score > 0: | ||
| mq_kwargs["min_score"] = args.min_score | ||
| if args.rerank_floor_ratio is not None: | ||
| mq_kwargs["rerank_floor_ratio"] = args.rerank_floor_ratio |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect the MemoryQuery defaults for min_score and rerank_floor_ratio.
rg -nP 'min_score|rerank_floor_ratio|strict_recall' --type=py -C2 src/hebb/models/memory.pyRepository: afx-team/hebb-mind
Length of output: 1468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== eval retrieval_lab relevant lines =="
sed -n '340,390p' eval/retrieval_lab.py
echo
sed -n '488,520p' eval/retrieval_lab.py
echo
echo "== memory.py relevant default =="
sed -n '75,105p' src/hebb/models/memory.pyRepository: afx-team/hebb-mind
Length of output: 6340
Apply --rerank-floor-ratio independently of --min-score.
--min-score defaults to 0.0, which is the MemoryQuery.no filter default, so --rerank-floor-ratio 0.5 alone is currently ignored. Set mq_kwargs["rerank_floor_ratio"] whenever the flag is provided; the ratio field only translates the floor when min_score > 0, so it still does not filter alone.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@eval/retrieval_lab.py` around lines 377 - 380, Update the argument handling
around args.min_score and args.rerank_floor_ratio so
mq_kwargs["rerank_floor_ratio"] is set whenever args.rerank_floor_ratio is not
None, independently of the args.min_score > 0 check. Preserve conditional
inclusion of mq_kwargs["min_score"] only for positive min_score values.
| _recall_and_print( | ||
| query=_SESSION_START_QUERY, | ||
| current_session_id=session_id, | ||
| timeout=20, | ||
| min_score=_resolve_hook_min_score(), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the public hook docstrings.
handle and handle_prompt omit Args, Returns, and Raises sections. Add the required sections, including that both functions return None and suppress operational failures.
As per coding guidelines, “All public APIs in Python MUST have docstrings with Args, Returns, and Raises sections.”
Also applies to: 85-90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/claude_code/recall.py` around lines 60 - 64, Complete
the public docstrings for handle and handle_prompt by adding Args, Returns, and
Raises sections. Document each function’s parameters, state that both return
None, and describe that operational failures are suppressed; do not alter their
behavior.
Source: Coding guidelines
| # rerank scale for the pool and keep the composite floor for the tail. | ||
| if query.min_score > 0.0: | ||
| rerank_floor = query.min_score * _RERANK_FLOOR_RATIO | ||
| rerank_floor = query.min_score * query.rerank_floor_ratio |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add an API docstring to MemorySearcher.search.
MemorySearcher.search is public and has no docstring. Add Args, Returns, and Raises sections.
As per coding guidelines, “All public APIs in Python MUST have docstrings with Args, Returns, and Raises sections.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/retrieval/searcher.py` at line 286, Add a PEP 257-style docstring to
the public MemorySearcher.search method, documenting its parameters in an Args
section, return value in Returns, and possible exceptions in Raises. Keep the
implementation unchanged and describe the method’s existing behavior accurately.
Source: Coding guidelines
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/unit/test_audit_retrieval.py (1)
149-240: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd return type annotations to the new test functions.
Add
-> Noneto each new test function. This keeps the test modules compatible with strict type checking.
tests/unit/test_audit_retrieval.py#L149-L240: add-> Noneto each newasync def test_*function.tests/unit/integrations/test_claude_code_hooks.py#L347-L376: add-> Nonetotest_handle_respects_hook_min_scoreandtest_handle_defaults_to_strict_recall_when_hook_min_score_none.tests/unit/integrations/test_claude_code_hooks.py#L416-L433: add-> Nonetotest_handle_prompt_respects_hook_min_score.As per coding guidelines, “
**/*.py: Add type hints on all public functions (mypy strict is enabled in pyproject.toml).”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_audit_retrieval.py` around lines 149 - 240, Add -> None return annotations to every new async test function in tests/unit/test_audit_retrieval.py lines 149-240. Also annotate test_handle_respects_hook_min_score and test_handle_defaults_to_strict_recall_when_hook_min_score_none in tests/unit/integrations/test_claude_code_hooks.py lines 347-376, plus test_handle_prompt_respects_hook_min_score in lines 416-433; no other changes are needed.Source: Coding guidelines
src/hebb/mcp/server.py (1)
96-107: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winComplete the public
search_memorydocstring.
search_memoryhas anArgssection, but it has noReturnsorRaisessections. Document the formatted result and the HTTP or response-decoding errors that propagate to the caller.Suggested docstring addition
Args: query: Natural language search query. top_k: Maximum number of results to return (1-100, default 5). min_score: Optional relevance floor (0-1). When set, overrides the server's configured ``recall_min_score``. When omitted, strict recall is enabled at the server's configured floor. + Returns: + Formatted search results, or "No memories found." + Raises: + httpx.HTTPError: If the search request fails or returns a non-2xx status. + ValueError: If the response body is not valid JSON. """As per coding guidelines, “All public APIs in Python MUST have docstrings with Args, Returns, and Raises sections.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/mcp/server.py` around lines 96 - 107, Complete the public search_memory docstring by adding Returns and Raises sections. Document the formatted search result returned to callers, and identify the HTTP errors and response-decoding errors that can propagate from the underlying request; leave the existing Args documentation unchanged.Source: Coding guidelines
♻️ Duplicate comments (3)
eval/threshold_probe.py (1)
627-634: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winValidate
stepand the range order in_parse_range.
step=0raisesZeroDivisionErrorat line 633. A negativesteporstop < startmakesnzero or negative, so the function returns an empty list and the sweep prints an empty table with no explanation. Reject both inputs.🛡️ Proposed fix
start, stop, step = parts + if step <= 0: + raise ValueError(f"step must be positive — got: {step}") + if stop < start: + raise ValueError(f"stop must be >= start — got: {start},{stop}") n = int(round((stop - start) / step)) + 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/threshold_probe.py` around lines 627 - 634, Update _parse_range to reject a zero or negative step and any range where stop is less than start before calculating n. Raise a clear ValueError for each invalid input, while preserving the existing parsing and list generation for valid ascending ranges.eval/retrieval_lab.py (1)
377-380: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply
--rerank-floor-ratioindependently of--min-score.
--min-scoredefaults to0.0, so--rerank-floor-ratio 0.5alone never reachesMemoryQuery. Setrerank_floor_ratiowhenever the flag is provided. The field only translates the floor whenmin_score > 0, so it still does not filter on its own.🐛 Proposed fix
if args.min_score > 0: mq_kwargs["min_score"] = args.min_score - if args.rerank_floor_ratio is not None: - mq_kwargs["rerank_floor_ratio"] = args.rerank_floor_ratio + if args.rerank_floor_ratio is not None: + mq_kwargs["rerank_floor_ratio"] = args.rerank_floor_ratio🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/retrieval_lab.py` around lines 377 - 380, Update the argument handling around min_score and rerank_floor_ratio so mq_kwargs["rerank_floor_ratio"] is set whenever args.rerank_floor_ratio is provided, regardless of args.min_score. Keep mq_kwargs["min_score"] conditional on args.min_score > 0, preserving the existing behavior that rerank_floor_ratio alone does not filter results.eval/reports/threshold_calibration/full-C_rerank_composite_floor.json (1)
2-10: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winBoth committed reports predate the probe's current
configschema.main_asyncnow always writesfloor_modeand setsrerank_floor_ratio_rangeto"N/A"in composite mode. Neither artifact containsfloor_mode, so a reader cannot tell which floor mode produced the numbers.
eval/reports/threshold_calibration/full-C_rerank_composite_floor.json#L2-L10: regenerate with the current probe soconfigcontains"floor_mode": "composite"and"rerank_floor_ratio_range": "N/A", because every cell in this file usesratio1.0.eval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.json#L2-L10: regenerate soconfigcontains"floor_mode": "sigmoid".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/reports/threshold_calibration/full-C_rerank_composite_floor.json` around lines 2 - 10, Regenerate eval/reports/threshold_calibration/full-C_rerank_composite_floor.json (lines 2-10) with the current main_async schema, adding config.floor_mode as "composite" and setting rerank_floor_ratio_range to "N/A"; all cells use ratio 1.0. Also regenerate eval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.json (lines 2-10) so its config includes floor_mode set to "sigmoid".
🧹 Nitpick comments (2)
CHANGELOG.md (1)
29-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new
MemoryQuery.rerank_floor_ratiofield.The
Addedsection lists theSettingsfield and the frontend item, but not the per-query override.eval/retrieval_lab.pyline 380 passesrerank_floor_ratiotoMemoryQuery, so the field is part of the public query model. Add an entry for it, and consider listing the committed calibration reports undereval/reports/threshold_calibration/.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CHANGELOG.md` around lines 29 - 38, The CHANGELOG Added section omits the public MemoryQuery.rerank_floor_ratio override and calibration reports. Add a concise entry documenting MemoryQuery.rerank_floor_ratio as a per-query override, and list the committed threshold calibration reports under eval/reports/threshold_calibration/ if they are present.eval/retrieval_lab.py (1)
457-460: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
empty_fracin the sweep header describes only the first config.Line 457 prints
empty_fraconce, inside theheader_doneguard. When you combine--sweepwith--min-score, each config can produce a different empty fraction, but the table shows the value from the first config only. Moveempty_fracinto the per-config row.♻️ Proposed refactor
if not header_done: - print(f" units={res['_units']} q={res['_total']} empty_frac={res['_empty_frac']:.3f}") - print(f" {'config':22s} " + " ".join(f"{k:>8s}" for k in kk)) + print(f" units={res['_units']} q={res['_total']}") + print(f" {'config':22s} " + " ".join(f"{k:>8s}" for k in kk) + f" {'empty':>8s}") header_done = True - print(f" {label:22s} " + " ".join(f"{res[k]:8.3f}" for k in kk)) + print(f" {label:22s} " + " ".join(f"{res[k]:8.3f}" for k in kk) + + f" {res['_empty_frac']:8.3f}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/retrieval_lab.py` around lines 457 - 460, Move the empty_frac output from the header block guarded by header_done into the per-config print row alongside each config’s other metrics. Update the row formatting to display res['_empty_frac'] for every result while keeping units, total, and the shared table header printed only once.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@eval/reports/threshold_calibration/full-A_composite_baseline.json`:
- Around line 2-10: Regenerate the threshold calibration artifact using the
current probe and its main_async output format. Ensure the config includes
floor_mode, and replace the stale composite default row around ratio 0.625 with
the current current_ratio=1.0 result, including the resolved min_score 0.8
metrics instead of null values.
In `@eval/reports/threshold_calibration/threshold_calibration_report.md`:
- Line 135: Update the recommended (0.6, composite) average result count from
7.3 to 7.4 in both
eval/reports/threshold_calibration/threshold_calibration_report.md at lines
135-135 and
eval/reports/threshold_calibration/threshold_calibration_report_EN.md at lines
135-135, keeping the Chinese and English tables consistent.
- Line 89: Update the mode C empty-recall value at min_score 0.50 from 0.0% to
0.2% in eval/reports/threshold_calibration/threshold_calibration_report.md at
line 89 and
eval/reports/threshold_calibration/threshold_calibration_report_EN.md at line
89, and apply the same correction in appendix table C at line 209 in both files.
In `@src/hebb/static/js/components/config-section.js`:
- Around line 176-185: Normalize whitespace for the score fields in the
configuration save flow before validation and before constructing newValue, so
whitespace-only input is treated as empty and follows the existing null
behavior. Update the logic around the rerank_floor_ratio and recall_min_score
handling while preserving range validation for non-empty numeric values.
---
Outside diff comments:
In `@src/hebb/mcp/server.py`:
- Around line 96-107: Complete the public search_memory docstring by adding
Returns and Raises sections. Document the formatted search result returned to
callers, and identify the HTTP errors and response-decoding errors that can
propagate from the underlying request; leave the existing Args documentation
unchanged.
In `@tests/unit/test_audit_retrieval.py`:
- Around line 149-240: Add -> None return annotations to every new async test
function in tests/unit/test_audit_retrieval.py lines 149-240. Also annotate
test_handle_respects_hook_min_score and
test_handle_defaults_to_strict_recall_when_hook_min_score_none in
tests/unit/integrations/test_claude_code_hooks.py lines 347-376, plus
test_handle_prompt_respects_hook_min_score in lines 416-433; no other changes
are needed.
---
Duplicate comments:
In `@eval/reports/threshold_calibration/full-C_rerank_composite_floor.json`:
- Around line 2-10: Regenerate
eval/reports/threshold_calibration/full-C_rerank_composite_floor.json (lines
2-10) with the current main_async schema, adding config.floor_mode as
"composite" and setting rerank_floor_ratio_range to "N/A"; all cells use ratio
1.0. Also regenerate
eval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.json (lines 2-10)
so its config includes floor_mode set to "sigmoid".
In `@eval/retrieval_lab.py`:
- Around line 377-380: Update the argument handling around min_score and
rerank_floor_ratio so mq_kwargs["rerank_floor_ratio"] is set whenever
args.rerank_floor_ratio is provided, regardless of args.min_score. Keep
mq_kwargs["min_score"] conditional on args.min_score > 0, preserving the
existing behavior that rerank_floor_ratio alone does not filter results.
In `@eval/threshold_probe.py`:
- Around line 627-634: Update _parse_range to reject a zero or negative step and
any range where stop is less than start before calculating n. Raise a clear
ValueError for each invalid input, while preserving the existing parsing and
list generation for valid ascending ranges.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 29-38: The CHANGELOG Added section omits the public
MemoryQuery.rerank_floor_ratio override and calibration reports. Add a concise
entry documenting MemoryQuery.rerank_floor_ratio as a per-query override, and
list the committed threshold calibration reports under
eval/reports/threshold_calibration/ if they are present.
In `@eval/retrieval_lab.py`:
- Around line 457-460: Move the empty_frac output from the header block guarded
by header_done into the per-config print row alongside each config’s other
metrics. Update the row formatting to display res['_empty_frac'] for every
result while keeping units, total, and the shared table header printed only
once.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a68755f-e5ad-4d1b-b60a-ef8a01a698ab
📒 Files selected for processing (21)
CHANGELOG.mdeval/reports/threshold_calibration/full-A_composite_baseline.jsoneval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.jsoneval/reports/threshold_calibration/full-C_rerank_composite_floor.jsoneval/reports/threshold_calibration/threshold_calibration_report.mdeval/reports/threshold_calibration/threshold_calibration_report_EN.mdeval/retrieval_lab.pyeval/threshold_probe.pysrc/hebb/config/settings.pysrc/hebb/integrations/claude_code/recall.pysrc/hebb/mcp/server.pysrc/hebb/models/memory.pysrc/hebb/retrieval/searcher.pysrc/hebb/server/routers/search.pysrc/hebb/static/js/components/activate.jssrc/hebb/static/js/components/config-section.jssrc/hebb/static/js/components/system.jssrc/hebb/static/js/i18n.jstests/integration/server/test_api.pytests/unit/integrations/test_claude_code_hooks.pytests/unit/test_audit_retrieval.py
| "config": { | ||
| "dataset": "locomo", | ||
| "reranker": "none", | ||
| "top_n": 0, | ||
| "top_k": 10, | ||
| "vector": false, | ||
| "min_score_range": "0.5,0.95,0.05", | ||
| "rerank_floor_ratio_range": "N/A" | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
This artifact does not match the probe's current output format.
main_async now always writes floor_mode into config, but lines 2-10 have no floor_mode key. Lines 174-182 record "ratio": 0.625 with every metric null, while the composite branch of the current code passes current_ratio=1.0 and would resolve the min_score 0.8 cell (R@1 0.5536, R@10 0.6355, empty_frac 0.2755). Regenerate this report with the current probe so the committed evidence matches the tool and the current-default row carries values.
Also applies to: 174-182
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@eval/reports/threshold_calibration/full-A_composite_baseline.json` around
lines 2 - 10, Regenerate the threshold calibration artifact using the current
probe and its main_async output format. Ensure the config includes floor_mode,
and replace the stale composite default row around ratio 0.625 with the current
current_ratio=1.0 result, including the resolved min_score 0.8 metrics instead
of null values.
| | min_score | A — R@10 | A — 空召回率 | C — R@10 | C — 空召回率 | C — 平均结果数 | | ||
| | ---------- | --------- | -------- | --------- | -------- | --------- | | ||
| | 无门槛 | 91.5% | 0.0% | 94.6% | 0.0% | 10.0 | | ||
| | **0.50** | **90.4%** | **0.3%** | **93.0%** | **0.0%** | **8.7** | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The mode C empty-recall rate at min_score 0.50 is rounded away. full-C_rerank_composite_floor.json records empty_frac 0.0020, which is 0.2%. Both reports print 0.0%, while the neighbouring mode A value 0.0025 is printed as 0.3%. The inconsistent rounding suggests mode C never returns an empty result at this floor.
eval/reports/threshold_calibration/threshold_calibration_report.md#L89-L89: 将 C 模式 0.50 行的空召回率改为 0.2%，并同步修正附录表 C（第 209 行）。eval/reports/threshold_calibration/threshold_calibration_report_EN.md#L89-L89: change the mode C empty recall at 0.50 to 0.2%, and apply the same correction in appendix table C at line 209.
📍 Affects 2 files
eval/reports/threshold_calibration/threshold_calibration_report.md#L89-L89(this comment)eval/reports/threshold_calibration/threshold_calibration_report_EN.md#L89-L89
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@eval/reports/threshold_calibration/threshold_calibration_report.md` at line
89, Update the mode C empty-recall value at min_score 0.50 from 0.0% to 0.2% in
eval/reports/threshold_calibration/threshold_calibration_report.md at line 89
and eval/reports/threshold_calibration/threshold_calibration_report_EN.md at
line 89, and apply the same correction in appendix table C at line 209 in both
files.
| const raw = input.type === 'checkbox' ? String(input.checked) : input.value; | ||
| // Frontend validation for 0-1 numeric fields before hitting the API. | ||
| if ((key === 'rerank_floor_ratio' || key === 'recall_min_score') && raw !== '') { | ||
| const num = Number(raw.trim()); | ||
| if (!Number.isFinite(num) || num < 0 || num > 1) { | ||
| error(t('settings.error.range_0_1', { key })); | ||
| return; | ||
| } | ||
| } | ||
| const newValue = raw || 'null'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle whitespace-only score input before validation.
For input such as " ", raw !== '' is true. Number(raw.trim()) becomes 0, so validation passes. raw || 'null' then sends the whitespace string to the API.
Normalize score fields before the empty check and before constructing newValue.
Suggested fix
const raw = input.type === 'checkbox' ? String(input.checked) : input.value;
+ const isScoreKey = key === 'rerank_floor_ratio' || key === 'recall_min_score';
+ const normalizedRaw = raw.trim();
// Frontend validation for 0-1 numeric fields before hitting the API.
- if ((key === 'rerank_floor_ratio' || key === 'recall_min_score') && raw !== '') {
- const num = Number(raw.trim());
+ if (isScoreKey && normalizedRaw !== '') {
+ const num = Number(normalizedRaw);
if (!Number.isFinite(num) || num < 0 || num > 1) {
error(t('settings.error.range_0_1', { key }));
return;
}
}
- const newValue = raw || 'null';
+ const newValue = (isScoreKey ? normalizedRaw : raw) || 'null';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const raw = input.type === 'checkbox' ? String(input.checked) : input.value; | |
| // Frontend validation for 0-1 numeric fields before hitting the API. | |
| if ((key === 'rerank_floor_ratio' || key === 'recall_min_score') && raw !== '') { | |
| const num = Number(raw.trim()); | |
| if (!Number.isFinite(num) || num < 0 || num > 1) { | |
| error(t('settings.error.range_0_1', { key })); | |
| return; | |
| } | |
| } | |
| const newValue = raw || 'null'; | |
| const raw = input.type === 'checkbox' ? String(input.checked) : input.value; | |
| const isScoreKey = key === 'rerank_floor_ratio' || key === 'recall_min_score'; | |
| const normalizedRaw = raw.trim(); | |
| // Frontend validation for 0-1 numeric fields before hitting the API. | |
| if (isScoreKey && normalizedRaw !== '') { | |
| const num = Number(normalizedRaw); | |
| if (!Number.isFinite(num) || num < 0 || num > 1) { | |
| error(t('settings.error.range_0_1', { key })); | |
| return; | |
| } | |
| } | |
| const newValue = (isScoreKey ? normalizedRaw : raw) || 'null'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/static/js/components/config-section.js` around lines 176 - 185,
Normalize whitespace for the score fields in the configuration save flow before
validation and before constructing newValue, so whitespace-only input is treated
as empty and follows the existing null behavior. Update the logic around the
rerank_floor_ratio and recall_min_score handling while preserving range
validation for non-empty numeric values.
…stead of sigmoid gating
ch-qiaoyongheng
changed the title
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hebb/config/settings.py (1)
172-178: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the
recall_hook_min_scoredescription.The recall hook sends this value as
filter_score, notmin_score. The fallback is the composite-score filter configuration, notrecall_min_score. This text can cause operators to configure the wrong threshold when the two settings differ.Proposed fix
- description="Per-deployment min_score override for the recall hook; " - "None = use strict_recall (global recall_min_score)", + description="Per-deployment composite-score filter override for the recall hook; " + "None = use the global filter_score",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/config/settings.py` around lines 172 - 178, Update the description for the recall_hook_min_score field in the settings configuration to state that the value is sent as filter_score and that None falls back to the composite-score filter configuration, replacing the incorrect min_score and recall_min_score references.
♻️ Duplicate comments (1)
src/hebb/static/js/components/config-section.js (1)
176-185: 🎯 Functional Correctness | 🟡 MinorNormalize whitespace-only
filter_scoreinput before validation.At Line 178,
raw !== ''is true for" ".Number(raw.trim())becomes0, so validation passes. At Line 185,raw || 'null'sends the whitespace string instead ofnull. Normalize the score before the empty check and before creatingnewValue.This is the same whitespace-validation defect previously reported for the legacy score fields, now present on
filter_score.Suggested fix
const raw = input.type === 'checkbox' ? String(input.checked) : input.value; + const normalizedRaw = key === 'filter_score' ? raw.trim() : raw; // Frontend validation for 0-1 numeric fields before hitting the API. - if (key === 'filter_score' && raw !== '') { - const num = Number(raw.trim()); + if (key === 'filter_score' && normalizedRaw !== '') { + const num = Number(normalizedRaw); if (!Number.isFinite(num) || num < 0 || num > 1) { error(t('settings.error.range_0_1', { key })); return; } } - const newValue = raw || 'null'; + const newValue = normalizedRaw || 'null';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hebb/static/js/components/config-section.js` around lines 176 - 185, Normalize the `filter_score` value by trimming whitespace before the empty check and before constructing `newValue`. Use the normalized value for numeric validation and ensure whitespace-only input is treated as empty so it is sent as `null`, while preserving existing validation for non-empty scores.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hebb/integrations/claude_code/recall.py`:
- Around line 56-68: Update _resolve_hook_filter_score so recall_hook_min_score
is not returned as filter_score; preserve filter_score as the sole
composite-score override and return it when configured, otherwise None. Route
recall_hook_min_score through the hook’s min_score rollback path or an
established migration/alias mechanism instead.
In `@src/hebb/mcp/server.py`:
- Around line 105-111: Complete the public search_memory docstring by adding
Returns and Raises sections alongside its existing Args section. Describe the
formatted string returned by the tool and document the request and response
errors callers must handle, using the function’s existing exception behavior and
symbols rather than inventing new error types.
In `@src/hebb/retrieval/searcher.py`:
- Around line 284-302: Update the preferred filter in the query retrieval flow
to compare each result’s pre_rerank_score against query.filter_score instead of
score, including the comparison inside the results loop. Keep score unchanged
for final reranked ordering, and leave the legacy min_score fallback behavior
intact.
In `@src/hebb/server/routers/search.py`:
- Around line 41-45: Update the public search_memories docstring to document the
precedence among filter_score, min_score, and strict_recall, including that an
explicitly provided filter_score overrides the strict_recall default. Add
complete Args, Returns, and Raises sections describing the method’s parameters,
result, and possible exceptions.
- Around line 41-45: Update the strict-recall filter-score injection condition
in the query handling logic so settings.filter_score is applied only when
neither filter_score nor the legacy min_score is explicitly supplied. Preserve
caller-provided min_score and filter_score values unchanged.
---
Outside diff comments:
In `@src/hebb/config/settings.py`:
- Around line 172-178: Update the description for the recall_hook_min_score
field in the settings configuration to state that the value is sent as
filter_score and that None falls back to the composite-score filter
configuration, replacing the incorrect min_score and recall_min_score
references.
---
Duplicate comments:
In `@src/hebb/static/js/components/config-section.js`:
- Around line 176-185: Normalize the `filter_score` value by trimming whitespace
before the empty check and before constructing `newValue`. Use the normalized
value for numeric validation and ensure whitespace-only input is treated as
empty so it is sent as `null`, while preserving existing validation for
non-empty scores.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fb9df99-513f-4736-af18-98c731e4c4f0
📒 Files selected for processing (12)
CHANGELOG.mdeval/reports/threshold_calibration/threshold_calibration_report.mdsrc/hebb/config/settings.pysrc/hebb/integrations/claude_code/recall.pysrc/hebb/mcp/server.pysrc/hebb/models/memory.pysrc/hebb/retrieval/searcher.pysrc/hebb/server/routers/search.pysrc/hebb/static/js/components/activate.jssrc/hebb/static/js/components/config-section.jssrc/hebb/static/js/i18n.jstests/unit/integrations/test_claude_code_hooks.py
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/unit/integrations/test_claude_code_hooks.py
- src/hebb/static/js/i18n.js
- eval/reports/threshold_calibration/threshold_calibration_report.md
| def _resolve_hook_filter_score() -> float | None: | ||
| """Return the per-deployment filter_score override, or None for default. | ||
|
|
||
| Priority: recall_hook_min_score > filter_score > None (strict_recall path). | ||
| """ | ||
| try: | ||
| settings = load_settings() | ||
| # If recall_hook_min_score is explicitly set, use it as filter_score | ||
| if settings.recall_hook_min_score is not None: | ||
| return settings.recall_hook_min_score | ||
| return settings.filter_score | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 'recall_hook_min_score|filter_score|min_score' src testsRepository: afx-team/hebb-mind
Length of output: 50374
Don’t map recall_hook_min_score directly to filter_score.
recall_hook_min_score is documented as the per-deployment min_score override, while filter_score is the composite-score filter. Since an existing hook config with recall_hook_min_score=0.7 currently sends filter_score=0.7, it changes the legacy override from the old dual-scale behavior to direct composite filtering. Send this value to min_score only for rollback, or add a migration/alias if this was intentional.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/integrations/claude_code/recall.py` around lines 56 - 68, Update
_resolve_hook_filter_score so recall_hook_min_score is not returned as
filter_score; preserve filter_score as the sole composite-score override and
return it when configured, otherwise None. Route recall_hook_min_score through
the hook’s min_score rollback path or an established migration/alias mechanism
instead.
|
https://github.com/coderabbitai full review |
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 9 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
eval/threshold_probe.py (1)
627-634: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
_parse_rangestill accepts an invalidstep.A
stepof0raisesZeroDivisionErrorat line 633. A negativesteporstop < startmakesnzero or negative, and the function returns an empty list. The sweep then produces no cells and the probe prints an empty table with no explanation. This was raised in a previous review and is still present.🛡️ Proposed fix
start, stop, step = parts + if step <= 0: + raise ValueError(f"step must be positive — got: {step}") + if stop < start: + raise ValueError(f"stop must be >= start — got: {start},{stop}") n = int(round((stop - start) / step)) + 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/threshold_probe.py` around lines 627 - 634, Validate inputs in _parse_range before calculating n: reject step values less than or equal to zero, and reject ranges where stop is less than start, by raising a clear ValueError. Preserve the existing parsing and list-generation behavior for valid ascending ranges.eval/reports/threshold_calibration/threshold_calibration_report.md (2)
135-135: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe comparison table in both reports still states 7.3 average results. Sections 3.2 and 4.2 in both documents state 7.4, and
full-C_rerank_composite_floor.jsonrecordsmean_results7.36 atmin_score0.6. Only the two-approach comparison table was left at 7.3.
eval/reports/threshold_calibration/threshold_calibration_report.md#L135-L135: 将"推荐方案 (0.6, composite)"列的平均结果数从 7.3 改为 7.4。eval/reports/threshold_calibration/threshold_calibration_report_EN.md#L135-L135: change the avg results value for "Recommended (0.6, composite)" from 7.3 to 7.4.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/reports/threshold_calibration/threshold_calibration_report.md` at line 135, Update the comparison table’s recommended composite approach average-results value from 7.3 to 7.4 in eval/reports/threshold_calibration/threshold_calibration_report.md at lines 135-135 and eval/reports/threshold_calibration/threshold_calibration_report_EN.md at lines 135-135; make no other changes.
89-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBoth language reports misprint two mode C empty-recall values.
full-C_rerank_composite_floor.jsonrecordsempty_frac0.0020 atmin_score0.50 and 0.0046 atmin_score0.55, which are 0.2% and 0.5%. Both documents print 0.0% and 0.1%. The neighbouring mode A value 0.0025 is already printed as 0.3%, so this is not consistent rounding.
eval/reports/threshold_calibration/threshold_calibration_report.md#L89-L90: 将 C 模式 0.50 行改为 0.2%、0.55 行改为 0.5%，并同步修正附录表 C 第 209 行。eval/reports/threshold_calibration/threshold_calibration_report_EN.md#L89-L90: set the mode C 0.50 empty recall to 0.2% and the 0.55 empty recall to 0.5%, and apply the same correction in appendix table C at line 209.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/reports/threshold_calibration/threshold_calibration_report.md` around lines 89 - 90, The mode C empty-recall values are misprinted in both language reports. Update eval/reports/threshold_calibration/threshold_calibration_report.md lines 89-90 and eval/reports/threshold_calibration/threshold_calibration_report_EN.md lines 89-90 to show 0.2% at min_score 0.50 and 0.5% at 0.55, and apply the same corrections to appendix table C at line 209 in both files.eval/retrieval_lab.py (1)
377-380: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
--rerank-floor-ratiois still ignored unless--min-scoreis positive.Line 377 gates both keyword arguments on
args.min_score > 0.--min-scoredefaults to0.0, so--rerank-floor-ratio 0.5alone never reachesMemoryQuery. Setrerank_floor_ratiowhenever the flag is provided. The ratio only scales the floor whenmin_score > 0, so it still cannot filter on its own. This repeats a previous review comment.🐛 Proposed fix
if args.min_score > 0: mq_kwargs["min_score"] = args.min_score - if args.rerank_floor_ratio is not None: - mq_kwargs["rerank_floor_ratio"] = args.rerank_floor_ratio + if args.rerank_floor_ratio is not None: + mq_kwargs["rerank_floor_ratio"] = args.rerank_floor_ratio🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/retrieval_lab.py` around lines 377 - 380, Update the argument-to-mq_kwargs handling in the retrieval query setup so rerank_floor_ratio is added whenever args.rerank_floor_ratio is not None, independently of args.min_score. Keep min_score gated by args.min_score > 0, preserving the behavior that rerank_floor_ratio alone does not filter results.eval/reports/threshold_calibration/full-C_rerank_composite_floor.json (1)
2-10: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAll three committed reports predate the current probe output format.
main_asyncineval/threshold_probe.pynow writesfloor_modeintoconfigand setsrerank_floor_ratio_rangeto"N/A"in composite mode. No committed artifact carriesfloor_mode, so none of them is self-describing. Regenerate all three with the current probe.
eval/reports/threshold_calibration/full-C_rerank_composite_floor.json#L2-L10: regenerate soconfiggainsfloor_mode: "composite"andrerank_floor_ratio_rangebecomes"N/A"instead of the unused"0.3,1.0,0.05"grid.eval/reports/threshold_calibration/full-A_composite_baseline.json#L174-L182: regenerate after fixing the default-resolution logic ateval/threshold_probe.pylines 670-690, so thecurrent_defaultblock carries values instead ofnull.eval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.json#L2-L10: regenerate soconfiggainsfloor_mode: "sigmoid".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/reports/threshold_calibration/full-C_rerank_composite_floor.json` around lines 2 - 10, Regenerate all three reports using the current eval/threshold_probe.py output: update eval/reports/threshold_calibration/full-C_rerank_composite_floor.json lines 2-10 with floor_mode "composite" and rerank_floor_ratio_range "N/A"; regenerate eval/reports/threshold_calibration/full-A_composite_baseline.json lines 174-182 after fixing main_async’s default-resolution logic around lines 670-690 so current_default contains values rather than null; and update eval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.json lines 2-10 with floor_mode "sigmoid".
🧹 Nitpick comments (1)
eval/retrieval_lab.py (1)
383-385: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
empty_fracmeasures missing metadata keys, not empty results.
_ranked_keysdrops any result whose metadata has nosession_id(or nosid/global_idx). It also folds inresp.related. A query that returns results without the metric key is therefore counted as empty, and a query whose only output is related memories is counted as non-empty.eval/threshold_probe.pycounts emptiness fromresp.resultsalone, so the two tools report different quantities under the same name.Track both counters if you want the numbers to be comparable.
♻️ Proposed change
total += 1 - if not keys: + if not keys: empty_count += 1 + if not kw_channel and not resp.results: + no_result_count += 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/retrieval_lab.py` around lines 383 - 385, Update the evaluation loop around _ranked_keys to distinguish empty retrieval results from missing metric keys: determine the empty-results count from resp.results alone, while separately counting queries whose results lack usable session_id/sid/global_idx metadata and those represented only by resp.related. Preserve total and expose both counters so empty_frac matches threshold_probe and missing-key behavior remains measurable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CHANGELOG.md`:
- Line 26: Update the changelog entry for the recommended filter_score=0.6
configuration to report R@10 as 91.7%, matching the calibration reports; do not
use the 91.5% unfiltered baseline value.
In `@eval/reports/threshold_calibration/threshold_calibration_report.md`:
- Around line 124-125: Update both
eval/reports/threshold_calibration/threshold_calibration_report.md (lines 19,
124-125, and 164) and
eval/reports/threshold_calibration/threshold_calibration_report_EN.md (lines 19,
124-125, and 164): replace the recommended 0.6 setting name with filter_score,
and explicitly state that recall_min_score is deprecated in the
backward-compatibility notes.
In `@src/hebb/config/settings.py`:
- Around line 173-178: Update the description on the recall_hook_min_score Field
to accurately document that None makes the recall hook send
settings.filter_score with strict_recall=True, while a configured value is
mapped to filter_score; remove the incorrect reference to recall_min_score.
---
Duplicate comments:
In `@eval/reports/threshold_calibration/full-C_rerank_composite_floor.json`:
- Around line 2-10: Regenerate all three reports using the current
eval/threshold_probe.py output: update
eval/reports/threshold_calibration/full-C_rerank_composite_floor.json lines 2-10
with floor_mode "composite" and rerank_floor_ratio_range "N/A"; regenerate
eval/reports/threshold_calibration/full-A_composite_baseline.json lines 174-182
after fixing main_async’s default-resolution logic around lines 670-690 so
current_default contains values rather than null; and update
eval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.json lines 2-10
with floor_mode "sigmoid".
In `@eval/reports/threshold_calibration/threshold_calibration_report.md`:
- Line 135: Update the comparison table’s recommended composite approach
average-results value from 7.3 to 7.4 in
eval/reports/threshold_calibration/threshold_calibration_report.md at lines
135-135 and
eval/reports/threshold_calibration/threshold_calibration_report_EN.md at lines
135-135; make no other changes.
- Around line 89-90: The mode C empty-recall values are misprinted in both
language reports. Update
eval/reports/threshold_calibration/threshold_calibration_report.md lines 89-90
and eval/reports/threshold_calibration/threshold_calibration_report_EN.md lines
89-90 to show 0.2% at min_score 0.50 and 0.5% at 0.55, and apply the same
corrections to appendix table C at line 209 in both files.
In `@eval/retrieval_lab.py`:
- Around line 377-380: Update the argument-to-mq_kwargs handling in the
retrieval query setup so rerank_floor_ratio is added whenever
args.rerank_floor_ratio is not None, independently of args.min_score. Keep
min_score gated by args.min_score > 0, preserving the behavior that
rerank_floor_ratio alone does not filter results.
In `@eval/threshold_probe.py`:
- Around line 627-634: Validate inputs in _parse_range before calculating n:
reject step values less than or equal to zero, and reject ranges where stop is
less than start, by raising a clear ValueError. Preserve the existing parsing
and list-generation behavior for valid ascending ranges.
---
Nitpick comments:
In `@eval/retrieval_lab.py`:
- Around line 383-385: Update the evaluation loop around _ranked_keys to
distinguish empty retrieval results from missing metric keys: determine the
empty-results count from resp.results alone, while separately counting queries
whose results lack usable session_id/sid/global_idx metadata and those
represented only by resp.related. Preserve total and expose both counters so
empty_frac matches threshold_probe and missing-key behavior remains measurable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d367dbac-b654-44bb-a763-7715683456a8
📒 Files selected for processing (21)
CHANGELOG.mdeval/reports/threshold_calibration/full-A_composite_baseline.jsoneval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.jsoneval/reports/threshold_calibration/full-C_rerank_composite_floor.jsoneval/reports/threshold_calibration/threshold_calibration_report.mdeval/reports/threshold_calibration/threshold_calibration_report_EN.mdeval/retrieval_lab.pyeval/threshold_probe.pysrc/hebb/config/settings.pysrc/hebb/integrations/claude_code/recall.pysrc/hebb/mcp/server.pysrc/hebb/models/memory.pysrc/hebb/retrieval/searcher.pysrc/hebb/server/routers/search.pysrc/hebb/static/js/components/activate.jssrc/hebb/static/js/components/config-section.jssrc/hebb/static/js/components/system.jssrc/hebb/static/js/i18n.jstests/integration/server/test_api.pytests/unit/integrations/test_claude_code_hooks.pytests/unit/test_audit_retrieval.py
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
eval/reports/threshold_calibration/threshold_calibration_report.md (2)
135-135: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe recommended-configuration avg-results figure was not actually corrected.
Line 135 states 7.3 average results for "推荐方案 (0.6, composite)", but
full-C_rerank_composite_floor.jsonrecordsmean_results7.36 (rounds to 7.4) atmin_score0.6, and Line 124 on the same page already states "平均 7.4 条" for the same configuration. A prior review flagged this exact line and it was marked as addressed, but the value shown here is still the old, incorrect one.🐛 Proposed fix
-| 平均结果数 | 1.4 | 7.3 | 8.7 | +| 平均结果数 | 1.4 | 7.4 | 8.7 |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/reports/threshold_calibration/threshold_calibration_report.md` at line 135, Update the recommended configuration’s average-results value in the report table from 7.3 to 7.4, matching the 7.36 mean_results value and the existing “平均 7.4 条” statement for the same configuration.
89-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMode C empty-recall values do not match the source data.
full-C_rerank_composite_floor.jsonrecordsempty_frac0.0020 (0.2%) atmin_score0.50 andempty_frac0.0046 (0.5% rounded) atmin_score0.55. Lines 89 and 90 both print the C column as effectively rounded to 0.0%/0.1%, and the appendix at line 209 repeats the 0.50 error. The neighboring A column at the same rows rounds correctly (0.3%, 0.5%), so this looks like a transcription slip rather than intentional rounding.🐛 Proposed fix
-| **0.50** | **90.4%** | **0.3%** | **93.0%** | **0.0%** | **8.7** | -| 0.55 | 90.0% | 0.5% | 92.8% | 0.1% | 8.1 | +| **0.50** | **90.4%** | **0.3%** | **93.0%** | **0.2%** | **8.7** | +| 0.55 | 90.0% | 0.5% | 92.8% | 0.5% | 8.1 |-| 0.50 | 72.9% | 87.0% | 90.7% | 93.0% | 0.0% | 8.7 | +| 0.50 | 72.9% | 87.0% | 90.7% | 93.0% | 0.2% | 8.7 |Also applies to: 209-209
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/reports/threshold_calibration/threshold_calibration_report.md` around lines 89 - 90, Update the Mode C empty-recall values in the threshold table and its appendix entry to match full-C_rerank_composite_floor.json: use 0.2% for min_score 0.50 and 0.5% for min_score 0.55, preserving the neighboring columns and existing formatting.eval/reports/threshold_calibration/full-C_rerank_composite_floor.json (1)
2-10: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winConfig still misrepresents this run.
Every sweep cell in this file has
"ratio": 1.0because--composite-floorroutes to_sweep_composite, which ignores the ratio grid, yet line 9 still records"rerank_floor_ratio_range": "0.3,1.0,0.05". The current probe code sets this field to"N/A"for composite mode and adds afloor_modekey, but neither change is reflected in this committed artifact.🐛 Proposed fix
"config": { "dataset": "locomo", + "floor_mode": "composite", "reranker": "BAAI/bge-reranker-base", "top_n": 30, "top_k": 10, "vector": false, "min_score_range": "0.5,0.95,0.05", - "rerank_floor_ratio_range": "0.3,1.0,0.05" + "rerank_floor_ratio_range": "N/A" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/reports/threshold_calibration/full-C_rerank_composite_floor.json` around lines 2 - 10, Update the committed composite-mode calibration artifact configuration to match the probe output: set rerank_floor_ratio_range to "N/A" and add the floor_mode key indicating composite mode. Preserve the existing dataset, reranker, and sweep settings.eval/retrieval_lab.py (1)
372-380: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winApply
--rerank-floor-ratioindependently of--min-score.Line 379 sets
mq_kwargs["rerank_floor_ratio"]only whenargs.min_score > 0.MemoryQuery.min_scorehas its own non-zero production default, so omittingmin_scorefrommq_kwargsdoes not mean "no floor" — the searcher still applies the model's default floor.--rerank-floor-ratio 0.5alone (without--min-score) is currently silently ignored even though it should still affect that default floor.🐛 Proposed fix
if args.min_score > 0: mq_kwargs["min_score"] = args.min_score - if args.rerank_floor_ratio is not None: - mq_kwargs["rerank_floor_ratio"] = args.rerank_floor_ratio + if args.rerank_floor_ratio is not None: + mq_kwargs["rerank_floor_ratio"] = args.rerank_floor_ratio🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/retrieval_lab.py` around lines 372 - 380, Update the mq_kwargs construction so args.rerank_floor_ratio is added whenever it is not None, independently of the args.min_score > 0 condition. Keep args.min_score handling unchanged, while ensuring a standalone --rerank-floor-ratio overrides the searcher’s default floor.eval/threshold_probe.py (1)
627-634: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winValidate
stepand range direction in_parse_range.
step=0raisesZeroDivisionErrorat Line 633. A negativesteporstop < startproducesn <= 0and an empty list, and the sweep then silently prints an empty table. Reject a non-positivestepand an inverted range before computingn.🛡️ Proposed fix
start, stop, step = parts + if step <= 0: + raise ValueError(f"step must be positive — got: {step}") + if stop < start: + raise ValueError(f"stop must be >= start — got: {start},{stop}") n = int(round((stop - start) / step)) + 1🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/threshold_probe.py` around lines 627 - 634, Update _parse_range to validate that step is positive and stop is not less than start before calculating n. Raise ValueError for invalid step or inverted ranges, while preserving the existing parsing and range generation behavior for valid inputs.eval/reports/threshold_calibration/full-A_composite_baseline.json (1)
2-10: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRegenerate this artifact with the current probe.
main_asyncnow always writesfloor_modeintoconfig, but lines 2-10 have nofloor_modekey. Lines 174-182 still record"ratio": 0.625with every metricnull, while the composite branch of the current code passescurrent_ratio=1.0and would resolve themin_score=0.8cell (R@1 0.5536, R@10 0.6355, empty_frac 0.2755) from the sweep at lines 123-132. Regenerate this report with the current probe so the committed evidence matches the tool and the current-default row carries values.🐛 Proposed fix (until regenerated)
"config": { "dataset": "locomo", + "floor_mode": "composite", "reranker": "none","current_default": { "min_score": 0.8, - "ratio": 0.625, - "R@1": null, - "R@3": null, - "R@5": null, - "R@10": null, - "empty_frac": null + "ratio": 1.0, + "R@1": 0.5536, + "R@3": 0.6158, + "R@5": 0.6289, + "R@10": 0.6355, + "empty_frac": 0.2755 }Also applies to: 174-182
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@eval/reports/threshold_calibration/full-A_composite_baseline.json` around lines 2 - 10, Regenerate full-A_composite_baseline.json using the current probe and commit the refreshed artifact. Ensure the config includes the floor_mode field written by main_async, and update the current-default composite result from the stale ratio 0.625/null metrics to the current_ratio=1.0 result resolved from the sweep, including its corresponding metrics.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@eval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.json`:
- Around line 2-10: Add the missing floor_mode configuration entry to the
artifact’s config, setting it to "sigmoid" to match the probe mode that produced
these results. Preserve the existing dataset, reranker, and threshold settings.
In `@eval/reports/threshold_calibration/threshold_calibration_report_EN.md`:
- Around line 89-90: Update the Mode C empty-recall values in the threshold
calibration report to match full-C_rerank_composite_floor.json: use 0.2% for
min_score 0.50 and 0.5% for 0.55, and correct the repeated 0.50 value in the
appendix.
- Line 135: Update the “Recommended (0.6, composite)” Avg Results value in the
report table from 7.3 to 7.4, matching the 7.36 mean_results value in
full-C_rerank_composite_floor.json rounded to one decimal place.
In `@src/hebb/config/settings.py`:
- Around line 133-145: The deprecated recall_min_score setting is documented as
a rollback switch but is not consumed by the search path. Add an explicit
rollback branch in the supplied search_memories flow that uses configured
recall_min_score for legacy filtering, and add an integration test proving that
changing the rollback value changes results; otherwise remove the setting and
its rollback documentation.
---
Duplicate comments:
In `@eval/reports/threshold_calibration/full-A_composite_baseline.json`:
- Around line 2-10: Regenerate full-A_composite_baseline.json using the current
probe and commit the refreshed artifact. Ensure the config includes the
floor_mode field written by main_async, and update the current-default composite
result from the stale ratio 0.625/null metrics to the current_ratio=1.0 result
resolved from the sweep, including its corresponding metrics.
In `@eval/reports/threshold_calibration/full-C_rerank_composite_floor.json`:
- Around line 2-10: Update the committed composite-mode calibration artifact
configuration to match the probe output: set rerank_floor_ratio_range to "N/A"
and add the floor_mode key indicating composite mode. Preserve the existing
dataset, reranker, and sweep settings.
In `@eval/reports/threshold_calibration/threshold_calibration_report.md`:
- Line 135: Update the recommended configuration’s average-results value in the
report table from 7.3 to 7.4, matching the 7.36 mean_results value and the
existing “平均 7.4 条” statement for the same configuration.
- Around line 89-90: Update the Mode C empty-recall values in the threshold
table and its appendix entry to match full-C_rerank_composite_floor.json: use
0.2% for min_score 0.50 and 0.5% for min_score 0.55, preserving the neighboring
columns and existing formatting.
In `@eval/retrieval_lab.py`:
- Around line 372-380: Update the mq_kwargs construction so
args.rerank_floor_ratio is added whenever it is not None, independently of the
args.min_score > 0 condition. Keep args.min_score handling unchanged, while
ensuring a standalone --rerank-floor-ratio overrides the searcher’s default
floor.
In `@eval/threshold_probe.py`:
- Around line 627-634: Update _parse_range to validate that step is positive and
stop is not less than start before calculating n. Raise ValueError for invalid
step or inverted ranges, while preserving the existing parsing and range
generation behavior for valid inputs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64e8d5c1-8085-47c2-819d-3b394c969430
📒 Files selected for processing (21)
CHANGELOG.mdeval/reports/threshold_calibration/full-A_composite_baseline.jsoneval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.jsoneval/reports/threshold_calibration/full-C_rerank_composite_floor.jsoneval/reports/threshold_calibration/threshold_calibration_report.mdeval/reports/threshold_calibration/threshold_calibration_report_EN.mdeval/retrieval_lab.pyeval/threshold_probe.pysrc/hebb/config/settings.pysrc/hebb/integrations/claude_code/recall.pysrc/hebb/mcp/server.pysrc/hebb/models/memory.pysrc/hebb/retrieval/searcher.pysrc/hebb/server/routers/search.pysrc/hebb/static/js/components/activate.jssrc/hebb/static/js/components/config-section.jssrc/hebb/static/js/components/system.jssrc/hebb/static/js/i18n.jstests/integration/server/test_api.pytests/unit/integrations/test_claude_code_hooks.pytests/unit/test_audit_retrieval.py
| # DEPRECATED: This field is retained only as an emergency rollback switch. | ||
| # New code should use ``filter_score`` for composite-score filtering instead. | ||
| # The router no longer reads this field; it was replaced by filter_score | ||
| # after eval (LoCoMo, 1978 queries) proved sigmoid scores unsuitable for | ||
| # hard filtering. See: eval/reports/threshold_calibration/threshold_calibration_review_v2.md | ||
| recall_min_score: float = Field( | ||
| default=0.8, | ||
| default=0.6, | ||
| ge=0.0, | ||
| le=1.0, | ||
| description="Min relevance score (0-1) for hook/MCP recall; results below are dropped (console Search unaffected)", | ||
| description="[DEPRECATED] Retained as emergency rollback only. Use filter_score instead. " | ||
| "Original purpose: min relevance score for hook/MCP recall. " | ||
| "Replaced by filter_score after eval proved sigmoid scores unsuitable for hard filtering.", | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make recall_min_score a working rollback setting.
Line 135 states that the router no longer reads this field. The supplied search_memories path injects only settings.filter_score for strict recall. Changing recall_min_score therefore cannot restore the legacy behavior promised here.
Add an explicit rollback path that consumes recall_min_score, or remove the setting and its rollback documentation. Add an integration test that proves a configured rollback value changes filtering.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/config/settings.py` around lines 133 - 145, The deprecated
recall_min_score setting is documented as a rollback switch but is not consumed
by the search path. Add an explicit rollback branch in the supplied
search_memories flow that uses configured recall_min_score for legacy filtering,
and add an integration test proving that changing the rollback value changes
results; otherwise remove the setting and its rollback documentation.
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
for file in \
src/hebb/server/routers/search.py \
src/hebb/retrieval/searcher.py \
src/hebb/mcp/server.py \
src/hebb/integrations/claude_code/recall.py \
src/hebb/config/settings.py \
src/hebb/models/memory.py \
tests/integration/server/test_api.py \
tests/unit/integrations/test_claude_code_hooks.py \
tests/unit/test_audit_retrieval.py
do
printf '\n===== OUTLINE: %s =====\n' "$file"
ast-grep outline "$file" --items all 2>/dev/null || true
done
printf '\n===== SCORE-RELATED IMPLEMENTATION =====\n'
rg -n -C 6 'filter_score|recall_min_score|rerank_floor_ratio|min_score|strict_recall' \
src/hebb/server/routers/search.py \
src/hebb/retrieval/searcher.py \
src/hebb/mcp/server.py \
src/hebb/integrations/claude_code/recall.py \
src/hebb/config/settings.py \
src/hebb/models/memory.py
printf '\n===== SCORE-RELATED TESTS =====\n'
rg -n -C 5 'filter_score|recall_min_score|rerank_floor_ratio|min_score|strict_recall' \
tests/integration/server/test_api.py \
tests/unit/integrations/test_claude_code_hooks.py \
tests/unit/test_audit_retrieval.pyLength of output: 73038
For MCP, For the Claude Code recall hook, The legacy branch requires both conditions: query.filter_score == 0.0 and query.min_score > 0.0The current MCP and hook request paths prevent that combination. Therefore, 🐇 🔎 ✏️ Learnings added
You are interacting with an AI system. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@eval/reports/threshold_calibration/threshold_calibration_report_EN.md`:
- Line 127: Update the lenient approach entries for filter_score/min_score 0.5
in the threshold calibration report, including the line 127 description and the
lenient table column at line 133, from 93.0% to the source-aligned 93.5%. Keep
the other metrics and existing references unchanged.
In `@src/hebb/server/routers/search.py`:
- Around line 61-67: Update the strict-recall threshold handling so an
explicitly supplied min_score remains on the legacy filtering path instead of
being copied into updates["filter_score"]. Only inject settings.filter_score
when neither filter_score nor min_score appears in query.model_fields_set,
preserving the existing behavior for callers that provide filter_score.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b7cea9e4-9371-47cd-a6c6-63284682e4f2
📒 Files selected for processing (21)
CHANGELOG.mdeval/reports/threshold_calibration/full-A_composite_baseline.jsoneval/reports/threshold_calibration/full-B_rerank_sigmoid_floor.jsoneval/reports/threshold_calibration/full-C_rerank_composite_floor.jsoneval/reports/threshold_calibration/threshold_calibration_report.mdeval/reports/threshold_calibration/threshold_calibration_report_EN.mdeval/retrieval_lab.pyeval/threshold_probe.pysrc/hebb/config/settings.pysrc/hebb/integrations/claude_code/recall.pysrc/hebb/mcp/server.pysrc/hebb/models/memory.pysrc/hebb/retrieval/searcher.pysrc/hebb/server/routers/search.pysrc/hebb/static/js/components/activate.jssrc/hebb/static/js/components/config-section.jssrc/hebb/static/js/components/system.jssrc/hebb/static/js/i18n.jstests/integration/server/test_api.pytests/unit/integrations/test_claude_code_hooks.pytests/unit/test_audit_retrieval.py
| | `filter_score` | — | **0.6** | R@10=91.7%, empty recall=1.0%, avg 7.4 results — balances filtering purity and recall | | ||
| | `rerank_floor_ratio` | 0.625 | **Retained but not used in default filtering** | Kept as a fallback switch; can be enabled when sigmoid filtering is needed | | ||
|
|
||
| **More lenient approach (if zero empty recall is required)**: `filter_score = 0.5`, R@10=93.0%, empty recall=0.0%, avg 8.7 results. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the lenient-approach R@10 with the source data.
Line 127 and the lenient column at line 133 state R@10 = 93.0% for filter_score = 0.5. full-C_rerank_composite_floor.json records R@10 0.9348 at min_score 0.50, and lines 89 and 209 of this document already print 93.5%. The document contradicts itself.
🐛 Proposed fix
-**More lenient approach (if zero empty recall is required)**: `filter_score = 0.5`, R@10=93.0%, empty recall=0.0%, avg 8.7 results.
+**More lenient approach (if near-zero empty recall is required)**: `filter_score = 0.5`, R@10=93.5%, empty recall=0.2%, avg 8.7 results.-| R@10 | 53.5% | **91.7%** | 93.0% |
+| R@10 | 53.5% | **91.7%** | 93.5% |Also applies to: 133-133
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@eval/reports/threshold_calibration/threshold_calibration_report_EN.md` at
line 127, Update the lenient approach entries for filter_score/min_score 0.5 in
the threshold calibration report, including the line 127 description and the
lenient table column at line 133, from 93.0% to the source-aligned 93.5%. Keep
the other metrics and existing references unchanged.
| if query.strict_recall and "filter_score" not in query.model_fields_set: | ||
| if "min_score" in query.model_fields_set: | ||
| # Backward compat: convert legacy min_score to filter_score | ||
| updates["filter_score"] = query.min_score | ||
| else: | ||
| # Default: use configured filter_score | ||
| updates["filter_score"] = settings.filter_score |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep an explicit min_score on the legacy filtering path.
When a strict-recall caller sends min_score, this code converts it to filter_score. The searcher then bypasses rerank_floor_ratio. This makes the documented emergency rollback path unavailable to strict-recall clients.
Only inject settings.filter_score when neither threshold was supplied.
Proposed fix
- if query.strict_recall and "filter_score" not in query.model_fields_set:
- if "min_score" in query.model_fields_set:
- # Backward compat: convert legacy min_score to filter_score
- updates["filter_score"] = query.min_score
- else:
- # Default: use configured filter_score
- updates["filter_score"] = settings.filter_score
+ if (
+ query.strict_recall
+ and "filter_score" not in query.model_fields_set
+ and "min_score" not in query.model_fields_set
+ ):
+ updates["filter_score"] = settings.filter_score🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hebb/server/routers/search.py` around lines 61 - 67, Update the
strict-recall threshold handling so an explicitly supplied min_score remains on
the legacy filtering path instead of being copied into updates["filter_score"].
Only inject settings.filter_score when neither filter_score nor min_score
appears in query.model_fields_set, preserving the existing behavior for callers
that provide filter_score.
|
@coderabbitai |
|
Tip For best results, initiate chat on the files or code changes.
The current behavior matches the intended design:
I withdraw the finding. 🐇 ✏️ Learnings added
You are interacting with an AI system. |
1 participant
Summary
Promote the hardcoded
_RERANK_FLOOR_RATIOconstant to a configurable Settings field, pass through min_score via MCP server and Claude Code recall hook, and calibrate the default threshold through a full LoCoMo evaluation (1,978 queries) — confirming that sigmoid scores are unsuitable for hard filtering and recommendingfilter_score = 0.6with composite score filtering.Closes #31
Background
Audit C5 / Recall F3 identified that the post-rerank candidate pool carries cross-encoder sigmoid scores while the tail retains composite
disp_score— two different distributions.Applying a single
recall_min_score=0.8threshold causes bge-reranker to silently clear strict recall on correct but non-literal short-conversation hits (39% of queries return empty results).Issue #31 divided the work into three parts:
rerank_floor_ratiohardcoded constant to a configurable fieldAfter a full LoCoMo evaluation (1,978 queries) and discussion with the team, the new approach was confirmed:
rerank_floor_ratioas the default production mechanism (existing parameters marked with DEPRECATED comments)filter_scorefor composite-score-based filteringTask 1: Add filter_score configuration to replace the old dual-scale filtering
Problem with the original approach:
Using
rerank_floor_ratioto map composite scores to the sigmoid scale for gating, but evaluation proved sigmoid scores are unsuitable for hard filtering.New approach:
filter_scoreconfiguration field (default0.6, range[0,1])rerank_floor_ratioandrecall_min_scoreas DEPRECATEDrecall_hook_min_scoreas a deployment-level overrideModified files:
src/hebb/config/settings.py: Add filter_score field, mark old fields as DEPRECATEDsrc/hebb/models/memory.py: Add filter_score field to MemoryQuerysrc/hebb/server/routers/search.py: Update routing logic to prioritize filter_scoresrc/hebb/retrieval/searcher.py: Update filtering logic to prioritize composite score filteringTask 2: Pass through min_score via MCP server and recall hook
Problem with the original approach:
MCP server and recall hook hardcode
strict_recall: True, unable to pass per-call overrides.New approach:
filter_scoreparameter, taking priority over min_scorerecall_hook_min_scoreconfiguration and uses it as filter_scoreModified files:
src/hebb/mcp/server.py: Add filter_score parameter, support min_score pass-throughsrc/hebb/integrations/claude_code/recall.py: Add filter_score configuration reading and passingTask 3: Threshold calibration — validate defaults based on evaluation data
Problem with the original approach:
recall_min_score=0.8and_RERANK_FLOOR_RATIO=0.625were heuristically derived and never validated on annotated datasets.Evaluation probe:
eval/threshold_probe.py: Offline probe for full min_score × rerank_floor_ratio parameter sweep, reporting R@k, empty recall ratio, and score distribution statisticsFull evaluation (LoCoMo, 1,978 queries)
Key finding
Sigmoid scores are unsuitable for hard filtering — the root cause is not "which threshold to choose" but "wrong filtering dimension":
Full 160-combination sigmoid sweep: no pair achieves R@10 ≥ 90% with empty result rate < 5%.
Recommended approach:
Recommended default: Use composite filter threshold
filter_score = 0.6.Task 4: Frontend configuration UI update
Modified files:
src/hebb/static/js/components/activate.js: Remove recall_min_score and rerank_floor_ratio, add filter_scoresrc/hebb/static/js/components/config-section.js: Update validation logic, remove old field handlingsrc/hebb/static/js/i18n.js: Add Chinese and English translations for filter_scoreChecklist
Review Notes
Evaluation artifacts:
eval/reports/threshold_calibration/full-{A,B,C}_*.jsoneval/reports/threshold_calibration/threshold_calibration_review_v2.mdAudit items:
Key decisions:
This change only adjusts the filtering threshold and filtering dimension (sigmoid → composite), without altering the relative ranking order of results — a monotonic threshold-only change that, per the issue's release rules, does not require A/B testing.
rerank_floor_ratiois retained as a configurable field as an emergency rollback switch — to restore sigmoid filtering, just modifyhebb.jsonwithout redeployment.CodeRabbit Summary
New features:
Bug fixes:
Tests:
Summary by CodeRabbit