Skip to content

fix(ui): identify a model row by the model, not by its array position - #255

Merged
argszero merged 1 commit into
mainfrom
fix/model-row-identity-is-not-a-position
Sep 15, 2026
Merged

argszero merged 1 commit into
mainfrom
fix/model-row-identity-is-not-a-position

Conversation

@argszero

@argszero argszero commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Summary

modelsToView() (ui/js/app.js) adapted /api/models rows for the UI and minted a per-row identity from the row's array index (id: i). That index was used as the model's identity in two places that outlive the render which produced it:

  1. it is persistedmarkRecentUsed(id) writes it to localStorage["atp-recent-models"] ("recently used" chips);
  2. it is resolved again in another arrayrenderRecent() / openChat() / consumeModel() look it up in Live.models ? modelsToView(Live.models) : D.MARKET, and for a guest session D.MARKET (ui/js/data.js) is a different table (7 rows, ids 1..7, different order and length) that merely looks like the same space; the backend also serves /api/models ORDER BY provider, model, so adding/removing/renaming any model shifts every index after it.

Because the two spaces overlap numerically, the failure is silent and looks like real data:

face reproduction what the screen shows
points at another model live, click "use" on xai/grok-4.6 (index 5) → log out → guest market chip reads google/gemini-3.1-pro — a model this user never used
the entry disappears same, but on the first live row anthropic/claude-opus-5 (index 0) the guest table is 1-based ⇒ nothing resolves ⇒ the chip vanishes silently
catalog drift use a model → an admin lists a model that sorts earlier → next session chip reads moonshot/kimi-k3 (index 5 after the shift)
the lie becomes an action click that chip after the drift the chat that opens is kimi-k3 (openChat resolves the same index)

