feat(search): make retrieval and fusion scores self-describing - #144
AhmedIrfan7 wants to merge 3 commits into
Conversation
Search responses returned several ranking numbers (a raw vector-store distance, a derived per-channel score, a per-channel rank, and a combined fusion score) with no statement of what each meant or which direction ranked better, and the derived scores could be mistaken for calibrated confidences. Add a RetrievalScoring descriptor, carried on both SearchResult and FusedSearchResult, that states the distance metric, the low/high ranking direction of raw_distance and score, the distance->score transform (negated_distance), and that both values are ordering_only rather than a probability. Extend FusionProvenance to declare the combined moment score as ordering_only and higher-is-better, and document requested vs searched channels. Add field descriptions to every ranking value on SearchHit, FusedMoment, and the evidence artifacts so the meaning is identical across the CLI, HTTP, MCP, stored job results, and evidence delivery. The CLI search table now labels the score column and captions the metric and ordering-only meaning. The change is additive: every new field is defaulted, so previously stored job results and existing FusedSearchResult payloads still validate. Scores remain uncalibrated; calibrated scoring is deferred to the end-to-end ranking evaluation in grayhatdevelopers#76. Closes grayhatdevelopers#90
|
Hey @AhmedIrfan7, |
|
hello, @SaadBazaz, I've starred the repo. Thanks. |
tulayha
left a comment
tulayha
left a comment
There was a problem hiding this comment.
The regular-search changes look reasonable. Three gaps remain in how score information carries through query answers, evidence, and older saved results. I've left the details inline.
The branch also currently has conflicts with main. Please sync it while making these changes; I can help resolve the conflicts if needed.
Could you also leave a quick comment on #90 confirming that you're working on it? There isn't one there yet, and that will let me assign the issue to you.
| query_id: str = Field(min_length=1) | ||
| query: SearchQuery | ||
| modalities: tuple[Identifier, ...] | ||
| scoring: RetrievalScoring = Field( |
There was a problem hiding this comment.
This adds the retrieval descriptor to regular search results, but video-query answers lose it. GroundedQueryService.answer() copies the moments and evidence into QueryAnswer, which has no scoring field, and the individual hits don’t carry the descriptor either.
Please preserve this information in query answers too. A test using cosine retrieval should verify that the metric and score conversion survive when the fused result becomes a query answer.
There was a problem hiding this comment.
Done. QueryAnswer now carries the same scoring descriptor as FusedSearchResult, copied from the fused search in GroundedQueryService.answer(), so it also survives in stored query job results. I kept it at the response level rather than copying it onto every hit, since each hit in the answer is described by that single descriptor, the same way search results already work. test_query_answer_keeps_cosine_retrieval_scoring runs a cosine index through search_embeddings, fusion, and the answer, then checks the metric and the score conversion, including after a JSON round trip.
| score: float | None = Field( | ||
| default=None, | ||
| description=( | ||
| "Combined ordering-only fusion score copied from the source moment; " |
There was a problem hiding this comment.
This description says the value is a combined fusion score, but _query_candidates() copies evidence.hit.score, which is the individual channel score. For example, I got an evidence score of -1.0 while the fused moment’s score was about 0.01639.
Please distinguish the score’s actual source and carry its scoring information into the evidence payload. The same description on EvidenceDeliveryItem.score needs attention. Cover both search and video-query evidence, since they currently supply different kinds of scores.
There was a problem hiding this comment.
Good catch, and thanks for the concrete example. EvidenceBoardCandidate and EvidenceDeliveryItem, and therefore board tiles, now have score_semantics. Search evidence is marked fused_moment and carries the fusion provenance, while video-query moment evidence is marked channel_hit and carries the answer's retrieval descriptor, including the metric. Actor evidence has no score, so its score_semantics stays null, and payloads saved before this change load with it null instead of a guessed source. Both score descriptions are now source-neutral. The tests cover search and video-query evidence using your -1.0 versus roughly 0.01639 case, along with actor evidence, board tiles, and a validator that rejects semantics without a score.
| """ | ||
|
|
||
| distance_metric: Literal["l2", "cosine", "ip"] = Field( | ||
| default="l2", |
There was a problem hiding this comment.
Defaulting this to l2 makes older saved results report a metric we don’t know. Those results may have used cosine or inner product, but they didn’t store this descriptor. Loading successfully shouldn’t change an unknown metric into a definite l2.
Please keep the metric absent or explicitly unknown when it wasn’t recorded, while continuing to populate it for new searches. The legacy-result test should verify that missing information stays unknown.
There was a problem hiding this comment.
Agreed. distance_metric now defaults to null, meaning not recorded, and search_embeddings still records the configured metric for every new search, including l2. Fusion also reports a metric only when every searched channel recorded the same one, so an unrecorded channel can't become a definite metric there either, and the CLI prints "not recorded" in that case. The legacy search-result and query-answer tests check that the metric stays null.
Keep the retrieval distance metric unknown when it was not recorded instead of defaulting to l2. New searches still record the configured metric, and fusion reports a metric only when every searched channel recorded the same one, so an unrecorded channel never becomes a definite metric. The CLI shows "not recorded" in that case. Carry the retrieval descriptor into QueryAnswer so a fused search keeps its metric and score conversion when it becomes a video-query answer, including in stored query job results. Describe evidence scores by their actual source. Search evidence copies the fused moment score, while video-query evidence copies one channel's hit score. EvidenceBoardCandidate, EvidenceBoardTile, and EvidenceDeliveryItem now carry score_semantics: fused_moment with the fusion provenance, or channel_hit with the retrieval descriptor. It is null for actor evidence and for evidence saved before the field existed. All new fields are optional, so stored job results, board requests, and evidence payloads saved earlier still load.
|
Thanks for the detailed review. I merged main and resolved the conflicts with #141 and #146, keeping the shared-overlap fusion and candidate-limit changes as they are. The three review points are addressed in the latest commit, and I've replied on each thread. |
3 participants
What
Make every ranking number in a search response self-describing, so callers never have to guess what a value means or which direction ranks better, and can't mistake an uncalibrated score for a confidence.
Closes #90.
Why
A search response carried several numbers — a raw vector-store
raw_distance, a derived per-channelscore, a per-channelrank, and a combined fusionscore— with nothing in the payload stating the distance metric, the low/high direction, howscoreis derived, or whether the value is calibrated. The negated-distance and RRF scores could read like probabilities.Changes
RetrievalScoringdescriptor on bothSearchResultandFusedSearchResult, stating:distance_metric(l2/cosine/ip), threaded fromIndexConfig.vector_distance;raw_distancedirection (lower is closer) andscoredirection (higher ranks better);score_transform(negated_distance, i.e.score = -raw_distance);score_calibration = ordering_only— valid for sorting one response, never a probability. Calibrated scoring is deferred to the end-to-end ranking evaluation in Add end-to-end retrieval evaluation #76.FusionProvenancenow declares the combined moment score asordering_onlyand higher-is-better, and documentsrequested_modalities(asked) vssearched_modalities(run); a moment's contributing channels remainFusedMoment.modalities.SearchHit.rank/score/raw_distance,FusedMoment.rank/score/modalities, and the evidence-artifactscores) so the meaning is identical across CLI, HTTP, MCP, stored job results, and evidence delivery.Score (RRF)and captions the metric plus the ordering-only meaning.Backward compatibility
Additive only — every new field is defaulted, so previously stored job results and existing
FusedSearchResultpayloads still validate (covered by a regression test that reloads a legacy payload without the descriptor). No field renames, no change to score math, no index-schema bump.Testing
ruff check src tests— clean.python -m unittest discover -s tests— 659 passed, 7 skipped.ordering_onlymarking, per-channel metric surfacing, and legacy stored-result reload.