add: with_lookup pitfalls - #61
Conversation
szabosteve
left a comment
There was a problem hiding this comment.
@Dylancouzon This is my first review in this repo and I'm still figuring this out, so please point out if I'm incorrect anywhere or made a wrong assumption. Also sorry for any inconvenience.
I left a few – mostly trivial – suggestions, please take or leave them. My only real concern is about the size of this skill. The new section adds a significant amount of content. Would it be worth splitting it into a sibling leaf, something like with-lookup-pitfalls/SKILL.md and linking that from the parent skill?
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@szabosteve Thanks for the review and feedback. No inconvenience at all, this is exactly the kind of question worth raising. Quick context on how I work in this repo: when I open a PR against On the split: we landed on keeping it as one leaf. The We’re also already four levels deep, so adding a fifth sibling creates another routing decision and a near-duplicate description competing for the same queries. For this repo, that’s a net negative because we optimize more for discoverability than readability. One caveat: the current evals don’t test discoverability. |
szabosteve
left a comment
There was a problem hiding this comment.
Thanks for the changes and for the clear explanation! LGTM!
|
|
||
| ### When to Split into a Sidecar Collection with Lookup in Groups | ||
|
|
||
| Duplicated document-level data across chunk points scales as `documents × chunks_per_document × shared_bytes_per_point` (e.g. 20k docs × 24 chunks × ~3 KB ≈ 1.4 GB duplicated vs ~60 MB once split). Whether to split depends on what the shared data is used for, not just its size. Classify the shared data before recommending [Lookup in Groups](https://search.qdrant.tech/md/documentation/search/search/?s=lookup-in-groups): |
There was a problem hiding this comment.
should not use absolute link to search. subdomain. This triggers agent's built-in protectins about using unknown domains
There was a problem hiding this comment.
Done, though flagging that this conflicts with our guidelines in CONTRIBUTING.md and AGENTS.md, both currently mandate search.qdrant.tech/md/documentation/.
|
|
||
| - End-to-end worked example fusing title, abstract, chunk, and sparse-title named vectors with RRF and document-level grouping in one Query API call: [Multi-Representation Search tutorial](https://search.qdrant.tech/md/documentation/tutorials-search-engineering/multi-representation-search/) | ||
| - If you have groups and subgroups of representations (document -> chunk, image -> patch), you could use [searching in groups](https://search.qdrant.tech/md/documentation/search/search/?s=search-groups). To not store identical payloads several times, check [Lookup in Groups](https://search.qdrant.tech/md/documentation/search/search/#lookup-in-groups). Index the grouping payload field (e.g. `document_id`) as a keyword payload index before grouping. | ||
| - If you have groups and subgroups of representations (document -> chunk, image -> patch), you could use [searching in groups](https://search.qdrant.tech/md/documentation/search/search/?s=search-groups): |
There was a problem hiding this comment.
maybe https://skills.qdrant.tech/md/documentation/search/search?s=grouping-api better?
| - When grouping chunk-level points back to documents, each prefetch only contributes the candidates it returned — so size per-prefetch `limit` well above the final document `limit` (rule of thumb: `prefetch_limit ≥ final_limit × expected_chunks_per_document`), otherwise a few documents with many chunks saturate the candidate pool and relevant documents drop silently. Validate grouped recall on a labeled sample. | ||
| - When per-document vectors (title, abstract) would be duplicated across every chunk-level point, the duplication can dominate storage at scale. Keeping them denormalized in one collection makes queries simpler (single Query API call, every representation reachable from any point); a sidecar collection joined via [Lookup in Groups](https://search.qdrant.tech/md/documentation/search/search/#lookup-in-groups) is the alternative when storage matters. | ||
|
|
||
| ### When to Split into a Sidecar Collection with Lookup in Groups |
There was a problem hiding this comment.
it feels unrelated to hybrid seach. Maybe a separate skill?
There was a problem hiding this comment.
Also keeping skills smaller seems more beneficial
There was a problem hiding this comment.
Pulled grouping + sidecar/with_lookup into its own leaf, search-strategies/document-grouping/, as a sibling to hybrid-search/.
Eval id 7 now targets the new leaf.
|
this PR has been inactive for 14 days. it will be closed in 7 days unless there is new activity. if you're still working on it, push an update or leave a comment. |
|
this PR has been inactive for 14 days. it will be closed in 7 days unless there is new activity. if you're still working on it, push an update or leave a comment. |
Propagates the operational caveats from landing_page #2362 into the
search-typesskill so agents recommendingwith_lookupflag the silent-failure modes the worked example introduced.What changed
skills/qdrant-search-quality/search-strategies/hybrid-search/search-types/SKILL.md:with_lookup/group_by/empty lookup/sidecar/chunk-to-documenttriggers to the frontmatter description.tenant_id/ACLs/recency dates (denormalize), and mixed.documents × chunks_per_document × shared_bytes_per_pointwith the worked PR #2362 example (~1.4 GB duplicated vs ~60 MB split) as a parenthetical anchor, not the decision rule.group_byvalues must be valid point IDs (unsigned int or UUID — arbitrary strings like DOIs need a stable join key); missing/mismatched ids return emptylookupsilently;with_lookup=\"documents\"shorthand returns payload only;group_byarray attaches one chunk to multiple groups.keywordfor strings,integerfor numeric) instead of unconditionalkeyword.#lookup-in-groupsanchors → canonical?s=lookup-in-groups.skills/qdrant-search-quality/search-strategies/SKILL.md:evals/evals.json+scripts/run_eval.py:with_lookupscenario, 7 expectations covering the silent-failure modes above) wired intoSKILL_MAPagainst the search-quality chain.Why
PR #2362 added the worked two-collection example because users were silently hitting empty
lookupresults, recall regressions from moving retrieval-relevant vectors behind the lookup, and filter behavior that broke under sidecar splits. The docs now teach the pattern; the skill should encode the failure-mode judgment so agents recommend the split safely.Test plan
Stacking
Stacks on #55 (link-multi-representation-search-tutorial). PR base is set accordingly; merge after #55 lands.