Repository navigation
Conversation
|
debt-src at d83a57c, every finding: |
trissim
force-pushed
the
work/commander-layout--P927
branch
from
October 29, 2026 07:16
9c4f518 to
fbc676a
Compare
trissim
marked this pull request as ready for review
October 29, 2026 18:24
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.
trissim
force-pushed
the
work/commander-layout--P927
branch
from
October 29, 2026 21:45
ebab03c to
9a56828
Compare
|
debt-tools at d83a57c, every finding: |
trissim
force-pushed
the
work/commander-layout--P927
branch
from
October 29, 2026 23:04
e35149a to
4a5e8b2
Compare
trissim
force-pushed
the
work/commander-layout--P927
branch
from
October 30, 2026 01:18
4a5e8b2 to
85a31ac
Compare
…o work/commander-layout--P927
…ed tag inheritance
…--P927 # Conflicts: # src/agent_comms/field_codec.py
PendingCommandBinding carried a command field and a __post_init__ that refused a command not inheriting the context (IMPL-10): the context's own class already declares which command inherits it. The binding now holds only the context, and CommandContext.command() reads the one CliCommand family member inheriting it. new gets NewThreadContext so each context has exactly one declared command. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu
test_pending_new_binding_offers_missing_tags_without_constructing_the_command read tags.editor_default, which UnsuppliedTargetField does not declare (only SuppliedTargetField owns a default); it failed on ce33201 before this branch's binding change. The field's type is the fact; json_schema still omits the default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu
trissim
enabled auto-merge
October 30, 2026 20:35
contexts() spelled type[CliCommand[CliResult]] twice (channel_targets.py:180, :396); Answer already declares default=CliResult (cli_command.py:64), and the restated default was the only reason channel_targets imported CliResult under TYPE_CHECKING. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu
trissim
disabled auto-merge
October 30, 2026 20:42
CommandTarget.bindings and its three overrides (thread, channel, project) existed only to pick thread_bindings/channel_bindings/project_bindings by the resolved value, and forced channel_targets to import CommandBinding under TYPE_CHECKING. CliCommand.bindings is now a singledispatchmethod classmethod keyed on ResolvedThread, ResolvedChannel and Checkout; the four target methods and both TYPE_CHECKING names (CommandBinding, CatalogAccess) are gone. test_fork_cli_default_inherits_audience_but_not_commander_role called the deleted TagChoice.for_new on a binding's tags; it now reads the child ForkSpec.child declares. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu
test_fork_cli_default_inherits_audience_but_not_commander_role called the deleted TagChoice.for_new and its notification_bus parent has no forkable runtime. The declared-parent test in test_thread_creation now tags that parent commander and reads ForkSpec.child: the default fork drops the role, GivenTags(parent.tags) keeps it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu
trissim
enabled auto-merge
October 30, 2026 21:14
…--P927 # Conflicts: # src/agent_comms/channel_targets.py # src/agent_comms/cli_commands.py # src/agent_comms/cli_verbs/fork.py # src/agent_comms/cli_verbs/new.py # src/agent_comms/cli_verbs/thread_tags.py # src/agent_comms/target_fields.py # tests/test_commander_notifications.py # tests/test_editor_defaults.py
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.
P927 — explicit New membership, actual-parent Fork inheritance
10 commits;17 files;+824/−223 against the fetched main used for this source checkpoint. Core head
85a31ac9872b949efa02f48a48b29ced057d8ee5. Core delivery claim is recorded. Companion PRs remain ToAD#1322 and ObjectState#65 under the same P927.Source checkpoint, not application acceptance. No author tests, type checker, ratchet, product imports, application/provider execution or installation for this correction. Root owns the matched qualification.
1. Creation: two sibling contracts, no imaginary parent
Before: New inherited Fork's
InheritedTagsdefault;TagChoice.for_newsupplied an imaginary empty parent. Opening Fork could freeze its parent's current tags as GivenTags.After:
New explicitly chooses membership; deliberate empty/private membership remains valid. Fork resolves its original default against the actual parent's tags when applied. Locked registration, backend selection and session-copy behavior remain. Deleted
for_new, empty-parent inheritance, premature parent-tag snapshot and requiredname=''placeholders. No forced channel roster or live registry migration.2. Binding: native context, one completion and authorization owner
A required answer may be absent while the form opens. Pending New/Fork retain only acquired native context and cannot apply. CommandBinding uses actual FieldPlans and existing FieldCodec construction to complete; no context encode/decode loop, hand argument roster or complete-command adapter. CommandContext is declaration-only, so it cannot shadow complete-command edit authorization through the MRO.
Deleted copied completion bodies, the leaked context mutation method and identity tag normalizers. Target-bound parent and unoffered-field ValueErrors remain programming/authorization refusals, not invalid-data feedback.
3. Presence: the field subtype owns it
Missing, supplied None, explicit empty and unchosen remain distinct. Required unsupplied inputs stay omitted until deliberate input. FieldPlan still owns requiredness/defaults/codec. The base and unsupplied
editor_default -> Nonedeclarations are deleted; absence is no longer encoded twice._ExactScalarDecoder.accepts_valuecallsacceptsinstead of restating its type test.4. Native member editor: original identity, metadata outside values
UnionDecoder.editor_membersderives alternatives.MemberSelectionhas nominal scalar/record/null members plus UnchosenMember; the latter has its own label and no-input draft, not a fake null selection. A committed union's actual native value remains its sole member identity.EditorMember metadata retains decoder/FieldPlans outside ObjectState values. Scalar drafts derive their actual represented input; record drafts derive fields; null needs none.
editor_wireowns represented editor operations. Public JSON schema/defaults remain JSON-safe, distinct from native in-process editor state. Finite/catalog choices retain their prior contracts.Deleted EditsAsText,
_TextEditedUnionDecoder, first-member preference, editor wrapper forwarders and FieldPlan editor forwarding. No type-name/tag switch, JSON-prefix guessing, copied committed selector or decoder stored as a draft value.5. Data boundary and actual form consumer
DecoderPlan.parse_editortranslates malformed JSON into MalformedRecord with cause. Native text is not forcibly JSON-quoted. ToAD's dialog catches only MalformedRecord, allowing ordinary authorization/programming ValueErrors to escape.ToAD#1322 supplies one generic UnionEditor/member transaction through existing ObjectState/InlineFormView. It retains MemberSelection objects through Select; Apply constructs one valid native member and commits once; Cancel leaves the original untouched. Old forms/subscriptions retire on replacement. Generic forms retain TYPE_CHECKING-only Core selection imports. Ancestor/descendant notifications use DottedFieldPath.intersects_path.
Provider#65 fixes actual subtree admission and saved/history topology; Core itself does not require it to merge. Transient command drafts remain unregistered: no invented registry undo/redo for those dialogs. Real provider registry undo/redo controls remain.
Source evidence and deletion claims
Retained AST source maps cover Core production/tests/tools, ToAD, ObjectState, python-introspect and formbridge. Reused Package/ParsedModule/FunctionFacts; no copied scanner. Coverage and omissions are retained in
p927-*-source*/binding/codec/provider maps. Dynamic metadata/MRO/lazy resolution remain semantic reads, not AST behavioral proof.for_new/shared default → sibling New/Fork declarations.Authored controls — UNRUN
Root's narrowed existing selection:
Controls cover missing/chosen/explicit-empty membership, actual-parent Fork, absent required child name, pending completion and no execution during preparation; native/wire member roundtrips including overlapping scalar forms; malformed data versus authorization refusal. Companion PRs list provider topology/history and actual form controls separately.
Integration and remaining proof
Merge Core#3595 first, then corrected ObjectState#65 and matching ToAD#1322. Install Core with the matching ToAD: main ToAD alone cannot supply the newly required input. Provider source must match the actual ToAD pin/lock. Preserve all unrelated pins. Root owns the combined qualification and private New/Fork application journey, including a real channel message/reply and actual inheritance. Passing source publication or earlier live tag repair does not establish future creation.
Assumed: no tested/installed/working claim. P916 remains separately open for its installed physical acceptance; P930 remains commander-screen's work. Original sessions and UNKNOWN inputs remain untouched.
Send-back fix: a pending binding no longer restates its command (2072c0d, c194d56)
PendingCommandBindingheldcommand: type[CliCommand]besidecontext, plus a__post_init__that refused a command not inheriting the context (IMPL-10, cli_command.py:154-162). The context's class already declares which command inherits it.context;CommandContext.command()returns the oneCliCommandfamily member inheriting that context ((command,) = CliCommand.members_with(cls)), anddeclarationreads it. Deleted: the field, the runtime check, and theclsargument at both construction sites.newgetsNewThreadContext(ThreadLaunchSettings)so each context has exactly one declared command (fork already hadForkContext).tests/test_thread_creation.py:55assertedtags.editor_default is Noneon anUnsuppliedTargetField, which declares no default; it failed at ce33201 before this change. It now asserts the field's type.channel_targets.py:26importsCommandBindingunderTYPE_CHECKING. A runtime import would create a cycle, becauseCommandBindingneedsconfig_options, which importsthreadsandbus, which importchannel_targets. Removing it means moving thebindingsdispatch out ofCommandTarget, which this PR does not do.test_overlapping_scalar_wire_does_not_change_selected_native_member("DID NOT RAISE MalformedRecord"), also fails at ce33201.🤖 Generated with Claude Code
https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu