Skip to content

The model keeps its own copy of what the unit is doing - #23

Merged
jonathanstokes merged 2 commits into
mainfrom
feat/om-m1.3-state-layer
Aug 15, 2026
Merged

The model keeps its own copy of what the unit is doing#23
jonathanstokes merged 2 commits into
mainfrom
feat/om-m1.3-state-layer

Conversation

@jonathanstokes

Copy link
Copy Markdown
Contributor

Closes #11

The problem this solves

You connect a script to your Quad Cortex. It reads a value. Then you reach over and
turn a knob on the touchscreen.

Until now the library had no idea. Whatever your script read stayed read, and
nothing told it otherwise. The only safe habit was to re-read everything before you
trusted it, and even that is a guess about how long ago the answer was true.

This adds the layer that fixes it. The unit says when things change, and the model
listens from the moment it connects. Where the unit says nothing - its firmware
version, for one - the model asks, once. So a value you read is what the unit is
doing, and no property ships with a "this might be out of date" note.

import pyquadcortex

with pyquadcortex.connect() as device:
    print(device.firmware)      # asks the unit
    print(device.firmware)      # free

How it works, in three sentences

Connecting is when the unit volunteers most of what it knows, in one burst about
nine seconds long. The model subscribes before the handshake starts, so it catches
all of that and usually has your answer before you ask. Anything the unit never
announces is read the first time you want it, and then remembered.

The rules that matter

A message the unit sends is treated as data, not as an alarm. The metronome
clock always runs, so the unit sends a message pair every single beat, forever, on
every connection. A cache that re-read whenever a message arrived would spend its
whole life re-reading. This one just applies what the message says. There is an
offline test that drives a minute of beats through it at 40 bpm and proves nothing
happens.

A message that mentions something the model does not keep makes it stop trusting
that part.
The next read goes to the unit. This is per field, not per message,
because applying the half of a message we understand and dropping the rest is the
one failure that leaves the model confidently wrong rather than obviously stale.
The check also fires on field numbers our bindings have never heard of, which is
not hypothetical here - the protobuf schema was recovered rather than published, so
a field the unit really sends and we cannot decode is ordinary.

Nothing is read from the thread that reads the USB device. That thread applies
what arrives and notes what needs re-reading; your thread does the reading. A read
from that thread could never be answered - it is the thread that would have to
deliver the answer - and would stall the whole connection for the length of its
timeout. The transport already refuses it outright, and there is a test that proves
the model gets refused rather than hanging.

A write updates the model straight away and is checked afterwards. The unit
echoes a change back, and a watcher compares the echo against what we sent. The bar
is one sentence: every field we sent must come back with the value we sent. Not
"the echo equals what we sent", because the unit legitimately changes things nobody
asked about - a meter, a mirrored parameter, dropdown values it recomputes on rows
you never touched. Three outcomes: confirmed, different, timed out. A timed-out
write marks that part of the model for re-reading, so a write the unit quietly
ignored corrects itself instead of poisoning the answer.

What the model remembers dies with the connection. A closed Device refuses to
answer rather than serving from what it remembers, because reporting a unit's state
through an object with no unit behind it is exactly the failure this layer exists to
prevent.

A protocol fact this turned up

While verifying on hardware, one write test failed in a way worth chasing. It turns
out PresetDirty - the unsaved-changes flag - announces a change of the flag,
not an edit. Measured as a controlled pair inside one connection: the same
parameter write produced a Grid echo and a PresetDirty when the preset was
clean, and a Grid echo and nothing else when it was already dirty.

The protocol notes said "also pushed unsolicited ... on edits", which was true of
the first edit and read as though it were true of all of them. docs/protocol.md
and the docstring now say what was measured, and the table shows both cases.

What is not covered yet

Testing

Offline: 1185 passed, 1 skipped (.venv/bin/python -m pytest).

Three new offline files. tests/test_state.py drives the cache through a loopback
link with the real protocol client above it, mirroring the two transport guarantees
the cache is built on. tests/test_state_rx.py runs it on a real transport and a
real read thread, because "never reads from that thread" is a claim about a thread
and no stand-in can test it. tests/test_watch.py covers the write watcher's rule
directly, since the cache's entries are too small to exercise the part that matters

  • what happens to fields we did not send.

Hardware (d14e, pytest tests/hardware --hardware): 18 passed, 3 skipped, and all
three skips are pre-existing operator-driven ones. What the new tests measured:

  • the connect burst warmed the unsaved-changes flag and reading it cost no round
    trip; the unit's identity was not in the burst, which is why the read path
    exists;
  • an edit made through the protocol layer - so, as far as the model is concerned,
    somebody else's edit - reached the model with no read issued;
  • six seconds of the tempo stream left every entry untouched and provoked no reads;
  • the write watcher confirmed in 612 ms on a clean preset, and on an already-dirty
    one it timed out, marked the entry, and the re-read recovered the unit's own
    answer.

Before opening this I mutated the new code 23 ways - dropping the mark, merging
where it should replace, clearing the mark unconditionally, confirming on the first
matching field, subscribing after the handshake instead of before, reading from the
RX thread - and confirmed the suite goes red for every one.

