FCL-M1: stabilize camera core - #2
Merged
Merged
Conversation
Give the analyzer an explicit resume path: any real session transition from stopped to running (Settings dismissal, interruption recovery, or a fresh launch) now clears analysisInFlight, bumps the analysis generation, resets the tracker/stabilizer/guidance engine, and republishes nil measurement/guidance so the next frame produces a clean detection. switchCamera() now shares the same reset path instead of duplicating it. The stale-generation guard in the video delegate is extracted into CameraSession.shouldAcceptAnalysis so it's independently testable. Added DEBUG-only os.Logger diagnostics at the lifecycle/analysis checkpoints needed for a physical retest, and regression tests covering: guidance preference defaults/persists independent of other settings, stale vs current generation acceptance, and a resume clearing in-flight analysis, bumping generation, and clearing published guidance. Also records FCL-002 as physically completed and FCL-003 as implemented and pending physical verification in docs/project-status.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Address independent review findings against 76b11e1: - Wire the Settings sheet's real onDismiss into a new CameraSession.refreshLocalGuidanceAfterSettings(), instead of only logging. It works on the still-running session (no stop/restart, no scenePhase assumption), invalidates in-flight/stale analysis, resets the tracker/stabilizer/guidance engine, and clears stale measurement/guidance so the next frame republishes them. Camera settings (zoom, exposure, ratio, filter, flash, timer) and guidanceEnabled are untouched. - Fix stale-completion ordering: acceptAnalysisCompletion() now validates the request generation before clearing analysisInFlight, so a stale completion (from before a reset) can no longer clear the flag a newer, still-in-flight request owns. - Reconcile docs/project-status.md's Next Gate section, which still called FCL-002 inactive after FCL-002 had already been recorded complete; FCL-003 is now the one documented active gate, still not physically verified. - Tests: exercise refreshLocalGuidanceAfterSettings() itself (the same method Settings' onDismiss calls) for state invalidation and idempotency across repeated calls, and a new stale-vs-current completion ordering test proving a stale result can't clear a newer request's in-flight flag. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
FCL-004. Physical evidence on iPhone 17e showed the toolbar claiming "24" while the saved photo was ~12 MP, and selecting "48 MP" changed nothing observable — the UI was showing a hardcoded label disconnected from what AVFoundation could actually deliver, and CameraSession.resolution could be set directly from the toolbar without updating the persisted AppSettings.maximumResolution, so the two could drift apart independently. Single source of truth: AppSettings.requestedResolution (a CameraResolution, replacing the old maximumResolution Bool) is now the only persisted selection. CameraSession.resolution is private(set); the toolbar and Settings both write through settings.requestedResolution, and CameraView mirrors it into the camera via the one entry point, setRequestedResolution(_:). Truthful three-way split: ResolvedResolution captures what was requested, what the active device/format actually resolves to (PhotoDimensions), and whether/why that's a downgrade (unsupportedByActiveFormat vs outputLimited) — computed by the pure, testable resolve() rule whenever capabilities or the requested mode change. The toolbar label, the two menu choices, and a non-modal inline "lower than requested" indicator are all driven by this instead of a fixed "24"/"48". CaptureResolutionRecord adds a privacy-safe (dimensions and enum labels only) record of requested/resolved/ saved dimensions after every capture, decoded from the saved bytes via pixelDimensions(of:) without touching pixel content. Confirmed the live filter/aspect-ratio pipeline does not silently reduce output size beyond a deliberate ratio crop (FilterRenderer.renderedData only crops and re-encodes; it never scales), so it's not a downgrade source and was left untouched. No zoom/exposure change: setRequestedResolution and refreshLocalGuidanceAfterSettings never touch either. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Addresses four independent-review findings against 2b3af0c, without amending it or the two cherry-picked FCL-003 commits. 1. Compile blocker: FocelleTests/SmokeTests.swift still set the deleted AppSettings.maximumResolution; switched to requestedResolution. 2. Legacy migration: AppSettings now reads an existing requestedResolution key first, otherwise maps a legacy maximumResolution Bool (true -> .maximum, false -> .standard) via the pure, testable migratedResolution(newRawValue:legacyValue:), persists the mapped value immediately, and drops the legacy key once it's been carried across. A new-format value already present is never overwritten, and repeated instantiation is idempotent. 3. Requested tier survives capability changes: configureCapabilities() no longer forces `resolution` to .standard when the active camera lacks a distinct maximum tier — that line was mutating the requested tier from inside CameraSession itself, bypassing setRequestedResolution(_:) and contradicting AppSettings.requestedResolution as sole intent. Only resolvedResolution's dimensions/downgradeReason reflect the camera's current limits; switching back to a capable camera resolves to maximum again with no further Settings/toolbar action. 4. One immutable capture snapshot: capture() now takes a single CaptureResolutionSnapshot (CameraSession.currentCaptureSnapshot(for:)) on `queue` at the moment of capture, computed from queue-owned standardDimensions/maximumDimensions/outputLimitDimensions (now PhotoDimensions, replacing the old CMVideoDimensions/PhotoDimensions dual state) plus a capabilityGeneration bumped on every configureCapabilities() call. That one snapshot configures AVCapturePhotoSettings.maxPhotoDimensions and later becomes CaptureResolutionRecord — they can no longer be independently re-derived and disagree, and a stale generation can't leak into a later capture. Also corrects docs/project-status.md, which still described FCL-004 as unreviewed and its "changeable only through setRequestedResolution(_:)" claim as unconditionally true — neither was accurate until this batch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CI-only evidence for immutable commit b381c53. No merge requested.