Skip to content

fix(lens): restart on lens change so the solver drops its pattern cache - #625

Merged
brickbots merged 1 commit into
mainfrom
lens-menu-restart
Aug 17, 2026
Merged

fix(lens): restart on lens change so the solver drops its pattern cache#625
brickbots merged 1 commit into
mainfrom
lens-menu-restart

Conversation

@brickbots

Copy link
Copy Markdown
Owner

Changing Settings → Advanced → Lens could silently fail to take effect. Independent of #624 — that PR is the design record for the rev4 12mm work; this is a standalone bug fix found while investigating it, and it can ship on its own.

The bug

Everything that reads the lens live does follow it on the next frame — the solver re-resolves the optical train per frame, so the FOV gate, SQM's radiometric field width and the chart frustum all move together. Tetra3's pattern cache does not.

Tetra3._pattern_cache is keyed on the pattern hash alone, but the value it stores was already pruned against whichever FOV gate was in force when it was computed — the prune happens inside the cached function (tetra3.py:2271, within _get_all_patterns_for_index). It is built once at tetra3.py:407 and nothing invalidates it — there is no clear() anywhere in tetra3 or in PiFinder, only LRU eviction. PiFinder holds one long-lived Tetra3 across lens changes.

So a lens change leaves entries pruned for the old window in place, and those can withhold the very patterns the new window needs.

The fix

Restart on change, as the neighbouring PiFinder Type and Camera Type settings already do.

Two things I verified rather than assumed:

  • The restart cannot lose the setting. text_menu.py persists via set_option (lines 187/234) before invoking post_callback (line 251). That's the same contract screen_direction's restart already relies on.
  • The regression guard isn't vacuous. Reverting just the restart_pifinder(ui_module) line makes 2 of the 3 new tests fail.

The shared-state publish is kept ahead of the restart so the change still lands when the restart is a no-op, as it is under sys_utils_fake on a dev machine.

Not fixed here

The underlying cache bug is upstream in tetra3 and remains: the cached value also depends on the per-frame image_pattern_largest_edge, so the cache is approximate across frames even under a fixed gate. This fixes the user-reachable symptom, not that.

Docs

  • menu_map.rst had no Lens entry at allDerive field of view from the optical train (sensor × lens) #609 added the menu without documenting it. Added, with the restart note in the local house style.
  • release_notes/2.6.2.md claimed "No restart. …a lens change takes effect on the very next frame". That claim is what this PR falsifies, so it's corrected.

Testing

1241 passed across unit + smoke (the 1,238 baseline plus 3 new tests). Ruff check and format clean. No new user-visible strings — it reuses the existing _("Restarting...") — so no i18n work.

🤖 Generated with Claude Code

https://claude.ai/code/session_012sYi5W1u5HsZCTKeUYHPxa

Changing Settings > Advanced > Lens could silently fail to take effect.
Everything that reads the lens live does follow it on the next frame -- the
solver re-resolves the optical train per frame, so the FOV gate, SQM's
radiometric field width and the chart frustum all move together. Tetra3's
pattern cache does not.

Tetra3._pattern_cache is keyed on the pattern hash alone, but the value it
stores was already pruned against whichever FOV gate was in force when it was
computed (the prune happens inside the cached function). It is built once per
Tetra3 instance and nothing invalidates it -- there is no clear() anywhere in
tetra3 or in PiFinder, only LRU eviction. A lens change therefore leaves
entries pruned for the old window in place, and those can withhold the very
patterns the new window needs.

Restart on change, as the neighbouring PiFinder Type and Camera Type settings
already do. text_menu persists via set_option before any post_callback runs,
so the restart cannot lose the setting. The shared-state publish is kept ahead
of it so the change still lands when the restart is a no-op, as under
sys_utils_fake on a development machine.

The underlying cache bug is upstream in tetra3 and remains: the cached value
also depends on the per-frame image_pattern_largest_edge, so the cache is
approximate across frames even under a fixed gate. This fixes the
user-reachable symptom, not that.

