feat(types): carry optional entity indices on Relation for span attribution - #262
sachinynaik wants to merge 1 commit into
Conversation
A relation's head and tail are plain strings, so a consumer cannot tell WHICH mention a relation came from when the same surface text appears more than once in the input. That breaks span-based anchoring: in a measured fixture "Dr Chen" occurs at offsets 0 and 24, and a relation naming "Dr Chen" is unattributable. Entity already carries start/end, and ExtractOutput already carries the per-item entities list, so the linkage exists on both sides and only the join is missing. Two optional integers restore it by reference: - no duplication of span data, so the copies cannot disagree - head/tail keep their type, so every existing consumer is unaffected - adapters that cannot supply indices simply omit them NotRequired rather than total=False: the four existing fields are required and should stay that way. Measured on knowledgator/gliner-relex-base-v1.0, which already returns entity_idx on both sides of each relation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe ChangesRelation response contract
Suggested reviewers: Merge Risk: ⚪ Minimal · up to Relation responses can now optionally identify their endpoint entities by index, allowing repeated entity text to be disambiguated while remaining compatible with producers that omit the new fields. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
| relation: str | ||
| score: float | ||
| head_idx: NotRequired[int] | ||
| tail_idx: NotRequired[int] |
There was a problem hiding this comment.
Thanks for this. The problem is real and index-by-reference is the right shape: no duplicated span data, and it also links bbox-only image entities that carry no character offsets at all. Happy to take it, but it needs to land as a working slice rather than a type declaration alone. As submitted nothing produces the fields, and every boundary between an adapter and a consumer rebuilds relations from the four existing keys, so the indices cannot reach a caller.
What the PR needs to be complete:
- Server API layer.
api/extract.pyrebuilds each relation withhead,tail,relationandscoreonly. Carryhead_idxandtail_idxthrough when present. This also keeps the direct path consistent with the queue path, which passes adapter dicts through unchanged. - Published contract. Add the fields to
RelationModelintypes/openapi.pyand to theRelationstruct insie_gateway/src/openapi.rs, then regenerate both committed specs withmise run openapi. - SDKs. Both
Relationtypes (sie_sdk/types.py,sie_ts_sdk/src/types.ts) and both parsers (client/_shared.py,internal/parsing.ts) drop the keys today. - A producer. GLiREL is the natural first one.
_relation_entity_textalready finds the matching entity by walkingzip(entities, ner_input), and the adapter echoes input entities in order, so the index into the response'sentitieslist is well-defined. GLiNER2 will have to omit the fields because upstream returns text only, which is worth stating in the description. - Tests. A GLiREL case with duplicated surface text (the "Dr Chen" fixture from the description) asserting the indices, plus coverage that the fields survive the API layer and both SDK parsers.
Problem
Relation'sheadandtailare plain strings, so a consumer cannot tell which mention a relation came from when the same surface text occurs more than once in the input.That is not cosmetic — it breaks span-based anchoring. In a measured fixture,
"Dr Chen"occurs at offsets 0 and 24; a relation naming"Dr Chen"is unattributable, and a consumer that anchors relations to caller-supplied entity spans has to either guess or drop it.The asymmetry is already visible in the types:
Why indices rather than nested objects or duplicated offsets
ExtractOutputalready carries the per-itementitieslist, andEntityalready carriesstart/end. So the linkage exists on both sides and only the join is missing. Two optional integers restore it by reference:GLiRELAdapter, which continues to emit four keys and stays valid.Nesting full objects under
head/tailwould be the alternative and is strictly worse: it changes the type of an existing field and breaks current consumers.NotRequiredrather than flipping the class tototal=False— the four existing fields are required and should stay that way.NotRequiredis already imported in this module.Prior art in the ecosystem
knowledgator/gliner-relex-base-v1.0already returnsentity_idxon both the head and tail of every relation, so for that model family the information exists at the adapter boundary and is currently discarded on the way out.Compatibility
🤖 Generated with Claude Code
Summary by CodeRabbit