Story OM-M1.3 (#11): the write-through cache every model read now goes
through, so a value the library reports is what the unit is doing rather
than what it was doing when somebody last asked.

Three modules under pyquadcortex/device/:

- state.py holds the cache. One persistent listener (ADR-0009), registered
  before the connect handshake so it hears the handshake's burst, merges
  what the unit pushes into a per-entry copy. Pushes are applied AS DATA:
  an absent field means "not mentioned", never "reset to default". A
  message that sets a field the entry does not keep - a schema field, or a
  field number the recovered bindings have never heard of - marks that
  entry, and the next read of it goes to the unit on the CALLER's thread.
  A message type no entry tracks is ignored outright, which is what makes
  the metronome's tempo stream free.

- entries.py is what is tracked, as data rather than code. Two of design
  section 9's rows so far: the unit's identity and the unsaved-changes
  flag. The rest arrive with the surfaces that read them.

- watch.py is the write side. A write updates the cache immediately and
  the unit's echo confirms it in the background against one sentence:
  every field we sent must come back with the value we sent. One watchdog
  thread per connection, started on the first write and never before it.

Device.firmware and .serial now read through the cache instead of holding
their own reply, so they cannot outlive the connection. ADR-0011 records
the trust rule and why the alternatives were rejected.

Corrects a protocol fact found while verifying this on hardware:
PresetDirty announces a CHANGE of the flag, not an edit. Measured as a
controlled pair in one connection - the same parameter write produced a
Grid echo AND a PresetDirty on a clean preset, and a Grid echo and nothing
else on an already-dirty one. The docstring said "on edits", which was
true of the first edit and read as though it were true of all of them.

Verified on hardware (d14e): the burst warms the cache and nothing it
delivered is re-read; an edit made outside the model reaches it with no
read issued; six seconds of the tempo stream cost nothing; the write
watcher confirmed in 612 ms, and timed out and self-corrected when the
unit had nothing to announce.

Not covered yet: reconnect and device loss (#15), the Directory, presets,
the grid and parameters (#12 and after), the counters and event taxonomy
(#16).

Closes #11
Eleven findings, all reproduced against the code before acting on any of
them. Nine were real and are fixed; two were right about the mechanism and
wrong about what it means, and those became tests and prose instead.

Fixed:

- A write the unit CONTRADICTED now marks its entry for re-reading. Section
  10 asks only for a log line there, on the reasoning that a disagreement is
  our bug rather than a stale cache - true of the field the unit named, whose
  value the echo just put right, and not true of the other fields in the same
  write. Those went in on our say-so, the echo did not carry them, and the
  write they belonged to is one the unit just disagreed with. It was the one
  path meaning "we have a bug" that cleaned up after itself least.

- cached() and needs_read() refuse once the cache is closed. They answered {}
  and False, which are answers, not refusals - and needs_read flipped True to
  False across close(), reporting "the next read is free" about a read that
  raises.

- A write still in flight when the connection closes is released with no
  outcome, and settled() reports whether there IS one rather than whether the
  event fired. It used to hang forever on its documented default, because
  nothing published those watches and the watchdog deliberately will not call
  them timed out - which would be a claim about the unit rather than a fact.

- A watchdog that has stopped no longer starts a fresh thread for a write that
  raced the close; it releases the write instead. Same reasoning.

- The watchdog firing as the connection closes returns quietly rather than
  raising on a thread with nobody to catch it.

- read_echoes is gone. It was a knob nothing set and nothing tested off its
  default; the entry that needs it can add it with the test that proves a
  number other than one works.

- Two docstrings that were wrong or weaker than the reason behind them. The
  SCAFFOLDING skip claimed action says nothing about the unit, which is
  already false for Grid - action: DELETE removes a block where an UPDATE with
  the same payload does nothing - so a Grid entry (#12) cannot inherit the
  skip. And the unknown-field probe named NotImplementedError as its reason
  when the load-bearing one is recursion: DiscardUnknownFields descends into
  submessages and UnknownFieldSet does not, measured, which is what the
  preset-dump entries will need.

- A test whose name overclaimed. Its "extra" field was request_id, which the
  scaffolding strips before the watcher sees it, so it passed under the exact
  mutation it named. Renamed to what it pins, pointing at the file that pins
  the rule.

Not fixed, with the reasoning recorded instead:

- "The check cannot see a presence-free field an entry does not keep" is true,
  and it is a limit of the wire rather than a hole here: proto3 writes such a
  field only when it differs from its default, so a message leaving one at its
  default carries no bytes for it - PresetDirty{is_dirty: false} serialises to
  two bytes, both of them action. Nothing could see it. What IS checkable is
  whether we keep every such field, which is a question about our code, and a
  new structural test asserts it.

- The aliasing risk in fields_applied is real for a composite field and
  impossible for today's scalars. A structural test now fires the first time an
  entry keeps a submessage or a repeated field, which is when it has to be
  answered rather than assumed.

Verified: offline 1199 passed, 1 skipped. Hardware 18 passed, 3 skipped (all
three pre-existing operator-driven skips), plus the write watcher's CONFIRMED
branch re-run on its own at 616 ms. The mutation set grew from 23 to 30 -
one per finding fixed - and all 30 turn the suite red.
@jonathanstokes

Copy link
Copy Markdown
Contributor Author

Review pass: 11 findings, all checked against the code first

A read-only triage raised eleven points. Nothing was posted as inline threads, so
this is the record of each one, what I found when I went and looked, and what
changed. Fixed in 9516bef.

Nine were real and are fixed. Two were right about the mechanism and wrong about
what it means, and those became tests and prose rather than code changes.

Fixed

1. A write the unit contradicted left unconfirmed fields behind with nothing to
correct them.
Real, and I reproduced it: write two fields, echo carries one of
them and disagrees, and the other stays in the cache on our say-so with no mark.
Section 10 asks only for a log line here, on the reasoning that a disagreement is
a bug in our code rather than a stale cache. That is true of the field the unit
named - the echo just put its value right. It is not true of the others in the
same write. So the one path that means "we have a bug" was the one that cleaned up
after itself least. It now marks the entry.

2. The action field is skipped everywhere, and it is not always the
transport's.
The code is right for the two message types tracked today and the
docstring's general claim was wrong. Grid is the counter-example, and this repo
already records it: action: DELETE removes a block where an UPDATE with the
same payload does nothing. So a Grid entry cannot inherit the skip - two pushes
with identical payloads and opposite meanings would apply identically and mark
nothing. The docstring now says that, ADR-0011 carries it as a consequence, and
CLAUDE.md says not to widen the shared set to quiet a new entry. Left as a
decision for #12 rather than a knob added now with nothing setting it.

3. cached() and needs_read() answered through a closed cache. Real.
cached() returned {} and needs_read() returned False - both answers, not
refusals, and needs_read flipped from True to False across close(), which
reports "the next read is free" about a read that raises. Both refuse now.

4. A write outstanding at close() never settled, and settled() would hang.
Real. Nothing published those watches, and the watchdog deliberately will not call
them timed out - that would be a claim about the unit rather than a fact. So the
default settled() waited forever. They are now released with no outcome, and
settled() reports whether there IS one rather than whether the event fired.

5. A watchdog could start a fresh thread after stop(). Real - I reproduced
the interleaving. A write that races the close now gets released instead of a
thread that waits out a deadline on a connection nobody can reach.

6. The watchdog firing as the connection closes could raise on its own thread.
Now returns quietly. There is no copy left to mark and nobody to catch it.

7. read_echoes was a knob nothing set and nothing tested off its default.
Removed. The entry that needs it can add it together with the test that proves a
number other than one works.

8. The unknown-field probe gave the weaker of its two reasons. Checking this
one changed my mind about the docstring. UnknownFieldSet does work, and it is
cheaper - but it reports only the top level, while the subtraction descends into
submessages. Measured on protobuf 7.35.1: for an unknown field nested inside a
known submessage, subtraction says yes and UnknownFieldSet counts zero. Nested
is the case that will matter most, because the entries fed by whole preset dumps
are the ones with submessages in them. The docstring now leads with that, so the
"simplification" that would silently lose nested detection has a sign on it.

9. A test whose name overclaimed. Correct. Its "extra" field was request_id,
which the scaffolding strips before the watcher ever sees it, so it passed under
the exact mutation its name describes. Renamed to what it actually pins, pointing
at the file that pins the rule.

Not changed in code, with the reasoning recorded

10. "The check cannot see a presence-free field an entry does not keep." The
mechanism is right and I do not think the conclusion is. Proto3 writes such a
field only when it differs from its default, so a message leaving one at its
default carries no bytes for it at all - PresetDirty{is_dirty: false} serialises
to two bytes, both of them action. It is invisible to our check because it is
invisible on the wire; no implementation of this check could see it, and calling
it a hidden "harmless field category" reads as though a better check would catch
it. What IS checkable is whether we keep every such field, which is a question
about our code rather than about the wire. A new structural test asserts that no
feeding type has one, in the same style as its neighbours, and ADR-0011 records
the distinction.

11. fields_applied stores whatever getattr returns. Real for a composite
field and impossible for today's scalars, which are copied by value. Rather than
guess at a copy nothing needs yet, a structural test now fires the first time an
entry keeps a submessage or a repeated field - which is when it has to be answered
rather than assumed.

Verification

Offline: 1199 passed, 1 skipped.

Hardware (d14e): 18 passed, 3 skipped, all three pre-existing operator-driven
skips. The write watcher's confirmed branch was re-run on its own at 616 ms, and
the full run exercised the timed-out branch and its self-correction.

The mutation set grew from 23 to 30 - one per finding fixed, so each of these
comes with a guard that goes red if it is undone. All 30 turn the suite red.

@jonathanstokes
jonathanstokes marked this pull request as ready for review August 14, 2026 23:10
@jonathanstokes
jonathanstokes merged commit 42bc2f0 into main Aug 15, 2026
4 checks passed
@jonathanstokes
jonathanstokes deleted the feat/om-m1.3-state-layer branch August 15, 2026 04:05
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.

OM-M1.3: The model stays current with the unit without asking twice

1 participant