Wire the bystander blur that was never called, plus four reliability fixes (Plans CN + CO) - #302
Merged
Merged
Conversation
…ixes The privacy filter shipped inert. `PrivacyFilterService.processFrame` and `filteredPublisher` had no call sites anywhere in the app: the service was constructed, its `isEnabled` tracked the setting, it suspended and resumed on lifecycle, and no frame was ever passed to it. Settings promised protection "during streaming or recording", the README said "auto-blurs bystander faces", and neither was true of any code path. Found while tracing whether agent attachments (Plan CN) would carry pre- or post-filter pixels -- the answer was that nothing in the app carried post-filter pixels. `PrivacyFilterScope` decides which consumers are filtered, in one tested place, because two constraints conflict. Face recognition must see raw pixels: the blur is indiscriminate since the known-contact exemption was removed as dead code (BK P6), so filtering ahead of it would blur the very faces the user enrolled. And a Vision pass plus a Core Image composite is affordable on the model-facing paths -- already throttled to roughly 1 fps -- but not at recording frame rates without an off-main pipeline that does not exist. So egress to third-party models is covered; recording and broadcast are asserted *not* covered in a test, so whoever builds that pipeline is reminded to update the user-facing copy, which now states the limit instead of implying the opposite. A pin is filtered once at pin time, so every downstream use inherits it. `filteredPublisher` is deleted: no callers, and it sampled `isEnabled` once at construction so a mid-session toggle could never have reached it. Then four items from a fork-network delta sweep, each verified against our own code before adoption rather than taken on the report: 1. `FaceMatcher.bestMatch` returned the top scorer over a threshold with no notion of a runner-up, so two close faceprints -- siblings, a parent and child -- resolved to whichever scored a hair higher and were spoken with full confidence. `match(...margin:)` returns confident/ambiguous/none and only confident may produce a name. Enrolment refuses on a near-tie too: that path renames an existing record, so acting on one would silently rewrite the other person's name. An ambiguous sighting logs no encounter, since a wrong entry outlives the moment and corrupts later recall. 2. The Gemini REST tool turn carried the full system prompt and tool declarations with `maxOutputTokens: 1024` and no `thinkingConfig`. Thinking tokens draw from the same allowance, so an unbounded pass can consume it and leave nothing for the answer -- a 200 with a STOP finish and zero output tokens. The live path has capped this since it was written; the REST path never did. `GeminiBudgetPolicy` gives the tool turn 512 thinking against 2048, so the answer keeps the 1024 it had. Empty completions already cascade as `retryOtherModel`, which rescues the turn and hides the cause, so they now carry a distinct marker. 3. The in-flight-turn guard stopped answers landing on the wrong question but discarded the utterance behind a debug print -- no tone, no HUD. The user cannot tell "didn't hear you" from "heard you and threw it away", so they repeat into the same guard. `TurnAdmissionPolicy` holds one utterance, expiring, replayed when the turn ends; anything still refused is refused audibly. 4. The flat 2.0s silence window is a statement's pause, not a question's. When the assistant's own answer is a question the conversation ended while the user was still thinking -- and item 1 makes question-shaped answers more common by design. `SpeechContinuationPolicy` widens it, never below 2.0s. Plan CN ships alongside: a delegated agent run received only the on-device model's paraphrase of the scene, so labels, serial plates and forms degraded to a sentence with no route back to the camera. The harness seam widens behind a default-implementation shim, both adapters carry an image, and the whole thing sits behind a consent setting distinct from Agent Mode -- enabling agents authorised dispatching text, not sending frames from a head-mounted camera to the same endpoint. Custom endpoints attach nothing until an image field is named. Full suite 2834 tests, 0 failures. Release configuration builds clean. Device smoke owed: filter cost at 1 fps, the ambiguity margin against real enrolments, the thinking budget against a live key, and the gateway round-trip. Co-Authored-By: Claude Opus 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
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.
The headline: the privacy filter has never run
PrivacyFilterService.processFrameandfilteredPublisherhad no call sites anywhere in the app. The service was constructed, itsisEnabledtracked the setting, it suspended and resumed on lifecycle — and no frame was ever passed to it. Settings promised protection "during streaming or recording", the README said "auto-blurs bystander faces",CLAUDE.mdlisted it Tier 2 DONE. None of it was true of any code path.Found while tracing Plan CN's blocking question — would agent attachments carry pre- or post-filter pixels? The answer was that nothing in the app carried post-filter pixels.
Where it now applies
A new
PrivacyFilterScopeanswers "which consumers?" in one tested place, because two constraints pull against each other:FrameThrottlerhas already cut to ~1 fps. Not affordable at recording or broadcast rates without an off-main pipeline that doesn't exist.So v1 covers egress to third-party models — live sessions, Direct-mode stills, pinned frames, CN attachments. Recording and broadcast are not covered, asserted in a test so that whoever builds that pipeline is reminded to update the user-facing copy, which now states the limit rather than implying the opposite. Applied at the chokepoints — the same shape
FramePinuses, and for the same reason.A pin is filtered once at pin time, so sharp-inject, heartbeat resends, Direct-mode reuse, the on-screen card and CN attachments all inherit it without repeating the Vision pass.
filteredPublisheris deleted: no callers (the same disease), and it sampledisEnabledonce at construction so a mid-session toggle would never have reached it.Four items from a fork-network delta sweep
Each verified against our own code before adoption, not taken on the report. Two reproduced, one was real in a different shape than reported, one was structural exposure.
1 — A confident wrong name.
FaceMatcher.bestMatchreturned the top scorer over a threshold with no notion of a runner-up, so two close faceprints (siblings, a parent and child) resolved to whichever scored marginally higher and were spoken with full confidence — indistinguishable from a correct answer.match(…margin:)now returns.confident/.ambiguous/.none, and only.confidentmay produce a name.Two things beyond the plan, found while wiring: enrolment refuses on a near-tie too (that path renames an existing record, so acting on one would silently rewrite the other sibling's name — the destructive form of the same bug), and no encounter is logged on an ambiguous sighting, since a wrong entry outlives the moment and corrupts every later "when did I last see…" answer.
2 — Unbounded thinking on the tool turn.
LLMService:2317sent the full system prompt and the entire tool-declaration set withmaxOutputTokens: 1024and nothinkingConfig. Thinking tokens draw from the same allowance, so an unbounded pass can consume it and leave nothing for the reply — a 200 with aSTOPfinish and zero output tokens. The asymmetry was the tell:GeminiLiveService:380has capped the live path atthinkingBudget: 0since it was written; the REST path never got the same treatment.GeminiBudgetPolicygives the tool turn 512 thinking against 2048, so the answer keeps the 1024 it used to have. Plain turns untouched.Empty completions already cascade as
retryOtherModel— good behaviour, bad observability, since a systematic defect reads as ordinary model churn. They now carry a distinct log marker on the way past.3 — The silently swallowed utterance. The in-flight guard does prevent answers landing a turn behind, but it discards the user's speech behind a debug
print. No tone, no HUD — the wearer can't tell "didn't hear you" from "heard you and threw it away", so they repeat into the same guard.TurnAdmissionPolicyholds one utterance (expiring, replayed when the turn ends); anything still refused is refused audibly.4 — A question needs a longer pause. The flat 2.0 s silence window is a statement's pause. When the assistant's own answer is a question the conversation ended while the user was still thinking — and item 1 makes question-shaped answers more common by design.
SpeechContinuationPolicywidens it, never below 2.0 s. This is also why the ambiguity prompt is phrased as a real question: a statement-shaped one wouldn't trip the wider window, so the app would ask and then hang up on the answer.Plan CN — a delegated run can see
AgentHarness.startwas text-only, so what reached a remote agent was the on-device model's one-sentence paraphrase. Dense visual content is exactly where a paraphrase loses most — digits transpose, part numbers normalise into plausible neighbours — and the agent has no route back to the camera.The seam widens behind a default-implementation shim, so existing adapters and tests are untouched. Both adapters carry an image. It sits behind a consent setting distinct from Agent Mode: enabling agents authorised dispatching text tasks, not shipping frames from a head-mounted camera to the same endpoint. Custom endpoints attach nothing until the user names an image field.
Verification
Localizable.xcstringschurn (SWIFT_EMIT_LOC_STRINGS=NOthroughout)Owed, and honestly still open
margindefault against real enrolments — a synthetic fixture proves the policy, never the numberimage_base64param. This is the main reason that setting ships offPlans:
docs/plans/CN-agent-vision-attachment.md,docs/plans/CO-identity-budget-turn-taking.md🤖 Generated with Claude Code