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", + }