From 1f92093150ec49dac98b99fa08829d69bef8c015 Mon Sep 17 00:00:00 2001 From: TMHSDigital <154358121+TMHSDigital@users.noreply.github.com> Date: Wed, 23 Sep 2026 16:56:32 -0400 Subject: [PATCH] fix: price unnamed responses by the requested model, key on option order where it matters, and send option descriptions #36: a row whose response named no model got cost None and basis model_not_priced, because pricing looked up only model_reported, while the pre-run guard priced the same run by the requested model. Such a row is now priced by the requested model and records that key. A response that names a model missing from the table stays unpriced: billing follows what answered, and a newer version never inherits an older rate. #37: cache_key sorted the labels, but generative and local_logits list the options in their prompts in the order given, so two orderings shared a key while sending different prompts. An adapter now declares label_order_matters, and for those two the key keeps the given order. Every other adapter still sorts, so reordering options reuses answers. #39: label_descriptions were parsed and then dropped. typesafe_wire sent criteria as None for every option; the runner never passed descriptions to any adapter; and dataset_hash ignored descriptions and question_type, so two datasets differing only in those hashed the same. An adapter now declares uses_label_descriptions; typesafe_wire does, sends them as the choice's criteria, and keys its cache on them. dataset_hash covers question_type and descriptions where a row carries them, so a dataset of plain choice rows keeps its hash. The artifact records how many rows carried descriptions and whether they were sent, and the report says so when they were not. The public fixture has yes/no rows and descriptions, so its dataset hash changes from c18e9496 to 1b96dc91. The example report is regenerated: it gains the descriptions note and the new hash, and no number moves. The README's sample artifact name follows. Fixes #36. Fixes #37. Fixes #39. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 17 +++++ README.md | 2 +- docs/datasets.md | 2 +- docs/example-report.md | 3 +- src/plumbline/adapters/base.py | 16 +++++ src/plumbline/adapters/generative.py | 1 + src/plumbline/adapters/local_logits.py | 3 + src/plumbline/adapters/typesafe_wire.py | 13 +++- src/plumbline/report/markdown.py | 6 ++ src/plumbline/runner/cache.py | 13 +++- src/plumbline/runner/execute.py | 67 ++++++++++++++----- tests/test_report.py | 14 ++++ tests/test_runner.py | 88 +++++++++++++++++++++++++ tests/test_typesafe_wire.py | 16 +++++ 14 files changed, 239 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e496e5..0626c50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -83,6 +83,23 @@ different event from one that moved because it was wrong. ### Fixed +- A row whose response named no model was never priced, because pricing + looked up only the model reported; the guard, which prices the requested + model, and the report then disagreed about the same run (#36). Such a row is + now priced by the requested model, and its pricing key says so. A response + that names a model missing from the table is still unpriced: a newer version + never inherits an older rate. +- The cache key sorted the options, but `generative` and `local_logits` list + them in their prompts in the order given, so a cached answer could be served + for a prompt that was never sent (#37). For those two the key keeps the order. +- Option descriptions were parsed and then dropped: `typesafe_wire` sent every + criterion as empty, and the dataset hash ignored descriptions and question + types (#39). `typesafe_wire` now sends them as the choice's criteria and keys + its cache on them, the dataset hash covers both where a row carries them (a + plain choice dataset keeps its hash), and the report says when an adapter + did not send the descriptions its rows carried. **The public fixture's + dataset hash changes, from `c18e9496` to `1b96dc91`, and the example report + gains that note; no number changes.** - `plumbline report` combined artifacts from different datasets under the first one's hash, as if their figures were comparable, and gave two runs of one adapter identical headings (#43). Different datasets are now refused unless diff --git a/README.md b/README.md index e5a8e44..ef5d182 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,7 @@ Expected output shape: ``` 111 rows read from datasets/public/jevbench-hard.jsonl, 111 loaded, 0 refused. ... -artifact: results/20260921T222053+0000-mock-c18e9496.json +artifact: results/20260921T222053+0000-mock-1b96dc91.json report: results/report.md ``` diff --git a/docs/datasets.md b/docs/datasets.md index a6d1c82..f328a96 100644 --- a/docs/datasets.md +++ b/docs/datasets.md @@ -17,7 +17,7 @@ Blank lines are skipped. Every other line is one case. | `labels` | yes | A list of at least two distinct strings: the options it chooses between. | | `gold_label` | yes | The correct option. It must be one of `labels`. | | `question_type` | no | `choice` (the default), `noul`, or `score`. See below. | -| `label_descriptions` | no | An object mapping an option to a sentence describing it. v0.1 loads these but does not yet send them to any adapter ([#39](https://github.com/TMHSDigital/plumbline/issues/39)). | +| `label_descriptions` | no | An object mapping an option to a sentence describing it. `typesafe_wire` sends them as the choice's criteria; the other adapters do not use them, and the report says so when the rows carried any. | `question_type` says what the row actually asks, and plumbline asks it that way: diff --git a/docs/example-report.md b/docs/example-report.md index 723c4fd..ad66363 100644 --- a/docs/example-report.md +++ b/docs/example-report.md @@ -48,7 +48,7 @@ Running the same command against a real vendor produces the same shape. See # plumbline report -Generated 2026-09-23 against dataset `c18e9496`, 105 rows. 1 arm(s). +Generated 2026-09-23 against dataset `1b96dc91`, 105 rows. 1 arm(s). ## How to read this @@ -72,6 +72,7 @@ The vendor asserts these probabilities are calibrated. Whether that survives con ### mock - **Model**: requested `mock-1`, reported `mock-1`. +- **Option descriptions**: 67 rows carried option descriptions, and this adapter does not send them, so they had no effect on its answers. - **Asked**: 67 choice rows asked as choice, 38 noul rows asked as choice. - 38 noul rows were asked as choice questions, which is a different question from the one the dataset states. Not comparable with an arm that asked them as noul. - Accuracy 0.7714 over 105 rows, against a chance null of 0.3416 (95th percentile 0.4190): better than chance at this sample size. diff --git a/src/plumbline/adapters/base.py b/src/plumbline/adapters/base.py index a5c21e9..a6e97da 100644 --- a/src/plumbline/adapters/base.py +++ b/src/plumbline/adapters/base.py @@ -76,6 +76,22 @@ class Adapter(ABC): the artifact so a blank cost column can be read rather than guessed at. """ + label_order_matters: bool = False + """Whether the order the options arrive in changes the question asked. + + True for an adapter whose prompt lists the options, since option order is a + known source of position bias. The cache then keys on the order as given; for + every other adapter it sorts them, so reordering the options reuses answers. + """ + + uses_label_descriptions: bool = False + """Whether this adapter sends option descriptions with the question. + + When True the runner passes a case's ``label_descriptions`` to ``classify`` + as ``descriptions``, and they join the cache key. When False they are not + sent, and the report says so for any run whose rows carried them. + """ + @property def call_params(self) -> Mapping[str, object]: """Call parameters that change the answer, for the Phase 4 cache key. diff --git a/src/plumbline/adapters/generative.py b/src/plumbline/adapters/generative.py index 6a12abc..ffc6e68 100644 --- a/src/plumbline/adapters/generative.py +++ b/src/plumbline/adapters/generative.py @@ -126,6 +126,7 @@ def __init__( raise ValueError(f"effort must be one of {EFFORTS!r}, got {effort!r}") self.name = "generative" + self.label_order_matters = True # the prompt lists the options in order self.model_requested = model_requested self.revision = None # Hosted models are not pinned by checkpoint. self.probability_semantics = "none" diff --git a/src/plumbline/adapters/local_logits.py b/src/plumbline/adapters/local_logits.py index 9cba931..d5351ee 100644 --- a/src/plumbline/adapters/local_logits.py +++ b/src/plumbline/adapters/local_logits.py @@ -170,6 +170,9 @@ def call_params(self) -> Mapping[str, object]: "option_prefix": self.option_prefix, } + #: The prompt lists the options in the order given. + label_order_matters = True + def classify( self, text: str, diff --git a/src/plumbline/adapters/typesafe_wire.py b/src/plumbline/adapters/typesafe_wire.py index 571c817..ec574c6 100644 --- a/src/plumbline/adapters/typesafe_wire.py +++ b/src/plumbline/adapters/typesafe_wire.py @@ -165,14 +165,22 @@ def call_params(self) -> Mapping[str, object]: "question_name": QUESTION_NAME, } + #: Option descriptions become the choice's criteria. + uses_label_descriptions = True + def classify( self, text: str, labels: list[str], *, question_type: QuestionType = "choice", + descriptions: Mapping[str, str] | None = None, ) -> Prediction: - """Ask one question of the kind the dataset says this row is.""" + """Ask one question of the kind the dataset says this row is. + + ``descriptions`` are the dataset's words for its options, sent as the + choice's criteria; an option without one is sent without one. + """ if not labels: raise ValueError("labels must not be empty") @@ -186,9 +194,10 @@ def classify( "answered as something else." ) + described = descriptions or {} question = Choice( instructions=self.instructions, - criteria=dict.fromkeys(labels), + criteria={label: described.get(label) for label in labels}, ) response, latency_ms = self._ask(text, question) diff --git a/src/plumbline/report/markdown.py b/src/plumbline/report/markdown.py index 6cc8ccf..c6b5be9 100644 --- a/src/plumbline/report/markdown.py +++ b/src/plumbline/report/markdown.py @@ -235,6 +235,12 @@ def _arm(result: RunResult, options: ReportOptions, heading: str) -> list[str]: failures = [record for record in scoreable if record.prediction is None] lines = ["", f"### {heading}", "", *_provenance(result, options)] + described = result.config.get("label_descriptions") or {} + if described.get("rows") and not described.get("sent"): + lines.append( + f"- **Option descriptions**: {described['rows']} rows carried option descriptions, " + "and this adapter does not send them, so they had no effect on its answers." + ) if excluded: lines.append( f"- **Excluded**: {excluded} rows of an unsupported question type were not scored." diff --git a/src/plumbline/runner/cache.py b/src/plumbline/runner/cache.py index 4e857bb..99ec9c1 100644 --- a/src/plumbline/runner/cache.py +++ b/src/plumbline/runner/cache.py @@ -27,7 +27,7 @@ import json import os import tempfile -from collections.abc import Sequence +from collections.abc import Mapping, Sequence from pathlib import Path from typing import Any @@ -42,6 +42,7 @@ def cache_key( text: str, labels: Sequence[str], question_type: QuestionType = "choice", + descriptions: Mapping[str, str] | None = None, ) -> str: """A stable fingerprint of everything that determines the answer. @@ -55,10 +56,18 @@ def cache_key( "model_requested": adapter.model_requested, "revision": adapter.revision, "text": text, - "labels": sorted(labels), + # Sorted, so reordering the options reuses answers, unless the adapter + # puts them in its prompt in the order given: then order is the question. + "labels": list(labels) + if getattr(adapter, "label_order_matters", False) + else sorted(labels), "question_type": question_type, "call_params": _canonical(dict(adapter.call_params)), } + if getattr(adapter, "uses_label_descriptions", False) and descriptions: + # Only where they are sent, and only when present, so every other key is + # exactly what it was. + payload["descriptions"] = dict(sorted(descriptions.items())) encoded = json.dumps(payload, sort_keys=True, separators=(",", ":"), default=str) return hashlib.blake2b(encoded.encode("utf-8"), digest_size=16).hexdigest() diff --git a/src/plumbline/runner/execute.py b/src/plumbline/runner/execute.py index 2cc63a2..e8fbde2 100644 --- a/src/plumbline/runner/execute.py +++ b/src/plumbline/runner/execute.py @@ -424,21 +424,27 @@ def _record_from_jsonable(row: dict[str, Any]) -> CaseRecord: def dataset_hash(cases: Sequence[Case]) -> str: """Fingerprint the dataset, so a result can be tied to the rows that produced it.""" encoded = json.dumps( - [ - { - "id": case.id, - "text": case.text, - "labels": sorted(case.labels), - "gold": case.gold_label, - } - for case in cases - ], - sort_keys=True, - separators=(",", ":"), + [_fingerprint(case) for case in cases], sort_keys=True, separators=(",", ":") ) return hashlib.blake2b(encoded.encode("utf-8"), digest_size=16).hexdigest() +def _fingerprint(case: Case) -> dict[str, Any]: + """What identifies one row. The question type and descriptions join it only + when a row carries them, so a dataset of plain choice rows keeps its hash.""" + row: dict[str, Any] = { + "id": case.id, + "text": case.text, + "labels": sorted(case.labels), + "gold": case.gold_label, + } + if case.question_type != "choice": + row["question_type"] = case.question_type + if case.label_descriptions: + row["descriptions"] = dict(sorted(case.label_descriptions.items())) + return row + + def redact(config: Mapping[str, Any]) -> dict[str, Any]: """Strip anything that looks like a credential before it reaches disk. @@ -577,6 +583,12 @@ def handle(index: int) -> None: # The server that answered, when it is not the vendor's default. It is # part of what was measured: a self-hosted endpoint is a different system. "endpoint": getattr(adapter, "base_url", None), + # Whether rows carried option descriptions and whether they were sent, + # so the report can say when the dataset's descriptions went nowhere. + "label_descriptions": { + "rows": sum(1 for case in cases if case.label_descriptions), + "sent": getattr(adapter, "uses_label_descriptions", False), + }, "pricing_table": {name: price.provenance(today) for name, price in table.items()}, **dict(extra_config or {}), } @@ -628,7 +640,11 @@ def _one_case( locks: _KeyLocks | None = None, ) -> CaseRecord: labels = list(case.labels) - key = cache_key(adapter, case.text, labels, case.question_type) if cache is not None else None + key = ( + cache_key(adapter, case.text, labels, case.question_type, case.label_descriptions) + if cache is not None + else None + ) if key is not None and locks is not None: with locks.for_key(key): return _answer(case, labels, key, adapter, cache, retry, table) @@ -655,6 +671,7 @@ def _answer( attempts=0, table=table, reports_tokens=adapter.reports_tokens, + model_requested=adapter.model_requested, ) last_error: Exception | None = None @@ -667,7 +684,7 @@ def _answer( retry.sleep(delay) attempts = attempt try: - prediction = adapter.classify(case.text, labels, question_type=case.question_type) + prediction = adapter.classify(case.text, labels, **_asked(adapter, case)) except CaseRefusedError as refusal: # A refusal is a decision, not a transient fault. Retrying it would # only produce the same refusal more slowly. @@ -701,6 +718,7 @@ def _answer( attempts=attempt, table=table, reports_tokens=adapter.reports_tokens, + model_requested=adapter.model_requested, ) return CaseRecord( @@ -718,6 +736,14 @@ def _answer( ) +def _asked(adapter: Adapter, case: Case) -> dict[str, Any]: + """The keyword arguments this adapter's classify takes for this case.""" + asked: dict[str, Any] = {"question_type": case.question_type} + if getattr(adapter, "uses_label_descriptions", False): + asked["descriptions"] = case.label_descriptions + return asked + + def _record( case: Case, prediction: Prediction, @@ -726,9 +752,14 @@ def _record( attempts: int, table: PricingTable, reports_tokens: bool, + model_requested: str, ) -> CaseRecord: cost, basis, key = _cost_of_record( - prediction, from_cache=from_cache, table=table, reports_tokens=reports_tokens + prediction, + from_cache=from_cache, + table=table, + reports_tokens=reports_tokens, + model_requested=model_requested, ) return CaseRecord( @@ -755,6 +786,7 @@ def _cost_of_record( from_cache: bool, table: PricingTable, reports_tokens: bool, + model_requested: str, ) -> tuple[float | None, CostBasis, str | None]: """The cost of one row, and the reason it is what it is. @@ -771,7 +803,12 @@ def _cost_of_record( # An adapter that was handed a cost by the vendor. Nothing to derive. return prediction.cost_usd, "priced", None - pricing, key = pricing_for(table, prediction.model_reported, "") + # Billing follows what answered. When the response names no model there is + # nothing else to go on, so the requested one prices it, and the key says so. + # A response that names a model the table lacks is not priced as the + # requested one: a newer version never inherits an older rate. + fallback = model_requested if prediction.model_reported is None else "" + pricing, key = pricing_for(table, prediction.model_reported, fallback) cost = cost_of(prediction.input_tokens, prediction.output_tokens, pricing) if cost is not None: return cost, "priced", key diff --git a/tests/test_report.py b/tests/test_report.py index 3ffe107..3e0d924 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -512,3 +512,17 @@ def test_a_backtick_in_a_model_name_cannot_break_out_of_its_code_span() -> None: line = next(line for line in render(odd).splitlines() if line.startswith("- **Model**")) assert "`` we`ird ``" in line + + +def test_the_report_says_when_option_descriptions_were_not_sent() -> None: + """An adapter that cannot use them must not leave the reader thinking it did (#39).""" + import dataclasses + + cases = [ + dataclasses.replace(case, label_descriptions={"billing": "charges and invoices"}) + for case in make_cases(40, labels=LABELS) + ] + text = render(execute.run(MockAdapter(gold_by_text(cases), seed=5), cases, workers=1)) + + assert "40 rows carried option descriptions" in text + assert "does not send them" in text diff --git a/tests/test_runner.py b/tests/test_runner.py index 9529e96..6d10125 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -811,3 +811,91 @@ def test_a_credential_inside_a_list_is_redacted_too() -> None: assert cleaned["endpoints"][0]["url"] == "https://a" assert cleaned["pair"][0]["token"] == "[redacted]" assert "sk-live-123" not in json.dumps(cleaned) + + +# Pricing a row the response did not name (#36), label order (#37), and what the +# dataset hash and the cache key cover (#39) + + +class UnnamedModelAdapter(Adapter): + """Answers with token counts but, like some APIs, without naming the model.""" + + def __init__(self, reported: str | None = None) -> None: + self.name = "unnamed" + self.model_requested = "mock-1" + self.revision = None + self.probability_semantics = "calibrated_claim" + self.reported = reported + + def classify(self, text: str, labels: list[str], **asked: object) -> Prediction: + return Prediction( + label=labels[0], + prob_selected=0.6, + distribution=None, + confidence=None, + latency_ms=5.0, + cost_usd=None, + input_tokens=1000, + output_tokens=10, + model_reported=self.reported, + ) + + +def test_a_row_whose_response_names_no_model_is_priced_by_the_requested_one() -> None: + result = execute.run( + UnnamedModelAdapter(), make_cases(3, labels=LABELS), pricing_table=PRICING, workers=1 + ) + + assert all(record.cost_basis == "priced" for record in result.records) + assert all(record.pricing_key == "mock-1" for record in result.records) + + +def test_a_named_model_missing_from_the_table_is_not_priced_as_the_requested_one() -> None: + """Billing follows what answered; a newer version never inherits an older rate.""" + result = execute.run( + UnnamedModelAdapter(reported="mock-2"), + make_cases(3, labels=LABELS), + pricing_table=PRICING, + workers=1, + ) + + assert all(record.cost_basis == "model_not_priced" for record in result.records) + + +def test_label_order_is_in_the_key_for_an_adapter_whose_prompt_lists_the_options() -> None: + adapter = an_adapter(make_cases(2, labels=LABELS)) + adapter.label_order_matters = True # type: ignore[misc] + + forward = cache_key(adapter, "t", ["approve", "escalate"]) + backward = cache_key(adapter, "t", ["escalate", "approve"]) + + assert forward != backward + + +def test_the_dataset_hash_covers_question_type_and_descriptions() -> None: + import dataclasses + + choice = make_cases(4, labels=("no", "yes")) + noul = [dataclasses.replace(case, question_type="noul") for case in choice] + described = [ + dataclasses.replace(case, label_descriptions={"yes": "it is", "no": "it is not"}) + for case in choice + ] + + assert len({execute.dataset_hash(rows) for rows in (choice, noul, described)}) == 3 + + +def test_a_plain_choice_dataset_keeps_the_hash_it_always_had() -> None: + """Only rows that carry a type or descriptions change their fingerprint.""" + import hashlib + + cases = make_cases(5, labels=LABELS) + old = json.dumps( + [ + {"id": c.id, "text": c.text, "labels": sorted(c.labels), "gold": c.gold_label} + for c in cases + ], + sort_keys=True, + separators=(",", ":"), + ) + assert execute.dataset_hash(cases) == hashlib.blake2b(old.encode(), digest_size=16).hexdigest() diff --git a/tests/test_typesafe_wire.py b/tests/test_typesafe_wire.py index d0e5cb7..787a942 100644 --- a/tests/test_typesafe_wire.py +++ b/tests/test_typesafe_wire.py @@ -422,3 +422,19 @@ def test_an_endpoint_set_in_the_environment_reaches_the_cache_key( assert self_hosted.base_url == "http://self-hosted.example" assert explicit.base_url == "http://other.example" # an argument beats the environment assert cache_key(hosted, "t", ["a", "b"]) != cache_key(self_hosted, "t", ["a", "b"]) + + +def test_option_descriptions_reach_the_request_as_criteria() -> None: + """They were parsed from the dataset and then sent as None for every option (#39).""" + adapter = an_adapter(a_response()) + client: FakeClient = adapter._client # type: ignore[assignment] + descriptions = {"billing": "charges and invoices", "sales": "buying something new"} + + adapter.classify("I was charged twice", LABELS, descriptions=descriptions) + + criteria = client.calls[0]["questions"][QUESTION_NAME].criteria + assert criteria == { + "billing": "charges and invoices", + "technical": None, + "sales": "buying something new", + }