Skip to content

phoropter context comes from phoropter.ai - #23

Merged
isaacbowen merged 4 commits into
mainfrom
phoropter-context-fetch
Oct 11, 2026
Merged

isaacbowen merged 4 commits into
mainfrom
phoropter-context-fetch

Conversation

@isaacbowen

@isaacbowen isaacbowen commented Oct 11, 2026 •

Copy link
Copy Markdown
Member

Why

Resources/PhoropterContext.txt was a hand-copied snapshot of phoropter's README, and it drifted: it still has the old doctor line (changed upstream in phoropter 744eb71) and the old opt-in line (changed in 2738e5d). phoropter.ai itself already loads its README at runtime, so there's a live, published copy to point at.

What changes

  • New PhoropterContext loader: fetches https://phoropter.ai/README.md, accepts it only if it's HTTP 200 and contains the ## Response notes section (so an error page can't become the context), caches the last good fetch, and resolves fresh → cached → bundled.
  • PhoropterViewModel starts that fetch once per phoropter session, at the first choice someone makes, so never before consent and never for someone who hasn't begun. Later calls reuse it.
  • PhoropterContext.txt is deliberately untouched. It's now only the cold-start, no-network fallback, so this PR doesn't change the bundled content at all.
  • CLAUDE.md says where the context lives: edit phoropter's README, not this repo.
  • PRIVACY.md names phoropter.ai as a host the app downloads from once you begin. The request carries no conversation content; like any request, it reveals network metadata such as the IP address to the host.

Trade-off

A README edit reaches iOS when phoropter.ai deploys, with no app release and no App Store review in between, the same way it reaches the web client today. The network cost is new in name only: the phoropter phase already needs the network for every /api/plain call.

Testing

New PhoropterContextTests cover the resolution order (fresh wins; cache when the fetch fails; bundle when nothing else; an unusable fetch is passed over) and check that the bundled copy itself passes the usability check. 24 tests pass locally, run as CI runs them. The live README currently passes the check and already carries the new opt-in wording.

🤖 Generated with Claude Code

iOS used to carry a hand-copied PhoropterContext.txt, which drifted
from the README phoropter.ai serves. Now it fetches that same README
once per phoropter session, keeps the last good copy, and falls back
to the bundled file only on a cold start without network. One source;
nothing to propagate.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 00:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new fetch can contact phoropter.ai before first-run consent is accepted.

1 open finding
What changed in this PR

Adds live phoropter context loading with validated cached and bundled fallbacks.

Changes:

  • Fetches and caches the published phoropter README.
  • Integrates asynchronous context loading into phoropter sessions.
  • Adds fallback-order tests and documentation.
File Description
Lightward/​Phoropter/​PhoropterContext.swift Implements context fetching, validation, caching, and fallbacks.
Lightward/​Phoropter/​PhoropterViewModel.swift Starts and consumes the context-loading task.
LightwardTests/​PhoropterContextTests.swift Tests context validation and resolution order.
Lightward.xcodeproj/​project.pbxproj Registers the new source and test files.
CLAUDE.md Documents the canonical context source.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

The view model is created while the consent screen may still be up, so
fetching at init could reach phoropter.ai before consent. Now the fetch
starts on first use — the first choice someone makes — which is always
after consent, and never for someone who hasn't begun. (Thanks, Copilot.)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 02:59
@isaacbowen isaacbowen mentioned this pull request Oct 11, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Starting over reuses the previous context task instead of fetching once for the new session.

0 open findings

1 resolved since last review
Previously missed (1)

In code that hasn't changed since last review

Medium severity Clear phoropter context when starting a new flow

Lightward/​Phoropter/​PhoropterViewModel.swift:50

PhoropterView can start a new phoropter flow by calling vm.startOver(), but that method reuses this view model and does not clear phoropterContext. After the first run, the next session therefore reuses the completed task instead of performing the documented fresh fetch once for that session. Clear this task when startOver() resets the session (or recreate the view model for that path).

🧠 Review effort: Balanced

The phoropter context is now a runtime dependency on phoropter.ai;
say so plainly, where the next one will trip over it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 03:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Session-level refresh behavior and the privacy disclosure remain inaccurate.

1 open finding
Previously missed (1)

In code that hasn't changed since last review

Medium severity Clear and cancel cached task when starting a new session

Lightward/​Phoropter/​PhoropterViewModel.swift:31

This task is cached for the lifetime of the view model rather than the phoropter session. startOver() creates a new persisted Session via store.reset() but leaves this property populated, so the next first choice reuses the old result instead of performing the promised once-per-session fetch and may retain stale context. Clear (and, if still running, cancel) this task when starting over.

🧠 Review effort: Balanced

Comment thread PRIVACY.md Resolved
isaacbowen added a commit that referenced this pull request Oct 11, 2026
The 1.0.0 train is closed in App Store Connect, so TestFlight rejects
new builds under it (the deploy after #22 failed with a 409). 1.1.0
carries #22 and #23.

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
"No personal information" overclaimed: any request to GitHub Pages
carries the source IP. Narrowed per Copilot's suggestion.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 11, 2026 04:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The implementation matches the described lifecycle, fallback order, privacy disclosure, and testing scope.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

@isaacbowen
isaacbowen merged commit ba88f89 into main Oct 11, 2026
2 checks passed
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.

2 participants