diff --git a/CHANGELOG.md b/CHANGELOG.md index f78df5de..d7a6ad64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,15 @@ history of this codebase and was never published. This release is `[0.4.1]`. ### Added -- Registry prior for VLM labeling (#61): a prompt pack's `registry_prior_top_k` (0 = off, max 50) - adds the top-k registry classes by validated count and the pending proposal names to the - open-vocabulary prompt as a hint. Served in the pack schema as `kind: "int"`; a new - `pack_field_out_of_range` validation code; the run refuses when the counts are unreadable. +- Detector hint for VLM labeling (#61): a prompt pack's `detector_hint_min_confidence_pct` + (0 = off, max 100) tells the VLM each item's stored detector class name, per image, when the + detector confidence is at least that percent. Served in the pack schema as `kind: "int"`; a new + `pack_field_out_of_range` validation code. Measured on a public COCO oracle (539 crops, local + VLM) it raised overall accuracy from 0.775 to 0.853 (0.7846 to 0.8478 in an arm with ten + validated classes seeded) and accuracy when answered from 0.850 to 0.918. Caveat: the detector + is COCO-trained and the oracle is COCO ground truth, so the gain may be optimistic for other + domains. A registry-prior variant (top validated classes plus pending proposals in the prompt) + was measured, showed no gain (and about -1 point with seeded classes) and was removed. - Gateway mode (#93): `OP_GATEWAY_SUBPATHS=true` makes Cropwright's nginx serve Grafana, Prometheus, OpenSearch Dashboards and MLflow under `/grafana/`, `/prometheus/`, `/dashboards/` and `/mlflow/` of its one published port, so the UIs @@ -36,6 +41,11 @@ history of this codebase and was never published. This release is `[0.4.1]`. `POST /pipeline/auto_label` never writes. Every residual-clustering run now records itself (`clustering/last_run.py`, called by `cluster_residuals` and `assign_only_residuals`) and the stats serve that record. +- The shipped proposal-name denylist now also covers the scene and quality words a live COCO + oracle run produced (#193): `abstract*`, `*_background`, `*_object`, `scene*`, `empty*`, + `blank_*`, `*_image`, `shadow*`, `outdoors` and similar. The text-free generic pack and + `examples/prompt_packs/vehicle_wheel.json` carry it too, tested against the COCO class names so + no glob eats a real class. - Cropwright's nginx now answers redirects with relative `Location` headers; the `/OpenProcessor` redirect used to name the container port 8080. - Config-store writes (prompt packs, region profiles, open-vocabulary sets, VLM endpoints, diff --git a/contracts/openapi/curation.json b/contracts/openapi/curation.json index a3442f3c..5c34ba86 100644 --- a/contracts/openapi/curation.json +++ b/contracts/openapi/curation.json @@ -15703,6 +15703,13 @@ "title": "Combined User Template", "type": "string" }, + "detector_hint_min_confidence_pct": { + "default": 0, + "maximum": 100.0, + "minimum": 0.0, + "title": "Detector Hint Min Confidence Pct", + "type": "integer" + }, "open_class_system": { "default": "", "title": "Open Class System", @@ -15750,13 +15757,6 @@ "title": "Region Visible User", "type": "string" }, - "registry_prior_top_k": { - "default": 0, - "maximum": 50.0, - "minimum": 0.0, - "title": "Registry Prior Top K", - "type": "integer" - }, "synonyms": { "additionalProperties": { "type": "string" diff --git a/docs-site/docs/guides/prompt-packs.mdx b/docs-site/docs/guides/prompt-packs.mdx index 77f1eb9e..e35a9051 100644 --- a/docs-site/docs/guides/prompt-packs.mdx +++ b/docs-site/docs/guides/prompt-packs.mdx @@ -34,14 +34,16 @@ code. Packs are editable per project, versioned, and can be rolled back. (`blurry_*`, `*_scene`) for proposed new-class names that are scene or image-quality words rather than classes. A matching proposal is dropped, so the crop counts as "nothing fits" and does not reach the new-class queue. It defaults to empty for a - stored pack that omits it; the shipped generic pack carries a starter list. - - An optional integer, `registry_prior_top_k` (`0` = off, the default; at most 50), adds a - registry prior to open-vocabulary labeling: the prompt lists that many registry classes - (most validated items first, ties by name) and the pending new-class proposal names, built - from the project's own state. It is a hint, not a constraint: a reply outside the list is - still accepted. If the counts cannot be read the run refuses rather than labeling without - the prior. + stored pack that omits it; the shipped generic packs carry a starter list covering blur, + quality, empty and scene words plus `abstract*`, `*_background` and `*_object`. + + An optional integer, `detector_hint_min_confidence_pct` (`0` = off, the default; at most + 100), adds one line per image naming the item's stored detector class when the detector + confidence is at least that percent. It is a hint, not a constraint. On a public COCO oracle + (539 crops) it raised overall accuracy from 0.775 to 0.853 (0.7846 to 0.8478 with ten + validated classes seeded). The detector is COCO-trained and the oracle is COCO ground truth, + so the gain may be optimistic for other domains. A registry-prior variant was measured, gave + no gain and was removed. - **Placeholders.** Only three fields are formatted with `str.format`: `class_user_template` and `open_class_user_template` accept `class_names_csv`; `combined_user_template` accepts `class_block` and @@ -114,9 +116,9 @@ named region profile (default: the active one). The response is |---|---|---| | `pack_name_invalid`, `pack_name_reserved` | error | Name must match `^[a-z0-9][a-z0-9_.-]{1,63}$`; `off`, `none` and `default` are reserved | | `name_conflict` | error | Name already used | -| `pack_field_missing`, `pack_field_empty` | error | A text field is absent or blank; `proposal_denylist` is not a list of strings, or holds a blank pattern; `registry_prior_top_k` is not an integer | +| `pack_field_missing`, `pack_field_empty` | error | A text field is absent or blank; `proposal_denylist` is not a list of strings, or holds a blank pattern; `detector_hint_min_confidence_pct` is not an integer | | `pack_field_too_long` | error | A field over 20000 characters, a map or `proposal_denylist` over 500 entries, or a denylist pattern over 200 characters | -| `pack_field_out_of_range` | error | `registry_prior_top_k` is outside 0 to 50 | +| `pack_field_out_of_range` | error | `detector_hint_min_confidence_pct` is outside 0 to 100 | | `pack_template_format_error` | error | The field cannot be formatted (for example a single brace) | | `pack_placeholder_missing`, `pack_placeholder_unknown` | error | A formatted field lacks a required placeholder or uses another | | `pack_placeholder_in_verbatim_field` | error | A placeholder in a field that is not formatted | diff --git a/docs/CURATION.md b/docs/CURATION.md index 9e37c3c4..3e7a6a21 100644 --- a/docs/CURATION.md +++ b/docs/CURATION.md @@ -501,14 +501,17 @@ Browse and cluster at box level: A prompt pack holds the VLM prompt templates and two vocabulary tables (`class_descriptions`, `synonyms`) and an optional `proposal_denylist` of glob patterns (`blurry_*`, `*_scene`) whose matching new-class proposals are -dropped. An optional `registry_prior_top_k` (integer, `0` = off, at most 50) -turns on a registry prior for open-vocabulary VLM labeling: the prompt then lists -that many registry classes, most validated items first (ties by name), plus the -pending new-class proposal names, built server-side from the project's own state -(never from a client list). It is a hint, not a constraint: a reply outside the -list is still accepted and resolved as usual. If the counts cannot be read the -run refuses (HTTP 503 on `vlm/label_batch`, a stage error in the auto-label -pipeline) instead of labeling without the prior. It is separate from the region profile: +dropped. The shipped default denylist covers blur, quality, empty and scene words plus +`abstract*`, `*_background` and `*_object`. An optional +`detector_hint_min_confidence_pct` (integer, `0` = off, at most 100) adds, per +image, the item's stored detector class name to the open-vocabulary prompt when the +detector confidence is at least that percent; it is a hint, not a constraint (a reply +outside it is still accepted). Measured on a 539-crop public COCO oracle with a local +VLM it raised overall accuracy from 0.775 to 0.853 (0.7846 to 0.8478 with ten validated +classes seeded). The detector is COCO-trained and the oracle is COCO ground truth, so the +gain may be optimistic for other domains. A registry-prior variant (top validated +classes plus pending proposals in the prompt) was measured, gave no gain and was +removed. It is separate from the region profile: a profile says where to look, a pack says what to ask. Either can be used without the other. diff --git a/docs/design/README.md b/docs/design/README.md index e9cade5d..b7584926 100644 --- a/docs/design/README.md +++ b/docs/design/README.md @@ -33,7 +33,7 @@ Rows marked implemented are merged for v0.4.0; the rest are open. | #57, #58, #59, #60, #64, #66 | Implemented in the v0.4.0 hardening merge (regions re-verify safety, config-store hardening, test fakes and dev tooling, silent failures, docs, GPU clustering overlay); the issues stay open until the owner closes them | | #53, #55 | Implemented (shard budget `capacity` block, combine preview warnings) | | #46 | Implemented (region-stage hit-rate gate, pause and resume) | -| #61 | VLM proposal noise, optional registry prior, label agreement vs cluster purity | +| #61 | VLM proposal noise, optional detector hint, label agreement vs cluster purity | | #62 | Split oversize files, unify job state files, rename the `api` compose service | | #63 | Release decisions (`:latest`, control-plane-only) and remaining local VLM catalog verification | | #65 | Roadmap: deferred any-domain features | diff --git a/examples/prompt_packs/vehicle_wheel.json b/examples/prompt_packs/vehicle_wheel.json index 6b4d34f5..0005a06d 100644 --- a/examples/prompt_packs/vehicle_wheel.json +++ b/examples/prompt_packs/vehicle_wheel.json @@ -17,6 +17,38 @@ "region_visible_user": "For each numbered crop, answer: is a wheel (tire plus rim) of the car visible anywhere in the image? Count partial, angled, or small wheels as visible; count fully hidden wheels as not visible. Respond as a JSON object whose ``results`` field is an array of per-image verdicts in input order:\n{\"results\": [{\"img\": 1, \"visible\": true|false}, ...]}", "class_descriptions": {}, "synonyms": {}, - "proposal_denylist": [], - "registry_prior_top_k": 0 + "proposal_denylist": [ + "abstract*", + "background*", + "*_background", + "object", + "objects", + "*_object", + "blurry*", + "blurred*", + "*_blurry", + "*_blur", + "blur_*", + "out_of_focus*", + "low_quality*", + "low_resolution*", + "unclear*", + "unidentified*", + "scene*", + "*_scene", + "empty*", + "unknown*", + "*_image", + "*_photo", + "*_abstract", + "blank", + "blank_*", + "indoor", + "indoors", + "outdoor", + "outdoors", + "shadow*", + "*_shadow" + ], + "detector_hint_min_confidence_pct": 0 } diff --git a/frontend/CHANGELOG.md b/frontend/CHANGELOG.md index 5ce162b1..015f2507 100644 --- a/frontend/CHANGELOG.md +++ b/frontend/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- The served pack schema gains a `registry_prior_top_k` row with `kind: "int"` (the optional VLM registry prior, 0 = off). The pack editor shows an unknown kind read-only and saves it unchanged, so the value round-trips; `PackSchemaField.kind` lists `int`. +- The served pack schema gains a `detector_hint_min_confidence_pct` row with `kind: "int"` (the optional per-item detector-name hint, 0 = off). The pack editor shows an unknown kind read-only and saves it unchanged, so the value round-trips; `PackSchemaField.kind` lists `int`. - Gateway mode: with `OP_GATEWAY_SUBPATHS=true` the entrypoint installs `nginx-gateway.conf`, which proxies `/grafana/`, `/prometheus/`, `/dashboards/` and `/mlflow/` to the compose services (bare service names only, overridable with diff --git a/frontend/src/lib/test/fixtures/promptPacks.ts b/frontend/src/lib/test/fixtures/promptPacks.ts index 6c871bee..0194e817 100644 --- a/frontend/src/lib/test/fixtures/promptPacks.ts +++ b/frontend/src/lib/test/fixtures/promptPacks.ts @@ -101,8 +101,8 @@ export function schemaFixture(): PromptPackSchema { help: '', }, { - field: 'registry_prior_top_k', - label: 'Registry prior top k', + field: 'detector_hint_min_confidence_pct', + label: 'Detector hint min confidence pct', group: 'open_classify', kind: 'int', formatted: false, diff --git a/frontend/src/lib/types_packs.ts b/frontend/src/lib/types_packs.ts index b1da0036..8b04b135 100644 --- a/frontend/src/lib/types_packs.ts +++ b/frontend/src/lib/types_packs.ts @@ -21,7 +21,7 @@ import type { * A pack body: every `PromptPack` field except `name`, keyed by the * schema's `field` ids. `kind: "text"` fields are strings, `kind: "map"` * fields (`class_descriptions`, `synonyms`) are string maps, and - * `kind: "list"` (`proposal_denylist`) is a list of case-insensitive glob strings, and `kind: "int"` (`registry_prior_top_k`, 0 = off) is shown read-only. + * `kind: "list"` (`proposal_denylist`) is a list of case-insensitive glob strings, and `kind: "int"` (`detector_hint_min_confidence_pct`, 0 = off) is shown read-only. * Kept as a record so a field the backend adds renders from the schema alone. */ export type PackFieldValue = string | number | string[] | Record; diff --git a/scripts/curation/bakeoff/vlm_prior_oracle.py b/scripts/curation/bakeoff/vlm_hint_oracle.py similarity index 95% rename from scripts/curation/bakeoff/vlm_prior_oracle.py rename to scripts/curation/bakeoff/vlm_hint_oracle.py index af6ce3cf..dca50ba0 100755 --- a/scripts/curation/bakeoff/vlm_prior_oracle.py +++ b/scripts/curation/bakeoff/vlm_hint_oracle.py @@ -1,14 +1,14 @@ #!/usr/bin/env python3 -"""Score the VLM registry prior on/off against an oracle (#61 item 2). +"""Score the VLM detector hint on/off against an oracle (#61 item 2). Two steps, run by hand around two labeling runs of the same crops (one with a -pack whose ``registry_prior_top_k`` is 0, one with it set): +pack whose ``detector_hint_min_confidence_pct`` is 0, one with it set): # after each run, dump (crop, oracle name, VLM answer) for that run's pack - .venv/bin/python scripts/curation/bakeoff/vlm_prior_oracle.py dump \\ + .venv/bin/python scripts/curation/bakeoff/vlm_hint_oracle.py dump \\ --project --pack --truth-field --out off.jsonl - .venv/bin/python scripts/curation/bakeoff/vlm_prior_oracle.py compare \\ + .venv/bin/python scripts/curation/bakeoff/vlm_hint_oracle.py compare \\ --off off.jsonl --on on.jsonl ``dump`` reads items whose ``vlm_prompt_pack`` stamp starts with ``--pack`` and diff --git a/src/routers/curation/_prompt_pack_models.py b/src/routers/curation/_prompt_pack_models.py index 958ec99a..874a6a48 100644 --- a/src/routers/curation/_prompt_pack_models.py +++ b/src/routers/curation/_prompt_pack_models.py @@ -11,7 +11,7 @@ ActiveRef, ValidationReport, ) -from src.services.labeling.registry_prior import MAX_REGISTRY_PRIOR_TOP_K +from src.services.labeling.detector_hint import MAX_DETECTOR_HINT_PCT from src.services.labeling.vlm_prompts import REPLY_KEY_CONTRACT @@ -52,14 +52,14 @@ class PromptPackBody(BaseModel): ] # Typed ``Any`` for the same reason: a bool/str/out-of-range value must come # back as a validation issue. 0 = off. - registry_prior_top_k: Annotated[ + detector_hint_min_confidence_pct: Annotated[ Any, Field( default=0, json_schema_extra={ 'type': 'integer', 'minimum': 0, - 'maximum': MAX_REGISTRY_PRIOR_TOP_K, + 'maximum': MAX_DETECTOR_HINT_PCT, }, ), ] diff --git a/src/routers/curation/pipeline.py b/src/routers/curation/pipeline.py index a01bb426..8cfd076d 100644 --- a/src/routers/curation/pipeline.py +++ b/src/routers/curation/pipeline.py @@ -152,7 +152,6 @@ async def _run_auto_label( from src.services.curation.autolabel.job import with_elapsed_tick from src.services.curation.clustering.auto_promote import gated_auto_promote from src.services.curation.image_serving import THUMBNAIL_CACHE - from src.services.labeling.vlm_models import ItemCrop reject_detection_profile(detection_profile) if isinstance(vlm, str) and not vlm_resolved and not run_vlm: @@ -397,8 +396,7 @@ async def _run_auto_label( summary['final'] = {'unvalidated': 0, 'human_required': 0} return summary - # Reuse the VLM label_batch logic by calling it directly (no HTTP - # hop). Build ItemCrops here so we can chunk. + # Calls the labeler directly (no HTTP hop); ItemCrops are built here to chunk. from src.services.curation.region_class import item_classes from src.services.labeling.vlm_class_names import ( format_class_catalog, @@ -441,13 +439,10 @@ async def _run_auto_label( labeler = _get_vlm_labeler(_labeler_pack, _labeler_revision, endpoint=endpoint) class_catalog = format_class_catalog(class_dicts, labeler._pack) - from src.services.curation.registry_prior_source import prior_or_error + from src.services.labeling.detector_hint import DETECTOR_HINT_FIELDS, hinted_crop from src.services.labeling.vlm_prompts import prompt_pack_stamp _pack_stamp = prompt_pack_stamp(labeler._pack, revision=_labeler_revision) - registry_prior, prior_error = await prior_or_error(opensearch, labeler._pack, class_names) - if prior_error: - return {**summary, 'stages_error': prior_error} # Count how many crops bypass the synonym/fuzzy force-fit because the # VLM's confidence is low — those route straight to the raw-label @@ -479,9 +474,9 @@ async def _run_chunk(ids: list[str]) -> tuple[int, int, list[dict[str, Any]]]: docs = await mget_crops( opensearch, ids, - source_includes=['image_path', 'bbox_norm'], + source_includes=['image_path', 'bbox_norm', *DETECTOR_HINT_FIELDS], ) - crops: list[ItemCrop] = [] + crops = [] for crop_id, doc in docs.items(): src = doc.get('_source') or {} image_path = src.get('image_path', '') @@ -496,7 +491,7 @@ async def _run_chunk(ids: list[str]) -> tuple[int, int, list[dict[str, Any]]]: except Exception as exc: logger.warning('pipeline_thumb_failed', crop_id=crop_id, error=str(exc)) continue - crops.append(ItemCrop(img_id=crop_id, jpeg_bytes=jpeg)) + crops.append(hinted_crop(crop_id, jpeg, src, labeler._pack)) # No item classes yet (a fresh project, or only the region class): # nothing to label items as. if not crops or not class_names: @@ -505,7 +500,6 @@ async def _run_chunk(ids: list[str]) -> tuple[int, int, list[dict[str, Any]]]: crops, class_names, class_catalog=class_catalog, - registry_prior=registry_prior, ) # Collect per-doc updates, then dispatch via # occ_skip_on_conflict_bulk so a concurrent human edit always diff --git a/src/routers/curation/prompt_packs.py b/src/routers/curation/prompt_packs.py index 6b863629..9a172702 100644 --- a/src/routers/curation/prompt_packs.py +++ b/src/routers/curation/prompt_packs.py @@ -50,7 +50,7 @@ rollback_pack, save_pack, ) -from src.services.labeling.registry_prior import MAX_REGISTRY_PRIOR_TOP_K +from src.services.labeling.detector_hint import MAX_DETECTOR_HINT_PCT from src.services.labeling.vlm_prompts import FORMATTED_PLACEHOLDERS, REPLY_KEY_CONTRACT @@ -81,21 +81,21 @@ 'class_descriptions': 'vocabulary', 'synonyms': 'vocabulary', 'proposal_denylist': 'vocabulary', - 'registry_prior_top_k': 'open_classify', + 'detector_hint_min_confidence_pct': 'open_classify', } _FIELD_KIND: dict[str, Literal['map', 'list', 'int']] = { 'class_descriptions': 'map', 'synonyms': 'map', 'proposal_denylist': 'list', - 'registry_prior_top_k': 'int', + 'detector_hint_min_confidence_pct': 'int', } _FIELD_HELP: dict[str, str] = { - 'registry_prior_top_k': ( + 'detector_hint_min_confidence_pct': ( 'Optional hint for open-vocabulary labeling: 0 = off; 1 to ' - f'{MAX_REGISTRY_PRIOR_TOP_K} lists that many registry classes (most validated first) ' - 'and pending proposal names in the prompt. A hint, not a constraint.' + f"{MAX_DETECTOR_HINT_PCT} tells the VLM each item's stored detector class name when " + 'the detector confidence is at least that many percent. A hint, not a constraint.' ), } diff --git a/src/routers/curation/vlm.py b/src/routers/curation/vlm.py index 202c2733..cf593be9 100644 --- a/src/routers/curation/vlm.py +++ b/src/routers/curation/vlm.py @@ -45,10 +45,7 @@ ) from src.services.curation.crop_bytes import load_region_jpeg, load_vlm_item_jpeg from src.services.curation.label_batch_write import label_batch_merge, label_batch_update -from src.services.curation.registry_prior_source import ( - RegistryPriorUnavailableError, - prior_for_pack, -) +from src.services.labeling.detector_hint import DETECTOR_HINT_FIELDS, hinted_crop _F = get_region_fields() @@ -190,23 +187,15 @@ async def vlm_label_batch( }, ) - # Before any crop is read: a pack that asks for a prior it cannot get refuses the call. - try: - registry_prior = await prior_for_pack(opensearch, labeler._pack, class_names) - except RegistryPriorUnavailableError as exc: - raise HTTPException(status_code=503, detail=str(exc)) from exc - # Pull image_path + bbox_norm for each crop, then build ItemCrop list # with the LRU-thumbnail JPEG bytes (128px is enough for the VLM). - from src.services.labeling.vlm_models import ItemCrop - # Same OP_CROP_CACHE_DIR / CurationConfig.crop_cache_dir the worker # (scripts/curation/worker/state.py) writes into -- this used to read a # different env var with a different default, which meant a 100% cache # miss out of the box. crop_cache_dir = str(get_curation_config().crop_cache_dir) - crops: list[ItemCrop] = [] + crops = [] cache_hits = 0 cache_misses = 0 # One mget_crops() call instead of N separate opensearch.get() @@ -219,7 +208,7 @@ async def vlm_label_batch( list(payload.crop_ids), index=items_index(), source_includes=sorted( - {'class_source', 'class_validated', 'image_path', 'bbox_norm'} + {'class_source', 'class_validated', 'image_path', 'bbox_norm', *DETECTOR_HINT_FIELDS} | set(CLASS_GUARD_SOURCE_FIELDS) ), ) @@ -248,7 +237,7 @@ async def vlm_label_batch( jpeg = load_vlm_item_jpeg(crop_id, image_path, tuple(bbox), cache_dir=crop_cache_dir) if jpeg is None: continue - crops.append(ItemCrop(img_id=crop_id, jpeg_bytes=jpeg)) + crops.append(hinted_crop(crop_id, jpeg, src, labeler._pack)) if cache_hits + cache_misses > 0: logger.info( @@ -267,9 +256,7 @@ async def vlm_label_batch( _pack_stamp = prompt_pack_stamp(labeler._pack) # Use the open-vocabulary path so the VLM can flag genuinely-unknown # items instead of silently snapping them to the wrong class. - predictions = await labeler.label_or_propose_batch( - crops, class_names, registry_prior=registry_prior - ) + predictions = await labeler.label_or_propose_batch(crops, class_names) name_to_id = {c.class_name: c.class_id for c in labelable} # Collect per-doc updates, then dispatch via occ_skip_on_conflict_bulk # so a concurrent human edit always wins. diff --git a/src/services/config_store/pack_validation.py b/src/services/config_store/pack_validation.py index 32de71cc..312b1c50 100644 --- a/src/services/config_store/pack_validation.py +++ b/src/services/config_store/pack_validation.py @@ -12,8 +12,8 @@ from dataclasses import fields as dc_fields from typing import TYPE_CHECKING, Any +from src.services.labeling.detector_hint import MAX_DETECTOR_HINT_PCT from src.services.labeling.region_overlay import REPLY_TEXT_KEY -from src.services.labeling.registry_prior import MAX_REGISTRY_PRIOR_TOP_K from src.services.labeling.vlm_prompts import FORMATTED_PLACEHOLDERS, REPLY_KEY_CONTRACT, PromptPack @@ -40,7 +40,13 @@ f.name for f in dc_fields(PromptPack) if f.name - not in ('name', 'class_descriptions', 'synonyms', 'proposal_denylist', 'registry_prior_top_k') + not in ( + 'name', + 'class_descriptions', + 'synonyms', + 'proposal_denylist', + 'detector_hint_min_confidence_pct', + ) ) _MAP_FIELDS = ('class_descriptions', 'synonyms') @@ -122,12 +128,13 @@ def _check_denylist(deny: Any) -> list[ValidationIssue]: return issues -def _check_registry_prior(body: dict[str, Any]) -> list[ValidationIssue]: - field_name = 'registry_prior_top_k' +def _check_bounded_int( + body: dict[str, Any], field_name: str, maximum: int +) -> list[ValidationIssue]: if field_name not in body: return [] - top_k = body[field_name] - if isinstance(top_k, bool) or not isinstance(top_k, int): + value = body[field_name] + if isinstance(value, bool) or not isinstance(value, int): return [ _issue( 'pack_field_missing', @@ -136,14 +143,14 @@ def _check_registry_prior(body: dict[str, Any]) -> list[ValidationIssue]: field=field_name, ) ] - if not 0 <= top_k <= MAX_REGISTRY_PRIOR_TOP_K: + if not 0 <= value <= maximum: return [ _issue( 'pack_field_out_of_range', 'error', - f'{field_name} must be between 0 (off) and {MAX_REGISTRY_PRIOR_TOP_K}', + f'{field_name} must be between 0 (off) and {maximum}', field=field_name, - detail={'value': top_k, 'max': MAX_REGISTRY_PRIOR_TOP_K}, + detail={'value': value, 'max': maximum}, ) ] return [] @@ -204,7 +211,9 @@ def _check_required_fields(body: dict[str, Any]) -> list[ValidationIssue]: ) ) issues.extend(_check_denylist(body.get('proposal_denylist'))) - issues.extend(_check_registry_prior(body)) + issues.extend( + _check_bounded_int(body, 'detector_hint_min_confidence_pct', MAX_DETECTOR_HINT_PCT) + ) return issues diff --git a/src/services/curation/registry_prior_source.py b/src/services/curation/registry_prior_source.py deleted file mode 100644 index 4a35daa6..00000000 --- a/src/services/curation/registry_prior_source.py +++ /dev/null @@ -1,80 +0,0 @@ -"""Build the registry prior from served project state (never from the client).""" - -from __future__ import annotations - -from typing import TYPE_CHECKING, Any - -from src.config.curation import items_index -from src.services.labeling.registry_prior import RegistryPrior, rank_registry_prior - - -if TYPE_CHECKING: - from collections.abc import Collection - -_PENDING_BUCKETS = 200 - - -class RegistryPriorUnavailableError(RuntimeError): - """The pack asks for a registry prior but the counts could not be read. - - Fail closed: labeling without the prior the pack asked for would silently - change the prompt the run is stamped with. - """ - - -def _buckets(aggs: dict[str, Any], key: str) -> dict[str, int]: - node = aggs.get(key, {}).get('by_name', {}) - return {str(b['key']): int(b['doc_count']) for b in node.get('buckets', [])} - - -async def load_registry_prior( - opensearch: Any, *, top_k: int, registry_names: Collection[str] -) -> RegistryPrior | None: - """The prior for ``top_k`` > 0 (``None`` when off or nothing to rank).""" - if top_k <= 0 or not registry_names: - return None - body = { - 'size': 0, - 'query': {'bool': {'must_not': [{'term': {'test_holdout': True}}]}}, - 'aggs': { - 'validated': { - 'filter': {'term': {'class_validated': True}}, - 'aggs': { - 'by_name': {'terms': {'field': 'class_name', 'size': len(registry_names)}} - }, - }, - 'pending': { - 'filter': {'term': {'class_source': 'vlm_new_class_pending'}}, - 'aggs': { - 'by_name': {'terms': {'field': 'vlm_proposed_class', 'size': _PENDING_BUCKETS}} - }, - }, - }, - } - try: - resp = await opensearch.search(index=items_index(), body=body) - aggs = resp['aggregations'] - validated = _buckets(aggs, 'validated') - pending = _buckets(aggs, 'pending') - except Exception as exc: - raise RegistryPriorUnavailableError(f'registry prior counts unavailable: {exc}') from exc - return rank_registry_prior(registry_names, validated, pending, top_k=top_k) - - -async def prior_for_pack( - opensearch: Any, pack: Any, registry_names: Collection[str] -) -> RegistryPrior | None: - """The prior the pack asks for (``None`` when its ``registry_prior_top_k`` is 0).""" - return await load_registry_prior( - opensearch, top_k=pack.registry_prior_top_k, registry_names=registry_names - ) - - -async def prior_or_error( - opensearch: Any, pack: Any, registry_names: Collection[str] -) -> tuple[RegistryPrior | None, str | None]: - """:func:`prior_for_pack`, with an unreadable-counts failure as its message.""" - try: - return await prior_for_pack(opensearch, pack, registry_names), None - except RegistryPriorUnavailableError as exc: - return None, str(exc) diff --git a/src/services/labeling/detector_hint.py b/src/services/labeling/detector_hint.py new file mode 100644 index 00000000..269b0b09 --- /dev/null +++ b/src/services/labeling/detector_hint.py @@ -0,0 +1,49 @@ +"""Optional per-item detector-name hint for the VLM prompt (#193). + +On the public COCO oracle the stored detector name was right about 96.6% of the time +against 72.7% for the VLM, so a pack can opt in to telling the VLM what the detector +saw. It is a hint only: the reply parser still accepts any answer, and the stored +detector fields are read, never written. Class identity is the name. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +from src.services.labeling.vlm_models import ItemCrop + + +if TYPE_CHECKING: + from src.services.labeling.vlm_prompts import PromptPack + + +#: Item fields a caller must read to build a hint. +DETECTOR_HINT_FIELDS = ('detector_class_name', 'detector_confidence') + +#: ``detector_hint_min_confidence_pct`` bounds; 0 is off. +MAX_DETECTOR_HINT_PCT = 100 + + +def detector_hint_for(src: dict[str, Any], min_confidence_pct: int) -> tuple[str, float | None]: + """The ``(detector class name, confidence)`` to hint, or ``('', None)``. + + Empty when the setting is off (0), the item has no stored detector name or + confidence, or the confidence is below ``min_confidence_pct`` percent. + """ + if min_confidence_pct <= 0: + return '', None + name = src.get('detector_class_name') + conf = src.get('detector_confidence') + if not isinstance(name, str) or not name: + return '', None + if isinstance(conf, bool) or not isinstance(conf, (int, float)): + return '', None + if conf * 100 < min_confidence_pct: + return '', None + return name, float(conf) + + +def hinted_crop(crop_id: str, jpeg: bytes, src: dict[str, Any], pack: PromptPack) -> ItemCrop: + """An :class:`ItemCrop` carrying the detector hint ``pack`` asks for, if any.""" + name, conf = detector_hint_for(src, pack.detector_hint_min_confidence_pct) + return ItemCrop(img_id=crop_id, jpeg_bytes=jpeg, detector_class=name, detector_confidence=conf) diff --git a/src/services/labeling/registry_prior.py b/src/services/labeling/registry_prior.py deleted file mode 100644 index 7f369746..00000000 --- a/src/services/labeling/registry_prior.py +++ /dev/null @@ -1,65 +0,0 @@ -"""Optional registry prior for VLM item labeling (#61 item 2). - -A prompt pack with ``registry_prior_top_k > 0`` tells the VLM which registry -classes are best established in the project (ranked by validated-item count) -and which new-class proposals are pending. The list is a hint: the reply parser -still accepts any answer, and the lock rule and test holdout are untouched -because only the prompt text changes. - -Class identity is the name; no index crosses this module. -""" - -from __future__ import annotations - -from dataclasses import dataclass -from typing import TYPE_CHECKING - - -if TYPE_CHECKING: - from collections.abc import Collection, Mapping - - -#: Hard ceiling on ``registry_prior_top_k``, enforced by pack validation. -MAX_REGISTRY_PRIOR_TOP_K = 50 - - -@dataclass(frozen=True) -class RegistryPrior: - """Candidate names for one VLM labeling run, already ordered.""" - - classes: tuple[str, ...] - pending: tuple[str, ...] - - def prompt_text(self) -> str: - parts = [f'Most established registry classes: {", ".join(self.classes)}.'] - if self.pending: - parts.append(f'Pending proposed classes: {", ".join(self.pending)}.') - parts.append( - 'Treat these as a prior, not a constraint: answer with any registry class or ' - 'a new class if the crop clearly belongs elsewhere.' - ) - return ' '.join(parts) - - -def rank_registry_prior( - registry_names: Collection[str], - validated_counts: Mapping[str, int], - pending_counts: Mapping[str, int], - *, - top_k: int, -) -> RegistryPrior | None: - """Top ``top_k`` registry names by validated count, ties by name. - - Pending proposals that already are registry names are dropped (they would - be a duplicate candidate). ``None`` when ``top_k`` is not positive or the - registry is empty: there is nothing to prime with. - """ - if top_k <= 0 or not registry_names: - return None - known = set(registry_names) - classes = sorted(known, key=lambda n: (-int(validated_counts.get(n, 0)), n))[:top_k] - pending = sorted( - (n for n in pending_counts if n and n not in known), - key=lambda n: (-int(pending_counts[n]), n), - )[:top_k] - return RegistryPrior(classes=tuple(classes), pending=tuple(pending)) diff --git a/src/services/labeling/vlm_labeler_classify.py b/src/services/labeling/vlm_labeler_classify.py index 9027bac7..03bb0672 100644 --- a/src/services/labeling/vlm_labeler_classify.py +++ b/src/services/labeling/vlm_labeler_classify.py @@ -23,7 +23,6 @@ if TYPE_CHECKING: from src.config import RegionFields - from src.services.labeling.registry_prior import RegistryPrior logger = get_logger(__name__) @@ -260,7 +259,6 @@ async def label_or_propose_batch( images_per_call: int | None = None, class_catalog: str | None = None, cluster_hint: str | None = None, - registry_prior: RegistryPrior | None = None, ) -> list[VlmClassPrediction]: """Label crops, but allow the VLM to propose new classes when nothing fits. @@ -276,10 +274,9 @@ async def label_or_propose_batch( replaces the bare CSV in the prompt with grouped + described classes, sharply improving accuracy on ambiguous slugs. ``cluster_hint`` adds a per-batch bias line — pass it when labeling a single cluster's members - to nudge the VLM toward the dominant class hypothesis. ``registry_prior`` - (built only when the pack's ``registry_prior_top_k`` is set) lists the - best-established classes and pending proposals as a hint; replies outside - it are still accepted. + to nudge the VLM toward the dominant class hypothesis. When the pack sets + ``detector_hint_min_confidence_pct``, crops carrying a detector class get a + per-image hint line; replies outside it are still accepted. """ if not crops: @@ -304,7 +301,6 @@ async def label_or_propose_batch( class_names, class_catalog=class_catalog, cluster_hint=cluster_hint, - registry_prior=registry_prior, ) for c in chunks ], @@ -322,7 +318,6 @@ async def _label_chunk_open( *, class_catalog: str | None = None, cluster_hint: str | None = None, - registry_prior: RegistryPrior | None = None, ) -> list[VlmClassPrediction]: if class_catalog: user_text = ( @@ -338,8 +333,19 @@ async def _label_chunk_open( class_names_csv=', '.join(class_names) ) user_text = f'{user_text}\n{_RESULTS_ENVELOPE}' - if registry_prior is not None: - user_text = f'{registry_prior.prompt_text()}\n\n{user_text}' + if self._pack.detector_hint_min_confidence_pct > 0: + hints = [ + f'image {i}: {c.detector_class} (detector confidence {c.detector_confidence:.2f})' + for i, c in enumerate(chunk, start=1) + if c.detector_class and c.detector_confidence is not None + ] + if hints: + user_text = ( + f'Detector hints, one per image where the detector saw something: ' + f'{"; ".join(hints)}. A hint, not a constraint: the detector can be wrong, ' + 'so answer from what the crop shows.\n\n' + f'{user_text}' + ) if cluster_hint: user_text = ( f'Hint: these crops were grouped together by visual similarity; the ' diff --git a/src/services/labeling/vlm_models.py b/src/services/labeling/vlm_models.py index 733ad735..288f60c6 100644 --- a/src/services/labeling/vlm_models.py +++ b/src/services/labeling/vlm_models.py @@ -34,6 +34,14 @@ class ItemCrop(BaseModel): description='JPEG-encoded crop bytes. Caller is responsible for resizing to a sane size ' '(e.g. ≤ 768 px on the long edge) before calling.', ) + detector_class: str = Field( + '', + description="The item's stored detector class name, set only when the pack opts in " + '(``detector_hint_min_confidence_pct``); empty = no hint.', + ) + detector_confidence: float | None = Field( + None, description='Detector confidence for ``detector_class``.' + ) class RegionCrop(BaseModel): diff --git a/src/services/labeling/vlm_prompts.py b/src/services/labeling/vlm_prompts.py index aabc45c3..b438276e 100644 --- a/src/services/labeling/vlm_prompts.py +++ b/src/services/labeling/vlm_prompts.py @@ -26,7 +26,7 @@ import re from dataclasses import asdict, dataclass, field, fields from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any from src.core.logging import get_logger from src.services.labeling.region_overlay import ( @@ -36,6 +36,9 @@ ) +if TYPE_CHECKING: + from collections.abc import Sequence + logger = get_logger(__name__) @@ -100,10 +103,10 @@ class PromptPack: # candidate. Empty = no filtering. proposal_denylist: list[str] = field(default_factory=list) - # Registry prior (#61): when > 0, open-vocabulary labeling tells the VLM - # the top-k registry classes (by validated count) and pending proposal - # names as a hint. 0 = off. Bounded by ``MAX_REGISTRY_PRIOR_TOP_K``. - registry_prior_top_k: int = 0 + # Detector hint (#193): when > 0, open-vocabulary labeling tells the VLM each + # item's stored detector class name when the detector confidence is at least + # this many percent. 0 = off. Bounded by ``MAX_DETECTOR_HINT_PCT`` (100). + detector_hint_min_confidence_pct: int = 0 def to_dict(self) -> dict[str, Any]: """Plain-dict serialization -- every field is a ``str`` or a @@ -141,12 +144,52 @@ def from_json(cls, path: str | Path) -> PromptPack: return cls.from_dict(data) -def proposal_denied(slug: str, patterns: list[str] | tuple[str, ...]) -> bool: +def proposal_denied(slug: str, patterns: Sequence[str]) -> bool: """True when ``slug`` matches any ``patterns`` glob (case-insensitive).""" low = slug.lower() return any(fnmatch.fnmatchcase(low, str(p).lower()) for p in patterns) +#: Shipped proposal-name denylist: scene / image-quality words that are never a +#: class. A live COCO oracle run (#193) put ``abstract_background`` at 60% of pending +#: proposals, so the abstract / background / generic-object families are listed beside +#: the blur and emptiness words. Globs are anchored (``fnmatch``, whole slug) and +#: tested against the COCO class names so none of them eats a real class. +DEFAULT_PROPOSAL_DENYLIST: tuple[str, ...] = ( + 'abstract*', + 'background*', + '*_background', + 'object', + 'objects', + '*_object', + 'blurry*', + 'blurred*', + '*_blurry', + '*_blur', + 'blur_*', + 'out_of_focus*', + 'low_quality*', + 'low_resolution*', + 'unclear*', + 'unidentified*', + 'scene*', + '*_scene', + 'empty*', + 'unknown*', + '*_image', + '*_photo', + '*_abstract', + 'blank', + 'blank_*', + 'indoor', + 'indoors', + 'outdoor', + 'outdoors', + 'shadow*', + '*_shadow', +) + + # --------------------------------------------------------------------------- # Neutral example pack — generic "product photo" domain. # @@ -285,20 +328,7 @@ def proposal_denied(slug: str, patterns: list[str] | tuple[str, ...]) -> bool: # Deliberately no class_descriptions/synonyms: they name registry classes, # and the shipped default must validate with zero warnings on any registry # (empty, COCO, ...). Domain vocabulary belongs in a stored pack. - proposal_denylist=[ - 'blurry*', - '*_blurry', - 'blur_*', - 'out_of_focus*', - 'low_quality*', - 'low_resolution*', - 'unclear*', - '*_scene', - 'scene_*', - 'background*', - 'empty_*', - 'unknown*', - ], + proposal_denylist=list(DEFAULT_PROPOSAL_DENYLIST), ) @@ -407,6 +437,7 @@ def proposal_denied(slug: str, patterns: list[str] | tuple[str, ...]) -> bool: 'is an array of per-image verdicts in input order:\n' '{"results": [{"img": 1, "visible": true|false}, ...]}' ), + proposal_denylist=list(DEFAULT_PROPOSAL_DENYLIST), ) # A quoted value in a prompt: "..." or '...'. Double-quoted strings are @@ -542,6 +573,7 @@ def prompt_text_examples(pack: PromptPack) -> frozenset[str]: __all__ = [ 'BUILT_IN_PACKS', + 'DEFAULT_PROPOSAL_DENYLIST', 'FORMATTED_PLACEHOLDERS', 'GENERIC_ITEM_PACK', 'GENERIC_REGION_PACK', diff --git a/tests/curation/test_detector_hint.py b/tests/curation/test_detector_hint.py new file mode 100644 index 00000000..97e9f48f --- /dev/null +++ b/tests/curation/test_detector_hint.py @@ -0,0 +1,238 @@ +"""#193: optional per-item detector-name hint in the open-vocabulary VLM prompt.""" + +from __future__ import annotations + +import dataclasses +from typing import Any +from unittest.mock import AsyncMock + +import pytest +from _curation_app import mount_curation_routers +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from curation._fake_config_opensearch import FakeConfigOpenSearch +from curation.test_proposal_denylist import _labeler +from src.services.labeling.detector_hint import detector_hint_for +from src.services.labeling.vlm_models import ItemCrop +from src.services.labeling.vlm_prompts import GENERIC_ITEM_PACK, PromptPack + + +PREFIX = '/curation/projects/default/prompt_packs' + + +@pytest.mark.parametrize( + ('src', 'min_pct', 'expected'), + [ + ({'detector_class_name': 'car', 'detector_confidence': 0.91}, 50, ('car', 0.91)), + ({'detector_class_name': 'car', 'detector_confidence': 0.91}, 0, ('', None)), + ({'detector_class_name': 'car', 'detector_confidence': 0.4}, 50, ('', None)), + ({'detector_class_name': 'car', 'detector_confidence': 0.5}, 50, ('car', 0.5)), + ({'detector_class_name': '', 'detector_confidence': 0.9}, 1, ('', None)), + ({'detector_confidence': 0.9}, 1, ('', None)), + ({'detector_class_name': 'car'}, 1, ('', None)), + ({'detector_class_name': 'car', 'detector_confidence': 'high'}, 1, ('', None)), + ], +) +def test_detector_hint_for(src: dict[str, Any], min_pct: int, expected: tuple[str, Any]) -> None: + assert detector_hint_for(src, min_pct) == expected + + +def test_default_pack_has_the_hint_off_and_round_trips() -> None: + assert GENERIC_ITEM_PACK.detector_hint_min_confidence_pct == 0 + pack = dataclasses.replace(GENERIC_ITEM_PACK, detector_hint_min_confidence_pct=40) + assert PromptPack.from_dict(pack.to_dict()).detector_hint_min_confidence_pct == 40 + + +def _sent_text(labeler: Any) -> str: + payload = labeler._client.post.await_args.kwargs['json'] + return payload['messages'][1]['content'][0]['text'] + + +_ENTRY = {'img': 1, 'class': 'car', 'confidence': 'high', 'proposed_class': ''} + + +@pytest.mark.asyncio +async def test_prompt_names_the_detector_class_per_image() -> None: + pack = dataclasses.replace(GENERIC_ITEM_PACK, detector_hint_min_confidence_pct=1) + labeler = _labeler([_ENTRY], pack) + crops = [ + ItemCrop(img_id='c1', jpeg_bytes=b'j', detector_class='car', detector_confidence=0.91), + ItemCrop(img_id='c2', jpeg_bytes=b'j'), + ItemCrop(img_id='c3', jpeg_bytes=b'j', detector_class='dog', detector_confidence=0.5), + ] + await labeler.label_or_propose_batch(crops, ['car', 'dog']) + text = _sent_text(labeler) + assert 'image 1: car (detector confidence 0.91)' in text + assert 'image 3: dog (detector confidence 0.50)' in text + assert 'image 2' not in text.split('Label each')[0] + assert 'hint, not a constraint' in text + + +@pytest.mark.asyncio +async def test_prompt_has_no_hint_when_the_pack_has_it_off_or_no_crop_has_one() -> None: + crop = ItemCrop(img_id='c1', jpeg_bytes=b'j', detector_class='car', detector_confidence=0.9) + off = _labeler([_ENTRY]) + await off.label_or_propose_batch([crop], ['car']) + assert 'detector' not in _sent_text(off).lower() + + on = _labeler( + [_ENTRY], dataclasses.replace(GENERIC_ITEM_PACK, detector_hint_min_confidence_pct=1) + ) + await on.label_or_propose_batch([ItemCrop(img_id='c1', jpeg_bytes=b'j')], ['car']) + assert 'detector' not in _sent_text(on).lower() + + +# --- pack REST --------------------------------------------------------------- + + +@pytest.fixture(autouse=True) +def _reset_caches(): + from src.services.config_store.store import reset_config_stores + + reset_config_stores() + yield + reset_config_stores() + + +@pytest.fixture +def app_client(monkeypatch: pytest.MonkeyPatch) -> TestClient: + from src.routers.curation import _raw_opensearch_dep, router as curation_router + + monkeypatch.setattr('src.routers.curation._ensure_indexes', AsyncMock(return_value=None)) + app = FastAPI() + mount_curation_routers(app, curation_router) + fake_os = FakeConfigOpenSearch() + app.dependency_overrides[_raw_opensearch_dep] = lambda: fake_os + return TestClient(app) + + +def _body(**over: object) -> dict[str, object]: + body = GENERIC_ITEM_PACK.to_dict() + body.pop('name') + body.update(over) + return body + + +def test_schema_serves_the_setting_as_an_int_row(app_client: TestClient) -> None: + fields = {f['field']: f for f in app_client.get(f'{PREFIX}/schema').json()['fields']} + row = fields['detector_hint_min_confidence_pct'] + assert row['kind'] == 'int' + assert row['group'] == 'open_classify' + assert '100' in row['help'] + + +@pytest.mark.parametrize('bad', [-1, 101, True, 'x', 2.5, None]) +def test_bad_value_is_a_validation_error(app_client: TestClient, bad: object) -> None: + body = _body(detector_hint_min_confidence_pct=bad) + report = app_client.post(f'{PREFIX}/validate', json={'body': body}).json() + assert report['ok'] is False, bad + assert any(e['field'] == 'detector_hint_min_confidence_pct' for e in report['errors']) + + +@pytest.mark.parametrize('good', [0, 1, 100]) +def test_value_in_range_round_trips(app_client: TestClient, good: int) -> None: + body = _body(detector_hint_min_confidence_pct=good) + r = app_client.post(PREFIX, json={'name': 'hint_pack', 'description': '', 'body': body}) + assert r.status_code == 201, r.text + got = app_client.get(f'{PREFIX}/hint_pack').json()['body'] + assert got['detector_hint_min_confidence_pct'] == good + + +def test_missing_value_defaults_off(app_client: TestClient) -> None: + body = _body() + body.pop('detector_hint_min_confidence_pct') + assert app_client.post(f'{PREFIX}/validate', json={'body': body}).json()['ok'] is True + + +# --- wiring: POST /vlm/label_batch ------------------------------------------- + + +def _wire_label_batch( + tmp_path: Any, + monkeypatch: pytest.MonkeyPatch, + pack: PromptPack, + seen: list[Any], + doc_extra: dict[str, Any] | None = None, +) -> tuple[Any, Any]: + import io + from types import SimpleNamespace + + from PIL import Image + + import src.routers.curation.vlm as vlm_mod + from curation.query_fakes import QueryFakeOpenSearch + from src.clients.curation_opensearch.registry import ClassRegistry + from src.config.curation import base_curation_config + + items = base_curation_config().items_index + buf = io.BytesIO() + Image.new('RGB', (32, 32), (1, 2, 3)).save(buf, format='JPEG') + (tmp_path / 'c1.jpg').write_bytes(buf.getvalue()) + monkeypatch.setattr( + vlm_mod, 'get_curation_config', lambda: SimpleNamespace(crop_cache_dir=tmp_path) + ) + reg = ClassRegistry(path=tmp_path / 'class_registry.json') + reg.add_class('widget') + monkeypatch.setattr(vlm_mod, 'get_class_registry', lambda: reg) + + async def _no_pack(_os: Any) -> None: + return None + + monkeypatch.setattr(vlm_mod, '_default_pack_name', _no_pack) + fake = QueryFakeOpenSearch( + { + items: { + 'c1': { + 'crop_id': 'c1', + 'image_path': '/data/c1.jpg', + 'bbox_norm': [0.1, 0.1, 0.5, 0.5], + 'class_source': 'item_proposal', + 'class_validated': False, + **(doc_extra or {}), + } + } + } + ) + + return vlm_mod, fake + + +class _RecordingLabeler: + def __init__(self, pack: PromptPack, crops: list[Any]) -> None: + from src.services.labeling.vlm_client import VlmIdentity + + self.identity = VlmIdentity('env@None', 'test-vlm') + self._pack = pack + self.crops = crops + + async def label_or_propose_batch( + self, crops: list[Any], _names: list[str], **_k: Any + ) -> list[Any]: + self.crops.extend(crops) + return [] + + +@pytest.mark.usefixtures('vlm_env') +@pytest.mark.asyncio +@pytest.mark.parametrize(('pct', 'hinted'), [(0, False), (50, True), (95, False)]) +async def test_label_batch_attaches_the_stored_detector_name_only_when_enabled( + tmp_path: Any, monkeypatch: pytest.MonkeyPatch, pct: int, hinted: bool +) -> None: + from src.routers.curation.vlm import VlmLabelBatchRequest + + pack = dataclasses.replace(GENERIC_ITEM_PACK, detector_hint_min_confidence_pct=pct) + vlm_mod, fake = _wire_label_batch( + tmp_path, + monkeypatch, + pack, + [], + doc_extra={'detector_class_name': 'car', 'detector_confidence': 0.9}, + ) + crops: list[Any] = [] + monkeypatch.setattr( + vlm_mod, '_get_vlm_labeler', lambda *_a, **_k: _RecordingLabeler(pack, crops) + ) + await vlm_mod.vlm_label_batch(VlmLabelBatchRequest(crop_ids=['c1']), fake) + assert len(crops) == 1 + assert crops[0].detector_class == ('car' if hinted else '') diff --git a/tests/curation/test_prompt_pack.py b/tests/curation/test_prompt_pack.py index 25432a75..25adeb9b 100644 --- a/tests/curation/test_prompt_pack.py +++ b/tests/curation/test_prompt_pack.py @@ -47,7 +47,7 @@ def test_prompt_pack_is_frozen_dataclass() -> None: 'class_descriptions', 'synonyms', 'proposal_denylist', - 'registry_prior_top_k', + 'detector_hint_min_confidence_pct', } diff --git a/tests/curation/test_proposal_denylist.py b/tests/curation/test_proposal_denylist.py index 8af1bf2f..0fcb00b6 100644 --- a/tests/curation/test_proposal_denylist.py +++ b/tests/curation/test_proposal_denylist.py @@ -72,3 +72,154 @@ async def test_empty_denylist_keeps_everything() -> None: labeler = _labeler([_entry(1, 'blurry_thing')], pack) preds = await labeler.label_or_propose_batch([ItemCrop(img_id='c1', jpeg_bytes=b'j')], ['box']) assert preds[0].proposed_class == 'blurry_thing' + + +# The shipped default must catch the scene / quality words the live COCO oracle run +# produced (abstract_background was 60% of pending proposals) and nothing that is a +# real object class. +_COCO_NAMES = [ + 'person', + 'bicycle', + 'car', + 'motorcycle', + 'airplane', + 'bus', + 'train', + 'truck', + 'boat', + 'traffic_light', + 'fire_hydrant', + 'stop_sign', + 'parking_meter', + 'bench', + 'bird', + 'cat', + 'dog', + 'horse', + 'sheep', + 'cow', + 'elephant', + 'bear', + 'zebra', + 'giraffe', + 'backpack', + 'umbrella', + 'handbag', + 'tie', + 'suitcase', + 'frisbee', + 'skis', + 'snowboard', + 'sports_ball', + 'kite', + 'baseball_bat', + 'baseball_glove', + 'skateboard', + 'surfboard', + 'tennis_racket', + 'bottle', + 'wine_glass', + 'cup', + 'fork', + 'knife', + 'spoon', + 'bowl', + 'banana', + 'apple', + 'sandwich', + 'orange', + 'broccoli', + 'carrot', + 'hot_dog', + 'pizza', + 'donut', + 'cake', + 'chair', + 'couch', + 'potted_plant', + 'bed', + 'dining_table', + 'toilet', + 'tv', + 'laptop', + 'mouse', + 'remote', + 'keyboard', + 'cell_phone', + 'microwave', + 'oven', + 'toaster', + 'sink', + 'refrigerator', + 'book', + 'clock', + 'vase', + 'scissors', + 'teddy_bear', + 'hair_drier', + 'toothbrush', + 'forklift', + 'crane', + 'pallet', + 'x_ray_scanner', + 'photo_booth', + 'image_sensor', + 'blanket', + 'dark_chocolate', + 'indoor_plant', + 'outdoor_grill', +] + + +@pytest.mark.parametrize( + 'noise', + [ + 'abstract_background', + 'abstract_object', + 'abstract', + 'abstract_pattern', + 'plain_background', + 'dark_background', + 'background', + 'unknown_object', + 'generic_object', + 'object', + 'blurry', + 'blurry_object', + 'blurred_image', + 'out_of_focus', + 'low_quality_image', + 'street_scene', + 'scene', + 'scene_unclear', + 'empty', + 'empty_frame', + 'unclear', + 'unidentified', + # residual noise left after the first live run (#193) + 'blank_image', + 'dark_image', + 'dark_abstract', + 'outdoors', + 'indoors', + 'outdoor', + 'blank', + 'shadow', + 'long_shadow', + 'street_photo', + ], +) +def test_default_pack_denies_scene_and_quality_words(noise: str) -> None: + assert proposal_denied(noise, GENERIC_ITEM_PACK.proposal_denylist), noise + + +@pytest.mark.parametrize('name', _COCO_NAMES) +def test_default_pack_keeps_real_class_names(name: str) -> None: + assert not proposal_denied(name, GENERIC_ITEM_PACK.proposal_denylist), name + + +def test_every_shipped_default_pack_inherits_the_denylist() -> None: + from src.services.labeling import vlm_prompts + + for pack in (vlm_prompts.GENERIC_ITEM_PACK, vlm_prompts.GENERIC_REGION_PACK): + assert proposal_denied('abstract_background', pack.proposal_denylist), pack.name diff --git a/tests/curation/test_registry_prior.py b/tests/curation/test_registry_prior.py deleted file mode 100644 index ec505093..00000000 --- a/tests/curation/test_registry_prior.py +++ /dev/null @@ -1,310 +0,0 @@ -"""#61 item 2: optional per-pack registry prior for the VLM labeling prompt.""" - -from __future__ import annotations - -import dataclasses -from typing import Any -from unittest.mock import AsyncMock - -import pytest -from _curation_app import mount_curation_routers -from fastapi import FastAPI -from fastapi.testclient import TestClient - -from curation._fake_config_opensearch import FakeConfigOpenSearch -from curation.test_proposal_denylist import _labeler -from src.services.curation.registry_prior_source import ( - RegistryPriorUnavailableError, - load_registry_prior, -) -from src.services.labeling.registry_prior import ( - MAX_REGISTRY_PRIOR_TOP_K, - RegistryPrior, - rank_registry_prior, -) -from src.services.labeling.vlm_models import ItemCrop -from src.services.labeling.vlm_prompts import GENERIC_ITEM_PACK, PromptPack - - -PREFIX = '/curation/projects/default/prompt_packs' - - -def test_rank_orders_by_validated_count_then_name_and_bounds() -> None: - prior = rank_registry_prior( - ['car', 'bus', 'truck', 'bike'], - {'truck': 9, 'car': 9, 'bus': 1}, - {'forklift': 3, 'bus': 8, 'crane': 3}, - top_k=3, - ) - assert prior == RegistryPrior( - classes=('car', 'truck', 'bus'), - # 'bus' is a registry name, so it is not a pending candidate - pending=('crane', 'forklift'), - ) - - -def test_rank_is_none_when_off_or_empty() -> None: - assert rank_registry_prior(['car'], {}, {}, top_k=0) is None - assert rank_registry_prior([], {}, {}, top_k=5) is None - - -def test_default_pack_has_prior_off_and_round_trips() -> None: - assert GENERIC_ITEM_PACK.registry_prior_top_k == 0 - pack = dataclasses.replace(GENERIC_ITEM_PACK, registry_prior_top_k=7) - assert PromptPack.from_dict(pack.to_dict()).registry_prior_top_k == 7 - - -class _Os: - def __init__(self, resp: dict[str, Any] | Exception) -> None: - self.resp = resp - self.bodies: list[dict[str, Any]] = [] - - async def search(self, *, index: str, body: dict[str, Any]) -> dict[str, Any]: - assert index - self.bodies.append(body) - if isinstance(self.resp, Exception): - raise self.resp - return self.resp - - -def _aggs(validated: dict[str, int], pending: dict[str, int]) -> dict[str, Any]: - def node(counts: dict[str, int]) -> dict[str, Any]: - return {'by_name': {'buckets': [{'key': k, 'doc_count': v} for k, v in counts.items()]}} - - return {'aggregations': {'validated': node(validated), 'pending': node(pending)}} - - -@pytest.mark.asyncio -async def test_source_ranks_served_counts_and_excludes_holdout() -> None: - os_ = _Os(_aggs({'bus': 2, 'car': 7}, {'forklift': 4})) - prior = await load_registry_prior(os_, top_k=2, registry_names=['bus', 'car', 'van']) - assert prior == RegistryPrior(classes=('car', 'bus'), pending=('forklift',)) - assert {'term': {'test_holdout': True}} in os_.bodies[0]['query']['bool']['must_not'] - - -@pytest.mark.asyncio -async def test_prior_or_error_reports_instead_of_raising() -> None: - from src.services.curation.registry_prior_source import prior_or_error - - on = dataclasses.replace(GENERIC_ITEM_PACK, registry_prior_top_k=3) - prior, err = await prior_or_error(_Os(RuntimeError('down')), on, ['car']) - assert prior is None - assert err is not None - assert 'down' in err - prior, err = await prior_or_error(_Os(_aggs({'car': 1}, {})), on, ['car']) - assert err is None - assert prior is not None - assert prior.classes == ('car',) - - -@pytest.mark.asyncio -async def test_source_off_does_not_query() -> None: - os_ = _Os(_aggs({}, {})) - assert await load_registry_prior(os_, top_k=0, registry_names=['car']) is None - assert os_.bodies == [] - - -@pytest.mark.asyncio -async def test_source_fails_closed_when_counts_unreadable() -> None: - with pytest.raises(RegistryPriorUnavailableError): - await load_registry_prior(_Os(RuntimeError('down')), top_k=3, registry_names=['car']) - - -def _sent_text(labeler: Any) -> str: - payload = labeler._client.post.await_args.kwargs['json'] - return payload['messages'][1]['content'][0]['text'] - - -@pytest.mark.asyncio -async def test_prompt_carries_the_prior_and_off_does_not() -> None: - entry = {'img': 1, 'class': 'car', 'confidence': 'high', 'proposed_class': ''} - crops = [ItemCrop(img_id='c1', jpeg_bytes=b'j')] - prior = RegistryPrior(classes=('car', 'bus'), pending=('forklift',)) - - on = _labeler([entry]) - await on.label_or_propose_batch(crops, ['car', 'bus', 'van'], registry_prior=prior) - text = _sent_text(on) - assert 'Most established registry classes: car, bus.' in text - assert 'Pending proposed classes: forklift.' in text - - off = _labeler([entry]) - await off.label_or_propose_batch(crops, ['car', 'bus', 'van']) - assert 'registry classes' not in _sent_text(off) - - -@pytest.mark.asyncio -async def test_answer_outside_the_candidates_is_still_accepted() -> None: - entry = {'img': 1, 'class': 'van', 'confidence': 'high', 'proposed_class': ''} - labeler = _labeler([entry]) - prior = RegistryPrior(classes=('car',), pending=()) - preds = await labeler.label_or_propose_batch( - [ItemCrop(img_id='c1', jpeg_bytes=b'j')], ['car', 'van'], registry_prior=prior - ) - assert preds[0].class_name == 'van' - assert preds[0].failure is None - - -# --- pack REST: schema, validation, round trip ------------------------------- - - -@pytest.fixture(autouse=True) -def _reset_caches(): - from src.services.config_store.store import reset_config_stores - - reset_config_stores() - yield - reset_config_stores() - - -@pytest.fixture -def app_client(monkeypatch: pytest.MonkeyPatch) -> TestClient: - from src.routers.curation import _raw_opensearch_dep, router as curation_router - - monkeypatch.setattr('src.routers.curation._ensure_indexes', AsyncMock(return_value=None)) - app = FastAPI() - mount_curation_routers(app, curation_router) - fake_os = FakeConfigOpenSearch() - app.dependency_overrides[_raw_opensearch_dep] = lambda: fake_os - return TestClient(app) - - -def _body(**over: object) -> dict[str, object]: - body = GENERIC_ITEM_PACK.to_dict() - body.pop('name') - body.update(over) - return body - - -def test_schema_serves_the_setting_as_an_int_row(app_client: TestClient) -> None: - fields = {f['field']: f for f in app_client.get(f'{PREFIX}/schema').json()['fields']} - assert fields['registry_prior_top_k']['kind'] == 'int' - assert fields['registry_prior_top_k']['group'] == 'open_classify' - assert str(MAX_REGISTRY_PRIOR_TOP_K) in fields['registry_prior_top_k']['help'] - - -@pytest.mark.parametrize('bad', [-1, MAX_REGISTRY_PRIOR_TOP_K + 1, True, 'ten', 2.5, None]) -def test_bad_top_k_is_a_validation_error(app_client: TestClient, bad: object) -> None: - r = app_client.post(f'{PREFIX}/validate', json={'body': _body(registry_prior_top_k=bad)}) - assert r.status_code == 200, r.text - report = r.json() - assert report['ok'] is False, bad - assert any(e['field'] == 'registry_prior_top_k' for e in report['errors']) - - -@pytest.mark.parametrize('good', [0, 1, MAX_REGISTRY_PRIOR_TOP_K]) -def test_top_k_in_range_validates_and_round_trips(app_client: TestClient, good: int) -> None: - r = app_client.post( - PREFIX, - json={'name': 'prior_pack', 'description': '', 'body': _body(registry_prior_top_k=good)}, - ) - assert r.status_code == 201, r.text - got = app_client.get(f'{PREFIX}/prior_pack').json()['body'] - assert got['registry_prior_top_k'] == good - - -def test_missing_top_k_defaults_off(app_client: TestClient) -> None: - body = _body() - body.pop('registry_prior_top_k') - r = app_client.post(f'{PREFIX}/validate', json={'body': body}) - assert r.json()['ok'] is True - - -# --- wiring: POST /vlm/label_batch ------------------------------------------ - - -def _wire_label_batch( - tmp_path: Any, monkeypatch: pytest.MonkeyPatch, pack: PromptPack, seen: list[Any] -) -> tuple[Any, Any]: - import io - from types import SimpleNamespace - - from PIL import Image - - import src.routers.curation.vlm as vlm_mod - from curation.query_fakes import QueryFakeOpenSearch - from src.clients.curation_opensearch.registry import ClassRegistry - from src.config.curation import base_curation_config - from src.services.labeling.vlm_client import VlmIdentity - - items = base_curation_config().items_index - buf = io.BytesIO() - Image.new('RGB', (32, 32), (1, 2, 3)).save(buf, format='JPEG') - (tmp_path / 'c1.jpg').write_bytes(buf.getvalue()) - monkeypatch.setattr( - vlm_mod, 'get_curation_config', lambda: SimpleNamespace(crop_cache_dir=tmp_path) - ) - reg = ClassRegistry(path=tmp_path / 'class_registry.json') - reg.add_class('widget') - monkeypatch.setattr(vlm_mod, 'get_class_registry', lambda: reg) - - async def _no_pack(_os: Any) -> None: - return None - - monkeypatch.setattr(vlm_mod, '_default_pack_name', _no_pack) - fake = QueryFakeOpenSearch( - { - items: { - 'c1': { - 'crop_id': 'c1', - 'image_path': '/data/c1.jpg', - 'bbox_norm': [0.1, 0.1, 0.5, 0.5], - 'class_source': 'item_proposal', - 'class_validated': False, - } - } - } - ) - - class _Labeler: - identity = VlmIdentity('env@None', 'test-vlm') - _pack = pack - - async def label_or_propose_batch( - self, _crops: list[Any], _names: list[str], *, registry_prior: Any = None - ) -> list[Any]: - seen.append(registry_prior) - return [] - - monkeypatch.setattr(vlm_mod, '_get_vlm_labeler', lambda *_a, **_k: _Labeler()) - return vlm_mod, fake - - -@pytest.mark.usefixtures('vlm_env') -@pytest.mark.asyncio -async def test_label_batch_passes_the_served_prior_only_when_the_pack_asks( - tmp_path: Any, monkeypatch: pytest.MonkeyPatch -) -> None: - from src.routers.curation.vlm import VlmLabelBatchRequest - - seen: list[Any] = [] - off = dataclasses.replace(GENERIC_ITEM_PACK, registry_prior_top_k=0) - vlm_mod, fake = _wire_label_batch(tmp_path, monkeypatch, off, seen) - await vlm_mod.vlm_label_batch(VlmLabelBatchRequest(crop_ids=['c1']), fake) - assert seen == [None] - - seen.clear() - on = dataclasses.replace(GENERIC_ITEM_PACK, registry_prior_top_k=5) - (tmp_path / 'on').mkdir() - vlm_mod, fake = _wire_label_batch(tmp_path / 'on', monkeypatch, on, seen) - await vlm_mod.vlm_label_batch(VlmLabelBatchRequest(crop_ids=['c1']), fake) - assert seen[0] is not None - assert seen[0].classes == ('widget',) - - -@pytest.mark.usefixtures('vlm_env') -@pytest.mark.asyncio -async def test_label_batch_refuses_when_the_prior_cannot_be_built( - tmp_path: Any, monkeypatch: pytest.MonkeyPatch -) -> None: - from fastapi import HTTPException - - from src.routers.curation.vlm import VlmLabelBatchRequest - - seen: list[Any] = [] - on = dataclasses.replace(GENERIC_ITEM_PACK, registry_prior_top_k=5) - vlm_mod, fake = _wire_label_batch(tmp_path, monkeypatch, on, seen) - fake.search = AsyncMock(side_effect=RuntimeError('down')) - with pytest.raises(HTTPException) as exc: - await vlm_mod.vlm_label_batch(VlmLabelBatchRequest(crop_ids=['c1']), fake) - assert exc.value.status_code == 503 - assert seen == [] diff --git a/tests/curation/test_vlm_prior_oracle.py b/tests/curation/test_vlm_hint_oracle.py similarity index 94% rename from tests/curation/test_vlm_prior_oracle.py rename to tests/curation/test_vlm_hint_oracle.py index d36b328f..bb0894d2 100644 --- a/tests/curation/test_vlm_prior_oracle.py +++ b/tests/curation/test_vlm_hint_oracle.py @@ -1,4 +1,4 @@ -"""Offline scoring of the registry-prior on/off comparison (#61 item 2).""" +"""Offline scoring of the detector-hint on/off comparison (#61 item 2).""" from __future__ import annotations @@ -10,8 +10,8 @@ import pytest -_PATH = Path(__file__).resolve().parents[2] / 'scripts/curation/bakeoff/vlm_prior_oracle.py' -_spec = importlib.util.spec_from_file_location('vlm_prior_oracle', _PATH) +_PATH = Path(__file__).resolve().parents[2] / 'scripts/curation/bakeoff/vlm_hint_oracle.py' +_spec = importlib.util.spec_from_file_location('vlm_hint_oracle', _PATH) assert _spec is not None assert _spec.loader is not None oracle = importlib.util.module_from_spec(_spec)