Skip to content

P55: narrow dialog catches and decide catalog fields from their typed state - #730

Merged
trissim merged 1 commit into
mainfrom
fix/p55-narrow-catches
Oct 16, 2026
Merged

trissim merged 1 commit into
mainfrom
fix/p55-narrow-catches

Conversation

@trissim

@trissim trissim commented Oct 16, 2026

Copy link
Copy Markdown

Follow-up to #729. It fixes the debt ratchet against the installed Toad 6be909d (ForeignAbsenceProbe +5 in comms_command_dialog.py) and codex's broad-catch review. I merged #729 without running the ratchet; this corrects it.

  • Catalog fields: the dialog chooses the catalog editor from Core's typed CatalogTargetField. Each CatalogField says whether its value is the one its catalogs were prepared for (CatalogField.prepared). The five parameter.catalog is not None probes are gone, and catalog refresh configures each editor from the newly prepared fields by name.
  • Narrowed catches: NativePiUnavailable for catalog discovery. Core's RouteChanged for TargetContext.current(), which now raises it instead of a plain ValueError (still a ValueError subclass, so other callers are unchanged). ValueError only around edited(), which decodes the form's input (for example "Unknown ThinkingLevel name: ''"). OSError around spawning the login program. Programming errors now raise.
  • Real bug fixed: the dialog called run_login(comms=...), P128: every toad test file collects against current owners #1007's signature. On Core main that raised TypeError, so "Connect a provider" from the dialog was broken. It now calls run_login(), and CommandDialog no longer holds a Comms.
  • The missing-choice message is returned instead of raised and caught.

Ratchet --root src/toad/ --base 6be909dab --head 624f9dd15: exit 0, no positive deltas (/home/ts/.cache/agent-scratch/rts-results/p55-toad-ratchet-claude-session.json). Real UI on isolated Xvfb :254 with plain st: the dialog opened, the model changed to gpt-5.4-mini through the shared picker, the catalog refresh re-enabled Apply, and there were no crash files (/home/ts/wt/toad-forms-migration/.scratch/p68-live/p55b/model-changed.png). Goes with Core #1071.

🤖 Generated with Claude Code

… typed state

The dialog chooses the catalog editor from Core's CatalogTargetField and
asks each CatalogField whether its value is the one its catalogs were
prepared for (CatalogField.prepared); the five 'parameter.catalog is not
None' probes are gone, and catalog refresh configures each editor from
the newly prepared fields by name.

Each catch now names the failure it expects: NativePiUnavailable from
catalog discovery, RouteChanged from TargetContext.current() (which now
raises Core's RouteChanged), ValueError from decoding the form's input in
edited(), and OSError from spawning the login program. Provider login
calls run_login() as Core main declares it; CommandDialog no longer holds
a Comms. The missing-choice line is returned instead of raised and caught.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 16, 2026 22:15

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@trissim
trissim merged commit bd99e8d into main Oct 16, 2026
2 checks passed
@trissim
trissim deleted the fix/p55-narrow-catches branch October 17, 2026 20:51
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