This is a defect, not a trade-off: the same file already identifies models by name elsewhere (Live.models.find((x) => x.model === model), the shares form's <option value="m.model">), and the host has already ruled on the same family once (session caches must be keyed by identity, not by the slot they happen to sit in).

Related Issue

(no linked issue — this change was driven by a jsdom instrument plus an A/B; the evidence is below)

Changes

  • ui/js/app.js: new modelKey(m) = provider + "/" + model; modelsToView() no longer emits a position (the .map( callback takes one parameter, the row object has no id); the three identity carriers (data-mk-expand, data-use-model, data-recent-model) are rendered from esc(modelKey(m)); the click sites pass the identity string through unchanged (no more Number(...)); getRecentKeys() accepts identity strings only — a value stored by an older version is an index, which cannot be honestly resolved back to a model, so it is dropped once (deliberate).
  • src/state_gate.rs: new static gate the_model_row_identity_is_the_model_not_its_position, four rules each with its own tooth — ① the .map( callback takes one parameter and the row object declares no id field; ② the three data-* carriers are produced by modelKey( and no click site re-wraps them in Number(; ③ modelKey is defined exactly once, mentions provider and model, never id; ④ every markRecentUsed(...) call site writes a modelKey(...) expression — plus extractor self-checks (the_model_identity_extractors_have_teeth, with synthetic and negative controls).
  • ui/README.md: new convention section ("cross-render / cross-session / cross-array model identity goes through modelKey(); an array index is only meaningful inside the array that produced it"), including the discriminator note that renders_attr must require both attr= and "this line is not a selector query" (the first version of the gate read the consumer's querySelector('[data-use-model="' + id + '"]') as a render site).
  • ui/index.html: cache-bust app.js?v=20260915-10.

Measured (jsdom boots the real ui/index.html + all four real scripts; only fetch is stubbed and logged)

tmp/c2138_probe.js — 17 checks, expect declared per check. Fixtures: two /api/models payloads, both ordered provider, model (W1 six rows; W2 = W1 plus anthropic/gpt-9, so every index from 1 shifts by one).

Before (6be548a, ui/js/app.js md5 7171e11f…) — exactly the 5 axis legs red:

leg reading
B1 (guest market) chips=["gemini-3.1-pro"] store=[5] — the chip names a model that was never used
D1 (catalog drift) chips=["kimi-k3"] store=[5]
D2 (same, clicking it) chat title="Use kimi-k3" — the display error becomes an action error
C2 (mechanism) store=[5] — the persisted identity is a position
E1 (row #1) store=[0] chips=[] — the entry is lost against the 1-based guest table

After — 17/17, store=["xai/grok-4.6"], chat title Use grok-4.6.

A competing fix (hand-aligning D.MARKET's ids to today's catalog order, app.js untouched) satisfies the two guest faces (B1/E1 turn green) but is rejected by the drift and mechanism legs (D1/D2/C2 stay red) — 15/17 as declared.

Why this needs a static gate

The instrument can only prove "the screen no longer shows a foreign payload". It cannot distinguish "the identity is now the model" from "somebody hand-aligned the two tables once". The gate pins the shape, and its A/B (in-place mutation, byte-for-byte restore checked by md5) shows each rule failing on its own leg:

leg mutation gate red on
b0 the tree before the fix (6be548a) rule ① (index parameter)
m1a re-add the index parameter rule ① (two parameters)
m1b put id: i back in the row object rule ① (field id)
m2a render data-use-model from m.model rule ② (not from modelKey()
m2b consumeModel(Number(b.dataset.useModel)) rule ② (re-numbered)
m3a modelKey drops provider rule ③
m3b modelKey appends m.id rule ③ (id)
m4 store m.model instead of modelKey(m) rule ④

Tests

  • cargo test279 → 281 passed
  • cargo fmt --check clean
  • cargo clippy — no new warnings (only the pre-existing src/protocol.rs:662 false positive)
  • new unit tests: the gate (four rules) + extractor self-checks with synthetic inputs

Checklist

  • Branch naming follows the convention (fix/…)
  • Commit message uses Conventional Commits
  • Single purpose, minimal diff (4 files, +420/−25)

…osition

`modelsToView()` used to mint a per-row identity from the row's array index
(`id: i`). That index is used as the model's identity in places that outlive
the render that produced it: it is persisted (`markRecentUsed()` →
`localStorage["atp-recent-models"]`) and resolved again in a *different* array
(`D.MARKET` for a guest session) or in a *different catalog order*
(`/api/models` is `ORDER BY provider, model`).

Because the two spaces overlap numerically, the failure is silent:

  - live index 5 (`xai/grok-4.6`) → `D.MARKET` id 5 = `google/gemini-3.1-pro`
  - live index 0 (`anthropic/claude-opus-5`) → the guest table is 1-based, so
    the entry disappears instead of pointing at something
  - an admin adds a model that sorts earlier → every index shifts, the chip
    names `moonshot/kimi-k3`, and the chat opened from that chip is kimi-k3

The identity becomes `modelKey(m)` = `provider/model`, everywhere: the row
object no longer carries a position, the three `data-*` carriers
(`data-mk-expand` / `data-use-model` / `data-recent-model`) are rendered from
`modelKey(m)`, the click sites pass the identity string through unchanged, and
the recent-use store accepts identity strings only (a stored *index* cannot be
honestly resolved back to a model, so it is dropped once — deliberately).

Tests: `src/state_gate.rs::the_model_row_identity_is_the_model_not_its_position`
(4 rules, each with its own tooth) plus extractor self-checks; the instruments
`tmp/c2138_probe.js` (17 checks) and its A/B legs.
@argszero
argszero merged commit e64ae6c into main Sep 15, 2026
1 check passed
@argszero
argszero deleted the fix/model-row-identity-is-not-a-position branch September 15, 2026 00:58
@argszero argszero mentioned this pull request Sep 15, 2026
10 tasks
argszero added a commit that referenced this pull request Sep 15, 2026
Ships the 18 PRs merged since v0.7.24 (#242-#259). Schema 14 -> 15 (two
covering indexes, applied at startup). No config change, so no deployment-side
config.toml edit is needed.

Two themes:

Perf on the NFS dev database
- #259: stop mapping the db (PRAGMA mmap_size 64MB -> 0) and stop a real write
  per request (dao::touch_api_key gains a 60s guard). Measured on the live dev
  db: mmap=64MB 1.7-3.1s per COUNT / 250 MiB read vs mmap=0 ~10.5ms / 80 KiB;
  mmap=0 alone still leaves ~1.2s behind any write, so the pair is required.
- #242: codify the two emergency indexes in a v15 migration and gate the
  conditional joins at the plan level.
- #243: read the sharing page's earn total from one batched aggregate.

Frontend: display must equal what it filters on, and one fact, one source
- #250 one writer for the transaction cache; #251 clear every session slot at
  the identity boundary and give the wallet view a loader; #253 one shared
  writer for the wallet/dashboard month-changes; #254 boot loads only the
  destination view; #255 a model row's identity is the model, not its index;
  #256 the marketplace source follows the session, not whether data arrived;
  #257 the sidebar advertises only digits that work; #258 the admin
  total-balance card sums the gift amount its caption names.

i18n
- #249 every backend error reaches the wordlist, and the comment stripper stops
  mangling UTF-8; #252 the backend stops inventing Chinese display labels in
  response data fields.

Forms and robustness
- #244 a non-auth boot failure no longer looks like being logged out; #245 a
  credential 401 is no longer read as a session expiry; #246 wire timestamps
  reach the renderer unsliced; #247 inline cards submit from every field; #248
  a market row's availability label comes from that row.

- Cargo.toml / Cargo.lock: 0.7.24 -> 0.7.25.
- CHANGELOG.md: v0.7.25 entry.
- ui/index.html: cache-bust left as-is; the UI PRs in this release already
  advanced it past the value deployed with v0.7.24 (app.js 20260915-13,
  i18n.js 20260915-3).

cargo test 288 passed; cargo fmt --check clean; clippy unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant