focus: the operator can be the motor - #39
Conversation
A camera focused by hand could read its zones and tune its filter, but not measure how sharply that filter peaks -- the sweep needed something to drive the lens. It never did. sweepZones works from the ORDER the readings arrived in and takes a median consensus; it has no idea a motor exists. A hand on the barrel is as good a sweep, only less evenly spaced, which is exactly what a median is robust to. So "Measure by hand" starts collecting, the operator turns the lens right through focus, and Done gives the same verdict the motorised sweep gives: the falls-to ratio, and the zones focusing at a different distance from the rest of the frame, ringed on the picture. That second one is the finding that matters in the field -- dirt on the dome drags autofocus onto the glass and is exactly as invisible on a hand-focused camera as a motorised one. Nothing is fetched twice. focusTick was already reading the grid for the live display and throwing all but the latest away; the sweep just keeps them, so the operator watches the numbers move while they turn. Offered even where there IS a motor, which is not the house rule but is what the measurements say. Measure it takes eight fixed steps -- a guess at how far this lens must travel to leave focus -- and on the 85H50AI those eight steps moved the reading 3% while the full travel moved it fourfold. The automatic sweep had nothing to measure. A hand covers the whole range. One verdict for both paths: sayVerdict() is shared, so a hand sweep and a motor sweep cannot word the same measurement differently, and the pinned counter refusal applies to both. Three guards, because a hand is less trustworthy than a motor: under eight readings is refused rather than averaged; a reading that barely changed is reported as a lens that did not move rather than as a filter that cannot see focus; and the collection is capped so a sweep someone walked away from does not grow for as long as the page is open.
PR Summary by QodoAdd operator-driven focus sweeps
AI Description
Diagram
High-Level Assessment
Files changed (3)
|
Code Review by Qodo
1. Hand sweeps ring the wrong zones
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| if (handFrames && handFrames.length < HAND_MAX) { | ||
| handFrames.push({ fv: sum.fv, state: sum.state, sat: sum.satZone, | ||
| rows: sum.rows, cols: sum.cols, peak: sum.peak, | ||
| pinned: sum.peakSaturated }); |
There was a problem hiding this comment.
1. Hand sweeps ring the wrong zones 🐞 Bug ≡ Correctness
focusTick stores only the order of manual readings, without the lens position represented by each frame. sweepZones measures separation as a fraction of frame indices, so pauses or speed changes distort physical focus distance and can either ring normal zones or miss genuinely distant ones.
Agent Prompt
## Issue description
Manual sweeps collect frame order but no lens position, while zone-distance detection treats frame-index separation as physical travel. Uneven hand speed therefore invalidates the claim that suspect zones focus at a different distance.
## Fix Focus Areas
- src/editor.js[4021-4024]
- src/editor.js[4423-4431]
- src/aftune.js[411-427]
- dist/editor.js[4021-4024]
## Recommended Fix
Do not run or report distance-based zone detection for hand sweeps unless the frames include a position or another validated travel coordinate. Either add position-aware sampling and make `sweepZones` compare that coordinate, or limit hand sweeps to the peak ratio and clearly omit zone rings.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Four from review, all real, and the first corrects something I wrote in the commit that introduced this. I said a median consensus is robust to uneven spacing. That is true of the CONSENSUS and false of the THRESHOLD, which was a fixed fraction of the reading count -- and an operator who slows down through focus piles most of the readings there, spreading the normal zones out in index until that fraction is either blind or trigger-happy. It now measures against how tightly the scene itself agrees: three times the median absolute deviation of the zones that responded. Every zone is sampled at the same instants, so their peak ORDER means something however fast the barrel turned, and the MAD is in the same distorted units as the thing being judged, so the distortion cancels. My first attempt kept the old fraction as a floor and the new test caught it immediately: on a 38-reading sweep that floor is 11 frames, which swallows a genuine 6-frame outlier. The more carefully someone measured, the blinder it got. The floor is two readings flat, there only to stop a scene that agrees to the frame from flagging its own noise. The rest: one shared sweepping() so the two sweeps cannot run at once -- the hand one lives off the live poll the motor one stops; handStop() wired into both leaving Focus and destroy, so an abandoned sweep does not keep a ticker and a growing array alive behind a Done button that is no longer in the document; and a grid-shape check that refuses the RATIO too, because peaks taken over different zone sizes are not comparable numbers and reporting one while dropping the zone findings hands back the half that is equally wrong. Two of the five guards came back vacuous on the first mutation pass. The mutual-exclusion one sits behind the disabled attribute, and a disabled button never fires its handler, so the test could not reach the code it was testing; it now re-enables the button first, which is also the real case, a panel rebuild clearing disabled. The teardown one asserted that readings stop, which stopFocusPoll already does on its own; it now holds the old Done button and checks it flipped back, because leaving Focus must END the sweep rather than merely detach its panel.
|
All four were real and are fixed in c8db186. The first corrects something I wrote in the commit that introduced this. 1 — Hand sweeps ring the wrong zones. Correct, and my justification was wrong in exactly the place it mattered. I claimed a median consensus is robust to uneven spacing: true of the consensus, false of the threshold, which was a fixed fraction of the reading count. An operator who slows down through focus piles most readings there, spreading the normal zones out in index until that fraction is either blind or trigger-happy. It now measures against how tightly the scene itself agrees — 3× the median absolute deviation of the zones that responded. Every zone is sampled at the same instants, so their peak order means something however fast the barrel turned, and the MAD is in the same distorted units as the thing being judged, so the distortion cancels. My first attempt kept the old fraction as a floor, and the new test caught it immediately: on a 38-reading sweep that floor is 11 frames, which swallows a genuine 6-frame outlier. The more carefully someone measured, the blinder it got. The floor is now two readings flat, there only to stop a scene that agrees to the frame from flagging its own noise. 2 — Two sweeps can control one lens at once. Correct. One shared 3 — Closed focus panels leak sweep timers. Correct. 4 — Grid changes still produce a verdict. Correct, and it applies to the ratio as much as the rings: peaks taken over different zone sizes are not comparable numbers, so reporting one while quietly dropping the zone findings would hand back the half that is equally wrong. A reshaped grid is now refused outright. On the testsTwo of the five guards came back vacuous on the first mutation pass:
All five now discriminate:
|
The review kept the ring finding open after the median-absolute-deviation fix, and it was right to. That fix makes the threshold survive UNEVEN spacing. It does nothing about spacing that is not monotonic. Everything sweepZones concludes about distance rests on one assumption: reading order is lens order. A motor guarantees it -- every step goes the same way. A hand does not. An operator who turns forward, back, and forward again visits the same position at three different indices, and two zones peaking at different indices may be at the same distance after all. Uneven speed stretches the mapping and a median absorbs that; turning back breaks the mapping, and no statistic recovers it. So the sweep is checked for going one way, from the scene's own curve rather than from a position the readings do not carry: swept once through focus the overall reading rises and falls once, and crossing the halfway mark upwards more than once means the lens came back. Half way up is high enough that noise around the trough does not register as a turn and low enough to catch a real second excursion. A wandering sweep keeps its ratio -- highest over lowest is the same whatever order they arrived in -- and loses only the claim it cannot support, with the reason said out loud rather than the rings quietly not appearing. Motor sweeps are unaffected: they are monotonic by construction.
|
You kept #1 open after the MAD fix, and you were right to. Fixed properly in e567198. The median-absolute-deviation change makes the threshold survive uneven spacing. It does nothing about spacing that is not monotonic, and that is the assumption the whole distance claim rests on: reading order is lens order. A motor guarantees it — every step goes the same way. A hand does not. An operator who turns forward, back, and forward again visits the same position at three different indices, and two zones peaking at different indices may be at the same distance after all. Uneven speed stretches the mapping and a median absorbs it; turning back breaks the mapping, and no statistic recovers it. So the sweep is now checked for going one way, from the scene's own curve rather than from a position the readings do not carry: /* swept once through focus, the overall reading rises and falls once.
* Crossing the halfway mark upwards more than once means the lens came back. */
const mid = lo + (hi - lo) / 2;
let ups = 0;
for (let i = 1; i < v.length; i++)
if (v[i - 1] < mid && v[i] >= mid) ups++;
return ups <= 1;Half way up is high enough that noise around the trough does not register as a turn, and low enough to catch a real second excursion. A wandering sweep keeps its ratio — highest over lowest is the same whatever order they arrived in — and loses only the claim it cannot support:
Said out loud, rather than the rings quietly not appearing. Motor sweeps are unaffected — monotonic by construction. On your alternative "limit hand sweeps to the peak ratio and clearly omit zone rings": that is exactly what this does, but only for the sweeps where the assumption actually fails, rather than for every hand sweep. A steady one-way turn supports the finding, and that is the common case worth keeping — dirt on the dome is the reason this feature exists and is just as invisible on a manual camera. TestsFive on the detector (one pass; there-and-back; a wobble that must still count as one sweep; too-short and flat both declining to answer) and three end to end (a wandering sweep keeps its ratio, drops the claim, rings nothing). All three mutations discriminate:
|
1 participant
A camera focused by hand could read its zones and tune its filter, but not measure how sharply that filter peaks — the sweep needed something to drive the lens.
It never did.
sweepZonesworks from the order the readings arrived in and takes a median consensus; it has no idea a motor exists. A hand on the barrel is as good a sweep, only less evenly spaced — which is exactly what a median is robust to.What it does
Measure by hand → "Turn the lens slowly from one end of its travel to the other, right through focus. Keep going — 23 readings so far." → Done → the same verdict the motorised sweep gives: the falls-to ratio, and the zones focusing at a different distance from the rest of the frame, ringed on the picture.
That second one is the finding that matters in the field. Dirt on the dome drags autofocus onto the glass, and it is exactly as invisible on a hand-focused camera as on a motorised one.
Nothing is fetched twice.
focusTickwas already reading the grid for the live display and throwing all but the latest away; the sweep just keeps them. So the operator watches the numbers move while they turn.Offered even where there is a motor
That is not the house rule, but it is what the measurements say.
Measure ittakes eight fixed steps — a guess at how far this lens must travel to leave focus — and on the 85H50AI those eight steps moved the reading 3%, while the full travel moved it fourfold. The automatic sweep had nothing to measure. A hand covers the whole range.One verdict for both
sayVerdict()is now shared, so a hand sweep and a motor sweep cannot word the same measurement differently, and the clamped-counter refusal from #34 applies to both.Three guards, because a hand is less trustworthy than a motor
The middle one is the same lesson as the clamped peak: say what you could not measure instead of returning a number that looks like a measurement.
Tests
Eleven new checks. Every guard mutation-tested:
The ring check was added after the first mutation pass came back vacuous — I had tested that the verdict mentions the odd zones but never that they are drawn on the picture.
node tools/smoke.mjsandnode tools/ui-check.mjsboth pass.