Also documents the Lens setting in menu_map.rst, which #609 never added, and
corrects the 2.6.2 release notes, which claimed a lens change needs no
restart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012sYi5W1u5HsZCTKeUYHPxa
brickbots added a commit that referenced this pull request Aug 17, 2026
The plan told the implementer to file it separately. It is filed and fixed:
callbacks.set_camera_lens now restarts, as the neighbouring PiFinder Type and
Camera Type settings do. The upstream tetra3 cache bug remains out of scope.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012sYi5W1u5HsZCTKeUYHPxa
@brickbots
brickbots marked this pull request as ready for review August 17, 2026 21:12
brickbots added a commit that referenced this pull request Aug 17, 2026
The plan told the implementer to file it separately. It is filed and fixed:
callbacks.set_camera_lens now restarts, as the neighbouring PiFinder Type and
Camera Type settings do. The upstream tetra3 cache bug remains out of scope.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012sYi5W1u5HsZCTKeUYHPxa
@brickbots
brickbots merged commit a9d68df into main Aug 17, 2026
4 checks passed
brickbots added a commit that referenced this pull request Aug 18, 2026
The plan told the implementer to file it separately. It is filed and fixed:
callbacks.set_camera_lens now restarts, as the neighbouring PiFinder Type and
Camera Type settings do. The upstream tetra3 cache bug remains out of scope.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012sYi5W1u5HsZCTKeUYHPxa
brickbots added a commit that referenced this pull request Aug 18, 2026
…eal (ADR 0029) (#624)

* docs: FOV gate width follows lens confidence (ADR 0029)

Some rev4 units shipped with a 12mm lens and no camera_lens in config.
resolve_lens falls back to the imx462's default_lens_key of 16mm, so the
device derives 10.40 deg and gates [8.84, 11.96] against a true field of
13.51 deg -- every frame rejected before verification. Those units stopped
solving on update having changed nothing. The pre-0027 window was the fixed
12.0 +/- 4.0, which spanned both lenses; that is why 2.6.1 worked.

ADR 0027 treated the camera profile's fallback lens as knowledge. It isn't.
Record the decision to make gate width proportional to what is actually known
about the train, and to let the fitted FOV promote an assumption to a
statement.

Adds ADR 0029 and the vocabulary the change turns on: stated lens vs assumed
lens (camera CONTEXT.md), and the gate-width and self-heal roles in
positioning CONTEXT.md. Also corrects the camera glossary's claim that a
wrong lens is "a configuration error the device cannot discover for itself"
-- positioning CONTEXT.md already said the fitted FOV diagnoses exactly that,
and the two entries contradicted each other.

Docs and plan only; no behaviour change yet. Implementation checklist in
lens-gate-confidence-plan.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012sYi5W1u5HsZCTKeUYHPxa

* docs: record what the narrow FOV gate was actually buying

Measured against the real solver and real frames rather than assumed. On
speed the narrow gate buys nothing: +-15%, +-30%, +-50% and the pre-0027
fixed window all solve in 1.1-1.4 ms, and even omitting the hint entirely
costs only +0.3 ms, with Prob bit-identical across widths.

It does buy mis-solve rejection, which is why the assumed gate is a union
rather than no hint at all. With injected noise, wide and no-hint gates
returned confident false solves at 20-23 deg that match_threshold did not
reject; the gate's upper bound is what catches those, and [8.84, 15.53]
keeps it. Also records the asymmetry that settles the trade: a good frame
outside the gate burns the whole solve_timeout every frame, forever, which
is three orders of magnitude worse than a wide gate's 0.3 ms.

Adds an implementation note on tetra3's _pattern_cache, which is keyed on
hash_index alone but stores the FOV-pruned result and is never cleared.
Self-heal moves wide -> tight, the safe direction (cached entries are a
superset and the post-fit rejection still applies). A Lens menu change moves
tight -> different-tight and may not take effect until solver restart --
pre-existing in 2.6.2, not introduced here, but it will mislead anyone
testing this change by hand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012sYi5W1u5HsZCTKeUYHPxa

* docs: note the pattern-cache staleness is fixed in #625

The plan told the implementer to file it separately. It is filed and fixed:
callbacks.set_camera_lens now restarts, as the neighbouring PiFinder Type and
Camera Type settings do. The upstream tetra3 cache bug remains out of scope.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012sYi5W1u5HsZCTKeUYHPxa

* feat(optics): FOV gate width follows lens confidence, plus self-heal

Some rev4 units shipped with a 12mm lens and no camera_lens in config.
resolve_lens fell back to the imx462's 16mm, so 2.6.2 derived 10.40 deg
and gated [8.84, 11.96] against frames that are actually 13.51 deg wide.
Those units stopped solving on update having changed nothing.

The fallback was never a claim, so the gate should never have narrowed
around it. Gate width now follows how the lens was obtained:

* stated lens (config names a registered key) -> +/-15%, as 0027 had it
* assumed lens -> the union of every lens that sensor shipped with

CameraProfile gains shipped_lens_keys; default_lens_key keeps its value
but a narrower job -- it is now specifically the lens to *assume*.

LensSelfHeal (integrator) then closes the assumption: a successful solve
reports the fitted FOV, the sensor is known, so what is left is the lens.
Three consecutive identifications agreeing to within 5% promote the
assumption to a statement and the gate narrows onto it. It never
overwrites a stated lens, writes nothing when the measurement matches no
shipped lens, skips replayed solves, and can never take the integrator
down -- a failed promotion costs the promotion, not the pointing.

The hq only ever shipped one lens, so it has no union to take. That case
returns early rather than falling through the union arithmetic, which
would re-derive the same window and land a float ULP off the value that
has been shipping; its gate is asserted bit-identical.

Also: the solver's "Optical train:" line now says stated or assumed, and
api_extensions' frustum comment says "derived" -- the glossary reserves
"fitted" for what tetra3 measures.

Docs: release notes, troubleshooting.rst (#613) and BOM.rst's
imx462-specific "10deg FOV" line (#613).

Implements docs/adr/0029-fov-gate-width-follows-lens-confidence.md.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* docs: drop the implementation checklist from the branch

lens-gate-confidence-plan.md was a working checklist for this change and
the work it describes is now done. The decisions it summarised live in
ADR 0029, the vocabulary in the camera and positioning CONTEXT.md files,
and the code and tests are on this branch. Keeping it would land a
stale to-do list on main, which is also why the project's other planning
docs stay untracked.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(lens): read the Lens menu's value from shared state, not stale config

`get_camera_lens` resolved the sensor half from `shared_state` but the lens
half from `config_object`. The UI process loads its `Config` at boot and
reloads it only on an explicit `reload_config`, so once self-heal writes a
lens from the integrator, this process still reports the *assumed* lens.

The menu therefore showed 16mm on a rev4 that had just measured itself as a
12mm. That is not only a display fault: `text_menu` writes the highlighted
entry on select, so confirming the stale value states a lens the device
measured itself out of, tightens the FOV gate around it, and stops solving
for good -- self-heal cannot undo it, because it only ever writes into an
absence. `troubleshooting.rst` sends a user with no solves to exactly this
screen.

`shared_state` is seeded from config when it is built and republished by both
`set_camera_lens` and self-heal, and nothing else writes `camera_lens`, so it
is strictly the more current of the two views.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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