Repository navigation
test(review): derive decision-test probabilities from the question registry - #58
Merged
Merged
Conversation
…gistry PR #57 re-derived contracts/model/decision-questions.json (accept/escalate thresholds changed; ids, fields and option keys did not) without touching the Rust tests, so 13 tests in review_model::scanning that hard-coded probabilities tuned to the old thresholds failed on main. Production code reads every threshold from the embedded registry at run time and is correct for the new values; only the fixtures were stale. Test module of crates/openloops-desktop/src/review_scan.rs only: - new helpers registry_accept(id), registry_gray(id) (escalate..accept midpoint) and micros(p), following the pattern every_rule_gray_reject_and_error_falls_back already used, so the next re-tune does not break these tests; - FixedDecisionClient::new sends a gray closure.outcome choice (was 0.5, now below the new 0.8 escalate cut) so the noul answers still decide; - each of the 13 tests takes its accept/gray values from the registry; no assertion removed. compare_prediction_flags_the_registry_gray_band_for_nouls_and_choices now asserts the fixed 0.5 noul label at the gray value and adds a 0.1 false-branch case. Verified: cargo fmt --check; clippy -D warnings (desktop native-ui,ui-screenshot); desktop 424 passed, 0 failed, 3 ignored; no Cargo, tools, or contracts changes. Registry values for the owner to review (unchanged here): closure.outcome accept 1.0 / escalate 0.8 means a choice below 0.8 ends the pair with no noul fallback; deadline_kind accept 1.0 almost never auto-applies; three passing tests sit exactly on new cuts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YCesN8ZHnhqa6n3rWFThas
urnlahzer
added a commit
that referenced
this pull request
Oct 2, 2026
* test(review): derive decision-test probabilities from the question registry PR #57 re-derived contracts/model/decision-questions.json (accept/escalate thresholds changed; ids, fields and option keys did not) without touching the Rust tests, so 13 tests in review_model::scanning that hard-coded probabilities tuned to the old thresholds failed on main. Production code reads every threshold from the embedded registry at run time and is correct for the new values; only the fixtures were stale. Test module of crates/openloops-desktop/src/review_scan.rs only: - new helpers registry_accept(id), registry_gray(id) (escalate..accept midpoint) and micros(p), following the pattern every_rule_gray_reject_and_error_falls_back already used, so the next re-tune does not break these tests; - FixedDecisionClient::new sends a gray closure.outcome choice (was 0.5, now below the new 0.8 escalate cut) so the noul answers still decide; - each of the 13 tests takes its accept/gray values from the registry; no assertion removed. compare_prediction_flags_the_registry_gray_band_for_nouls_and_choices now asserts the fixed 0.5 noul label at the gray value and adds a 0.1 false-branch case. Verified: cargo fmt --check; clippy -D warnings (desktop native-ui,ui-screenshot); desktop 424 passed, 0 failed, 3 ignored; no Cargo, tools, or contracts changes. Registry values for the owner to review (unchanged here): closure.outcome accept 1.0 / escalate 0.8 means a choice below 0.8 ends the pair with no noul fallback; deadline_kind accept 1.0 almost never auto-applies; three passing tests sit exactly on new cuts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YCesN8ZHnhqa6n3rWFThas * feat(graph): mail-provider abstraction, shipped-registration hooks, per-provider sessions Design doc: docs/plans/2026-09-27-google-provider-and-shared-registration.md (approved 2026-09-27). This is the graph-crate half of PR-1. - live/provider.rs: MailProvider {Microsoft, Google}, AccountConfig enum dispatch, ProviderLoad and load_all (continues after one provider fails). - live/registration.rs: build-time injected shared registrations via option_env! (OPENLOOPS_MS_CLIENT_ID, OPENLOOPS_GOOGLE_CLIENT_ID, OPENLOOPS_GOOGLE_CLIENT_SECRET), BYO-over-shipped precedence, and the tenant admin-consent URL. Nothing is committed to the repository; a source build without the variables behaves as before. - live/google/mod.rs: GoogleConfig validation only; every Google call returns ProviderUnavailable until PR-2. - live.rs: SESSIONS has one slot per provider (clear_session_for, clear_all_sessions, has_session_for; the Microsoft-named wrappers keep their behaviour); OAuthEndpoints + authorize_with parametrise authority, token URI, redirect host, optional installed-app client secret, extra params and scope normalisation, with MICROSOFT reproducing the current flow exactly; any refresh_token in a token response is removed and zeroed before the OAuth library parses it; AADSTS65001/90094 -> AdminConsentRequired and AADSTS650052/650056 -> PublisherNotTrusted, content-free; ConnectionError text is provider-neutral. - live/callback.rs: RedirectHost {Localhost, LoopbackIp}; the Host check follows it. - live/review.rs: provider field on MailItem, SourceReview, UserIdentity (Microsoft default, account string unchanged so decision fingerprints are stable); fetch_from_origin_with_headers, add_hydrated and cutoff made crate-shareable. - live/reminders.rs: ReminderRequest.provider; valid_graph_id -> valid_remote_id. - live/test_support.rs: shared fake servers plus a path-routed server for PR-2. - desktop: struct literals gain provider: Microsoft; the reminder-status test's three expected strings follow the neutral ConnectionError wording (no behaviour change). Verified: cargo fmt --check; clippy -D warnings (graph live-connection, desktop native-ui,ui-screenshot); graph 161 passed; desktop 424 passed, 0 failed, 3 ignored (the branch sits on PR #58's scanning-test fix, 4d969d4, so the suite is green); Cargo.lock unchanged; the twelve tools/check-*.ps1 checkers match the origin/main baseline (the four that fail there fail identically here). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YCesN8ZHnhqa6n3rWFThas * feat(desktop): provider-aware review model, account-qualified scan scope, shipped-registration wiring Desktop half of PR-1 (design: docs/plans/2026-09-27-google-provider-and-shared-registration.md). - review_scan/review_model/app_model/slint_review: ConversationKey = (account, conversation) replaces every bare conversation-id set (ScanScope::Incremental, retry/check outcomes, append_sources, merge_scan, retryable_conversations, prior_open_items); closes the cross-account collision before a second provider exists. Exchange split-thread merging is Microsoft-only: a ThreadGroup with non-Microsoft messages is authoritative and never merged. ReviewMessage carries provider. - loop_state: Reminder::Created/Completed carry provider; wire bytes 0..3 unchanged (decode as Microsoft), 4/5 = Google; legacy-bytes test pins the old layout. - Links: is_trusted_message_link accepts the Outlook hosts and https://mail.google.com/; open-external accepts any provider's tasks URL; "Open message in {Outlook|Gmail}" labels are bound per card (url-label) instead of literal. - AccountDisplay::connected(&[MailProvider]) names one or both providers; the title bar binds account-text. - Outcome::Connection(provider, ..), Service::Mailbox; AppModel gains google: Status, effective_microsoft_client_id (BYO field > shipped registration > none), shared_registration_active, admin_consent_url, exposed to the Sources screen as shared-registration-active / admin-consent-url (layout lands in PR-1c). - settings: fields 13 google_client_id, 14 google_client_secret (zeroizing), 15 mail_providers tag (ms | google | ms,google; absent -> ms) on the append-only record; boundary, round-trip, unknown-tag and size tests. - graph nits from the PR-1a review: exchange_token_request takes one token URI; GoogleConfig exposes client_id()/client_secret() instead of a filler assertion. - Review fixes (Opus read-only review): the sign-in button gates on the effective client ID, not the BYO field; the Microsoft reminder sync/complete paths bind provider: Microsoft so a Google record can never reach Graph; apply_reconcile keeps the record's provider; conversation_quality / notes / rejection_reasons are keyed by ConversationKey; per-message key clones removed; shared borrowed_filter helper; AccountDisplay::connected(&[]) keeps the empty-name contract; TODO_URL derives from MailProvider::Microsoft.tasks_url(). Deferred to PR-2 (noted in the plan): Service::Mailbox(MailProvider) and the microsoft_status() rename. Microsoft decision fingerprints and relation keys are byte-identical to before. Verified: cargo fmt --check; clippy -D warnings (graph live-connection, desktop native-ui,ui-screenshot); graph 161 passed; desktop 436 passed, 0 failed, 3 ignored; Cargo.lock unchanged; public-repo gate on staged files. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YCesN8ZHnhqa6n3rWFThas --------- Co-authored-by: Claude Fable 5.1 <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.
Why
mainhas had 13 failing desktop tests since PR #57 merged. CI runs onlyscan-history, so nothing caught it. PR #57 re-derivedcontracts/model/decision-questions.json(new accept/escalate thresholds; same ids, fields and option keys) and changed no Rust. The 13 tests inreview_model::scanninghard-coded probabilities tuned to the old thresholds.Root cause
Production reads every threshold from the embedded registry at run time (
Registry::get(),RuleDecisions::noul/choice,classify_triage,accepted_update,pair_has_escalation,recombine_decisions,compare_prediction); no Rust constant mirrors a threshold. The loader and logic are correct for the new registry. Only the fixtures were stale. Example:FixedDecisionClient::newsent aclosure.outcomechoice at confidence 0.5, which was gray under the old 0.3 escalate cut and is now a hard reject under 0.8, dropping every closure pair.Change
Test module of
crates/openloops-desktop/src/review_scan.rsonly (+78/-30). Three helpers (registry_accept,registry_gray,micros) replace the literals, following the patternevery_rule_gray_reject_and_error_falls_backalready used. No assertion removed;compare_prediction_flags_the_registry_gray_band_for_nouls_and_choicesnow asserts the fixed 0.5 noul label at the gray value and adds a 0.1 false-branch case.Verification
cargo fmt --all -- --check: cleancargo clippy -p openloops-desktop --features native-ui,ui-screenshot --all-targets -- -D warnings: cleancargo test -p openloops-desktop --features native-ui -- --test-threads=1: 424 passed, 0 failed, 3 ignoredCargo.*,tools/, orcontracts/.For the owner (registry values, not changed here)
closure.outcomeaccept 1.0 / escalate 0.8:recombine_decisionslets the choice win only at confidence ≥ 1.0, and any choice below 0.8 now ends the pair with no noul fallback and no escalation. Check against the--compare-decisionsprobe before relying on decision-model closures.rules.deadline_kindaccept 1.0: the deadline-kind hint will almost never apply automatically.asks_recipient0.8 in two triage tests;deadline_kind0.69 vs escalate 0.65). Left unchanged; the next re-tune may break them.asks_recipient0.8..0.9 every gray answer counts as "true" in the comparison counts.🤖 Generated with Claude Code
https://claude.ai/code/session_01YCesN8ZHnhqa6n3rWFThas