fix(annotation): IAA pivot keys retrieval rows by (record_uuid, chunk_id)#225
Merged
Conversation
…_id) Retrieval annotation items are one-per-chunk, not one-per-record. The previous IAA pivot keyed rows by ``record_uuid`` alone, conflating annotator agreement across all chunks of the same record into a single slot. Switch to a composite ``(record_uuid, chunk_id)`` key so the pivot reflects the actual annotation unit. Grounding and generation pivots are unaffected (one item per record).
github-actions
Bot
added
feature
This was referenced May 27, 2026
lramir14
approved these changes
Jun 3, 2026
lramir14
left a comment
lramir14
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the work and nice implementation. I only have one comment or clarification. In the tests what happens when we have missing or empty chunk_id in retrieval rows. If the retrieval export/schema guarantees chunk_id is always present and non-empty, then this is fine. If not, _item_key could still collapse rows as record_uuid:None or record_uuid:. Not blocking from my side, but it may be worth either relying explicitly on the schema guarantee or adding a defensive check/error for missing chunk_id.
lramir14
approved these changes
Jun 3, 2026
lramir14
left a comment
lramir14
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the quick implementation. Looks good to me. I don't see any blocking issue. Approved.
saschagobel
added
fix
saschagobel
pushed a commit
that referenced
this pull request
Jun 17, 2026
**Stack (4 PRs):** #226 → #227 → #228 → #229 (companion to standalone IAA fix #225) **Chain goal:** Switch calibration vs production assignment from per-`record_uuid` to per annotation item (chunk for retrieval, record for grounding / generation), with inheritable per-task fraction and absolute cap. **This PR (1/4):** Lands the ADR and the matching design-doc section before the implementation, so subsequent PRs can reference the decision and the schema shape. --- ## Scope - `docs/decisions/0012-per-item-calibration-partition.md` (new): the decision, the option-set considered, Kish design-effect arithmetic justifying per-item over per-record under the existing naive Krippendorff bootstrap, strategic positioning vs LLM-judge mainstream + industry annotation services, consequences, out-of-scope future work, and references. - `docs/decisions/README.md`: index entry for the new ADR. - `docs/design/annotation-import-pipeline.md`: new "Per-record manifest schema" section describing the `PartitionManifestEntry` shape that #227 implements (`grounding_generation_calibration` + `retrieval_chunk_calibration`). ## Test plan - [x] Docs only - no test changes.
saschagobel
pushed a commit
that referenced
this pull request
Jun 19, 2026
…ion assignment (#227) **Stack (4 PRs):** #226 → #227 → #228 → #229 (companion to standalone IAA fix #225) **Chain goal:** Switch calibration vs production assignment from per-`record_uuid` to per annotation item (chunk for retrieval, record for grounding / generation), with inheritable per-task fraction and absolute cap. **This PR (2/4):** Per-item partition logic + schema reshape + per-task fraction cascade. Ships the user-visible feature without the absolute cap (#229) or the API-hardening pass (#228). --- ## Scope ### Schema - `core/schemas/annotation_import.py`: `PartitionManifestEntry` replaces `calibration: bool` with `grounding_generation_calibration: dict[Task, bool]` (keyed by task) and `retrieval_chunk_calibration: dict[str, bool]` (keyed by `chunk_id`). Per-task `calibration_fraction_at_import` provenance. ### Partition logic - `core/annotation/record_builder.py`: `assign_partitions` buckets units via per-(task, unit) digest `hash(seed || task || unit_id)` against `fraction * 2^32` so per-task draws are statistically independent. `_enumerate_units` resolves the per-task unit list, `_write_unit` routes the bool back into the right per-record dict. - Returns `dict[str, PartitionManifestEntry]` as before; the `PartitionResult` bundle is extracted in #228. ### Settings cascade - `core/settings/annotation_settings.py`: `calibration_fraction` is now inheritable across deployment / workspace / task scopes via the `Inherit` sentinel pattern established in #206. `_check_calibration_topology` walks per-(ws, task) using resolved values. The `_inherit` walk is hoisted into a single helper so `resolved_task` no longer repeats the chain four times. ### API / CLI - `api/annotation_import.py`: `ImportResult` per-task dicts (`calibration_count`, `production_count`, `calibration_fraction`, `realised_calibration_fraction`) replace the scalar fields. - `cli/commands/annotation.py`: CLI output prints one line per task. ### Tests - Per-chunk routing independence, per-task seed independence, inheritance cascade, and per-(ws, task) topology validation. Manifest IO round-trip with the new shape. ## Breaking change Pre-v0 on-disk manifests with `calibration: bool` no longer load. Pragmata is pre-1.0; affected workspaces re-bootstrap by deleting `partition.meta.json` and re-importing. ## Why Per ADR-0012 (#226): per-item partition is the natural fit for the existing naive Krippendorff bootstrap and matches Argilla's per-item record creation. Per-task fraction cascade lets operators right-size each task independently (e.g. retrieval at 5%, grounding at 20%). ## Test plan - [x] `uv run python -m pytest tests/unit` (770 passing in this branch's worktree)
saschagobel
pushed a commit
that referenced
this pull request
Jun 30, 2026
… helpers (#228) **Stack (4 PRs):** #226 → #227 → #228 → #229 (companion to standalone IAA fix #225) **Chain goal:** Switch calibration vs production assignment from per-`record_uuid` to per annotation item (chunk for retrieval, record for grounding / generation), with inheritable per-task fraction and absolute cap. **This PR (3/4):** Pure refactor with no behaviour change. Tightens the api / core seam introduced in #227 so per-task fraction reporting and per-pair `record_uuid` derivation are each computed exactly once. --- ## Scope - `core/annotation/record_builder.py`: introduce `PartitionResult(assignments, pairs_by_rid, calibration_fraction)`. `assign_partitions` now returns this bundle; `fan_out_records(client, settings, *, partition)` consumes it and `_build_batches(pairs_by_rid, assignments)` iterates the rid map instead of recomputing `derive_record_uuid(pair)` per pair. - `core/annotation/record_builder.py`: replace `count_calibration_per_task` (returning `dict[Task, int]`) with `count_units_per_task` returning a `TaskUnitCounts(calibration, total)` frozen dataclass so the api layer gets both tallies in a single pass. - `api/annotation_import.py`: drop `_total_units_per_task` and `_resolve_per_task_fraction`. The api now reads `partition.calibration_fraction` directly and uses the merged `count_units_per_task` helper. ## Why After #227 lands, the api layer recomputes per-task fraction via its own workspace walk (`_resolve_per_task_fraction`), and `_build_batches` recomputes `record_uuid` for every pair even though `assign_partitions` already derived it. Both are wasted work; the natural fix is to surface the precomputed values via a bundle return type. This also unblocks the cap-related fields in #229 - `PartitionResult` is the obvious place to carry per-task cap resolution. Reviewer sanity check: assertions in `test_partition.py` move from `result[rid]` to `partition.assignments[rid]`; the values they check are unchanged. ## Test plan - [x] `uv run python -m pytest tests/unit` (770 passing)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Standalone bug fix that pairs naturally with the per-item-calibration stack (#226 → #227 → #228 → #229 → #230) but ships independently (no functional dependency).
Scope
core/annotation/iaa_runner.py:_item_keyfor retrieval rows now composes(record_uuid, chunk_id)instead ofrecord_uuidalone. Grounding and generation keys are unchanged (one item per record).tests/unit/core/annotation/test_iaa_runner.py: pin the per-chunk pivot key behaviour and exercise the multi-chunk path.Why
Retrieval annotation items are one-per-chunk on the Argilla side (each chunk is its own record). Keying the IAA pivot by
record_uuidalone conflated annotator agreement across all chunks of the same record into a single slot, inflating apparent agreement on records with multiple chunks. The fix realigns the pivot with the actual annotation unit.Test plan
uv run python -m pytest tests/unit/core/annotation/test_iaa_runner.py