refactor(models): give duplicated private primitives one owner or distinct names

[ethan-scitix]

Type

• refactor — code restructuring, no behavior change

Summary

Six private names were defined twice each inside core/models. I compared the normalised bodies rather than assuming, and they split cleanly into two halves that want opposite fixes:

Byte-identical → one owner

• _copy_json_value (20 lines ×2) and _validate_nonempty_string (3 ×2), shared by capabilities.py + requirements.py → core/models/_shared.py
• _validate_top_logprobs (7 ×2), shared by both OpenAI dialects → core/models/dialects/_shared.py
• _choice_index — the two copies differed only in whether the message said "chat" or "completion", so it joins the dialect shared module and takes that label as an argument

Genuinely different → names that say so, no behaviour change

• _usage_stats → _chat_usage_stats / _completions_usage_stats. The chat copy enforces total_tokens == prompt_tokens + completion_tokens; the completions copy does not. One spelling for two different contracts is exactly the failure mode a shared name invites.
• _json_value → _named_json_value in model.py. It threads a dotted path through its errors and accepts any non-str/bytes Iterable, where ir.py's copy is unlabelled and takes only list/tuple.

The shared functions are public names inside private modules, rather than underscored names imported across module lines — the import policy calls the latter a smell.

Related Issues

Refs #25 — follow-up from the PR #45 review (N3).

Test Plan

Automated

• Lint/format clean (ruff check && ruff format --check)
• Type check clean (ty check)
• Unit tests pass (pdm run pytest) — 5237 passed (unit + integration), preflight 24/24, no FAIL/WARN

Manual

• Mutation-tested both shared modules, since a move refactor passes trivially if nothing exercises the moved code. Five breaks, all caught:

  mutation	result
  copy_json_value accepts non-finite floats	2 failed
  copy_json_value accepts non-JSON values	2 failed
  validate_nonempty_string accepts empty	7 failed
  validate_top_logprobs accepts any shape	1 failed
  resolve_choice_index drops the range check	2 failed

• Verified no leftover references to any of the six original names anywhere under sieval/.

Question for review

Is the _usage_stats asymmetry deliberate? Chat validates that the reported total decomposes into prompt + completion; completions accepts whatever the server reports. Plausibly intentional — an echo-mode completions response may not decompose cleanly — but nothing records that, and the shared name actively hid it. This PR only makes the difference visible; it does not change either behaviour. If the omission is an oversight, the fix belongs in its own PR with a note about which servers it affects.

One implementation note: choice_index had to become resolve_choice_index, because openai_chat's streaming loop already binds a local variable named choice_index. Caught by lint on a self-assignment, not by the tests.

Checklist

Required (all PRs)

• PR title follows conventional format (type(scope): description)
• No internal paths, credentials, or personal info in committed files
• AI-generated code has AI-Generated Code - <model> (<provider>) in module docstring — both new modules carry it
• No new upper-layer dependencies added to core/ — both shared modules import only from core/
• Deleted code verified — no remaining call sites depend on it

[ethan-scitix]

Pushed 14713372 — the review follow-up, applying the revised architecture conclusion rather than the original tier list.

Two extractions were made sideways instead of upward. dialects/_shared.py sat peered with the dialects, which breaks their cohesion without buying enforcement — a new dialect can still forget to call the helper.

• validate_top_logprobs validates the IR shape tuple[tuple[TopKEntry, ...], ...], which no dialect may legitimately disagree about. dialect.py already hosts validate_reasoning, validate_tool_calls, validate_input_scoring and validate_structured_output, so it moved there with its siblings.
• _choice_index went back to one copy per dialect. The two copies were never identical, so unifying them reintroduced the difference as a kind argument — a label that used to be a literal and became a parameter nothing checked. Confirmed by mutation before reverting: passing "chat" from the completions dialect, and deleting {kind} entirely, each left the suite green.

dialects/_shared.py is empty once both leave, so it is gone.

Going the other way, reconcile's copy of the JSON coercion folded into _shared.py — it encodes a policy three modules must agree on, and it had already drifted ("non-finite floats" vs "a non-finite float"; "keys must be strings" vs "mapping keys must be strings").

reconcile._nonempty deliberately stays. This PR'''s own data is the argument: the 20-line policy had 3 copies and 1 drifted; the 3-line validator had 3 copies and 0 drifted. Drift tracks how much policy a helper encodes, not how often it repeats.

ruff / ty clean, 5173 unit tests pass. 40007c3f then trims the docstrings.

⚠️ The PR description above still describes the pre-revision design (six names, _choice_index taking a label, _shared in both places). Worth a refresh before merge — I have left it alone since it is yours.

Related: #100 stacks on this branch and still merges cleanly.