Repository navigation
Expose standard-by-default service tier without running-thread locks - #3635
Merged
Merged
Conversation
|
debt-src at f604e92, every finding: |
trissim
marked this pull request as ready for review
October 30, 2026 05:46
trissim
enabled auto-merge
October 30, 2026 05:46
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
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.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 30, 2026
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.
P980: standard billing by default; change the next request without restarting
21 files, +225/-111 against main 46b208a; pushed head ca409c8. Toad companion #1350 is merged and unchanged. This is source delivery, not installed/provider/UI acceptance.
Default and existing configuration family
Before: Thread defaulted to PRIORITY; tool/CLI refused running targets even though NextTurnConfiguration already existed. CatalogSelection advertised
None, Thread/RunningSelection exposed string service_tier_value twins, so a creation menu advertised an impossible absence.After: ServiceTier.default() declares STANDARD once. Thread.service_tier uses its default factory, CatalogSelection.service_tier returns it, and RunningSelection returns Thread's typed field. ServiceTierConfigOption reads .value only at the wire boundary and derives choices from the existing ServiceTier enum. SessionConfigOptions discovers the existing option family. No second enum, default registry or settings store.
Explicit priority and RegistrationPolicy(InheritPrevious) remain. The running refusal and its dead permits_reconfiguration/matches_thread/may_reconfigure predicates are deleted; require_inactive stays for lifecycle callers. Tool/CLI caller authority, registry, native-execution and captured-configuration checks remain. KeptModel means tier-only writes preserve model/provider/thinking.
Catalog publication: one fence for every producer
Before: index.ts kept mutable serviceTier set by startup and its own tool result. External CLI/menu changes left it stale. The first PR version fenced only the configure caller, while launch could republish an older Thread snapshot.
After: ThreadCatalogFile in threads.py declares the existing native-sessions/{lookup}/comms-catalog.json path. Its subclass ThreadCatalog in thread_catalog.py acquires RegistryStore.current_entry, checks original-project identity, and holds the registry guard through atomic replacement. It selects the current registered Thread, never the passed launch snapshot. Both configuration writes and launch use that publisher. Registry-dependent writing no longer lives on the Thread record, avoiding a reverse registry import cycle.
ThreadManagement.native_launch_arguments owns launch preparation. Both PiNativeBackend paths and CodingToolMode call it; the old Thread.native_launch_arguments and record-owned publisher are removed, not aliased. NativeArguments retains extension-loading policy. Unchanged catalog content does not replace the file.
The extension reads the same catalog before supported provider requests, caching decode by (mtimeNs,size). Its mutable tier and own-tool-result assignment are deleted. Priority adds service_tier to a fresh payload; standard leaves it untouched. No registry subprocess, extra RPC, independent setting or ACP-only refresh path.
Native tier-only calls
Omitted model previously became an empty string and failed provider/model validation. Model validation now runs only when params.model is supplied. No native model command is issued for a tier-only change. In-flight captured payloads are not mutated.
Native snapshot and pin correction
The previous head's native-scripts failure was this PR's defect: index.ts moved without its import manifest. The corrected manifest records index.ts SHA256 5153e0b671b0887ceb8cf8b15b23975d0ff4281672ebb9c46396e02394ee93e5, 7285 bytes.
Ran the explicitly ordered existing preparer once:
Retained log model-tier-correction-native-build.log ends RC=0. stack/pi-native.sha256 now records regenerated extension/import artifact hashes and tree/input commitments. This built the artifact; it did not install the live runtime. The subsequent main integration touched no native inputs.
Controls and actual evidence limits
Behavior controls remain UNRUN. No author pytest, full suite, local NRA gate, profile, provider/UI journey or live installation was run. Root owns the necessary combined selection and real acceptance.
Source parsing/imports were executed, despite the author-checker prohibition; this is disclosed rather than calling everything UNRUN. Nine source modules parsed. The first import command failed (NRA source-path package lacked audit); the corrected shared interpreter with PYTHONPATH=src imported thread_catalog, thread_management, channel_coding_tools and pi_native_backend from this checkout, RC0, and inspected the STANDARD default factory. These are source-load observations, not behavioral acceptance. The retained Python map covers 1511 modules with zero parse omissions; reflective/external dynamic consumers remain unresolved.
Retained controls cover STANDARD default and explicit PRIORITY; running/idle tier-only public tool updates with unchanged model/thinking/process/turn; captured versus new declaration; ACP new_session/set_config_option choices/readback; and stale launch snapshots after priority→standard changes. The native catalog control launches and publishes using the pre-change snapshot and checks the catalog still reflects current registry tier, with unchanged (mtime_ns,size) on repeated identical publication.
The old head's dead RelationViolationError import and new long configuration expression are corrected. The refactor-audit directory remains untracked material retained after main removed its submodule; it is neither committed nor deleted.
Hosted follow-through and missed stack caller
At fc79428, run41037958497 reported debt-src SUCCESS and debt-tools SUCCESS. Native artifact preparation also succeeded, establishing the corrected manifest/pin builds on the hosted runner. Native-scripts then FAILED at stack/test-native-global-extensions.py:117: the control still called the deleted Thread.native_launch_arguments. My earlier inventory omitted stack, so its consumer coverage was incomplete.
The next commit migrates that caller to comms.threads.native_launch_arguments without restoring an alias. A full textual search over src/tests/tools/stack finds no remaining old caller. NativeBuildInputs.paths inventories stack/native and declared repository inputs, not this stack/test-* control, so changing that caller does not change the native artifact input pin. No author test was executed.
Current ca409c8 checks are queued in run41038453598; prior-head passes do not count as current-head success. Core recorded delivery claim fc79428; updating it to the corrected head was refused with
Only a pending order can be claimed by its holder. Commander notified to refresh the claim and review; published source is not held for that bookkeeping.Acceptance still owed
On a saved-session fork: change priority from the real menu while a turn runs; verify the next supported before_provider_request payload has service_tier=priority. Change standard and verify no override. Check external tool/CLI changes, unchanged in-flight request, picker refresh/reopen/reattach, model/provider/thinking preservation and explicit priority retention. No author payload/UI acceptance exists.
Assumed: root owns combined qualification and installation; a supplied Thread belongs to the same registered original project; captured request configuration stays unchanged; existing explicit priority is intentional and is not rewritten by carry. No live-performance or completion claim.