From 5933cd95dbe2f21c1de6b108a0ba85be778f41d8 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 03:01:03 -0400 Subject: [PATCH 01/26] Separate explicit new-thread tags from parent-derived fork membership --- .../cli_support/thread_creation.py | 53 ++++++++++++++++ src/agent_comms/cli_verbs/fork.py | 14 ++--- src/agent_comms/cli_verbs/new.py | 62 ++----------------- src/agent_comms/thread_management.py | 37 ++++++----- tests/test_cli.py | 5 +- 5 files changed, 89 insertions(+), 82 deletions(-) create mode 100644 src/agent_comms/cli_support/thread_creation.py diff --git a/src/agent_comms/cli_support/thread_creation.py b/src/agent_comms/cli_support/thread_creation.py new file mode 100644 index 0000000000..6c34493c72 --- /dev/null +++ b/src/agent_comms/cli_support/thread_creation.py @@ -0,0 +1,53 @@ +"""The launch settings and field-derived construction shared by new and fork.""" +from __future__ import annotations + +import asyncio +from abc import abstractmethod +from dataclasses import dataclass, fields +from pathlib import Path +from typing import Annotated + +from agent_comms.cli_command import CliCommand +from agent_comms.cli_commands import NameArgument, ThreadCreatedResult, _tag_choice_text +from agent_comms.comms import Comms +from agent_comms.config_options import ConfigOptions, ModelConfigOption, ThinkingLevelConfigOption +from agent_comms.field_codec import Argument, Catalog, EditorFormat, FieldCodec, Multiline, Normalize +from agent_comms.native_settings import NativeThreadDefaults +from agent_comms.pi_vocabulary import ThinkingLevelField +from agent_comms.repo import Checkout, LaunchCheckout +from agent_comms.thread_management import ThreadCreationSpec +from agent_comms.thread_model import CatalogSelection, ThreadModel + + +type TagArgument[T] = Annotated[ + T, EditorFormat(FieldCodec.encode), Normalize(_tag_choice_text), Argument(flags=("--tags",)), +] + + +@dataclass(frozen=True, kw_only=True) +class ThreadCreationCliCommand(NameArgument, CatalogSelection, CliCommand, declared_name=None): + task: Annotated[str, Multiline(), Argument(flags=("--task",))] = "" + project: Annotated[Checkout, Argument(flags=("--project",))] + model: Annotated[ + ThreadModel, Catalog(ModelConfigOption), Argument(flags=("--model",)), + ] = NativeThreadDefaults.model + thinking_level: Annotated[ + ThinkingLevelField, Catalog(ThinkingLevelConfigOption), Argument(flags=("--thinking-level",)), + ] = NativeThreadDefaults.thinking_level + + @property + @abstractmethod + def spec_type(self) -> type[ThreadCreationSpec]: ... + + @property + def project_path(self) -> Path: + return self.project.path + + def apply(self, ctx: Comms) -> ThreadCreatedResult: + spec = self.spec_type.from_task(**{ + **{item.name: getattr(self, item.name) for item in fields(self.spec_type)}, + 'project': LaunchCheckout.checked(self.project.path), + }) + asyncio.run(ConfigOptions.configured().validate(self)) + child = ctx.threads.create(spec) + return ThreadCreatedResult(child.name, child.process_identity) diff --git a/src/agent_comms/cli_verbs/fork.py b/src/agent_comms/cli_verbs/fork.py index b36253a47a..e6b9c1637f 100644 --- a/src/agent_comms/cli_verbs/fork.py +++ b/src/agent_comms/cli_verbs/fork.py @@ -1,33 +1,29 @@ from __future__ import annotations -from agent_comms.cli_verbs.new import NewThreadCliCommand +from agent_comms.cli_support.thread_creation import TagArgument, ThreadCreationCliCommand from agent_comms.cli_support.thread_bound import ThreadBoundCliCommand -from agent_comms.comms import Comms from agent_comms.field_codec import Argument from agent_comms.field_codec import TargetBound from agent_comms.thread_execution import NativeOwnerCommand from agent_comms.thread_management import ForkSpec -from agent_comms.thread_management import GivenTags +from agent_comms.thread_management import InheritedTags, TagChoice from agent_comms.threads import Thread -from agent_comms.repo import Checkout from dataclasses import dataclass from typing import Annotated from typing import Self @dataclass(frozen=True, kw_only=True) -class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, NewThreadCliCommand): +class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ThreadCreationCliCommand): help = "Fork a child pi thread" spec_type = ForkSpec parent: Annotated[ str, TargetBound(), Argument(flags=("--parent",)) ] - @classmethod - def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[Self, ...]: - return () + tags: TagArgument[TagChoice] = InheritedTags() @classmethod def thread_binding(cls, thread: Thread) -> Self: - return cls(name='', parent=thread.name, tags=GivenTags(thread.tags), project=thread.worktree, + return cls(name='', parent=thread.name, project=thread.worktree, model=thread.model, thinking_level=thread.thinking_level) diff --git a/src/agent_comms/cli_verbs/new.py b/src/agent_comms/cli_verbs/new.py index d24a4b42a9..e7661e5b44 100644 --- a/src/agent_comms/cli_verbs/new.py +++ b/src/agent_comms/cli_verbs/new.py @@ -1,76 +1,24 @@ from __future__ import annotations -from agent_comms.cli_command import CliCommand -from agent_comms.cli_commands import NameArgument -from agent_comms.cli_commands import ThreadCreatedResult -from agent_comms.cli_commands import _tag_choice_text +from agent_comms.cli_support.thread_creation import TagArgument, ThreadCreationCliCommand from agent_comms.comms import Comms -from agent_comms.config_options import ConfigOptions -from agent_comms.config_options import ModelConfigOption -from agent_comms.config_options import ThinkingLevelConfigOption -from agent_comms.field_codec import Argument -from agent_comms.field_codec import Catalog -from agent_comms.field_codec import EditorFormat -from agent_comms.field_codec import FieldCodec -from agent_comms.field_codec import Multiline -from agent_comms.field_codec import Normalize from agent_comms.native_settings import NativeThreadDefaults -from agent_comms.repo import Checkout, LaunchCheckout -from agent_comms.pi_vocabulary import ThinkingLevelField -from agent_comms.thread_management import InheritedTags +from agent_comms.repo import Checkout +from agent_comms.thread_management import GivenTags from agent_comms.thread_management import NewThreadSpec -from agent_comms.thread_management import TagChoice -from agent_comms.thread_model import CatalogSelection, ThreadModel -from pathlib import Path from dataclasses import dataclass -from dataclasses import fields -from typing import Annotated from typing import ClassVar from typing import Self -import asyncio @dataclass(frozen=True, kw_only=True) -class NewThreadCliCommand(NameArgument, CatalogSelection, CliCommand, declared_name="new"): +class NewThreadCliCommand(ThreadCreationCliCommand, declared_name="new"): help = "Create a new pi thread" spec_type: ClassVar[type[NewThreadSpec]] = NewThreadSpec - task: Annotated[ - str, Multiline(), Argument(flags=("--task",)) - ] = "" - tags: Annotated[ - TagChoice, - EditorFormat(FieldCodec.encode), - Normalize(_tag_choice_text), - Argument(flags=("--tags",)), - ] = InheritedTags() - project: Annotated[ - Checkout, - Argument(flags=("--project",)), - ] - model: Annotated[ - ThreadModel, Catalog(ModelConfigOption), Argument(flags=("--model",)), - ] = NativeThreadDefaults.model - thinking_level: Annotated[ - ThinkingLevelField, - Catalog(ThinkingLevelConfigOption), - Argument(flags=("--thinking-level",)), - ] = NativeThreadDefaults.thinking_level + tags: TagArgument[GivenTags] @classmethod def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[Self, ...]: defaults = NativeThreadDefaults() return (cls(name='', project=project, model=defaults.model, thinking_level=defaults.thinking_level),) - - @property - def project_path(self) -> Path: - return self.project.path - - def apply(self, ctx: Comms) -> ThreadCreatedResult: - spec = self.spec_type.from_task(**{ - **{item.name: getattr(self, item.name) for item in fields(self.spec_type)}, - 'project': LaunchCheckout.checked(self.project.path), - }) - asyncio.run(ConfigOptions.configured().validate(self)) - child = ctx.threads.create(spec) - return ThreadCreatedResult(child.name, child.process_identity) diff --git a/src/agent_comms/thread_management.py b/src/agent_comms/thread_management.py index 2f79f5b457..cddd5e9f97 100644 --- a/src/agent_comms/thread_management.py +++ b/src/agent_comms/thread_management.py @@ -4,7 +4,7 @@ import asyncio import re -from abc import abstractmethod +from abc import ABC, abstractmethod from collections.abc import Sequence from contextlib import ExitStack from dataclasses import dataclass, field, replace @@ -72,9 +72,6 @@ class RenameThreadResult(ToolResult): class TagChoice(SubclassUnion, affix="Tags"): """Select explicit, inherited or changed tags from an existing set.""" - def for_new(self) -> frozenset[str]: - return self.for_child(frozenset()) - @abstractmethod def for_child(self, parent: frozenset[str]) -> frozenset[str]: ... @@ -144,27 +141,36 @@ def local_directory(self) -> Path: @dataclass(frozen=True, slots=True, kw_only=True) -class NewThreadSpec: - """Prepare the session, then register the thread under its registry lock.""" +class ThreadCreationSpec(ABC): + """Shared launch settings and construction after membership has been decided.""" name: str project: LaunchCheckout model: ThreadModel thinking_level: ThinkingLevelField task: LaunchInput = field(default_factory=LaunchInput) - tags: TagChoice = InheritedTags() @classmethod - def from_task(cls, *, task: str = "", **values) -> NewThreadSpec: + def from_task(cls, *, task: str = "", **values) -> Self: return cls(task=TaskLaunchInput(task) if task else LaunchInput(), **values) - def declaration(self) -> Thread: - return Thread(name=self.name, task=self.task.thread_text(), tags=self.tags.for_new(), + def declaration(self, tags: frozenset[str]) -> Thread: + return Thread(name=self.name, task=self.task.thread_text(), tags=tags, worktree=self.project, model=self.model, thinking_level=self.thinking_level) + @abstractmethod + def declare(self, threads: ThreadManagement, pi_bin: str, runtime: LaunchRuntime) -> Thread: ... + + +@dataclass(frozen=True, slots=True, kw_only=True) +class NewThreadSpec(ThreadCreationSpec): + """A new thread has no parent; its creator supplies its membership.""" + + tags: GivenTags + def declare(self, threads: ThreadManagement, pi_bin: str, runtime: LaunchRuntime) -> Thread: project = self.project.path - thread = self.declaration() + thread = self.declaration(self.tags.tags) threads.registry.require_declaration_available(thread) location = thread.runtime.new_location(threads.root, project) saved = RegistrySessionLocation.from_path(location.read_key, threads.root) @@ -172,10 +178,11 @@ def declare(self, threads: ThreadManagement, pi_bin: str, runtime: LaunchRuntime @dataclass(frozen=True, slots=True, kw_only=True) -class ForkSpec(NewThreadSpec): +class ForkSpec(ThreadCreationSpec): """A fork differs only in its parent-derived source and inherited settings.""" parent: str + tags: TagChoice = InheritedTags() def _parent(self, threads: ThreadManagement) -> Thread: parent = threads.registry.require(self.parent) @@ -185,8 +192,8 @@ def _parent(self, threads: ThreadManagement) -> Thread: def child(self, parent: Thread, runtime: RuntimeLocation, threads: ThreadManagement) -> Thread: saved = RegistrySessionLocation.from_path(runtime.read_key, threads.root) - return replace(self.declaration(), - parent=ForkedParent(parent.name), tags=self.tags.for_child(parent.tags), + return replace(self.declaration(self.tags.for_child(parent.tags)), + parent=ForkedParent(parent.name), backend=parent.backend, session_file=saved) def declare(self, threads: ThreadManagement, pi_bin: str, runtime: LaunchRuntime) -> Thread: @@ -541,7 +548,7 @@ def _delete_originals(self, originals: Sequence[Thread]) -> tuple[ThreadDeleted, for original in originals ) - def create(self, spec: NewThreadSpec) -> Thread: + def create(self, spec: ThreadCreationSpec) -> Thread: """Prepare the session, then register and launch the thread with its task as first input.""" resolved_bin = ConfiguredBinary().configured() runtime = LaunchRuntime.running() diff --git a/tests/test_cli.py b/tests/test_cli.py index 73d5586a73..e4fda55671 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -670,7 +670,10 @@ def test_result_kind_decides_exit_code_and_navigation_targets(): assert not launched.open_targets() and not batch.open_targets() created = ThreadCreatedResult("child", NoProcess()) assert created.open_targets() == ("child",) - command = NewThreadCliCommand(name="child", project=LaunchCheckout(Path("/project")), model=UndeclaredModel()) + from agent_comms.thread_management import GivenTags + + command = NewThreadCliCommand(name="child", tags=GivenTags(frozenset()), + project=LaunchCheckout(Path("/project")), model=UndeclaredModel()) from agent_comms.channel_targets import ProjectTarget completed = TargetCompleted(ProjectTarget(Checkout(Path("/project"))), command, created) From 984d8f45cc68d1dbf4d4f9a5866c0739998824c7 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 03:15:56 -0400 Subject: [PATCH 02/26] Collect required creation inputs through native command bindings --- src/agent_comms/channel_targets.py | 10 +- src/agent_comms/cli_command.py | 156 ++++++++++++++---- .../cli_support/thread_creation.py | 31 ++-- src/agent_comms/cli_verbs/fork.py | 4 +- src/agent_comms/cli_verbs/new.py | 15 +- src/agent_comms/target_fields.py | 8 +- tests/test_checkout_submodules.py | 2 +- tests/test_end_to_end.py | 16 +- tests/test_thread_creation.py | 84 ++++++++++ 9 files changed, 262 insertions(+), 64 deletions(-) create mode 100644 tests/test_thread_creation.py diff --git a/src/agent_comms/channel_targets.py b/src/agent_comms/channel_targets.py index fdea37d61a..0023b5bdc7 100644 --- a/src/agent_comms/channel_targets.py +++ b/src/agent_comms/channel_targets.py @@ -23,7 +23,7 @@ from .audience_manifest import AudiencedMessage, FrozenRecipient from .catalog_document import CatalogAccess, CatalogDocument from .channels import Channel - from .cli_command import CliCommand + from .cli_command import CliCommand, CommandBinding from .cli_result import CliResult from .comms import Comms from .goal_management import Goals @@ -166,7 +166,7 @@ def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], C def bindings[C: CliCommand]( self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[C, ...]: ... + ) -> tuple[CommandBinding, ...]: ... def history_view(self, viewer: DisplayViewer, require_channel: Callable[[str], Channel], channel: type[ChannelHistory], direct: type[DMHistory]) -> HistoryView: @@ -413,7 +413,7 @@ def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], C def bindings[C: CliCommand]( self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved: ResolvedThread, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[C, ...]: + ) -> tuple[CommandBinding, ...]: return declaration.thread_bindings(comms, resolved.thread, resolved.status, channel, catalog=catalog) def validate(self, message: Message) -> None: @@ -562,7 +562,7 @@ def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], C def bindings[C: CliCommand]( self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[C, ...]: + ) -> tuple[CommandBinding, ...]: return declaration.channel_bindings(comms, resolved, project=project, snapshot=snapshot, catalog=catalog) def validate(self, sender: Message | None = None): @@ -653,7 +653,7 @@ def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], C def bindings[C: CliCommand]( self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[C, ...]: + ) -> tuple[CommandBinding, ...]: return declaration.project_bindings(comms, resolved) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index 55778a9a38..f5a6b3bb14 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -16,6 +16,7 @@ from agent_comms.lifecycle_capability import LifecycleCapability, Successful from agent_comms.registry_document import RegistrySnapshot from agent_comms.target_fields import TargetField +from agent_comms.thread_model import CatalogSelection from dataclasses import dataclass, field, replace from pydantic import JsonValue from collections.abc import Mapping @@ -63,15 +64,128 @@ def __init_subclass__(cls) -> None: Answer = TypeVar("Answer", bound=CliResult, covariant=True, default=CliResult) +class CommandBinding(ABC): + """A declared operation can collect fields before a complete command exists.""" + + @property + @abstractmethod + def declaration(self) -> type[CliCommand]: ... + + @property + @abstractmethod + def editable_fields(self) -> tuple[TargetField, ...]: ... + + def for_editor(self, comms: Comms, target: str, project: Checkout) -> Self: + return self + + @property + def catalog_context(self) -> object: + return self + + def for_fields(self, arguments: Mapping[str, object]) -> CommandBinding: + return self.edited(self.declaration.editor_arguments(arguments)) if arguments else self + + async def prepare_fields(self, arguments: Mapping[str, object] = {}) -> tuple[TargetField, ...]: + binding = self.for_fields(arguments) + catalogs = ConfigOptions.configured() + return tuple([await item.prepare(catalogs, binding.catalog_context) + for item in binding.editable_fields]) + + def authorize_edits(self, arguments: Mapping[str, object]) -> frozenset[str]: + bound = frozenset(entry.key for entry in FieldCodec.argument_fields(self.declaration) + if entry.setting(TargetBound, False)) + if bound & arguments.keys(): + raise ValueError("Target-bound parameters cannot be overridden") + if arguments.keys() - {item.name for item in self.editable_fields}: + raise ValueError("Parameters not offered by this target command cannot be overridden") + return bound + + @abstractmethod + def edited(self, arguments: Mapping[str, object]) -> CliCommand: ... + + @abstractmethod + def confirmation(self) -> str: ... + + @classmethod + @abstractmethod + def selection_confirmation(cls, commands: tuple[Self, ...]) -> str: ... + + @abstractmethod + def with_confirmation(self, confirmed: bool) -> CommandBinding: ... + + +@dataclass(frozen=True) +class CommandContext(CatalogSelection): + """A partial command's supplied fields, with native edits decoded by their plans.""" + + def edited(self, arguments: Mapping[str, object]) -> Self: + changes = {name: value + for entry in FieldCodec.argument_fields(type(self)) if entry.key in arguments + for name, value in entry.decoded({ + entry.key: entry.editor_payload(arguments[entry.key]), + }).items()} + return replace(self, **changes) if changes else self + + +@dataclass(frozen=True) +class PendingCommandBinding(CommandBinding): + """Only supplied context, borrowing the concrete command's declared fields.""" + + command: type[CliCommand] + context: CommandContext + + @property + def declaration(self) -> type[CliCommand]: + return self.command + + @property + def catalog_context(self) -> CatalogSelection: + return self.context + + @property + def editable_fields(self) -> tuple[TargetField, ...]: + supplied = {entry.name: entry.current(self.context) + for entry in FieldCodec.argument_fields(type(self.context))} + return tuple(TargetField.of(entry, supplied[entry.name] if entry.name in supplied else entry.default()) + for entry in FieldCodec.argument_fields(self.declaration) + if not entry.setting(TargetBound, False)) + + def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: + return replace(self, context=self.context.edited(arguments)) + + def edited(self, arguments: Mapping[str, object]) -> CliCommand: + self.authorize_edits(arguments) + return self.declaration.from_payload({ + **FieldCodec.encode(self.context), + self.declaration.family_discriminator: FieldCodec.encode(self.declaration), + **arguments, + }) + + def confirmation(self) -> str: + return '' + + @classmethod + def selection_confirmation(cls, commands: tuple[Self, ...]) -> str: + return '' + + def with_confirmation(self, confirmed: bool) -> CliCommand: + return self.edited({}).with_confirmation(confirmed) + + @dataclass(frozen=True, kw_only=True) class CliCommand( ModuleMembers, DeclaredCommand, CliExecution[Answer], + CommandBinding, Generic[Answer], affix="CliCommand", discovery_package="agent_comms.cli_verbs", ): + @property + def declaration(self) -> type[Self]: + return type(self) + def schedule(self, batch: TargetBatchSchedule, comms: Comms) -> None: result = TargetCompleted(batch.target, self, self.apply(comms)) batch.outcomes.append((batch.position, result)) @@ -125,21 +239,21 @@ def selection_channels(cls, channel: ChannelSelection, return channel.channels_for(target) @classmethod - def thread_bindings(cls, comms: Comms, thread: Thread, status: ThreadStatus, channel: SurroundingChannel, *, catalog: CatalogAccess) -> tuple[Self, ...]: + def thread_bindings(cls, comms: Comms, thread: Thread, status: ThreadStatus, channel: SurroundingChannel, *, catalog: CatalogAccess) -> tuple[CommandBinding, ...]: return () @classmethod - def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[Self, ...]: + def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[CommandBinding, ...]: return () @classmethod - def channel_bindings(cls, comms: Comms, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess) -> tuple[Self, ...]: + def channel_bindings(cls, comms: Comms, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess) -> tuple[CommandBinding, ...]: return () @classmethod def bindings(cls, comms: Comms, target: CommandTarget, channel: SurroundingChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - resolved: ResolvedThread | ResolvedChannel | Checkout) -> tuple[Self, ...]: + resolved: ResolvedThread | ResolvedChannel | Checkout) -> tuple[CommandBinding, ...]: return target.bindings( cls, comms, channel, resolved, project=project, snapshot=snapshot, catalog=catalog) @@ -160,7 +274,7 @@ def target_catalog(cls, comms: Comms, target: TargetSelection, catalog = (comms.channels.catalog.read() if channel.needs_catalog or any(item.needs_channel_state for item in selected) else NoCatalog()) resolved_targets = tuple((name, name.resolve(snapshot, comms.bus.require_channel)) for name in available_targets) - grouped: dict[type[CliCommand], dict[CliCommand, None]] = {} + grouped: dict[type[CliCommand], dict[CommandBinding, None]] = {} for member in target.command_members(cls): selected_channels = channel if member.channel_fields() else NoChannel() for name, resolved in resolved_targets: @@ -176,9 +290,6 @@ def target_catalog(cls, comms: Comms, target: TargetSelection, channel if declaration.channel_fields() else NoChannel()) for declaration, commands in grouped.items()) - def for_editor(self, comms: Comms, target: str, project: Checkout) -> Self: - return self - @property def editable_fields(self) -> tuple[TargetField, ...]: """The command owns the parameters offered by its target action.""" @@ -189,11 +300,6 @@ def editable_fields(self) -> tuple[TargetField, ...]: if not entry.setting(TargetBound, False) ) - async def prepare_fields(self) -> tuple[TargetField, ...]: - - catalogs = ConfigOptions.configured() - return tuple([await item.prepare(catalogs, self) for item in self.editable_fields]) - def describe(self, comms: Comms, target: CommandTarget, project: Checkout) -> TargetAction: bound = self.for_editor(comms, target.name, project) @@ -236,15 +342,7 @@ def with_confirmation(self, confirmed: bool) -> Self: return self def edited(self, arguments: Mapping[str, object]) -> Self: - bound = { - entry.key for entry in FieldCodec.argument_fields(type(self)) - if entry.setting(TargetBound, False) - } - if bound & arguments.keys(): - raise ValueError("Target-bound parameters cannot be overridden") - editable = {item.name for item in self.editable_fields} - if arguments.keys() - editable: - raise ValueError("Parameters not offered by this target command cannot be overridden") + bound = self.authorize_edits(arguments) encoded = FieldCodec.encode(self) return type(self).from_payload({ **{key: encoded[key] for key in bound & encoded.keys()}, @@ -271,7 +369,7 @@ def execute_target(cls, comms: Comms, target: TargetSelection, arguments: Mappin @dataclass(frozen=True) class TargetAction: """A selection keeps the bound commands, not a separate permission list.""" - bound: tuple[CliCommand, ...] + bound: tuple[CommandBinding, ...] targets: tuple[str, ...] declaration: type[CliCommand] channels: ChannelSelection = NoChannel() @@ -280,8 +378,8 @@ class TargetAction: def editable_fields(self) -> tuple[TargetField, ...]: return self.bound[0].editable_fields - async def prepare_fields(self) -> tuple[TargetField, ...]: - return await self.bound[0].prepare_fields() + async def prepare_fields(self, arguments: Mapping[str, object] = {}) -> tuple[TargetField, ...]: + return await self.bound[0].prepare_fields(arguments) @property def label(self) -> str: @@ -289,14 +387,14 @@ def label(self) -> str: @property def confirmation(self) -> str: - groups: dict[type[CliCommand], list[CliCommand]] = {} + groups: dict[type[CommandBinding], list[CommandBinding]] = {} for command in self.bound: groups.setdefault(type(command), []).append(command) return '\n'.join(filter(None, (declaration.selection_confirmation(tuple(commands)) for declaration, commands in groups.items()))) def edited(self, arguments: Mapping[str, object]) -> TargetAction: - return replace(self, bound=tuple(command.edited(type(command).editor_arguments(arguments)) + return replace(self, bound=tuple(command.edited(command.declaration.editor_arguments(arguments)) for command in self.bound)) def with_confirmation(self, confirmed: bool) -> TargetAction: @@ -480,7 +578,7 @@ def run(self, declaration: type[CliCommand], comms: Comms, arguments: Mapping[st if member.catalog_declaration() is declaration) if declaration.catalog_declaration() is declaration else (declaration,)) outcomes: list[tuple[int, TargetCompleted | TargetFailed]] = [] - planned: dict[CliCommand, tuple[int, CommandTarget, SurroundingChannel]] = {} + planned: dict[CommandBinding, tuple[int, CommandTarget, SurroundingChannel]] = {} together: dict[type[AppliedTogether], list[tuple[int, CommandTarget, AppliedTogether]]] = {} for name in self.targets: try: @@ -511,7 +609,7 @@ def run(self, declaration: type[CliCommand], comms: Comms, arguments: Mapping[st current = comms.registry.snapshot() current_catalog = (comms.channels.catalog.read() if context.needs_catalog or name.needs_channel_state else NoCatalog()) - if bound_command not in type(bound_command).bindings( + if bound_command not in bound_command.declaration.bindings( comms, name, context, project=project, snapshot=current, catalog=current_catalog, resolved=name.resolve(current, comms.bus.require_channel)): raise CommandSelectionChanged( diff --git a/src/agent_comms/cli_support/thread_creation.py b/src/agent_comms/cli_support/thread_creation.py index 6c34493c72..8e964102a7 100644 --- a/src/agent_comms/cli_support/thread_creation.py +++ b/src/agent_comms/cli_support/thread_creation.py @@ -7,7 +7,7 @@ from pathlib import Path from typing import Annotated -from agent_comms.cli_command import CliCommand +from agent_comms.cli_command import CliCommand, CommandContext from agent_comms.cli_commands import NameArgument, ThreadCreatedResult, _tag_choice_text from agent_comms.comms import Comms from agent_comms.config_options import ConfigOptions, ModelConfigOption, ThinkingLevelConfigOption @@ -16,16 +16,17 @@ from agent_comms.pi_vocabulary import ThinkingLevelField from agent_comms.repo import Checkout, LaunchCheckout from agent_comms.thread_management import ThreadCreationSpec -from agent_comms.thread_model import CatalogSelection, ThreadModel +from agent_comms.thread_model import ThreadModel -type TagArgument[T] = Annotated[ - T, EditorFormat(FieldCodec.encode), Normalize(_tag_choice_text), Argument(flags=("--tags",)), -] +TAG_ARGUMENT = ( + EditorFormat(FieldCodec.encode), Normalize(_tag_choice_text), Argument(flags=("--tags",)), +) @dataclass(frozen=True, kw_only=True) -class ThreadCreationCliCommand(NameArgument, CatalogSelection, CliCommand, declared_name=None): +class ThreadCreationFields(NameArgument, CommandContext): + """Supplied launch context is also the complete command's field declaration.""" task: Annotated[str, Multiline(), Argument(flags=("--task",))] = "" project: Annotated[Checkout, Argument(flags=("--project",))] model: Annotated[ @@ -35,19 +36,25 @@ class ThreadCreationCliCommand(NameArgument, CatalogSelection, CliCommand, decla ThinkingLevelField, Catalog(ThinkingLevelConfigOption), Argument(flags=("--thinking-level",)), ] = NativeThreadDefaults.thinking_level - @property - @abstractmethod - def spec_type(self) -> type[ThreadCreationSpec]: ... - @property def project_path(self) -> Path: return self.project.path - def apply(self, ctx: Comms) -> ThreadCreatedResult: - spec = self.spec_type.from_task(**{ + +@dataclass(frozen=True, kw_only=True) +class ThreadCreationCliCommand(ThreadCreationFields, CliCommand, declared_name=None): + @property + @abstractmethod + def spec_type(self) -> type[ThreadCreationSpec]: ... + + def specification(self) -> ThreadCreationSpec: + return self.spec_type.from_task(**{ **{item.name: getattr(self, item.name) for item in fields(self.spec_type)}, 'project': LaunchCheckout.checked(self.project.path), }) + + def apply(self, ctx: Comms) -> ThreadCreatedResult: + spec = self.specification() asyncio.run(ConfigOptions.configured().validate(self)) child = ctx.threads.create(spec) return ThreadCreatedResult(child.name, child.process_identity) diff --git a/src/agent_comms/cli_verbs/fork.py b/src/agent_comms/cli_verbs/fork.py index e6b9c1637f..548bf1b13a 100644 --- a/src/agent_comms/cli_verbs/fork.py +++ b/src/agent_comms/cli_verbs/fork.py @@ -1,6 +1,6 @@ from __future__ import annotations -from agent_comms.cli_support.thread_creation import TagArgument, ThreadCreationCliCommand +from agent_comms.cli_support.thread_creation import TAG_ARGUMENT, ThreadCreationCliCommand from agent_comms.cli_support.thread_bound import ThreadBoundCliCommand from agent_comms.field_codec import Argument from agent_comms.field_codec import TargetBound @@ -21,7 +21,7 @@ class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ThreadCreationCl str, TargetBound(), Argument(flags=("--parent",)) ] - tags: TagArgument[TagChoice] = InheritedTags() + tags: Annotated[TagChoice, *TAG_ARGUMENT] = InheritedTags() @classmethod def thread_binding(cls, thread: Thread) -> Self: diff --git a/src/agent_comms/cli_verbs/new.py b/src/agent_comms/cli_verbs/new.py index e7661e5b44..349a389107 100644 --- a/src/agent_comms/cli_verbs/new.py +++ b/src/agent_comms/cli_verbs/new.py @@ -1,24 +1,25 @@ from __future__ import annotations -from agent_comms.cli_support.thread_creation import TagArgument, ThreadCreationCliCommand +from agent_comms.cli_command import PendingCommandBinding +from agent_comms.cli_support.thread_creation import TAG_ARGUMENT, ThreadCreationCliCommand, ThreadCreationFields from agent_comms.comms import Comms from agent_comms.native_settings import NativeThreadDefaults from agent_comms.repo import Checkout from agent_comms.thread_management import GivenTags from agent_comms.thread_management import NewThreadSpec from dataclasses import dataclass -from typing import ClassVar -from typing import Self +from typing import Annotated, ClassVar @dataclass(frozen=True, kw_only=True) class NewThreadCliCommand(ThreadCreationCliCommand, declared_name="new"): help = "Create a new pi thread" spec_type: ClassVar[type[NewThreadSpec]] = NewThreadSpec - tags: TagArgument[GivenTags] + tags: Annotated[GivenTags, *TAG_ARGUMENT] @classmethod - def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[Self, ...]: + def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[PendingCommandBinding, ...]: defaults = NativeThreadDefaults() - return (cls(name='', project=project, model=defaults.model, - thinking_level=defaults.thinking_level),) + return (PendingCommandBinding(cls, ThreadCreationFields( + name='', project=project, model=defaults.model, + thinking_level=defaults.thinking_level)),) diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index f07a82f7a5..85e3ea0b73 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -6,7 +6,7 @@ from agent_comms import field_codec from agent_comms.field_codec import FieldCodec from collections.abc import Callable -from dataclasses import dataclass +from dataclasses import MISSING, dataclass from dataclasses import replace from agent_comms.config_options import CatalogChoice, CatalogConfigOption, ConfigOptions from agent_comms.thread_model import CatalogSelection @@ -44,9 +44,13 @@ def multiline(self) -> bool: def description(self) -> str: return self.plan.description + @property + def supplied(self) -> bool: + return self.value is not MISSING + @property def editor_default(self) -> JsonValue: - return self.plan.editor_default(self.value) + return self.plan.editor_default(self.value) if self.supplied else None def editor_hints(self, choices: Callable[[ChoiceTargetField], object]) -> tuple[object, ...]: """An unrestricted field does not declare a finite-choice editor.""" diff --git a/tests/test_checkout_submodules.py b/tests/test_checkout_submodules.py index 60700222a2..b4166031fc 100644 --- a/tests/test_checkout_submodules.py +++ b/tests/test_checkout_submodules.py @@ -136,7 +136,7 @@ def test_empty_project_does_not_become_the_callers_checkout(tmp_path, monkeypatc git(tmp_path, 'init', '-q') monkeypatch.chdir(tmp_path) comms = wire(tmp_path / 'wire') - result = core('--root', str(comms.root), 'new', '--name', 'empty-project', '--project', '') + result = core('--root', str(comms.root), 'new', '--name', 'empty-project', '--tags', '', '--project', '') assert result.returncode != 0 assert 'Project path must not be empty' in result.stdout + result.stderr assert 'empty-project' not in comms.registry.snapshot().threads diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index 9b41eb166a..12af3959cd 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -42,19 +42,21 @@ async def cli(programs, root, *arguments): class TestEndToEndLifecycle: - @pytest.mark.parametrize('operation,parent_arguments', [ - ('new', ()), ('fork', ('--parent', 'parent')), + @pytest.mark.parametrize('operation,creation_arguments,expected_tags', [ + ('new', ('--tags', 'chosen'), frozenset({'chosen'})), + ('new', ('--tags', ''), frozenset()), + ('fork', ('--parent', 'parent'), frozenset({'inherited'})), ]) async def test_new_and_fork_require_a_declared_checkout( - self, native, source_programs, operation, parent_arguments, + self, native, source_programs, operation, creation_arguments, expected_tags, ): await native.author_history() comms = wire(native.root) await cli(source_programs, native.root, 'register', '--name', 'parent', - '--worktree', str(native.project), '--execution', 'native') + '--tags', 'inherited', '--worktree', str(native.project), '--execution', 'native') parent = comms.registry.require('parent') comms.threads.attach_session(parent, str(native.session), process=parent.process_identity) - command = (operation, '--name', 'probe', *parent_arguments) + command = (operation, '--name', 'probe', *creation_arguments) plain = native.project.parent / 'plain' plain.mkdir() for project_arguments in ((), ('--project', ''), ('--project', str(plain))): @@ -66,7 +68,9 @@ async def test_new_and_fork_require_a_declared_checkout( '--project', str(native.project), '--model', 'response-local/fixture', '--thinking-level', 'off') assert created['created'] == 'probe' - assert comms.registry.require('probe').worktree == Checkout(native.project.resolve()) + child = comms.registry.require('probe') + assert child.worktree == Checkout(native.project.resolve()) + assert child.tags == expected_tags finally: for thread in comms.registry.snapshot().threads.values(): if thread.process_alive: diff --git a/tests/test_thread_creation.py b/tests/test_thread_creation.py new file mode 100644 index 0000000000..b4bea079c4 --- /dev/null +++ b/tests/test_thread_creation.py @@ -0,0 +1,84 @@ +"""Creation contracts through the actual CLI decoder, editor binding and registry. + +Author does not execute these controls; the integration owner selects them once. +""" +from dataclasses import replace + +import pytest + +from agent_comms.cli import build_parser +from agent_comms.cli_command import PendingCommandBinding +from agent_comms.cli_verbs.fork import ForkCliCommand +from agent_comms.cli_verbs.new import NewThreadCliCommand +from agent_comms.field_codec import MalformedRecord +from agent_comms.launch_runtime import LaunchRuntime +from agent_comms.repo import Checkout +from agent_comms.thread_management import GivenTags, InheritedTags + + +def parsed(arguments): + namespace = build_parser(arguments).parse_args(arguments) + return namespace.command_declaration.from_arguments(namespace) + + +def test_new_cli_requires_an_explicit_tag_argument(source_repository): + with pytest.raises(MalformedRecord): + parsed(['new', '--name', 'child', '--project', str(source_repository.checkout)]) + + +@pytest.mark.parametrize('text,expected', ( + ('auth,review', frozenset({'auth', 'review'})), + ('', frozenset()), +)) +def test_new_cli_membership_reaches_registration(comms, source_repository, text, expected): + command = parsed(['new', '--name', 'child', '--project', str(source_repository.checkout), + '--tags', text]) + assert command.tags == GivenTags(expected) + # New's declaration path reserves its session but does not launch a provider. + child = command.specification().declare(comms.threads, '', LaunchRuntime.running()) + assert child.tags == expected + assert comms.registry.require(child.name).tags == expected + + +def test_pending_new_binding_offers_missing_tags_without_constructing_the_command(comms, source_repository): + binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) + assert type(binding) is PendingCommandBinding + tags = next(item for item in binding.editable_fields if item.name == 'tags') + assert tags.required and not tags.supplied + assert tags.editor_default is None + with pytest.raises(MalformedRecord): + binding.edited(NewThreadCliCommand.editor_arguments({'name': 'child'})) + + +@pytest.mark.parametrize('text,expected', ( + ('auth', frozenset({'auth'})), + ('', frozenset()), +)) +def test_pending_new_binding_distinguishes_explicit_empty_from_missing(comms, source_repository, text, expected): + binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) + command = binding.edited(NewThreadCliCommand.editor_arguments({'name': 'child', 'tags': text})) + assert command.tags == GivenTags(expected) + assert command.project == Checkout(source_repository.checkout) + + +def test_pending_catalog_context_can_change_before_tags_are_supplied(comms, source_repository): + binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) + prepared = binding.for_fields({'name': 'child'}) + assert prepared.context.name == 'child' + assert not next(item for item in prepared.editable_fields if item.name == 'tags').supplied + + +def test_fork_cli_defaults_to_actual_parent_membership(comms, source_repository): + parent_command = parsed(['new', '--name', 'parent', '--project', str(source_repository.checkout), + '--tags', 'original']) + parent = parent_command.specification().declare(comms.threads, '', LaunchRuntime.running()) + command = parsed(['fork', '--name', 'child', '--parent', parent.name, + '--project', str(source_repository.checkout)]) + assert command.tags == InheritedTags() + editor = ForkCliCommand.thread_binding(parent) + assert editor.tags == InheritedTags() + # A real later parent record, not the stale tags at editor opening, supplies membership. + current = comms.registry.declare(replace(parent, tags=frozenset({'current'}))) + child = command.specification().child(current, current.runtime, comms.threads) + assert child.tags == current.tags == frozenset({'current'}) + assert editor.tags.for_child(current.tags) == current.tags From f9d18ba8c47e48812026e98678963ebac45cd9bf Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 03:31:25 -0400 Subject: [PATCH 03/26] Keep native context edits distinct from complete command authorization --- src/agent_comms/cli_command.py | 4 ++-- tests/test_thread_creation.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index f5a6b3bb14..afb03cfe71 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -118,7 +118,7 @@ def with_confirmation(self, confirmed: bool) -> CommandBinding: ... class CommandContext(CatalogSelection): """A partial command's supplied fields, with native edits decoded by their plans.""" - def edited(self, arguments: Mapping[str, object]) -> Self: + def with_editor_values(self, arguments: Mapping[str, object]) -> Self: changes = {name: value for entry in FieldCodec.argument_fields(type(self)) if entry.key in arguments for name, value in entry.decoded({ @@ -151,7 +151,7 @@ def editable_fields(self) -> tuple[TargetField, ...]: if not entry.setting(TargetBound, False)) def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: - return replace(self, context=self.context.edited(arguments)) + return replace(self, context=self.context.with_editor_values(arguments)) def edited(self, arguments: Mapping[str, object]) -> CliCommand: self.authorize_edits(arguments) diff --git a/tests/test_thread_creation.py b/tests/test_thread_creation.py index b4bea079c4..9f435cead0 100644 --- a/tests/test_thread_creation.py +++ b/tests/test_thread_creation.py @@ -68,6 +68,18 @@ def test_pending_catalog_context_can_change_before_tags_are_supplied(comms, sour assert not next(item for item in prepared.editable_fields if item.name == 'tags').supplied +@pytest.mark.parametrize('creation_arguments,forbidden', ( + (('new', '--tags', 'auth'), {'not_a_field': 'value'}), + (('fork', '--parent', 'parent'), {'not_a_field': 'value'}), + (('fork', '--parent', 'parent'), {'parent': 'different-parent'}), +)) +def test_complete_creation_commands_keep_command_edit_authorization(source_repository, creation_arguments, forbidden): + command = parsed([*creation_arguments, '--name', 'child', + '--project', str(source_repository.checkout)]) + with pytest.raises(ValueError): + command.edited(forbidden) + + def test_fork_cli_defaults_to_actual_parent_membership(comms, source_repository): parent_command = parsed(['new', '--name', 'parent', '--project', str(source_repository.checkout), '--tags', 'original']) From ee7b54782a1111ff1bdbb2d29e46a7a236b9aeff Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 14:22:19 -0400 Subject: [PATCH 04/26] Complete creation bindings through native field and presence owners --- src/agent_comms/cli_command.py | 118 ++++++++++-------- src/agent_comms/cli_commands.py | 12 +- .../cli_support/thread_creation.py | 21 ++-- src/agent_comms/cli_verbs/fork.py | 4 +- src/agent_comms/cli_verbs/new.py | 5 +- src/agent_comms/cli_verbs/thread_tags.py | 8 +- src/agent_comms/field_codec.py | 87 +++++++++---- src/agent_comms/target_fields.py | 95 ++++++++++---- src/agent_comms/thread_management.py | 3 +- tests/test_thread_creation.py | 9 +- 10 files changed, 230 insertions(+), 132 deletions(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index afb03cfe71..d6f9b42fc7 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -28,7 +28,7 @@ from agent_comms.field_codec import FieldPlan, TargetBound from agent_comms.launch_root import LaunchRoot from inspect import iscoroutinefunction, signature -from types import get_original_bases +from types import MappingProxyType, get_original_bases from typing import TYPE_CHECKING, TextIO, cast, get_args, get_origin if TYPE_CHECKING: @@ -72,23 +72,33 @@ class CommandBinding(ABC): def declaration(self) -> type[CliCommand]: ... @property + def editable_fields(self) -> tuple[TargetField, ...]: + return tuple(self.field_value(entry) + for entry in FieldCodec.argument_fields(self.declaration) + if not entry.setting(TargetBound, False)) + @abstractmethod - def editable_fields(self) -> tuple[TargetField, ...]: ... + def field_value(self, entry: FieldPlan) -> TargetField: ... - def for_editor(self, comms: Comms, target: str, project: Checkout) -> Self: - return self + @abstractmethod + def retained_payload(self) -> dict[str, object]: + """Already decoded constructor values, never serialized for editing.""" - @property - def catalog_context(self) -> object: + def for_editor(self, comms: Comms, target: str, project: Checkout) -> Self: return self def for_fields(self, arguments: Mapping[str, object]) -> CommandBinding: return self.edited(self.declaration.editor_arguments(arguments)) if arguments else self - async def prepare_fields(self, arguments: Mapping[str, object] = {}) -> tuple[TargetField, ...]: + async def prepare_field(self, item: TargetField, catalogs: ConfigOptions) -> TargetField: + return await item.prepare(catalogs) + + async def prepare_fields( + self, arguments: Mapping[str, object] = MappingProxyType({}), + ) -> tuple[TargetField, ...]: binding = self.for_fields(arguments) catalogs = ConfigOptions.configured() - return tuple([await item.prepare(catalogs, binding.catalog_context) + return tuple([await binding.prepare_field(item, catalogs) for item in binding.editable_fields]) def authorize_edits(self, arguments: Mapping[str, object]) -> frozenset[str]: @@ -100,8 +110,12 @@ def authorize_edits(self, arguments: Mapping[str, object]) -> frozenset[str]: raise ValueError("Parameters not offered by this target command cannot be overridden") return bound - @abstractmethod - def edited(self, arguments: Mapping[str, object]) -> CliCommand: ... + def edited(self, arguments: Mapping[str, object]) -> CliCommand: + self.authorize_edits(arguments) + decoded = {name: value + for entry in FieldCodec.argument_fields(self.declaration) if entry.key in arguments + for name, value in entry.decoded(arguments).items()} + return FieldCodec.construct(self.declaration, {**self.retained_payload(), **decoded}) @abstractmethod def confirmation(self) -> str: ... @@ -114,26 +128,33 @@ def selection_confirmation(cls, commands: tuple[Self, ...]) -> str: ... def with_confirmation(self, confirmed: bool) -> CommandBinding: ... +class CatalogBinding(CommandBinding): + """Only a binding declaring catalog inputs has a catalog question to answer.""" + + @property + @abstractmethod + def catalog_context(self) -> CatalogSelection: ... + + async def prepare_field(self, item: TargetField, catalogs: ConfigOptions) -> TargetField: + return await item.prepare_catalog(catalogs, self.catalog_context) + + @dataclass(frozen=True) class CommandContext(CatalogSelection): - """A partial command's supplied fields, with native edits decoded by their plans.""" - - def with_editor_values(self, arguments: Mapping[str, object]) -> Self: - changes = {name: value - for entry in FieldCodec.argument_fields(type(self)) if entry.key in arguments - for name, value in entry.decoded({ - entry.key: entry.editor_payload(arguments[entry.key]), - }).items()} - return replace(self, **changes) if changes else self + """A declaration of supplied fields, not an alternate command editor.""" @dataclass(frozen=True) -class PendingCommandBinding(CommandBinding): +class PendingCommandBinding(CatalogBinding): """Only supplied context, borrowing the concrete command's declared fields.""" command: type[CliCommand] context: CommandContext + def __post_init__(self) -> None: + if not issubclass(self.command, type(self.context)): + raise TypeError('A pending context must be a declared parent of its command') + @property def declaration(self) -> type[CliCommand]: return self.command @@ -142,24 +163,24 @@ def declaration(self) -> type[CliCommand]: def catalog_context(self) -> CatalogSelection: return self.context - @property - def editable_fields(self) -> tuple[TargetField, ...]: - supplied = {entry.name: entry.current(self.context) - for entry in FieldCodec.argument_fields(type(self.context))} - return tuple(TargetField.of(entry, supplied[entry.name] if entry.name in supplied else entry.default()) - for entry in FieldCodec.argument_fields(self.declaration) - if not entry.setting(TargetBound, False)) + def retained_payload(self) -> dict[str, object]: + return {entry.name: entry.current(self.context) + for entry in FieldCodec.field_plans(type(self.context))} - def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: - return replace(self, context=self.context.with_editor_values(arguments)) + def field_value(self, entry: FieldPlan) -> TargetField: + for supplied in FieldCodec.field_plans(type(self.context)): + if supplied.declared is entry.declared: + return TargetField.of(entry, supplied.current(self.context)) + return TargetField.omitted(entry) - def edited(self, arguments: Mapping[str, object]) -> CliCommand: + def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: self.authorize_edits(arguments) - return self.declaration.from_payload({ - **FieldCodec.encode(self.context), - self.declaration.family_discriminator: FieldCodec.encode(self.declaration), - **arguments, - }) + changes = {name: value + for entry in FieldCodec.argument_fields(type(self.context)) if entry.key in arguments + for name, value in entry.decoded({ + entry.key: entry.editor_payload(arguments[entry.key]), + }).items()} + return replace(self, context=replace(self.context, **changes)) if changes else self def confirmation(self) -> str: return '' @@ -290,15 +311,13 @@ def target_catalog(cls, comms: Comms, target: TargetSelection, channel if declaration.channel_fields() else NoChannel()) for declaration, commands in grouped.items()) - @property - def editable_fields(self) -> tuple[TargetField, ...]: - """The command owns the parameters offered by its target action.""" + def field_value(self, entry: FieldPlan) -> TargetField: + return TargetField.of(entry, entry.current(self)) - return tuple( - TargetField.of(entry, entry.current(self)) - for entry in FieldCodec.argument_fields(type(self)) - if not entry.setting(TargetBound, False) - ) + def retained_payload(self) -> dict[str, object]: + return {entry.name: entry.current(self) + for entry in FieldCodec.argument_fields(type(self)) + if entry.setting(TargetBound, False)} def describe(self, comms: Comms, target: CommandTarget, project: Checkout) -> TargetAction: @@ -341,15 +360,6 @@ def with_confirmation(self, confirmed: bool) -> Self: raise ValueError(self.confirmation()) return self - def edited(self, arguments: Mapping[str, object]) -> Self: - bound = self.authorize_edits(arguments) - encoded = FieldCodec.encode(self) - return type(self).from_payload({ - **{key: encoded[key] for key in bound & encoded.keys()}, - type(self).family_discriminator: FieldCodec.encode(type(self)), - **arguments, - }) - @classmethod def execute_target(cls, comms: Comms, target: TargetSelection, arguments: Mapping[str, object], *, confirmed: bool = False, @@ -378,7 +388,9 @@ class TargetAction: def editable_fields(self) -> tuple[TargetField, ...]: return self.bound[0].editable_fields - async def prepare_fields(self, arguments: Mapping[str, object] = {}) -> tuple[TargetField, ...]: + async def prepare_fields( + self, arguments: Mapping[str, object] = MappingProxyType({}), + ) -> tuple[TargetField, ...]: return await self.bound[0].prepare_fields(arguments) @property diff --git a/src/agent_comms/cli_commands.py b/src/agent_comms/cli_commands.py index 7f067c70e8..5ff848499d 100644 --- a/src/agent_comms/cli_commands.py +++ b/src/agent_comms/cli_commands.py @@ -29,7 +29,7 @@ from .cli_result import CliResult from .comms import Comms from .declared_family import DeclaredFamily -from .field_codec import Argument, PathText, WireForm, WireOmitDefault, boundary_decoder +from .field_codec import Argument, PathText, WireForm, WireOmitDefault from .history_result import HistoryPageResult from .goals import Goal from .lifecycle_capability import Complete @@ -76,16 +76,6 @@ def _tags(value: str) -> list[str]: return [tag.strip() for tag in value.split(",") if tag.strip()] -@boundary_decoder -def _tag_choice_text(value: str | None) -> str | None: - """Command-line and editor text is the given tags' wire; no text inherits.""" - return value - - -def _tag_text(value: frozenset[str] | None) -> str | None: - return None if value is None else ', '.join(sorted(value)) - - def _shell_words(value: str) -> list[str]: return shlex.split(value) diff --git a/src/agent_comms/cli_support/thread_creation.py b/src/agent_comms/cli_support/thread_creation.py index 8e964102a7..07c617a558 100644 --- a/src/agent_comms/cli_support/thread_creation.py +++ b/src/agent_comms/cli_support/thread_creation.py @@ -7,21 +7,16 @@ from pathlib import Path from typing import Annotated -from agent_comms.cli_command import CliCommand, CommandContext -from agent_comms.cli_commands import NameArgument, ThreadCreatedResult, _tag_choice_text +from agent_comms.cli_command import CatalogBinding, CliCommand, CommandContext +from agent_comms.cli_commands import NameArgument, ThreadCreatedResult from agent_comms.comms import Comms from agent_comms.config_options import ConfigOptions, ModelConfigOption, ThinkingLevelConfigOption -from agent_comms.field_codec import Argument, Catalog, EditorFormat, FieldCodec, Multiline, Normalize +from agent_comms.field_codec import Argument, Catalog, Multiline from agent_comms.native_settings import NativeThreadDefaults from agent_comms.pi_vocabulary import ThinkingLevelField from agent_comms.repo import Checkout, LaunchCheckout from agent_comms.thread_management import ThreadCreationSpec -from agent_comms.thread_model import ThreadModel - - -TAG_ARGUMENT = ( - EditorFormat(FieldCodec.encode), Normalize(_tag_choice_text), Argument(flags=("--tags",)), -) +from agent_comms.thread_model import CatalogSelection, ThreadModel @dataclass(frozen=True, kw_only=True) @@ -42,11 +37,17 @@ def project_path(self) -> Path: @dataclass(frozen=True, kw_only=True) -class ThreadCreationCliCommand(ThreadCreationFields, CliCommand, declared_name=None): +class ThreadCreationCliCommand( + ThreadCreationFields, CatalogBinding, CliCommand[ThreadCreatedResult], declared_name=None, +): @property @abstractmethod def spec_type(self) -> type[ThreadCreationSpec]: ... + @property + def catalog_context(self) -> CatalogSelection: + return self + def specification(self) -> ThreadCreationSpec: return self.spec_type.from_task(**{ **{item.name: getattr(self, item.name) for item in fields(self.spec_type)}, diff --git a/src/agent_comms/cli_verbs/fork.py b/src/agent_comms/cli_verbs/fork.py index 548bf1b13a..c39dd652cc 100644 --- a/src/agent_comms/cli_verbs/fork.py +++ b/src/agent_comms/cli_verbs/fork.py @@ -1,6 +1,6 @@ from __future__ import annotations -from agent_comms.cli_support.thread_creation import TAG_ARGUMENT, ThreadCreationCliCommand +from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand from agent_comms.cli_support.thread_bound import ThreadBoundCliCommand from agent_comms.field_codec import Argument from agent_comms.field_codec import TargetBound @@ -21,7 +21,7 @@ class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ThreadCreationCl str, TargetBound(), Argument(flags=("--parent",)) ] - tags: Annotated[TagChoice, *TAG_ARGUMENT] = InheritedTags() + tags: Annotated[TagChoice, Argument()] = InheritedTags() @classmethod def thread_binding(cls, thread: Thread) -> Self: diff --git a/src/agent_comms/cli_verbs/new.py b/src/agent_comms/cli_verbs/new.py index 349a389107..c0ecc5e229 100644 --- a/src/agent_comms/cli_verbs/new.py +++ b/src/agent_comms/cli_verbs/new.py @@ -1,8 +1,9 @@ from __future__ import annotations from agent_comms.cli_command import PendingCommandBinding -from agent_comms.cli_support.thread_creation import TAG_ARGUMENT, ThreadCreationCliCommand, ThreadCreationFields +from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand, ThreadCreationFields from agent_comms.comms import Comms +from agent_comms.field_codec import Argument from agent_comms.native_settings import NativeThreadDefaults from agent_comms.repo import Checkout from agent_comms.thread_management import GivenTags @@ -15,7 +16,7 @@ class NewThreadCliCommand(ThreadCreationCliCommand, declared_name="new"): help = "Create a new pi thread" spec_type: ClassVar[type[NewThreadSpec]] = NewThreadSpec - tags: Annotated[GivenTags, *TAG_ARGUMENT] + tags: Annotated[GivenTags, Argument()] @classmethod def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[PendingCommandBinding, ...]: diff --git a/src/agent_comms/cli_verbs/thread_tags.py b/src/agent_comms/cli_verbs/thread_tags.py index e7eff77318..0f031b7a9f 100644 --- a/src/agent_comms/cli_verbs/thread_tags.py +++ b/src/agent_comms/cli_verbs/thread_tags.py @@ -4,13 +4,10 @@ from agent_comms.catalog_document import CatalogAccess from agent_comms.cli_commands import ThreadTagsResult -from agent_comms.cli_commands import _tag_text -from agent_comms.cli_commands import _tags from agent_comms.cli_support.named import NamedCliCommand from agent_comms.comms import Comms from agent_comms.field_codec import Argument -from agent_comms.field_codec import EditorFormat -from agent_comms.field_codec import Normalize +from agent_comms.launch_runtime import TagListText from agent_comms.thread_status import ThreadStatus from agent_comms.threads import Thread from dataclasses import dataclass @@ -23,8 +20,7 @@ class ThreadTagsCliCommand(NamedCliCommand, declared_name="thread-tags"): help = "Edit thread tags" tags: Annotated[ frozenset[str], - Normalize(_tags), - EditorFormat(_tag_text), + TagListText, Argument( flags=("--tags",), options={"help": "Complete tags, separated by commas"} ), diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index db0eb0ab3f..aa34645a68 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -369,8 +369,8 @@ class Normalize(ValueDirective[Callable[[object], object]]): pass -class EditorFormat(ValueDirective[Callable[[object], JsonValue]]): - pass +class EditsAsText: + """This member declares the typed editor of its untagged wire family.""" class Editor(ValueDirective[type]): @@ -619,10 +619,6 @@ def argument_normalizer(self) -> Callable[[object], object]: def editor_normalizer(self) -> Callable[[object], object]: return self.setting(Normalize, self.value.decode_editor) - @cached_property - def editor_formatter(self) -> Callable[[object], JsonValue]: - return self.setting(EditorFormat, self.value.encode_editor) - def argument_payload(self, value: object) -> object: return self.argument_normalizer(value) @@ -630,7 +626,7 @@ def editor_payload(self, value: object) -> object: return self.editor_normalizer(value) def editor_default(self, value: object) -> JsonValue: - return self.editor_formatter(value) + return self.value.encode_editor(value) @property def description(self) -> str: @@ -957,6 +953,17 @@ class DecoderPlan(ABC): target: object + @property + def editor_annotation(self) -> object: + return str + + def format_editor(self, wire: object) -> JsonValue: + return json.dumps(wire) + + @boundary_decoder + def parse_editor(self, text: object) -> object: + return None if text is None else json.loads(text) + def sql_checks(self, column: str, quote: Callable[[object], str]) -> tuple[str, ...]: """Stored-value predicates; the SQL owner quotes its column and constants.""" return () @@ -1053,10 +1060,14 @@ def representation_plan( @classmethod @cache def for_members[M](cls, members: tuple[type[M], ...]) -> UnionDecoder[M]: - return UnionDecoder( - reduce(or_, members), - tuple(compile_decoder(member) for member in members), - ) + target = reduce(or_, members) + alternatives = tuple(compile_decoder(member) for member in members) + editors = tuple(member for member in members if EditsAsText in member.__mro__) + if len(editors) > 1: + raise TypeError(f'{target} declares more than one typed-text editor: {editors}') + if editors: + return _TextEditedUnionDecoder(target, alternatives, compile_decoder(editors[0]).editor_wire) + return UnionDecoder(target, alternatives) @abstractmethod def decode(self, data: object) -> T: @@ -1123,15 +1134,15 @@ def decode_selected(self, data, selections): def decode_keys(self, data, keys, selections): raise TypeError("Selected keys require a mapping field") - editor_annotation = str + @property + def editor_wire(self) -> DecoderPlan: + return self - def encode_editor(self, value: T) -> str: - encoded = FieldCodec.encode(value, self.target) - return json.dumps(encoded) + def encode_editor(self, value: T) -> JsonValue: + return self.format_editor(self.encode(value)) - @boundary_decoder - def decode_editor(self, value): - return None if value is None else json.loads(value) + def decode_editor(self, value: object) -> object: + return self.parse_editor(value) def editor_choices(self) -> tuple[tuple[str, object], ...]: raise TypeError("The declared decoder does not offer finite choices") @@ -1166,6 +1177,20 @@ def sql_checks(self, column: str, quote: Callable[[object], str]) -> tuple[str, def wire_structure(self) -> DecoderPlan: return self.representation.wire_structure(self.structural) + @property + def editor_wire(self) -> DecoderPlan: + return self.wire_structure + + @property + def editor_annotation(self) -> object: + return self.wire_structure.editor_annotation + + def format_editor(self, wire: object) -> JsonValue: + return self.wire_structure.format_editor(wire) + + def parse_editor(self, text: object) -> object: + return self.wire_structure.parse_editor(text) + @property def accepts_null(self) -> bool: return self.wire_structure.accepts_null @@ -2793,11 +2818,11 @@ class _TextDecoder(_ExactScalarDecoder[str]): def encode(self, value: str) -> str: return str(super().encode(value)) - def encode_editor(self, value: str) -> str: - return value + def format_editor(self, wire: object) -> JsonValue: + return cast(JsonValue, wire) - def decode_editor(self, value): - return value + def parse_editor(self, text: object) -> object: + return text class _FlagDecoder(_ExactScalarDecoder[bool]): @@ -3088,6 +3113,24 @@ def decode_candidates( first_error = None +@dataclass(frozen=True, slots=True) +class _TextEditedUnionDecoder[T](UnionDecoder[T]): + """One member's declaration owns typed text; other members remain wire choices.""" + + editor: DecoderPlan + + @property + def editor_annotation(self) -> object: + annotation = self.editor.editor_annotation + return Union[tuple((annotation, type(None)))] if self.accepts_null else annotation + + def format_editor(self, wire: object) -> JsonValue: + return self.editor.format_editor(wire) + + def parse_editor(self, text: object) -> object: + return self.editor.parse_editor(text) + + @dataclass(frozen=True, slots=True) class _ArrayDecoder[T: Iterable[object]](ValueDecoder[T]): container: ClassVar[type] diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index 85e3ea0b73..aa40e9b7a9 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -1,37 +1,46 @@ from __future__ import annotations +from abc import ABC, abstractmethod +from collections.abc import Callable, Mapping +from dataclasses import dataclass, replace + from pydantic import JsonValue -from typing import cast from agent_comms import field_codec -from agent_comms.field_codec import FieldCodec -from collections.abc import Callable -from dataclasses import MISSING, dataclass -from dataclasses import replace +from agent_comms.field_codec import FieldCodec, boundary_decoder from agent_comms.config_options import CatalogChoice, CatalogConfigOption, ConfigOptions from agent_comms.thread_model import CatalogSelection @dataclass(frozen=True) -class TargetField: - """The declared field owns parsing, editor hints and requiredness.""" +class TargetField(ABC): + """The declared field owns parsing, editor hints and input presence.""" plan: field_codec.FieldPlan - value: object @classmethod def of(cls, plan: field_codec.FieldPlan, value: object) -> TargetField: - editor: type[TargetField] = ( + editor: type[SuppliedTargetField] = ( CatalogTargetField if field_codec.Catalog.single(plan.annotation) - else plan.setting(field_codec.Editor, cls) + else plan.setting(field_codec.Editor, SuppliedTargetField) ) return editor(plan, value) + @classmethod + def omitted(cls, plan: field_codec.FieldPlan) -> TargetField: + if plan.constructor_required: + return UnsuppliedTargetField(plan) + return cls.of(plan, plan.default()) + @property def annotation(self) -> object: return self.plan.annotation + @property + def editor_annotation(self) -> object: + return field_codec.compile_decoder(self.annotation).editor_annotation + @property def name(self) -> str: return self.plan.key @@ -45,20 +54,26 @@ def description(self) -> str: return self.plan.description @property - def supplied(self) -> bool: - return self.value is not MISSING + @abstractmethod + def editor_default(self) -> JsonValue: ... - @property - def editor_default(self) -> JsonValue: - return self.plan.editor_default(self.value) if self.supplied else None + @abstractmethod + def default_schema(self) -> dict[str, JsonValue]: ... + + def argument(self, value: object) -> Mapping[str, object]: + return {self.name: value} def editor_hints(self, choices: Callable[[ChoiceTargetField], object]) -> tuple[object, ...]: """An unrestricted field does not declare a finite-choice editor.""" return () - async def prepare(self, catalogs: ConfigOptions, selection: object) -> TargetField: + async def prepare(self, catalogs: ConfigOptions) -> TargetField: return self + async def prepare_catalog(self, catalogs: ConfigOptions, + selection: CatalogSelection) -> TargetField: + return await self.prepare(catalogs) + @property def required(self) -> bool: return self.plan.constructor_required @@ -68,10 +83,44 @@ def value_schema(self) -> dict[str, JsonValue]: def json_schema(self) -> dict[str, JsonValue]: return {**self.value_schema(), 'multiline': self.multiline, - 'description': self.description, 'editor_default': self.editor_default} + 'description': self.description, **self.default_schema()} + + +@dataclass(frozen=True) +class SuppliedTargetField(TargetField): + value: object + + @property + def editor_default(self) -> JsonValue: + return self.plan.editor_default(self.value) + + def default_schema(self) -> dict[str, JsonValue]: + return {'editor_default': self.editor_default} + @dataclass(frozen=True) -class ChoiceTargetField(TargetField): +class UnsuppliedTargetField(TargetField): + """A required answer has no value; the optional editor owns its unset control.""" + + @property + def editor_annotation(self) -> object: + return field_codec.compile_decoder(self.annotation).editor_annotation | type(None) + + @property + def editor_default(self) -> JsonValue: + return None + + def default_schema(self) -> dict[str, JsonValue]: + return {} + + @boundary_decoder + def argument(self, value: object) -> Mapping[str, object]: + """Decode the optional control's raw presence once, not the field's wire value.""" + return {} if value is None else super().argument(value) + + +@dataclass(frozen=True) +class ChoiceTargetField(SuppliedTargetField): """A prepared finite declaration, from its annotation or configuration catalog.""" options: tuple[CatalogChoice, ...] = () @@ -83,8 +132,7 @@ def editor_declarations(self) -> tuple[object, ...]: def choices(self) -> tuple[tuple[str, str], ...]: return tuple((choice.name, choice.editor_value) for choice in self.options) - async def prepare(self, catalogs: ConfigOptions, - selection: object) -> ChoiceTargetField: + async def prepare(self, catalogs: ConfigOptions) -> ChoiceTargetField: decoder = field_codec.compile_decoder(self.annotation) options = (CatalogChoice( name=label, value=decoder.encode_editor(decoder.decode(value))) @@ -94,6 +142,7 @@ async def prepare(self, catalogs: ConfigOptions, def editor_hints(self, choices: Callable[[ChoiceTargetField], object]) -> tuple[object, ...]: return (choices(self),) + class CatalogTargetField(ChoiceTargetField): @property def catalog(self) -> type[CatalogConfigOption]: @@ -103,10 +152,10 @@ def catalog(self) -> type[CatalogConfigOption]: def editor_declarations(self) -> tuple[object, ...]: return (self.catalog,) - async def prepare(self, catalogs: ConfigOptions, - selection: object) -> CatalogTargetField: + async def prepare_catalog(self, catalogs: ConfigOptions, + selection: CatalogSelection) -> CatalogTargetField: catalog = catalogs.catalog_for(self.catalog) - options = await catalog.display_choices(cast(CatalogSelection, selection)) + options = await catalog.display_choices(selection) decoder = field_codec.compile_decoder(self.annotation) return replace(self, options=tuple(CatalogChoice(name=item.name, value=decoder.encode_editor(decoder.decode(item.value))) for item in options)) diff --git a/src/agent_comms/thread_management.py b/src/agent_comms/thread_management.py index cddd5e9f97..5be0b40458 100644 --- a/src/agent_comms/thread_management.py +++ b/src/agent_comms/thread_management.py @@ -38,6 +38,7 @@ from .errors import RelationViolationError from .field_codec import ( DeclaredWireType, + EditsAsText, FieldCodec, NullMember, RecordMember, @@ -77,7 +78,7 @@ def for_child(self, parent: frozenset[str]) -> frozenset[str]: ... @dataclass(frozen=True, slots=True) -class GivenTags(DeclaredWireType, TagChoice): +class GivenTags(EditsAsText, DeclaredWireType, TagChoice): """Tags spelled as comma-separated text on the command line, in the editor and in tools.""" wire_type = str diff --git a/tests/test_thread_creation.py b/tests/test_thread_creation.py index 9f435cead0..e7cf080452 100644 --- a/tests/test_thread_creation.py +++ b/tests/test_thread_creation.py @@ -44,8 +44,11 @@ def test_pending_new_binding_offers_missing_tags_without_constructing_the_comman binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) assert type(binding) is PendingCommandBinding tags = next(item for item in binding.editable_fields if item.name == 'tags') - assert tags.required and not tags.supplied + assert tags.required assert tags.editor_default is None + assert 'editor_default' not in tags.json_schema() + assert tags.argument(None) == {} + assert tags.argument('') == {'tags': ''} with pytest.raises(MalformedRecord): binding.edited(NewThreadCliCommand.editor_arguments({'name': 'child'})) @@ -65,7 +68,9 @@ def test_pending_catalog_context_can_change_before_tags_are_supplied(comms, sour binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) prepared = binding.for_fields({'name': 'child'}) assert prepared.context.name == 'child' - assert not next(item for item in prepared.editable_fields if item.name == 'tags').supplied + tags = next(item for item in prepared.editable_fields if item.name == 'tags') + assert tags.argument(None) == {} + assert tags.argument('') == {'tags': ''} @pytest.mark.parametrize('creation_arguments,forbidden', ( From 89a4397f953604d4e1333aea3edf185b36c4009c Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 17:45:55 -0400 Subject: [PATCH 05/26] Leave child names unsupplied until creation form completion --- src/agent_comms/cli_command.py | 23 +++++++------ src/agent_comms/cli_support/named_thread.py | 3 +- src/agent_comms/cli_support/thread_bound.py | 7 ++-- .../cli_support/thread_creation.py | 6 ++-- src/agent_comms/cli_verbs/fork.py | 21 ++++++------ src/agent_comms/cli_verbs/new.py | 6 ++-- src/agent_comms/field_codec.py | 2 +- tests/test_thread_creation.py | 32 ++++++++++++++++--- 8 files changed, 66 insertions(+), 34 deletions(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index d6f9b42fc7..08e274f0e5 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -110,12 +110,17 @@ def authorize_edits(self, arguments: Mapping[str, object]) -> frozenset[str]: raise ValueError("Parameters not offered by this target command cannot be overridden") return bound + @staticmethod + def decoded_edits(declaration: type, arguments: Mapping[str, object]) -> dict[str, object]: + return {name: value + for entry in FieldCodec.argument_fields(declaration) if entry.key in arguments + for name, value in entry.decoded(arguments).items()} + def edited(self, arguments: Mapping[str, object]) -> CliCommand: self.authorize_edits(arguments) - decoded = {name: value - for entry in FieldCodec.argument_fields(self.declaration) if entry.key in arguments - for name, value in entry.decoded(arguments).items()} - return FieldCodec.construct(self.declaration, {**self.retained_payload(), **decoded}) + return FieldCodec.construct(self.declaration, { + **self.retained_payload(), **self.decoded_edits(self.declaration, arguments), + }) @abstractmethod def confirmation(self) -> str: ... @@ -175,11 +180,11 @@ def field_value(self, entry: FieldPlan) -> TargetField: def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: self.authorize_edits(arguments) - changes = {name: value - for entry in FieldCodec.argument_fields(type(self.context)) if entry.key in arguments - for name, value in entry.decoded({ - entry.key: entry.editor_payload(arguments[entry.key]), - }).items()} + context_fields = {entry.key for entry in FieldCodec.argument_fields(type(self.context))} + wire = FieldCodec.editor_arguments(type(self.context), { + key: value for key, value in arguments.items() if key in context_fields, + }) + changes = self.decoded_edits(type(self.context), wire) return replace(self, context=replace(self.context, **changes)) if changes else self def confirmation(self) -> str: diff --git a/src/agent_comms/cli_support/named_thread.py b/src/agent_comms/cli_support/named_thread.py index badadaaecd..be9f7808d8 100644 --- a/src/agent_comms/cli_support/named_thread.py +++ b/src/agent_comms/cli_support/named_thread.py @@ -1,6 +1,7 @@ from __future__ import annotations from agent_comms.catalog_document import CatalogAccess +from agent_comms.cli_command import CommandBinding from agent_comms.channel_targets import OneChannel, ResolvedChannel from agent_comms.channel_targets import ResolvedThread from agent_comms.cli_support.named import NamedCliCommand @@ -24,7 +25,7 @@ def channel_members(cls, resolved: ResolvedChannel) -> tuple[ResolvedThread, ... return () @classmethod - def channel_bindings(cls, comms: Comms, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess) -> tuple[Self, ...]: + def channel_bindings(cls, comms: Comms, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess) -> tuple[CommandBinding, ...]: return tuple(bound for member in cls.channel_members(resolved) for bound in cls.thread_bindings( comms, member.thread, member.status, OneChannel(resolved.channel.name), catalog=catalog)) diff --git a/src/agent_comms/cli_support/thread_bound.py b/src/agent_comms/cli_support/thread_bound.py index cac53354a5..6b6e2d8129 100644 --- a/src/agent_comms/cli_support/thread_bound.py +++ b/src/agent_comms/cli_support/thread_bound.py @@ -4,13 +4,12 @@ from abc import abstractmethod from agent_comms.catalog_document import CatalogAccess -from agent_comms.cli_command import CliCommand +from agent_comms.cli_command import CliCommand, CommandBinding from agent_comms.comms import Comms from agent_comms.thread_execution import ThreadCommandAvailability from agent_comms.thread_status import ThreadStatus from agent_comms.threads import Thread from dataclasses import dataclass -from typing import Self @dataclass(frozen=True, kw_only=True) @@ -18,9 +17,9 @@ class ThreadBoundCliCommand(ThreadCommandAvailability, CliCommand): @classmethod def thread_bindings(cls, comms: Comms, thread: Thread, status: ThreadStatus, channel: SurroundingChannel, *, - catalog: CatalogAccess) -> tuple[Self, ...]: + catalog: CatalogAccess) -> tuple[CommandBinding, ...]: return (cls.thread_binding(thread),) if cls.available_for_thread(thread, status) else () @classmethod @abstractmethod - def thread_binding(cls, thread: Thread) -> Self: ... + def thread_binding(cls, thread: Thread) -> CommandBinding: ... diff --git a/src/agent_comms/cli_support/thread_creation.py b/src/agent_comms/cli_support/thread_creation.py index 07c617a558..8f8f1aeae4 100644 --- a/src/agent_comms/cli_support/thread_creation.py +++ b/src/agent_comms/cli_support/thread_creation.py @@ -20,8 +20,8 @@ @dataclass(frozen=True, kw_only=True) -class ThreadCreationFields(NameArgument, CommandContext): - """Supplied launch context is also the complete command's field declaration.""" +class ThreadLaunchSettings(CommandContext): + """Acquired launch settings; the creator has not supplied a child name.""" task: Annotated[str, Multiline(), Argument(flags=("--task",))] = "" project: Annotated[Checkout, Argument(flags=("--project",))] model: Annotated[ @@ -38,7 +38,7 @@ def project_path(self) -> Path: @dataclass(frozen=True, kw_only=True) class ThreadCreationCliCommand( - ThreadCreationFields, CatalogBinding, CliCommand[ThreadCreatedResult], declared_name=None, + NameArgument, ThreadLaunchSettings, CatalogBinding, CliCommand[ThreadCreatedResult], declared_name=None, ): @property @abstractmethod diff --git a/src/agent_comms/cli_verbs/fork.py b/src/agent_comms/cli_verbs/fork.py index c39dd652cc..14bdedb8db 100644 --- a/src/agent_comms/cli_verbs/fork.py +++ b/src/agent_comms/cli_verbs/fork.py @@ -1,6 +1,7 @@ from __future__ import annotations -from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand +from agent_comms.cli_command import PendingCommandBinding +from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand, ThreadLaunchSettings from agent_comms.cli_support.thread_bound import ThreadBoundCliCommand from agent_comms.field_codec import Argument from agent_comms.field_codec import TargetBound @@ -10,20 +11,22 @@ from agent_comms.threads import Thread from dataclasses import dataclass from typing import Annotated -from typing import Self @dataclass(frozen=True, kw_only=True) -class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ThreadCreationCliCommand): +class ForkContext(ThreadLaunchSettings): + parent: Annotated[str, TargetBound(), Argument(flags=("--parent",))] + + +@dataclass(frozen=True, kw_only=True) +class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ForkContext, ThreadCreationCliCommand): help = "Fork a child pi thread" spec_type = ForkSpec - parent: Annotated[ - str, TargetBound(), Argument(flags=("--parent",)) - ] tags: Annotated[TagChoice, Argument()] = InheritedTags() @classmethod - def thread_binding(cls, thread: Thread) -> Self: - return cls(name='', parent=thread.name, project=thread.worktree, - model=thread.model, thinking_level=thread.thinking_level) + def thread_binding(cls, thread: Thread) -> PendingCommandBinding: + return PendingCommandBinding(cls, ForkContext( + parent=thread.name, project=thread.worktree, + model=thread.model, thinking_level=thread.thinking_level)) diff --git a/src/agent_comms/cli_verbs/new.py b/src/agent_comms/cli_verbs/new.py index c0ecc5e229..aad00c4ff7 100644 --- a/src/agent_comms/cli_verbs/new.py +++ b/src/agent_comms/cli_verbs/new.py @@ -1,7 +1,7 @@ from __future__ import annotations from agent_comms.cli_command import PendingCommandBinding -from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand, ThreadCreationFields +from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand, ThreadLaunchSettings from agent_comms.comms import Comms from agent_comms.field_codec import Argument from agent_comms.native_settings import NativeThreadDefaults @@ -21,6 +21,6 @@ class NewThreadCliCommand(ThreadCreationCliCommand, declared_name="new"): @classmethod def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[PendingCommandBinding, ...]: defaults = NativeThreadDefaults() - return (PendingCommandBinding(cls, ThreadCreationFields( - name='', project=project, model=defaults.model, + return (PendingCommandBinding(cls, ThreadLaunchSettings( + project=project, model=defaults.model, thinking_level=defaults.thinking_level)),) diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index aa34645a68..776c48255e 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -962,7 +962,7 @@ def format_editor(self, wire: object) -> JsonValue: @boundary_decoder def parse_editor(self, text: object) -> object: - return None if text is None else json.loads(text) + return None if text is None else FieldCodec.parse(FieldCodec.decode(str, text)) def sql_checks(self, column: str, quote: Callable[[object], str]) -> tuple[str, ...]: """Stored-value predicates; the SQL owner quotes its column and constants.""" diff --git a/tests/test_thread_creation.py b/tests/test_thread_creation.py index e7cf080452..cedf3eb331 100644 --- a/tests/test_thread_creation.py +++ b/tests/test_thread_creation.py @@ -3,6 +3,7 @@ Author does not execute these controls; the integration owner selects them once. """ from dataclasses import replace +import json import pytest @@ -10,7 +11,8 @@ from agent_comms.cli_command import PendingCommandBinding from agent_comms.cli_verbs.fork import ForkCliCommand from agent_comms.cli_verbs.new import NewThreadCliCommand -from agent_comms.field_codec import MalformedRecord +from agent_comms.field_codec import MalformedRecord, compile_decoder +from agent_comms.target_fields import UnsuppliedTargetField from agent_comms.launch_runtime import LaunchRuntime from agent_comms.repo import Checkout from agent_comms.thread_management import GivenTags, InheritedTags @@ -43,6 +45,11 @@ def test_new_cli_membership_reaches_registration(comms, source_repository, text, def test_pending_new_binding_offers_missing_tags_without_constructing_the_command(comms, source_repository): binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) assert type(binding) is PendingCommandBinding + name = next(item for item in binding.editable_fields if item.name == 'name') + assert type(name) is UnsuppliedTargetField + assert name.argument(None) == {} + with pytest.raises(MalformedRecord): + binding.edited(NewThreadCliCommand.editor_arguments({'tags': ''})) tags = next(item for item in binding.editable_fields if item.name == 'tags') assert tags.required assert tags.editor_default is None @@ -66,8 +73,9 @@ def test_pending_new_binding_distinguishes_explicit_empty_from_missing(comms, so def test_pending_catalog_context_can_change_before_tags_are_supplied(comms, source_repository): binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) - prepared = binding.for_fields({'name': 'child'}) - assert prepared.context.name == 'child' + prepared = binding.for_fields({'task': 'child task', 'name': 'child'}) + assert prepared.context.task == 'child task' + assert type(next(item for item in prepared.editable_fields if item.name == 'name')) is UnsuppliedTargetField tags = next(item for item in prepared.editable_fields if item.name == 'tags') assert tags.argument(None) == {} assert tags.argument('') == {'tags': ''} @@ -85,6 +93,14 @@ def test_complete_creation_commands_keep_command_edit_authorization(source_repos command.edited(forbidden) +def test_structured_editor_reports_malformed_json_at_the_data_boundary(): + decoder = compile_decoder(tuple[int, ...]) + with pytest.raises(MalformedRecord) as refused: + decoder.decode_editor('[1,') + assert type(refused.value.__cause__) is json.JSONDecodeError + assert decoder.decode(decoder.decode_editor('[1, 2]')) == (1, 2) + + def test_fork_cli_defaults_to_actual_parent_membership(comms, source_repository): parent_command = parsed(['new', '--name', 'parent', '--project', str(source_repository.checkout), '--tags', 'original']) @@ -92,7 +108,15 @@ def test_fork_cli_defaults_to_actual_parent_membership(comms, source_repository) command = parsed(['fork', '--name', 'child', '--parent', parent.name, '--project', str(source_repository.checkout)]) assert command.tags == InheritedTags() - editor = ForkCliCommand.thread_binding(parent) + binding = ForkCliCommand.thread_binding(parent) + assert type(binding) is PendingCommandBinding + assert binding.context.parent == parent.name + assert type(next(item for item in binding.editable_fields if item.name == 'name')) is UnsuppliedTargetField + with pytest.raises(MalformedRecord): + binding.edited({}) + with pytest.raises(ValueError, match='Target-bound'): + binding.edited({'name': 'child', 'parent': 'other'}) + editor = binding.edited(ForkCliCommand.editor_arguments({'name': 'child'})) assert editor.tags == InheritedTags() # A real later parent record, not the stale tags at editor opening, supplies membership. current = comms.registry.declare(replace(parent, tags=frozenset({'current'}))) From 8c08089255bd42c5ce32f88249e8c01c6fa72b73 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 17:52:02 -0400 Subject: [PATCH 06/26] Remove invalid trailing comma from pending context edits --- src/agent_comms/cli_command.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index 08e274f0e5..760d788bfe 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -182,7 +182,7 @@ def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: self.authorize_edits(arguments) context_fields = {entry.key for entry in FieldCodec.argument_fields(type(self.context))} wire = FieldCodec.editor_arguments(type(self.context), { - key: value for key, value in arguments.items() if key in context_fields, + key: value for key, value in arguments.items() if key in context_fields }) changes = self.decoded_edits(type(self.context), wire) return replace(self, context=replace(self.context, **changes)) if changes else self From acf0a67b6d696f38c65bc2efd90347e1b5933a65 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 18:21:09 -0400 Subject: [PATCH 07/26] Keep native union members and derive their editor transactions --- src/agent_comms/cli_command.py | 7 +- src/agent_comms/field_codec.py | 195 ++++++++++++++++++++------- src/agent_comms/target_fields.py | 53 +++++++- src/agent_comms/thread_management.py | 3 +- tests/test_editor_defaults.py | 62 ++++++++- 5 files changed, 257 insertions(+), 63 deletions(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index 760d788bfe..d06b70a9c6 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -118,9 +118,10 @@ def decoded_edits(declaration: type, arguments: Mapping[str, object]) -> dict[st def edited(self, arguments: Mapping[str, object]) -> CliCommand: self.authorize_edits(arguments) - return FieldCodec.construct(self.declaration, { - **self.retained_payload(), **self.decoded_edits(self.declaration, arguments), - }) + values = {**self.retained_payload(), **self.decoded_edits(self.declaration, arguments)} + for parameter in self.editable_fields: + parameter.plan.require_constructor_input(values) + return FieldCodec.construct(self.declaration, values) @abstractmethod def confirmation(self) -> str: ... diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index 776c48255e..589d084a8f 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -12,7 +12,7 @@ import types from abc import ABC, abstractmethod from collections.abc import Callable, Iterable, Iterator, Mapping -from dataclasses import MISSING, Field, dataclass, field, fields, is_dataclass +from dataclasses import MISSING, Field, dataclass, field, fields, is_dataclass, make_dataclass from datetime import datetime from enum import Enum, IntFlag from functools import cache, cached_property, lru_cache, partial, reduce @@ -369,10 +369,6 @@ class Normalize(ValueDirective[Callable[[object], object]]): pass -class EditsAsText: - """This member declares the typed editor of its untagged wire family.""" - - class Editor(ValueDirective[type]): pass @@ -625,9 +621,16 @@ def argument_payload(self, value: object) -> object: def editor_payload(self, value: object) -> object: return self.editor_normalizer(value) - def editor_default(self, value: object) -> JsonValue: + def editor_default(self, value: object) -> str: return self.value.encode_editor(value) + def editor_initial(self, value: object) -> object: + return self.value.editor_initial(value) + + def require_constructor_input(self, values: Mapping[str, object]) -> None: + if self.constructor_required and self.name not in values: + raise MalformedRecord(f'Missing required field {self.key}') + @property def description(self) -> str: return Argument.declared(self.annotation).description(self.name) @@ -957,7 +960,34 @@ class DecoderPlan(ABC): def editor_annotation(self) -> object: return str - def format_editor(self, wire: object) -> JsonValue: + @property + def native_editor_annotation(self) -> object: + return self.editor_annotation + + @property + def editor_members(self) -> tuple[EditorMember, ...]: + return () + + def editor_member(self, declared: DecoderPlan) -> EditorMember: + return ScalarEditorMember(declared) + + def selected_editor_member(self, value: object) -> EditorMember: + raise TypeError(f'{self.target} does not declare a union editor') + + def project_editor(self, native: object, wire: Callable[[], object]) -> object: + return self.format_editor(wire()) + + def editor_argument(self, value: object) -> object: + return value + + def accepts_value(self, value: object) -> bool: + """Select a native member at this decoder boundary.""" + return isinstance(value, self.target) + + @abstractmethod + def decode_bound(self, data: object, context: Mapping[str, object]) -> object: ... + + def format_editor(self, wire: object) -> str: return json.dumps(wire) @boundary_decoder @@ -995,6 +1025,73 @@ def external_decoders(self, data: object) -> Iterable[ExternalDecoder]: return () +@dataclass(frozen=True) +class EditorMember(ABC): + """Read-only member metadata; only actual inputs enter a form's ObjectState.""" + + decoder: DecoderPlan + + @property + def declaration(self) -> object: + return self.decoder.target + + @property + def label(self) -> str: + return str(self.declaration) + + @property + @abstractmethod + def fields(self) -> tuple[FieldPlan, ...]: ... + + def current(self, entry: FieldPlan, value: object) -> object: + return value + + @abstractmethod + def wire_payload(self, arguments: Mapping[str, object]) -> object: ... + + def complete(self, arguments: Mapping[str, object]) -> object: + return self.decoder.decode_bound(self.wire_payload(arguments), {}) + + +class ScalarEditorMember(EditorMember): + @cached_property + def fields(self) -> tuple[FieldPlan, ...]: + declaration = make_dataclass('MemberInput', [('value', self.declaration)], + frozen=True, kw_only=True) + return FieldCodec.field_plans(declaration) + + def wire_payload(self, arguments: Mapping[str, object]) -> object: + entry, = self.fields + return entry.editor_payload(entry.raw(dict(arguments))) + + +@dataclass(frozen=True) +class RecordEditorMember(EditorMember): + plans: tuple[FieldPlan, ...] + + @property + def fields(self) -> tuple[FieldPlan, ...]: + return self.plans + + def current(self, entry: FieldPlan, value: object) -> object: + return entry.current(value) + + def wire_payload(self, arguments: Mapping[str, object]) -> object: + return {entry.key: entry.editor_payload(arguments[entry.key]) + for entry in self.fields if entry.key in arguments} + + +class NullEditorMember(EditorMember): + @property + def fields(self) -> tuple[FieldPlan, ...]: + return () + + def wire_payload(self, arguments: Mapping[str, object]) -> object: + if arguments: + raise MalformedRecord('A null member has no editor inputs') + return None + + @dataclass(frozen=True, slots=True) class ValueDecoder[T](DecoderPlan, JsonSchemaOwner): """One annotation owns its wire conversion, draft projection and choice domain.""" @@ -1062,11 +1159,6 @@ def representation_plan( def for_members[M](cls, members: tuple[type[M], ...]) -> UnionDecoder[M]: target = reduce(or_, members) alternatives = tuple(compile_decoder(member) for member in members) - editors = tuple(member for member in members if EditsAsText in member.__mro__) - if len(editors) > 1: - raise TypeError(f'{target} declares more than one typed-text editor: {editors}') - if editors: - return _TextEditedUnionDecoder(target, alternatives, compile_decoder(editors[0]).editor_wire) return UnionDecoder(target, alternatives) @abstractmethod @@ -1107,10 +1199,6 @@ def decode_resolved(self, value: T) -> T: raise MalformedRecord(f"Resolved value does not match {self.target}") return value - def accepts_value(self, value: object) -> bool: - """Select an in-process union case using its compiled declaration.""" - return isinstance(value, self.target) - def accepts_external_value(self, value: object) -> bool: """A structural container match does not validate its contents.""" return False @@ -1138,11 +1226,14 @@ def decode_keys(self, data, keys, selections): def editor_wire(self) -> DecoderPlan: return self - def encode_editor(self, value: T) -> JsonValue: - return self.format_editor(self.encode(value)) + def editor_initial(self, value: T) -> object: + return self.editor_wire.project_editor(value, lambda: self.encode(value)) + + def encode_editor(self, value: T) -> str: + return self.editor_wire.format_editor(self.encode(value)) def decode_editor(self, value: object) -> object: - return self.parse_editor(value) + return self.editor_wire.parse_editor(value) def editor_choices(self) -> tuple[tuple[str, object], ...]: raise TypeError("The declared decoder does not offer finite choices") @@ -1181,16 +1272,6 @@ def wire_structure(self) -> DecoderPlan: def editor_wire(self) -> DecoderPlan: return self.wire_structure - @property - def editor_annotation(self) -> object: - return self.wire_structure.editor_annotation - - def format_editor(self, wire: object) -> JsonValue: - return self.wire_structure.format_editor(wire) - - def parse_editor(self, text: object) -> object: - return self.wire_structure.parse_editor(text) - @property def accepts_null(self) -> bool: return self.wire_structure.accepts_null @@ -2780,7 +2861,13 @@ def fits_wire(self, data: object) -> bool: return self.accepts(data) def accepts_value(self, value: object) -> bool: - return self.accepts(value) + return type(value) is self.target + + def parse_editor(self, text: object) -> object: + value = super().parse_editor(text) + if not self.accepts_value(value): + raise MalformedRecord(f'Expected native {self.target}, received {type(value).__name__}') + return value def accepts_external_value(self, value: object) -> bool: return self.accepts(value) @@ -2806,6 +2893,9 @@ class _NullDecoder(_ExactScalarDecoder[None]): annotations = (type(None),) json_type = "null" + def editor_member(self, declared: DecoderPlan) -> EditorMember: + return NullEditorMember(declared) + def sql_checks(self, column: str, quote: Callable[[object], str]) -> tuple[str, ...]: return (f"{column} IS NULL",) @@ -2818,8 +2908,8 @@ class _TextDecoder(_ExactScalarDecoder[str]): def encode(self, value: str) -> str: return str(super().encode(value)) - def format_editor(self, wire: object) -> JsonValue: - return cast(JsonValue, wire) + def format_editor(self, wire: object) -> str: + return self.decode(wire) def parse_editor(self, text: object) -> object: return text @@ -3050,13 +3140,31 @@ def decode_external(self, data: object, record: Mapping[str, object]) -> T: def editor_annotation(self): return Union[tuple(item.editor_annotation for item in self.alternatives)] + @property + def native_editor_annotation(self) -> object: + return self.target + + @property + def editor_members(self) -> tuple[EditorMember, ...]: + return tuple(member.editor_wire.editor_member(member) for member in self.alternatives) + + def selected_editor_member(self, value: object) -> EditorMember: + member = self.selected(value) + return member.editor_wire.editor_member(member) + + def project_editor(self, native: object, wire: Callable[[], object]) -> object: + return native + + def editor_argument(self, value: object) -> object: + return self.format_editor(FieldCodec.encode(value, self.target)) + def argument_options(self, argument: Argument) -> dict[str, object]: return self.alternatives[0].argument_options(argument) def accepts_value(self, value: object) -> bool: return any(alternative.accepts_value(value) for alternative in self.alternatives) - def selected(self, value: T) -> ValueDecoder[T]: + def selected(self, value: object) -> ValueDecoder[T]: for alternative in self.alternatives: if alternative.accepts_value(value): return alternative @@ -3113,24 +3221,6 @@ def decode_candidates( first_error = None -@dataclass(frozen=True, slots=True) -class _TextEditedUnionDecoder[T](UnionDecoder[T]): - """One member's declaration owns typed text; other members remain wire choices.""" - - editor: DecoderPlan - - @property - def editor_annotation(self) -> object: - annotation = self.editor.editor_annotation - return Union[tuple((annotation, type(None)))] if self.accepts_null else annotation - - def format_editor(self, wire: object) -> JsonValue: - return self.editor.format_editor(wire) - - def parse_editor(self, text: object) -> object: - return self.editor.parse_editor(text) - - @dataclass(frozen=True, slots=True) class _ArrayDecoder[T: Iterable[object]](ValueDecoder[T]): container: ClassVar[type] @@ -3465,6 +3555,9 @@ class _RecordDecoder[T](_ContextDecoder[T]): def plan(self) -> FieldCodec._RecordPlan: return FieldCodec._record_plan(self.target) + def editor_member(self, declared: DecoderPlan) -> EditorMember: + return RecordEditorMember(declared, self.plan.bindings) + def fits_wire(self, data: object) -> TypeGuard[dict[str, object]]: if not isinstance(data, dict): return False diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index aa40e9b7a9..29e4adda58 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -39,7 +39,7 @@ def annotation(self) -> object: @property def editor_annotation(self) -> object: - return field_codec.compile_decoder(self.annotation).editor_annotation + return self.plan.value.editor_wire.native_editor_annotation @property def name(self) -> str: @@ -53,15 +53,29 @@ def multiline(self) -> bool: def description(self) -> str: return self.plan.description + @property + def editor_members(self) -> tuple[field_codec.EditorMember, ...]: + return self.plan.value.editor_wire.editor_members + + def editor_selection(self, value: object) -> object: + return self.plan.value.editor_wire.selected_editor_member(value).declaration + + @property + @abstractmethod + def editor_initial(self) -> object: ... + @property @abstractmethod - def editor_default(self) -> JsonValue: ... + def editor_default(self) -> str | None: ... @abstractmethod def default_schema(self) -> dict[str, JsonValue]: ... def argument(self, value: object) -> Mapping[str, object]: - return {self.name: value} + return {self.name: self.editor_argument(value)} + + def editor_argument(self, value: object) -> object: + return self.plan.value.editor_wire.editor_argument(value) def editor_hints(self, choices: Callable[[ChoiceTargetField], object]) -> tuple[object, ...]: """An unrestricted field does not declare a finite-choice editor.""" @@ -91,7 +105,11 @@ class SuppliedTargetField(TargetField): value: object @property - def editor_default(self) -> JsonValue: + def editor_initial(self) -> object: + return self.plan.editor_initial(self.value) + + @property + def editor_default(self) -> str: return self.plan.editor_default(self.value) def default_schema(self) -> dict[str, JsonValue]: @@ -104,15 +122,23 @@ class UnsuppliedTargetField(TargetField): @property def editor_annotation(self) -> object: - return field_codec.compile_decoder(self.annotation).editor_annotation | type(None) + return self.plan.value.editor_wire.native_editor_annotation | type(None) + + @property + def editor_initial(self) -> object: + return None @property - def editor_default(self) -> JsonValue: + def editor_default(self) -> None: return None def default_schema(self) -> dict[str, JsonValue]: return {} + @boundary_decoder + def editor_selection(self, value: object) -> object: + return None if value is None else super().editor_selection(value) + @boundary_decoder def argument(self, value: object) -> Mapping[str, object]: """Decode the optional control's raw presence once, not the field's wire value.""" @@ -124,6 +150,21 @@ class ChoiceTargetField(SuppliedTargetField): """A prepared finite declaration, from its annotation or configuration catalog.""" options: tuple[CatalogChoice, ...] = () + @property + def editor_annotation(self) -> object: + return self.plan.value.editor_wire.editor_annotation + + @property + def editor_members(self) -> tuple[field_codec.EditorMember, ...]: + return () + + @property + def editor_initial(self) -> object: + return self.editor_default + + def editor_argument(self, value: object) -> object: + return value + @property def editor_declarations(self) -> tuple[object, ...]: return () diff --git a/src/agent_comms/thread_management.py b/src/agent_comms/thread_management.py index 5be0b40458..cddd5e9f97 100644 --- a/src/agent_comms/thread_management.py +++ b/src/agent_comms/thread_management.py @@ -38,7 +38,6 @@ from .errors import RelationViolationError from .field_codec import ( DeclaredWireType, - EditsAsText, FieldCodec, NullMember, RecordMember, @@ -78,7 +77,7 @@ def for_child(self, parent: frozenset[str]) -> frozenset[str]: ... @dataclass(frozen=True, slots=True) -class GivenTags(EditsAsText, DeclaredWireType, TagChoice): +class GivenTags(DeclaredWireType, TagChoice): """Tags spelled as comma-separated text on the command line, in the editor and in tools.""" wire_type = str diff --git a/tests/test_editor_defaults.py b/tests/test_editor_defaults.py index c4b6e38e8a..1ce950cf2e 100644 --- a/tests/test_editor_defaults.py +++ b/tests/test_editor_defaults.py @@ -4,7 +4,9 @@ import pytest from agent_comms.cli_verbs.fork import ForkCliCommand -from agent_comms.field_codec import FieldCodec, compile_decoder +from agent_comms.field_codec import FieldCodec, MalformedRecord, compile_decoder +from agent_comms.target_fields import TargetField +from agent_comms.thread_management import ChangedTags, GivenTags, InheritedTags, TagChoice from agent_comms.pi_vocabulary import HighThinkingLevel, NoThinkingOverride, ThinkingLevelField from agent_comms.repo import LaunchCheckout from agent_comms.thread_execution import ExternalThreadExecution, ThreadExecution @@ -19,6 +21,10 @@ (str | None, None, None), (bool, True, True), (tuple[int, ...], (1, 2), [1, 2]), + (TagChoice, GivenTags(frozenset({'auth'})), 'auth'), + (TagChoice, InheritedTags(), None), + (TagChoice, ChangedTags(frozenset({'auth'}), frozenset({'old'})), + {'add': ['auth'], 'remove': ['old']}), )) def test_declared_default_editor_round_trip(annotation, value, encoded): decoder = compile_decoder(annotation) @@ -41,3 +47,57 @@ def test_fork_default_retains_annotated_thinking_representation(source_repositor assert compile_decoder(str).encode_editor('literal text') == 'literal text' +@pytest.mark.parametrize('annotation,value', ( + (TagChoice, GivenTags(frozenset())), + (TagChoice, GivenTags(frozenset({'auth'}))), + (TagChoice, InheritedTags()), + (TagChoice, ChangedTags(frozenset({'auth'}), frozenset({'old'}))), + (int | float, 3), + (int | float, 3.0), + (int | float | None, None), +)) +def test_native_union_member_and_field_derived_transaction(annotation: object, value: object) -> None: + decoder = compile_decoder(annotation) + assert decoder.editor_initial(value) is value + member = decoder.editor_wire.selected_editor_member(value) + assert member.declaration is type(value) + assert tuple(item.declaration for item in decoder.editor_wire.editor_members).count(type(value)) == 1 + parameters = tuple(TargetField.of(entry, member.current(entry, value)) for entry in member.fields) + arguments = {key: item for parameter in parameters + for key, item in parameter.argument(parameter.editor_initial).items()} + result = member.complete(arguments) + assert type(result) is type(value) + assert result == value + json.dumps(decoder.encode_editor(value)) + assert decoder.decode_editor(decoder.editor_wire.editor_argument(value)) == FieldCodec.encode(value, annotation) + + +def test_overlapping_scalar_wire_does_not_change_selected_native_member() -> None: + decoder = compile_decoder(int | float) + member = decoder.editor_wire.selected_editor_member(3.0) + assert member.declaration is float + assert member.complete({'value': '3.0'}) == 3.0 + with pytest.raises(MalformedRecord): + member.complete({'value': '3'}) + with pytest.raises(MalformedRecord): + FieldCodec.decode(int | float, 3) + + +def test_native_union_default_does_not_leak_into_public_json_schema() -> None: + plan = next(entry for entry in FieldCodec.argument_fields(ForkCliCommand) if entry.name == 'tags') + value = ChangedTags(frozenset({'auth'}), frozenset()) + parameter = TargetField.of(plan, value) + assert parameter.editor_initial is value + assert parameter.editor_selection(value) is ChangedTags + assert isinstance(parameter.editor_default, str) + json.dumps(parameter.json_schema()) + + +def test_record_member_inputs_remain_required_without_invented_values() -> None: + decoder = compile_decoder(TagChoice) + member = decoder.editor_wire.selected_editor_member(ChangedTags(frozenset(), frozenset())) + assert all(entry.constructor_required for entry in member.fields) + with pytest.raises(MalformedRecord): + member.complete({}) + + From 7d3d6ac4f9e8c70271d73198691b2abe15dc467a Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 18:40:03 -0400 Subject: [PATCH 08/26] Make unchosen a nominal member of editor selection --- src/agent_comms/field_codec.py | 42 +++++++++++++++++++++++++++++++- src/agent_comms/target_fields.py | 23 ++++++++++++++--- tests/test_editor_defaults.py | 23 +++++++++++++++-- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index 589d084a8f..19f69af185 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -1025,8 +1025,45 @@ def external_decoders(self, data: object) -> Iterable[ExternalDecoder]: return () +class MemberDraftFactory[Draft](ABC): + """An integration constructs drafts; selections own which construction applies.""" + + @abstractmethod + def unchosen(self, member: UnchosenMember) -> Draft: ... + + @abstractmethod + def member(self, member: EditorMember, value: object) -> Draft: ... + + +class MemberSelection(ABC): + @property + @abstractmethod + def declaration(self) -> object: ... + + @property + @abstractmethod + def label(self) -> str: ... + + @abstractmethod + def draft[Draft](self, factory: MemberDraftFactory[Draft], value: object) -> Draft: ... + + +@dataclass(frozen=True) +class UnchosenMember(MemberSelection): + @property + def declaration(self) -> object: + return type(self) + + @property + def label(self) -> str: + return 'Choose a member…' + + def draft[Draft](self, factory: MemberDraftFactory[Draft], value: object) -> Draft: + return factory.unchosen(self) + + @dataclass(frozen=True) -class EditorMember(ABC): +class EditorMember(MemberSelection): """Read-only member metadata; only actual inputs enter a form's ObjectState.""" decoder: DecoderPlan @@ -1039,6 +1076,9 @@ def declaration(self) -> object: def label(self) -> str: return str(self.declaration) + def draft[Draft](self, factory: MemberDraftFactory[Draft], value: object) -> Draft: + return factory.member(self, value) + @property @abstractmethod def fields(self) -> tuple[FieldPlan, ...]: ... diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index 29e4adda58..782edb78d0 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -3,6 +3,7 @@ from abc import ABC, abstractmethod from collections.abc import Callable, Mapping from dataclasses import dataclass, replace +from functools import singledispatchmethod from pydantic import JsonValue @@ -57,8 +58,12 @@ def description(self) -> str: def editor_members(self) -> tuple[field_codec.EditorMember, ...]: return self.plan.value.editor_wire.editor_members - def editor_selection(self, value: object) -> object: - return self.plan.value.editor_wire.selected_editor_member(value).declaration + @property + def editor_selections(self) -> tuple[field_codec.MemberSelection, ...]: + return self.editor_members + + def editor_selection(self, value: object) -> field_codec.MemberSelection: + return self.plan.value.editor_wire.selected_editor_member(value) @property @abstractmethod @@ -135,9 +140,19 @@ def editor_default(self) -> None: def default_schema(self) -> dict[str, JsonValue]: return {} + @property + def editor_selections(self) -> tuple[field_codec.MemberSelection, ...]: + return (field_codec.UnchosenMember(), *super().editor_selections) + + @singledispatchmethod + @boundary_decoder + def editor_selection(self, value: object) -> field_codec.MemberSelection: + return super().editor_selection(value) + + @editor_selection.register @boundary_decoder - def editor_selection(self, value: object) -> object: - return None if value is None else super().editor_selection(value) + def _unchosen_selection(self, value: None) -> field_codec.MemberSelection: + return field_codec.UnchosenMember() @boundary_decoder def argument(self, value: object) -> Mapping[str, object]: diff --git a/tests/test_editor_defaults.py b/tests/test_editor_defaults.py index 1ce950cf2e..9a2a1dd2fa 100644 --- a/tests/test_editor_defaults.py +++ b/tests/test_editor_defaults.py @@ -4,7 +4,9 @@ import pytest from agent_comms.cli_verbs.fork import ForkCliCommand -from agent_comms.field_codec import FieldCodec, MalformedRecord, compile_decoder +from dataclasses import dataclass + +from agent_comms.field_codec import FieldCodec, MalformedRecord, UnchosenMember, compile_decoder from agent_comms.target_fields import TargetField from agent_comms.thread_management import ChangedTags, GivenTags, InheritedTags, TagChoice from agent_comms.pi_vocabulary import HighThinkingLevel, NoThinkingOverride, ThinkingLevelField @@ -88,11 +90,28 @@ def test_native_union_default_does_not_leak_into_public_json_schema() -> None: value = ChangedTags(frozenset({'auth'}), frozenset()) parameter = TargetField.of(plan, value) assert parameter.editor_initial is value - assert parameter.editor_selection(value) is ChangedTags + assert parameter.editor_selection(value).declaration is ChangedTags assert isinstance(parameter.editor_default, str) json.dumps(parameter.json_schema()) +def test_unchosen_selection_is_not_a_supplied_null_member() -> None: + @dataclass(frozen=True) + class RequiredUnion: + value: int | float | None + + plan, = FieldCodec.argument_fields(RequiredUnion) + omitted = TargetField.omitted(plan) + unchosen = omitted.editor_selection(omitted.editor_initial) + assert isinstance(unchosen, UnchosenMember) + assert unchosen.declaration is UnchosenMember + assert unchosen.label == 'Choose a member…' + assert unchosen.declaration in tuple(member.declaration for member in omitted.editor_selections) + assert omitted.editor_selection(3).declaration is int + supplied_null = TargetField.of(plan, None).editor_selection(None) + assert supplied_null.declaration is type(None) + + def test_record_member_inputs_remain_required_without_invented_values() -> None: decoder = compile_decoder(TagChoice) member = decoder.editor_wire.selected_editor_member(ChangedTags(frozenset(), frozenset())) From 502402a5294b42bcd22dea4250588b0ef43b9f91 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 19:04:38 -0400 Subject: [PATCH 09/26] Ask the value decoder directly for editor projections --- src/agent_comms/field_codec.py | 6 ------ src/agent_comms/target_fields.py | 4 ++-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index 19f69af185..6cf5718230 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -621,12 +621,6 @@ def argument_payload(self, value: object) -> object: def editor_payload(self, value: object) -> object: return self.editor_normalizer(value) - def editor_default(self, value: object) -> str: - return self.value.encode_editor(value) - - def editor_initial(self, value: object) -> object: - return self.value.editor_initial(value) - def require_constructor_input(self, values: Mapping[str, object]) -> None: if self.constructor_required and self.name not in values: raise MalformedRecord(f'Missing required field {self.key}') diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index 782edb78d0..5cd068266e 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -111,11 +111,11 @@ class SuppliedTargetField(TargetField): @property def editor_initial(self) -> object: - return self.plan.editor_initial(self.value) + return self.plan.value.editor_initial(self.value) @property def editor_default(self) -> str: - return self.plan.editor_default(self.value) + return self.plan.value.encode_editor(self.value) def default_schema(self) -> dict[str, JsonValue]: return {'editor_default': self.editor_default} From 85a31ac9872b949efa02f48a48b29ced057d8ee5 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 21:18:34 -0400 Subject: [PATCH 10/26] Remove absent editor defaults and reuse scalar admission --- src/agent_comms/field_codec.py | 2 +- src/agent_comms/target_fields.py | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index 6cf5718230..a852d7e989 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -2895,7 +2895,7 @@ def fits_wire(self, data: object) -> bool: return self.accepts(data) def accepts_value(self, value: object) -> bool: - return type(value) is self.target + return self.accepts(value) def parse_editor(self, text: object) -> object: value = super().parse_editor(text) diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index 5cd068266e..48192859a6 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -69,10 +69,6 @@ def editor_selection(self, value: object) -> field_codec.MemberSelection: @abstractmethod def editor_initial(self) -> object: ... - @property - @abstractmethod - def editor_default(self) -> str | None: ... - @abstractmethod def default_schema(self) -> dict[str, JsonValue]: ... @@ -133,10 +129,6 @@ def editor_annotation(self) -> object: def editor_initial(self) -> object: return None - @property - def editor_default(self) -> None: - return None - def default_schema(self) -> dict[str, JsonValue]: return {} From f4f0750598f7f51ae0799f122cce4b87fabd2523 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 03:01:03 -0400 Subject: [PATCH 11/26] Separate explicit new-thread tags from parent-derived fork membership --- .../cli_support/thread_creation.py | 53 ++++++++++++++++ src/agent_comms/cli_verbs/fork.py | 14 ++--- src/agent_comms/cli_verbs/new.py | 62 ++----------------- src/agent_comms/thread_management.py | 37 ++++++----- tests/test_cli.py | 5 +- 5 files changed, 89 insertions(+), 82 deletions(-) create mode 100644 src/agent_comms/cli_support/thread_creation.py diff --git a/src/agent_comms/cli_support/thread_creation.py b/src/agent_comms/cli_support/thread_creation.py new file mode 100644 index 0000000000..6c34493c72 --- /dev/null +++ b/src/agent_comms/cli_support/thread_creation.py @@ -0,0 +1,53 @@ +"""The launch settings and field-derived construction shared by new and fork.""" +from __future__ import annotations + +import asyncio +from abc import abstractmethod +from dataclasses import dataclass, fields +from pathlib import Path +from typing import Annotated + +from agent_comms.cli_command import CliCommand +from agent_comms.cli_commands import NameArgument, ThreadCreatedResult, _tag_choice_text +from agent_comms.comms import Comms +from agent_comms.config_options import ConfigOptions, ModelConfigOption, ThinkingLevelConfigOption +from agent_comms.field_codec import Argument, Catalog, EditorFormat, FieldCodec, Multiline, Normalize +from agent_comms.native_settings import NativeThreadDefaults +from agent_comms.pi_vocabulary import ThinkingLevelField +from agent_comms.repo import Checkout, LaunchCheckout +from agent_comms.thread_management import ThreadCreationSpec +from agent_comms.thread_model import CatalogSelection, ThreadModel + + +type TagArgument[T] = Annotated[ + T, EditorFormat(FieldCodec.encode), Normalize(_tag_choice_text), Argument(flags=("--tags",)), +] + + +@dataclass(frozen=True, kw_only=True) +class ThreadCreationCliCommand(NameArgument, CatalogSelection, CliCommand, declared_name=None): + task: Annotated[str, Multiline(), Argument(flags=("--task",))] = "" + project: Annotated[Checkout, Argument(flags=("--project",))] + model: Annotated[ + ThreadModel, Catalog(ModelConfigOption), Argument(flags=("--model",)), + ] = NativeThreadDefaults.model + thinking_level: Annotated[ + ThinkingLevelField, Catalog(ThinkingLevelConfigOption), Argument(flags=("--thinking-level",)), + ] = NativeThreadDefaults.thinking_level + + @property + @abstractmethod + def spec_type(self) -> type[ThreadCreationSpec]: ... + + @property + def project_path(self) -> Path: + return self.project.path + + def apply(self, ctx: Comms) -> ThreadCreatedResult: + spec = self.spec_type.from_task(**{ + **{item.name: getattr(self, item.name) for item in fields(self.spec_type)}, + 'project': LaunchCheckout.checked(self.project.path), + }) + asyncio.run(ConfigOptions.configured().validate(self)) + child = ctx.threads.create(spec) + return ThreadCreatedResult(child.name, child.process_identity) diff --git a/src/agent_comms/cli_verbs/fork.py b/src/agent_comms/cli_verbs/fork.py index b36253a47a..e6b9c1637f 100644 --- a/src/agent_comms/cli_verbs/fork.py +++ b/src/agent_comms/cli_verbs/fork.py @@ -1,33 +1,29 @@ from __future__ import annotations -from agent_comms.cli_verbs.new import NewThreadCliCommand +from agent_comms.cli_support.thread_creation import TagArgument, ThreadCreationCliCommand from agent_comms.cli_support.thread_bound import ThreadBoundCliCommand -from agent_comms.comms import Comms from agent_comms.field_codec import Argument from agent_comms.field_codec import TargetBound from agent_comms.thread_execution import NativeOwnerCommand from agent_comms.thread_management import ForkSpec -from agent_comms.thread_management import GivenTags +from agent_comms.thread_management import InheritedTags, TagChoice from agent_comms.threads import Thread -from agent_comms.repo import Checkout from dataclasses import dataclass from typing import Annotated from typing import Self @dataclass(frozen=True, kw_only=True) -class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, NewThreadCliCommand): +class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ThreadCreationCliCommand): help = "Fork a child pi thread" spec_type = ForkSpec parent: Annotated[ str, TargetBound(), Argument(flags=("--parent",)) ] - @classmethod - def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[Self, ...]: - return () + tags: TagArgument[TagChoice] = InheritedTags() @classmethod def thread_binding(cls, thread: Thread) -> Self: - return cls(name='', parent=thread.name, tags=GivenTags(thread.tags), project=thread.worktree, + return cls(name='', parent=thread.name, project=thread.worktree, model=thread.model, thinking_level=thread.thinking_level) diff --git a/src/agent_comms/cli_verbs/new.py b/src/agent_comms/cli_verbs/new.py index d24a4b42a9..e7661e5b44 100644 --- a/src/agent_comms/cli_verbs/new.py +++ b/src/agent_comms/cli_verbs/new.py @@ -1,76 +1,24 @@ from __future__ import annotations -from agent_comms.cli_command import CliCommand -from agent_comms.cli_commands import NameArgument -from agent_comms.cli_commands import ThreadCreatedResult -from agent_comms.cli_commands import _tag_choice_text +from agent_comms.cli_support.thread_creation import TagArgument, ThreadCreationCliCommand from agent_comms.comms import Comms -from agent_comms.config_options import ConfigOptions -from agent_comms.config_options import ModelConfigOption -from agent_comms.config_options import ThinkingLevelConfigOption -from agent_comms.field_codec import Argument -from agent_comms.field_codec import Catalog -from agent_comms.field_codec import EditorFormat -from agent_comms.field_codec import FieldCodec -from agent_comms.field_codec import Multiline -from agent_comms.field_codec import Normalize from agent_comms.native_settings import NativeThreadDefaults -from agent_comms.repo import Checkout, LaunchCheckout -from agent_comms.pi_vocabulary import ThinkingLevelField -from agent_comms.thread_management import InheritedTags +from agent_comms.repo import Checkout +from agent_comms.thread_management import GivenTags from agent_comms.thread_management import NewThreadSpec -from agent_comms.thread_management import TagChoice -from agent_comms.thread_model import CatalogSelection, ThreadModel -from pathlib import Path from dataclasses import dataclass -from dataclasses import fields -from typing import Annotated from typing import ClassVar from typing import Self -import asyncio @dataclass(frozen=True, kw_only=True) -class NewThreadCliCommand(NameArgument, CatalogSelection, CliCommand, declared_name="new"): +class NewThreadCliCommand(ThreadCreationCliCommand, declared_name="new"): help = "Create a new pi thread" spec_type: ClassVar[type[NewThreadSpec]] = NewThreadSpec - task: Annotated[ - str, Multiline(), Argument(flags=("--task",)) - ] = "" - tags: Annotated[ - TagChoice, - EditorFormat(FieldCodec.encode), - Normalize(_tag_choice_text), - Argument(flags=("--tags",)), - ] = InheritedTags() - project: Annotated[ - Checkout, - Argument(flags=("--project",)), - ] - model: Annotated[ - ThreadModel, Catalog(ModelConfigOption), Argument(flags=("--model",)), - ] = NativeThreadDefaults.model - thinking_level: Annotated[ - ThinkingLevelField, - Catalog(ThinkingLevelConfigOption), - Argument(flags=("--thinking-level",)), - ] = NativeThreadDefaults.thinking_level + tags: TagArgument[GivenTags] @classmethod def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[Self, ...]: defaults = NativeThreadDefaults() return (cls(name='', project=project, model=defaults.model, thinking_level=defaults.thinking_level),) - - @property - def project_path(self) -> Path: - return self.project.path - - def apply(self, ctx: Comms) -> ThreadCreatedResult: - spec = self.spec_type.from_task(**{ - **{item.name: getattr(self, item.name) for item in fields(self.spec_type)}, - 'project': LaunchCheckout.checked(self.project.path), - }) - asyncio.run(ConfigOptions.configured().validate(self)) - child = ctx.threads.create(spec) - return ThreadCreatedResult(child.name, child.process_identity) diff --git a/src/agent_comms/thread_management.py b/src/agent_comms/thread_management.py index 2f79f5b457..cddd5e9f97 100644 --- a/src/agent_comms/thread_management.py +++ b/src/agent_comms/thread_management.py @@ -4,7 +4,7 @@ import asyncio import re -from abc import abstractmethod +from abc import ABC, abstractmethod from collections.abc import Sequence from contextlib import ExitStack from dataclasses import dataclass, field, replace @@ -72,9 +72,6 @@ class RenameThreadResult(ToolResult): class TagChoice(SubclassUnion, affix="Tags"): """Select explicit, inherited or changed tags from an existing set.""" - def for_new(self) -> frozenset[str]: - return self.for_child(frozenset()) - @abstractmethod def for_child(self, parent: frozenset[str]) -> frozenset[str]: ... @@ -144,27 +141,36 @@ def local_directory(self) -> Path: @dataclass(frozen=True, slots=True, kw_only=True) -class NewThreadSpec: - """Prepare the session, then register the thread under its registry lock.""" +class ThreadCreationSpec(ABC): + """Shared launch settings and construction after membership has been decided.""" name: str project: LaunchCheckout model: ThreadModel thinking_level: ThinkingLevelField task: LaunchInput = field(default_factory=LaunchInput) - tags: TagChoice = InheritedTags() @classmethod - def from_task(cls, *, task: str = "", **values) -> NewThreadSpec: + def from_task(cls, *, task: str = "", **values) -> Self: return cls(task=TaskLaunchInput(task) if task else LaunchInput(), **values) - def declaration(self) -> Thread: - return Thread(name=self.name, task=self.task.thread_text(), tags=self.tags.for_new(), + def declaration(self, tags: frozenset[str]) -> Thread: + return Thread(name=self.name, task=self.task.thread_text(), tags=tags, worktree=self.project, model=self.model, thinking_level=self.thinking_level) + @abstractmethod + def declare(self, threads: ThreadManagement, pi_bin: str, runtime: LaunchRuntime) -> Thread: ... + + +@dataclass(frozen=True, slots=True, kw_only=True) +class NewThreadSpec(ThreadCreationSpec): + """A new thread has no parent; its creator supplies its membership.""" + + tags: GivenTags + def declare(self, threads: ThreadManagement, pi_bin: str, runtime: LaunchRuntime) -> Thread: project = self.project.path - thread = self.declaration() + thread = self.declaration(self.tags.tags) threads.registry.require_declaration_available(thread) location = thread.runtime.new_location(threads.root, project) saved = RegistrySessionLocation.from_path(location.read_key, threads.root) @@ -172,10 +178,11 @@ def declare(self, threads: ThreadManagement, pi_bin: str, runtime: LaunchRuntime @dataclass(frozen=True, slots=True, kw_only=True) -class ForkSpec(NewThreadSpec): +class ForkSpec(ThreadCreationSpec): """A fork differs only in its parent-derived source and inherited settings.""" parent: str + tags: TagChoice = InheritedTags() def _parent(self, threads: ThreadManagement) -> Thread: parent = threads.registry.require(self.parent) @@ -185,8 +192,8 @@ def _parent(self, threads: ThreadManagement) -> Thread: def child(self, parent: Thread, runtime: RuntimeLocation, threads: ThreadManagement) -> Thread: saved = RegistrySessionLocation.from_path(runtime.read_key, threads.root) - return replace(self.declaration(), - parent=ForkedParent(parent.name), tags=self.tags.for_child(parent.tags), + return replace(self.declaration(self.tags.for_child(parent.tags)), + parent=ForkedParent(parent.name), backend=parent.backend, session_file=saved) def declare(self, threads: ThreadManagement, pi_bin: str, runtime: LaunchRuntime) -> Thread: @@ -541,7 +548,7 @@ def _delete_originals(self, originals: Sequence[Thread]) -> tuple[ThreadDeleted, for original in originals ) - def create(self, spec: NewThreadSpec) -> Thread: + def create(self, spec: ThreadCreationSpec) -> Thread: """Prepare the session, then register and launch the thread with its task as first input.""" resolved_bin = ConfiguredBinary().configured() runtime = LaunchRuntime.running() diff --git a/tests/test_cli.py b/tests/test_cli.py index 73d5586a73..e4fda55671 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -670,7 +670,10 @@ def test_result_kind_decides_exit_code_and_navigation_targets(): assert not launched.open_targets() and not batch.open_targets() created = ThreadCreatedResult("child", NoProcess()) assert created.open_targets() == ("child",) - command = NewThreadCliCommand(name="child", project=LaunchCheckout(Path("/project")), model=UndeclaredModel()) + from agent_comms.thread_management import GivenTags + + command = NewThreadCliCommand(name="child", tags=GivenTags(frozenset()), + project=LaunchCheckout(Path("/project")), model=UndeclaredModel()) from agent_comms.channel_targets import ProjectTarget completed = TargetCompleted(ProjectTarget(Checkout(Path("/project"))), command, created) From 51928ec9937bd1e7e54aa11dfd0eedad92d80498 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 03:15:56 -0400 Subject: [PATCH 12/26] Collect required creation inputs through native command bindings --- src/agent_comms/channel_targets.py | 10 +- src/agent_comms/cli_command.py | 156 ++++++++++++++---- .../cli_support/thread_creation.py | 31 ++-- src/agent_comms/cli_verbs/fork.py | 4 +- src/agent_comms/cli_verbs/new.py | 15 +- src/agent_comms/target_fields.py | 8 +- tests/test_checkout_submodules.py | 2 +- tests/test_end_to_end.py | 16 +- tests/test_thread_creation.py | 84 ++++++++++ 9 files changed, 262 insertions(+), 64 deletions(-) create mode 100644 tests/test_thread_creation.py diff --git a/src/agent_comms/channel_targets.py b/src/agent_comms/channel_targets.py index fdea37d61a..0023b5bdc7 100644 --- a/src/agent_comms/channel_targets.py +++ b/src/agent_comms/channel_targets.py @@ -23,7 +23,7 @@ from .audience_manifest import AudiencedMessage, FrozenRecipient from .catalog_document import CatalogAccess, CatalogDocument from .channels import Channel - from .cli_command import CliCommand + from .cli_command import CliCommand, CommandBinding from .cli_result import CliResult from .comms import Comms from .goal_management import Goals @@ -166,7 +166,7 @@ def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], C def bindings[C: CliCommand]( self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[C, ...]: ... + ) -> tuple[CommandBinding, ...]: ... def history_view(self, viewer: DisplayViewer, require_channel: Callable[[str], Channel], channel: type[ChannelHistory], direct: type[DMHistory]) -> HistoryView: @@ -413,7 +413,7 @@ def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], C def bindings[C: CliCommand]( self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved: ResolvedThread, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[C, ...]: + ) -> tuple[CommandBinding, ...]: return declaration.thread_bindings(comms, resolved.thread, resolved.status, channel, catalog=catalog) def validate(self, message: Message) -> None: @@ -562,7 +562,7 @@ def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], C def bindings[C: CliCommand]( self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[C, ...]: + ) -> tuple[CommandBinding, ...]: return declaration.channel_bindings(comms, resolved, project=project, snapshot=snapshot, catalog=catalog) def validate(self, sender: Message | None = None): @@ -653,7 +653,7 @@ def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], C def bindings[C: CliCommand]( self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[C, ...]: + ) -> tuple[CommandBinding, ...]: return declaration.project_bindings(comms, resolved) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index 55778a9a38..f5a6b3bb14 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -16,6 +16,7 @@ from agent_comms.lifecycle_capability import LifecycleCapability, Successful from agent_comms.registry_document import RegistrySnapshot from agent_comms.target_fields import TargetField +from agent_comms.thread_model import CatalogSelection from dataclasses import dataclass, field, replace from pydantic import JsonValue from collections.abc import Mapping @@ -63,15 +64,128 @@ def __init_subclass__(cls) -> None: Answer = TypeVar("Answer", bound=CliResult, covariant=True, default=CliResult) +class CommandBinding(ABC): + """A declared operation can collect fields before a complete command exists.""" + + @property + @abstractmethod + def declaration(self) -> type[CliCommand]: ... + + @property + @abstractmethod + def editable_fields(self) -> tuple[TargetField, ...]: ... + + def for_editor(self, comms: Comms, target: str, project: Checkout) -> Self: + return self + + @property + def catalog_context(self) -> object: + return self + + def for_fields(self, arguments: Mapping[str, object]) -> CommandBinding: + return self.edited(self.declaration.editor_arguments(arguments)) if arguments else self + + async def prepare_fields(self, arguments: Mapping[str, object] = {}) -> tuple[TargetField, ...]: + binding = self.for_fields(arguments) + catalogs = ConfigOptions.configured() + return tuple([await item.prepare(catalogs, binding.catalog_context) + for item in binding.editable_fields]) + + def authorize_edits(self, arguments: Mapping[str, object]) -> frozenset[str]: + bound = frozenset(entry.key for entry in FieldCodec.argument_fields(self.declaration) + if entry.setting(TargetBound, False)) + if bound & arguments.keys(): + raise ValueError("Target-bound parameters cannot be overridden") + if arguments.keys() - {item.name for item in self.editable_fields}: + raise ValueError("Parameters not offered by this target command cannot be overridden") + return bound + + @abstractmethod + def edited(self, arguments: Mapping[str, object]) -> CliCommand: ... + + @abstractmethod + def confirmation(self) -> str: ... + + @classmethod + @abstractmethod + def selection_confirmation(cls, commands: tuple[Self, ...]) -> str: ... + + @abstractmethod + def with_confirmation(self, confirmed: bool) -> CommandBinding: ... + + +@dataclass(frozen=True) +class CommandContext(CatalogSelection): + """A partial command's supplied fields, with native edits decoded by their plans.""" + + def edited(self, arguments: Mapping[str, object]) -> Self: + changes = {name: value + for entry in FieldCodec.argument_fields(type(self)) if entry.key in arguments + for name, value in entry.decoded({ + entry.key: entry.editor_payload(arguments[entry.key]), + }).items()} + return replace(self, **changes) if changes else self + + +@dataclass(frozen=True) +class PendingCommandBinding(CommandBinding): + """Only supplied context, borrowing the concrete command's declared fields.""" + + command: type[CliCommand] + context: CommandContext + + @property + def declaration(self) -> type[CliCommand]: + return self.command + + @property + def catalog_context(self) -> CatalogSelection: + return self.context + + @property + def editable_fields(self) -> tuple[TargetField, ...]: + supplied = {entry.name: entry.current(self.context) + for entry in FieldCodec.argument_fields(type(self.context))} + return tuple(TargetField.of(entry, supplied[entry.name] if entry.name in supplied else entry.default()) + for entry in FieldCodec.argument_fields(self.declaration) + if not entry.setting(TargetBound, False)) + + def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: + return replace(self, context=self.context.edited(arguments)) + + def edited(self, arguments: Mapping[str, object]) -> CliCommand: + self.authorize_edits(arguments) + return self.declaration.from_payload({ + **FieldCodec.encode(self.context), + self.declaration.family_discriminator: FieldCodec.encode(self.declaration), + **arguments, + }) + + def confirmation(self) -> str: + return '' + + @classmethod + def selection_confirmation(cls, commands: tuple[Self, ...]) -> str: + return '' + + def with_confirmation(self, confirmed: bool) -> CliCommand: + return self.edited({}).with_confirmation(confirmed) + + @dataclass(frozen=True, kw_only=True) class CliCommand( ModuleMembers, DeclaredCommand, CliExecution[Answer], + CommandBinding, Generic[Answer], affix="CliCommand", discovery_package="agent_comms.cli_verbs", ): + @property + def declaration(self) -> type[Self]: + return type(self) + def schedule(self, batch: TargetBatchSchedule, comms: Comms) -> None: result = TargetCompleted(batch.target, self, self.apply(comms)) batch.outcomes.append((batch.position, result)) @@ -125,21 +239,21 @@ def selection_channels(cls, channel: ChannelSelection, return channel.channels_for(target) @classmethod - def thread_bindings(cls, comms: Comms, thread: Thread, status: ThreadStatus, channel: SurroundingChannel, *, catalog: CatalogAccess) -> tuple[Self, ...]: + def thread_bindings(cls, comms: Comms, thread: Thread, status: ThreadStatus, channel: SurroundingChannel, *, catalog: CatalogAccess) -> tuple[CommandBinding, ...]: return () @classmethod - def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[Self, ...]: + def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[CommandBinding, ...]: return () @classmethod - def channel_bindings(cls, comms: Comms, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess) -> tuple[Self, ...]: + def channel_bindings(cls, comms: Comms, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess) -> tuple[CommandBinding, ...]: return () @classmethod def bindings(cls, comms: Comms, target: CommandTarget, channel: SurroundingChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - resolved: ResolvedThread | ResolvedChannel | Checkout) -> tuple[Self, ...]: + resolved: ResolvedThread | ResolvedChannel | Checkout) -> tuple[CommandBinding, ...]: return target.bindings( cls, comms, channel, resolved, project=project, snapshot=snapshot, catalog=catalog) @@ -160,7 +274,7 @@ def target_catalog(cls, comms: Comms, target: TargetSelection, catalog = (comms.channels.catalog.read() if channel.needs_catalog or any(item.needs_channel_state for item in selected) else NoCatalog()) resolved_targets = tuple((name, name.resolve(snapshot, comms.bus.require_channel)) for name in available_targets) - grouped: dict[type[CliCommand], dict[CliCommand, None]] = {} + grouped: dict[type[CliCommand], dict[CommandBinding, None]] = {} for member in target.command_members(cls): selected_channels = channel if member.channel_fields() else NoChannel() for name, resolved in resolved_targets: @@ -176,9 +290,6 @@ def target_catalog(cls, comms: Comms, target: TargetSelection, channel if declaration.channel_fields() else NoChannel()) for declaration, commands in grouped.items()) - def for_editor(self, comms: Comms, target: str, project: Checkout) -> Self: - return self - @property def editable_fields(self) -> tuple[TargetField, ...]: """The command owns the parameters offered by its target action.""" @@ -189,11 +300,6 @@ def editable_fields(self) -> tuple[TargetField, ...]: if not entry.setting(TargetBound, False) ) - async def prepare_fields(self) -> tuple[TargetField, ...]: - - catalogs = ConfigOptions.configured() - return tuple([await item.prepare(catalogs, self) for item in self.editable_fields]) - def describe(self, comms: Comms, target: CommandTarget, project: Checkout) -> TargetAction: bound = self.for_editor(comms, target.name, project) @@ -236,15 +342,7 @@ def with_confirmation(self, confirmed: bool) -> Self: return self def edited(self, arguments: Mapping[str, object]) -> Self: - bound = { - entry.key for entry in FieldCodec.argument_fields(type(self)) - if entry.setting(TargetBound, False) - } - if bound & arguments.keys(): - raise ValueError("Target-bound parameters cannot be overridden") - editable = {item.name for item in self.editable_fields} - if arguments.keys() - editable: - raise ValueError("Parameters not offered by this target command cannot be overridden") + bound = self.authorize_edits(arguments) encoded = FieldCodec.encode(self) return type(self).from_payload({ **{key: encoded[key] for key in bound & encoded.keys()}, @@ -271,7 +369,7 @@ def execute_target(cls, comms: Comms, target: TargetSelection, arguments: Mappin @dataclass(frozen=True) class TargetAction: """A selection keeps the bound commands, not a separate permission list.""" - bound: tuple[CliCommand, ...] + bound: tuple[CommandBinding, ...] targets: tuple[str, ...] declaration: type[CliCommand] channels: ChannelSelection = NoChannel() @@ -280,8 +378,8 @@ class TargetAction: def editable_fields(self) -> tuple[TargetField, ...]: return self.bound[0].editable_fields - async def prepare_fields(self) -> tuple[TargetField, ...]: - return await self.bound[0].prepare_fields() + async def prepare_fields(self, arguments: Mapping[str, object] = {}) -> tuple[TargetField, ...]: + return await self.bound[0].prepare_fields(arguments) @property def label(self) -> str: @@ -289,14 +387,14 @@ def label(self) -> str: @property def confirmation(self) -> str: - groups: dict[type[CliCommand], list[CliCommand]] = {} + groups: dict[type[CommandBinding], list[CommandBinding]] = {} for command in self.bound: groups.setdefault(type(command), []).append(command) return '\n'.join(filter(None, (declaration.selection_confirmation(tuple(commands)) for declaration, commands in groups.items()))) def edited(self, arguments: Mapping[str, object]) -> TargetAction: - return replace(self, bound=tuple(command.edited(type(command).editor_arguments(arguments)) + return replace(self, bound=tuple(command.edited(command.declaration.editor_arguments(arguments)) for command in self.bound)) def with_confirmation(self, confirmed: bool) -> TargetAction: @@ -480,7 +578,7 @@ def run(self, declaration: type[CliCommand], comms: Comms, arguments: Mapping[st if member.catalog_declaration() is declaration) if declaration.catalog_declaration() is declaration else (declaration,)) outcomes: list[tuple[int, TargetCompleted | TargetFailed]] = [] - planned: dict[CliCommand, tuple[int, CommandTarget, SurroundingChannel]] = {} + planned: dict[CommandBinding, tuple[int, CommandTarget, SurroundingChannel]] = {} together: dict[type[AppliedTogether], list[tuple[int, CommandTarget, AppliedTogether]]] = {} for name in self.targets: try: @@ -511,7 +609,7 @@ def run(self, declaration: type[CliCommand], comms: Comms, arguments: Mapping[st current = comms.registry.snapshot() current_catalog = (comms.channels.catalog.read() if context.needs_catalog or name.needs_channel_state else NoCatalog()) - if bound_command not in type(bound_command).bindings( + if bound_command not in bound_command.declaration.bindings( comms, name, context, project=project, snapshot=current, catalog=current_catalog, resolved=name.resolve(current, comms.bus.require_channel)): raise CommandSelectionChanged( diff --git a/src/agent_comms/cli_support/thread_creation.py b/src/agent_comms/cli_support/thread_creation.py index 6c34493c72..8e964102a7 100644 --- a/src/agent_comms/cli_support/thread_creation.py +++ b/src/agent_comms/cli_support/thread_creation.py @@ -7,7 +7,7 @@ from pathlib import Path from typing import Annotated -from agent_comms.cli_command import CliCommand +from agent_comms.cli_command import CliCommand, CommandContext from agent_comms.cli_commands import NameArgument, ThreadCreatedResult, _tag_choice_text from agent_comms.comms import Comms from agent_comms.config_options import ConfigOptions, ModelConfigOption, ThinkingLevelConfigOption @@ -16,16 +16,17 @@ from agent_comms.pi_vocabulary import ThinkingLevelField from agent_comms.repo import Checkout, LaunchCheckout from agent_comms.thread_management import ThreadCreationSpec -from agent_comms.thread_model import CatalogSelection, ThreadModel +from agent_comms.thread_model import ThreadModel -type TagArgument[T] = Annotated[ - T, EditorFormat(FieldCodec.encode), Normalize(_tag_choice_text), Argument(flags=("--tags",)), -] +TAG_ARGUMENT = ( + EditorFormat(FieldCodec.encode), Normalize(_tag_choice_text), Argument(flags=("--tags",)), +) @dataclass(frozen=True, kw_only=True) -class ThreadCreationCliCommand(NameArgument, CatalogSelection, CliCommand, declared_name=None): +class ThreadCreationFields(NameArgument, CommandContext): + """Supplied launch context is also the complete command's field declaration.""" task: Annotated[str, Multiline(), Argument(flags=("--task",))] = "" project: Annotated[Checkout, Argument(flags=("--project",))] model: Annotated[ @@ -35,19 +36,25 @@ class ThreadCreationCliCommand(NameArgument, CatalogSelection, CliCommand, decla ThinkingLevelField, Catalog(ThinkingLevelConfigOption), Argument(flags=("--thinking-level",)), ] = NativeThreadDefaults.thinking_level - @property - @abstractmethod - def spec_type(self) -> type[ThreadCreationSpec]: ... - @property def project_path(self) -> Path: return self.project.path - def apply(self, ctx: Comms) -> ThreadCreatedResult: - spec = self.spec_type.from_task(**{ + +@dataclass(frozen=True, kw_only=True) +class ThreadCreationCliCommand(ThreadCreationFields, CliCommand, declared_name=None): + @property + @abstractmethod + def spec_type(self) -> type[ThreadCreationSpec]: ... + + def specification(self) -> ThreadCreationSpec: + return self.spec_type.from_task(**{ **{item.name: getattr(self, item.name) for item in fields(self.spec_type)}, 'project': LaunchCheckout.checked(self.project.path), }) + + def apply(self, ctx: Comms) -> ThreadCreatedResult: + spec = self.specification() asyncio.run(ConfigOptions.configured().validate(self)) child = ctx.threads.create(spec) return ThreadCreatedResult(child.name, child.process_identity) diff --git a/src/agent_comms/cli_verbs/fork.py b/src/agent_comms/cli_verbs/fork.py index e6b9c1637f..548bf1b13a 100644 --- a/src/agent_comms/cli_verbs/fork.py +++ b/src/agent_comms/cli_verbs/fork.py @@ -1,6 +1,6 @@ from __future__ import annotations -from agent_comms.cli_support.thread_creation import TagArgument, ThreadCreationCliCommand +from agent_comms.cli_support.thread_creation import TAG_ARGUMENT, ThreadCreationCliCommand from agent_comms.cli_support.thread_bound import ThreadBoundCliCommand from agent_comms.field_codec import Argument from agent_comms.field_codec import TargetBound @@ -21,7 +21,7 @@ class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ThreadCreationCl str, TargetBound(), Argument(flags=("--parent",)) ] - tags: TagArgument[TagChoice] = InheritedTags() + tags: Annotated[TagChoice, *TAG_ARGUMENT] = InheritedTags() @classmethod def thread_binding(cls, thread: Thread) -> Self: diff --git a/src/agent_comms/cli_verbs/new.py b/src/agent_comms/cli_verbs/new.py index e7661e5b44..349a389107 100644 --- a/src/agent_comms/cli_verbs/new.py +++ b/src/agent_comms/cli_verbs/new.py @@ -1,24 +1,25 @@ from __future__ import annotations -from agent_comms.cli_support.thread_creation import TagArgument, ThreadCreationCliCommand +from agent_comms.cli_command import PendingCommandBinding +from agent_comms.cli_support.thread_creation import TAG_ARGUMENT, ThreadCreationCliCommand, ThreadCreationFields from agent_comms.comms import Comms from agent_comms.native_settings import NativeThreadDefaults from agent_comms.repo import Checkout from agent_comms.thread_management import GivenTags from agent_comms.thread_management import NewThreadSpec from dataclasses import dataclass -from typing import ClassVar -from typing import Self +from typing import Annotated, ClassVar @dataclass(frozen=True, kw_only=True) class NewThreadCliCommand(ThreadCreationCliCommand, declared_name="new"): help = "Create a new pi thread" spec_type: ClassVar[type[NewThreadSpec]] = NewThreadSpec - tags: TagArgument[GivenTags] + tags: Annotated[GivenTags, *TAG_ARGUMENT] @classmethod - def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[Self, ...]: + def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[PendingCommandBinding, ...]: defaults = NativeThreadDefaults() - return (cls(name='', project=project, model=defaults.model, - thinking_level=defaults.thinking_level),) + return (PendingCommandBinding(cls, ThreadCreationFields( + name='', project=project, model=defaults.model, + thinking_level=defaults.thinking_level)),) diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index f07a82f7a5..85e3ea0b73 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -6,7 +6,7 @@ from agent_comms import field_codec from agent_comms.field_codec import FieldCodec from collections.abc import Callable -from dataclasses import dataclass +from dataclasses import MISSING, dataclass from dataclasses import replace from agent_comms.config_options import CatalogChoice, CatalogConfigOption, ConfigOptions from agent_comms.thread_model import CatalogSelection @@ -44,9 +44,13 @@ def multiline(self) -> bool: def description(self) -> str: return self.plan.description + @property + def supplied(self) -> bool: + return self.value is not MISSING + @property def editor_default(self) -> JsonValue: - return self.plan.editor_default(self.value) + return self.plan.editor_default(self.value) if self.supplied else None def editor_hints(self, choices: Callable[[ChoiceTargetField], object]) -> tuple[object, ...]: """An unrestricted field does not declare a finite-choice editor.""" diff --git a/tests/test_checkout_submodules.py b/tests/test_checkout_submodules.py index 60700222a2..b4166031fc 100644 --- a/tests/test_checkout_submodules.py +++ b/tests/test_checkout_submodules.py @@ -136,7 +136,7 @@ def test_empty_project_does_not_become_the_callers_checkout(tmp_path, monkeypatc git(tmp_path, 'init', '-q') monkeypatch.chdir(tmp_path) comms = wire(tmp_path / 'wire') - result = core('--root', str(comms.root), 'new', '--name', 'empty-project', '--project', '') + result = core('--root', str(comms.root), 'new', '--name', 'empty-project', '--tags', '', '--project', '') assert result.returncode != 0 assert 'Project path must not be empty' in result.stdout + result.stderr assert 'empty-project' not in comms.registry.snapshot().threads diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py index 9b41eb166a..12af3959cd 100644 --- a/tests/test_end_to_end.py +++ b/tests/test_end_to_end.py @@ -42,19 +42,21 @@ async def cli(programs, root, *arguments): class TestEndToEndLifecycle: - @pytest.mark.parametrize('operation,parent_arguments', [ - ('new', ()), ('fork', ('--parent', 'parent')), + @pytest.mark.parametrize('operation,creation_arguments,expected_tags', [ + ('new', ('--tags', 'chosen'), frozenset({'chosen'})), + ('new', ('--tags', ''), frozenset()), + ('fork', ('--parent', 'parent'), frozenset({'inherited'})), ]) async def test_new_and_fork_require_a_declared_checkout( - self, native, source_programs, operation, parent_arguments, + self, native, source_programs, operation, creation_arguments, expected_tags, ): await native.author_history() comms = wire(native.root) await cli(source_programs, native.root, 'register', '--name', 'parent', - '--worktree', str(native.project), '--execution', 'native') + '--tags', 'inherited', '--worktree', str(native.project), '--execution', 'native') parent = comms.registry.require('parent') comms.threads.attach_session(parent, str(native.session), process=parent.process_identity) - command = (operation, '--name', 'probe', *parent_arguments) + command = (operation, '--name', 'probe', *creation_arguments) plain = native.project.parent / 'plain' plain.mkdir() for project_arguments in ((), ('--project', ''), ('--project', str(plain))): @@ -66,7 +68,9 @@ async def test_new_and_fork_require_a_declared_checkout( '--project', str(native.project), '--model', 'response-local/fixture', '--thinking-level', 'off') assert created['created'] == 'probe' - assert comms.registry.require('probe').worktree == Checkout(native.project.resolve()) + child = comms.registry.require('probe') + assert child.worktree == Checkout(native.project.resolve()) + assert child.tags == expected_tags finally: for thread in comms.registry.snapshot().threads.values(): if thread.process_alive: diff --git a/tests/test_thread_creation.py b/tests/test_thread_creation.py new file mode 100644 index 0000000000..b4bea079c4 --- /dev/null +++ b/tests/test_thread_creation.py @@ -0,0 +1,84 @@ +"""Creation contracts through the actual CLI decoder, editor binding and registry. + +Author does not execute these controls; the integration owner selects them once. +""" +from dataclasses import replace + +import pytest + +from agent_comms.cli import build_parser +from agent_comms.cli_command import PendingCommandBinding +from agent_comms.cli_verbs.fork import ForkCliCommand +from agent_comms.cli_verbs.new import NewThreadCliCommand +from agent_comms.field_codec import MalformedRecord +from agent_comms.launch_runtime import LaunchRuntime +from agent_comms.repo import Checkout +from agent_comms.thread_management import GivenTags, InheritedTags + + +def parsed(arguments): + namespace = build_parser(arguments).parse_args(arguments) + return namespace.command_declaration.from_arguments(namespace) + + +def test_new_cli_requires_an_explicit_tag_argument(source_repository): + with pytest.raises(MalformedRecord): + parsed(['new', '--name', 'child', '--project', str(source_repository.checkout)]) + + +@pytest.mark.parametrize('text,expected', ( + ('auth,review', frozenset({'auth', 'review'})), + ('', frozenset()), +)) +def test_new_cli_membership_reaches_registration(comms, source_repository, text, expected): + command = parsed(['new', '--name', 'child', '--project', str(source_repository.checkout), + '--tags', text]) + assert command.tags == GivenTags(expected) + # New's declaration path reserves its session but does not launch a provider. + child = command.specification().declare(comms.threads, '', LaunchRuntime.running()) + assert child.tags == expected + assert comms.registry.require(child.name).tags == expected + + +def test_pending_new_binding_offers_missing_tags_without_constructing_the_command(comms, source_repository): + binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) + assert type(binding) is PendingCommandBinding + tags = next(item for item in binding.editable_fields if item.name == 'tags') + assert tags.required and not tags.supplied + assert tags.editor_default is None + with pytest.raises(MalformedRecord): + binding.edited(NewThreadCliCommand.editor_arguments({'name': 'child'})) + + +@pytest.mark.parametrize('text,expected', ( + ('auth', frozenset({'auth'})), + ('', frozenset()), +)) +def test_pending_new_binding_distinguishes_explicit_empty_from_missing(comms, source_repository, text, expected): + binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) + command = binding.edited(NewThreadCliCommand.editor_arguments({'name': 'child', 'tags': text})) + assert command.tags == GivenTags(expected) + assert command.project == Checkout(source_repository.checkout) + + +def test_pending_catalog_context_can_change_before_tags_are_supplied(comms, source_repository): + binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) + prepared = binding.for_fields({'name': 'child'}) + assert prepared.context.name == 'child' + assert not next(item for item in prepared.editable_fields if item.name == 'tags').supplied + + +def test_fork_cli_defaults_to_actual_parent_membership(comms, source_repository): + parent_command = parsed(['new', '--name', 'parent', '--project', str(source_repository.checkout), + '--tags', 'original']) + parent = parent_command.specification().declare(comms.threads, '', LaunchRuntime.running()) + command = parsed(['fork', '--name', 'child', '--parent', parent.name, + '--project', str(source_repository.checkout)]) + assert command.tags == InheritedTags() + editor = ForkCliCommand.thread_binding(parent) + assert editor.tags == InheritedTags() + # A real later parent record, not the stale tags at editor opening, supplies membership. + current = comms.registry.declare(replace(parent, tags=frozenset({'current'}))) + child = command.specification().child(current, current.runtime, comms.threads) + assert child.tags == current.tags == frozenset({'current'}) + assert editor.tags.for_child(current.tags) == current.tags From 5c66f0ae36d12260bd6b6885a2185bd728122598 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 03:31:25 -0400 Subject: [PATCH 13/26] Keep native context edits distinct from complete command authorization --- src/agent_comms/cli_command.py | 4 ++-- tests/test_thread_creation.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index f5a6b3bb14..afb03cfe71 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -118,7 +118,7 @@ def with_confirmation(self, confirmed: bool) -> CommandBinding: ... class CommandContext(CatalogSelection): """A partial command's supplied fields, with native edits decoded by their plans.""" - def edited(self, arguments: Mapping[str, object]) -> Self: + def with_editor_values(self, arguments: Mapping[str, object]) -> Self: changes = {name: value for entry in FieldCodec.argument_fields(type(self)) if entry.key in arguments for name, value in entry.decoded({ @@ -151,7 +151,7 @@ def editable_fields(self) -> tuple[TargetField, ...]: if not entry.setting(TargetBound, False)) def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: - return replace(self, context=self.context.edited(arguments)) + return replace(self, context=self.context.with_editor_values(arguments)) def edited(self, arguments: Mapping[str, object]) -> CliCommand: self.authorize_edits(arguments) diff --git a/tests/test_thread_creation.py b/tests/test_thread_creation.py index b4bea079c4..9f435cead0 100644 --- a/tests/test_thread_creation.py +++ b/tests/test_thread_creation.py @@ -68,6 +68,18 @@ def test_pending_catalog_context_can_change_before_tags_are_supplied(comms, sour assert not next(item for item in prepared.editable_fields if item.name == 'tags').supplied +@pytest.mark.parametrize('creation_arguments,forbidden', ( + (('new', '--tags', 'auth'), {'not_a_field': 'value'}), + (('fork', '--parent', 'parent'), {'not_a_field': 'value'}), + (('fork', '--parent', 'parent'), {'parent': 'different-parent'}), +)) +def test_complete_creation_commands_keep_command_edit_authorization(source_repository, creation_arguments, forbidden): + command = parsed([*creation_arguments, '--name', 'child', + '--project', str(source_repository.checkout)]) + with pytest.raises(ValueError): + command.edited(forbidden) + + def test_fork_cli_defaults_to_actual_parent_membership(comms, source_repository): parent_command = parsed(['new', '--name', 'parent', '--project', str(source_repository.checkout), '--tags', 'original']) From b2f766f924e2b6a17ec2b6261cc181ae0c314eef Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 14:22:19 -0400 Subject: [PATCH 14/26] Complete creation bindings through native field and presence owners --- src/agent_comms/cli_command.py | 118 ++++++++++-------- src/agent_comms/cli_commands.py | 12 +- .../cli_support/thread_creation.py | 21 ++-- src/agent_comms/cli_verbs/fork.py | 4 +- src/agent_comms/cli_verbs/new.py | 5 +- src/agent_comms/cli_verbs/thread_tags.py | 8 +- src/agent_comms/field_codec.py | 87 +++++++++---- src/agent_comms/target_fields.py | 95 ++++++++++---- src/agent_comms/thread_management.py | 3 +- tests/test_thread_creation.py | 9 +- 10 files changed, 230 insertions(+), 132 deletions(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index afb03cfe71..d6f9b42fc7 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -28,7 +28,7 @@ from agent_comms.field_codec import FieldPlan, TargetBound from agent_comms.launch_root import LaunchRoot from inspect import iscoroutinefunction, signature -from types import get_original_bases +from types import MappingProxyType, get_original_bases from typing import TYPE_CHECKING, TextIO, cast, get_args, get_origin if TYPE_CHECKING: @@ -72,23 +72,33 @@ class CommandBinding(ABC): def declaration(self) -> type[CliCommand]: ... @property + def editable_fields(self) -> tuple[TargetField, ...]: + return tuple(self.field_value(entry) + for entry in FieldCodec.argument_fields(self.declaration) + if not entry.setting(TargetBound, False)) + @abstractmethod - def editable_fields(self) -> tuple[TargetField, ...]: ... + def field_value(self, entry: FieldPlan) -> TargetField: ... - def for_editor(self, comms: Comms, target: str, project: Checkout) -> Self: - return self + @abstractmethod + def retained_payload(self) -> dict[str, object]: + """Already decoded constructor values, never serialized for editing.""" - @property - def catalog_context(self) -> object: + def for_editor(self, comms: Comms, target: str, project: Checkout) -> Self: return self def for_fields(self, arguments: Mapping[str, object]) -> CommandBinding: return self.edited(self.declaration.editor_arguments(arguments)) if arguments else self - async def prepare_fields(self, arguments: Mapping[str, object] = {}) -> tuple[TargetField, ...]: + async def prepare_field(self, item: TargetField, catalogs: ConfigOptions) -> TargetField: + return await item.prepare(catalogs) + + async def prepare_fields( + self, arguments: Mapping[str, object] = MappingProxyType({}), + ) -> tuple[TargetField, ...]: binding = self.for_fields(arguments) catalogs = ConfigOptions.configured() - return tuple([await item.prepare(catalogs, binding.catalog_context) + return tuple([await binding.prepare_field(item, catalogs) for item in binding.editable_fields]) def authorize_edits(self, arguments: Mapping[str, object]) -> frozenset[str]: @@ -100,8 +110,12 @@ def authorize_edits(self, arguments: Mapping[str, object]) -> frozenset[str]: raise ValueError("Parameters not offered by this target command cannot be overridden") return bound - @abstractmethod - def edited(self, arguments: Mapping[str, object]) -> CliCommand: ... + def edited(self, arguments: Mapping[str, object]) -> CliCommand: + self.authorize_edits(arguments) + decoded = {name: value + for entry in FieldCodec.argument_fields(self.declaration) if entry.key in arguments + for name, value in entry.decoded(arguments).items()} + return FieldCodec.construct(self.declaration, {**self.retained_payload(), **decoded}) @abstractmethod def confirmation(self) -> str: ... @@ -114,26 +128,33 @@ def selection_confirmation(cls, commands: tuple[Self, ...]) -> str: ... def with_confirmation(self, confirmed: bool) -> CommandBinding: ... +class CatalogBinding(CommandBinding): + """Only a binding declaring catalog inputs has a catalog question to answer.""" + + @property + @abstractmethod + def catalog_context(self) -> CatalogSelection: ... + + async def prepare_field(self, item: TargetField, catalogs: ConfigOptions) -> TargetField: + return await item.prepare_catalog(catalogs, self.catalog_context) + + @dataclass(frozen=True) class CommandContext(CatalogSelection): - """A partial command's supplied fields, with native edits decoded by their plans.""" - - def with_editor_values(self, arguments: Mapping[str, object]) -> Self: - changes = {name: value - for entry in FieldCodec.argument_fields(type(self)) if entry.key in arguments - for name, value in entry.decoded({ - entry.key: entry.editor_payload(arguments[entry.key]), - }).items()} - return replace(self, **changes) if changes else self + """A declaration of supplied fields, not an alternate command editor.""" @dataclass(frozen=True) -class PendingCommandBinding(CommandBinding): +class PendingCommandBinding(CatalogBinding): """Only supplied context, borrowing the concrete command's declared fields.""" command: type[CliCommand] context: CommandContext + def __post_init__(self) -> None: + if not issubclass(self.command, type(self.context)): + raise TypeError('A pending context must be a declared parent of its command') + @property def declaration(self) -> type[CliCommand]: return self.command @@ -142,24 +163,24 @@ def declaration(self) -> type[CliCommand]: def catalog_context(self) -> CatalogSelection: return self.context - @property - def editable_fields(self) -> tuple[TargetField, ...]: - supplied = {entry.name: entry.current(self.context) - for entry in FieldCodec.argument_fields(type(self.context))} - return tuple(TargetField.of(entry, supplied[entry.name] if entry.name in supplied else entry.default()) - for entry in FieldCodec.argument_fields(self.declaration) - if not entry.setting(TargetBound, False)) + def retained_payload(self) -> dict[str, object]: + return {entry.name: entry.current(self.context) + for entry in FieldCodec.field_plans(type(self.context))} - def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: - return replace(self, context=self.context.with_editor_values(arguments)) + def field_value(self, entry: FieldPlan) -> TargetField: + for supplied in FieldCodec.field_plans(type(self.context)): + if supplied.declared is entry.declared: + return TargetField.of(entry, supplied.current(self.context)) + return TargetField.omitted(entry) - def edited(self, arguments: Mapping[str, object]) -> CliCommand: + def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: self.authorize_edits(arguments) - return self.declaration.from_payload({ - **FieldCodec.encode(self.context), - self.declaration.family_discriminator: FieldCodec.encode(self.declaration), - **arguments, - }) + changes = {name: value + for entry in FieldCodec.argument_fields(type(self.context)) if entry.key in arguments + for name, value in entry.decoded({ + entry.key: entry.editor_payload(arguments[entry.key]), + }).items()} + return replace(self, context=replace(self.context, **changes)) if changes else self def confirmation(self) -> str: return '' @@ -290,15 +311,13 @@ def target_catalog(cls, comms: Comms, target: TargetSelection, channel if declaration.channel_fields() else NoChannel()) for declaration, commands in grouped.items()) - @property - def editable_fields(self) -> tuple[TargetField, ...]: - """The command owns the parameters offered by its target action.""" + def field_value(self, entry: FieldPlan) -> TargetField: + return TargetField.of(entry, entry.current(self)) - return tuple( - TargetField.of(entry, entry.current(self)) - for entry in FieldCodec.argument_fields(type(self)) - if not entry.setting(TargetBound, False) - ) + def retained_payload(self) -> dict[str, object]: + return {entry.name: entry.current(self) + for entry in FieldCodec.argument_fields(type(self)) + if entry.setting(TargetBound, False)} def describe(self, comms: Comms, target: CommandTarget, project: Checkout) -> TargetAction: @@ -341,15 +360,6 @@ def with_confirmation(self, confirmed: bool) -> Self: raise ValueError(self.confirmation()) return self - def edited(self, arguments: Mapping[str, object]) -> Self: - bound = self.authorize_edits(arguments) - encoded = FieldCodec.encode(self) - return type(self).from_payload({ - **{key: encoded[key] for key in bound & encoded.keys()}, - type(self).family_discriminator: FieldCodec.encode(type(self)), - **arguments, - }) - @classmethod def execute_target(cls, comms: Comms, target: TargetSelection, arguments: Mapping[str, object], *, confirmed: bool = False, @@ -378,7 +388,9 @@ class TargetAction: def editable_fields(self) -> tuple[TargetField, ...]: return self.bound[0].editable_fields - async def prepare_fields(self, arguments: Mapping[str, object] = {}) -> tuple[TargetField, ...]: + async def prepare_fields( + self, arguments: Mapping[str, object] = MappingProxyType({}), + ) -> tuple[TargetField, ...]: return await self.bound[0].prepare_fields(arguments) @property diff --git a/src/agent_comms/cli_commands.py b/src/agent_comms/cli_commands.py index 7f067c70e8..5ff848499d 100644 --- a/src/agent_comms/cli_commands.py +++ b/src/agent_comms/cli_commands.py @@ -29,7 +29,7 @@ from .cli_result import CliResult from .comms import Comms from .declared_family import DeclaredFamily -from .field_codec import Argument, PathText, WireForm, WireOmitDefault, boundary_decoder +from .field_codec import Argument, PathText, WireForm, WireOmitDefault from .history_result import HistoryPageResult from .goals import Goal from .lifecycle_capability import Complete @@ -76,16 +76,6 @@ def _tags(value: str) -> list[str]: return [tag.strip() for tag in value.split(",") if tag.strip()] -@boundary_decoder -def _tag_choice_text(value: str | None) -> str | None: - """Command-line and editor text is the given tags' wire; no text inherits.""" - return value - - -def _tag_text(value: frozenset[str] | None) -> str | None: - return None if value is None else ', '.join(sorted(value)) - - def _shell_words(value: str) -> list[str]: return shlex.split(value) diff --git a/src/agent_comms/cli_support/thread_creation.py b/src/agent_comms/cli_support/thread_creation.py index 8e964102a7..07c617a558 100644 --- a/src/agent_comms/cli_support/thread_creation.py +++ b/src/agent_comms/cli_support/thread_creation.py @@ -7,21 +7,16 @@ from pathlib import Path from typing import Annotated -from agent_comms.cli_command import CliCommand, CommandContext -from agent_comms.cli_commands import NameArgument, ThreadCreatedResult, _tag_choice_text +from agent_comms.cli_command import CatalogBinding, CliCommand, CommandContext +from agent_comms.cli_commands import NameArgument, ThreadCreatedResult from agent_comms.comms import Comms from agent_comms.config_options import ConfigOptions, ModelConfigOption, ThinkingLevelConfigOption -from agent_comms.field_codec import Argument, Catalog, EditorFormat, FieldCodec, Multiline, Normalize +from agent_comms.field_codec import Argument, Catalog, Multiline from agent_comms.native_settings import NativeThreadDefaults from agent_comms.pi_vocabulary import ThinkingLevelField from agent_comms.repo import Checkout, LaunchCheckout from agent_comms.thread_management import ThreadCreationSpec -from agent_comms.thread_model import ThreadModel - - -TAG_ARGUMENT = ( - EditorFormat(FieldCodec.encode), Normalize(_tag_choice_text), Argument(flags=("--tags",)), -) +from agent_comms.thread_model import CatalogSelection, ThreadModel @dataclass(frozen=True, kw_only=True) @@ -42,11 +37,17 @@ def project_path(self) -> Path: @dataclass(frozen=True, kw_only=True) -class ThreadCreationCliCommand(ThreadCreationFields, CliCommand, declared_name=None): +class ThreadCreationCliCommand( + ThreadCreationFields, CatalogBinding, CliCommand[ThreadCreatedResult], declared_name=None, +): @property @abstractmethod def spec_type(self) -> type[ThreadCreationSpec]: ... + @property + def catalog_context(self) -> CatalogSelection: + return self + def specification(self) -> ThreadCreationSpec: return self.spec_type.from_task(**{ **{item.name: getattr(self, item.name) for item in fields(self.spec_type)}, diff --git a/src/agent_comms/cli_verbs/fork.py b/src/agent_comms/cli_verbs/fork.py index 548bf1b13a..c39dd652cc 100644 --- a/src/agent_comms/cli_verbs/fork.py +++ b/src/agent_comms/cli_verbs/fork.py @@ -1,6 +1,6 @@ from __future__ import annotations -from agent_comms.cli_support.thread_creation import TAG_ARGUMENT, ThreadCreationCliCommand +from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand from agent_comms.cli_support.thread_bound import ThreadBoundCliCommand from agent_comms.field_codec import Argument from agent_comms.field_codec import TargetBound @@ -21,7 +21,7 @@ class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ThreadCreationCl str, TargetBound(), Argument(flags=("--parent",)) ] - tags: Annotated[TagChoice, *TAG_ARGUMENT] = InheritedTags() + tags: Annotated[TagChoice, Argument()] = InheritedTags() @classmethod def thread_binding(cls, thread: Thread) -> Self: diff --git a/src/agent_comms/cli_verbs/new.py b/src/agent_comms/cli_verbs/new.py index 349a389107..c0ecc5e229 100644 --- a/src/agent_comms/cli_verbs/new.py +++ b/src/agent_comms/cli_verbs/new.py @@ -1,8 +1,9 @@ from __future__ import annotations from agent_comms.cli_command import PendingCommandBinding -from agent_comms.cli_support.thread_creation import TAG_ARGUMENT, ThreadCreationCliCommand, ThreadCreationFields +from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand, ThreadCreationFields from agent_comms.comms import Comms +from agent_comms.field_codec import Argument from agent_comms.native_settings import NativeThreadDefaults from agent_comms.repo import Checkout from agent_comms.thread_management import GivenTags @@ -15,7 +16,7 @@ class NewThreadCliCommand(ThreadCreationCliCommand, declared_name="new"): help = "Create a new pi thread" spec_type: ClassVar[type[NewThreadSpec]] = NewThreadSpec - tags: Annotated[GivenTags, *TAG_ARGUMENT] + tags: Annotated[GivenTags, Argument()] @classmethod def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[PendingCommandBinding, ...]: diff --git a/src/agent_comms/cli_verbs/thread_tags.py b/src/agent_comms/cli_verbs/thread_tags.py index e7eff77318..0f031b7a9f 100644 --- a/src/agent_comms/cli_verbs/thread_tags.py +++ b/src/agent_comms/cli_verbs/thread_tags.py @@ -4,13 +4,10 @@ from agent_comms.catalog_document import CatalogAccess from agent_comms.cli_commands import ThreadTagsResult -from agent_comms.cli_commands import _tag_text -from agent_comms.cli_commands import _tags from agent_comms.cli_support.named import NamedCliCommand from agent_comms.comms import Comms from agent_comms.field_codec import Argument -from agent_comms.field_codec import EditorFormat -from agent_comms.field_codec import Normalize +from agent_comms.launch_runtime import TagListText from agent_comms.thread_status import ThreadStatus from agent_comms.threads import Thread from dataclasses import dataclass @@ -23,8 +20,7 @@ class ThreadTagsCliCommand(NamedCliCommand, declared_name="thread-tags"): help = "Edit thread tags" tags: Annotated[ frozenset[str], - Normalize(_tags), - EditorFormat(_tag_text), + TagListText, Argument( flags=("--tags",), options={"help": "Complete tags, separated by commas"} ), diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index db0eb0ab3f..aa34645a68 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -369,8 +369,8 @@ class Normalize(ValueDirective[Callable[[object], object]]): pass -class EditorFormat(ValueDirective[Callable[[object], JsonValue]]): - pass +class EditsAsText: + """This member declares the typed editor of its untagged wire family.""" class Editor(ValueDirective[type]): @@ -619,10 +619,6 @@ def argument_normalizer(self) -> Callable[[object], object]: def editor_normalizer(self) -> Callable[[object], object]: return self.setting(Normalize, self.value.decode_editor) - @cached_property - def editor_formatter(self) -> Callable[[object], JsonValue]: - return self.setting(EditorFormat, self.value.encode_editor) - def argument_payload(self, value: object) -> object: return self.argument_normalizer(value) @@ -630,7 +626,7 @@ def editor_payload(self, value: object) -> object: return self.editor_normalizer(value) def editor_default(self, value: object) -> JsonValue: - return self.editor_formatter(value) + return self.value.encode_editor(value) @property def description(self) -> str: @@ -957,6 +953,17 @@ class DecoderPlan(ABC): target: object + @property + def editor_annotation(self) -> object: + return str + + def format_editor(self, wire: object) -> JsonValue: + return json.dumps(wire) + + @boundary_decoder + def parse_editor(self, text: object) -> object: + return None if text is None else json.loads(text) + def sql_checks(self, column: str, quote: Callable[[object], str]) -> tuple[str, ...]: """Stored-value predicates; the SQL owner quotes its column and constants.""" return () @@ -1053,10 +1060,14 @@ def representation_plan( @classmethod @cache def for_members[M](cls, members: tuple[type[M], ...]) -> UnionDecoder[M]: - return UnionDecoder( - reduce(or_, members), - tuple(compile_decoder(member) for member in members), - ) + target = reduce(or_, members) + alternatives = tuple(compile_decoder(member) for member in members) + editors = tuple(member for member in members if EditsAsText in member.__mro__) + if len(editors) > 1: + raise TypeError(f'{target} declares more than one typed-text editor: {editors}') + if editors: + return _TextEditedUnionDecoder(target, alternatives, compile_decoder(editors[0]).editor_wire) + return UnionDecoder(target, alternatives) @abstractmethod def decode(self, data: object) -> T: @@ -1123,15 +1134,15 @@ def decode_selected(self, data, selections): def decode_keys(self, data, keys, selections): raise TypeError("Selected keys require a mapping field") - editor_annotation = str + @property + def editor_wire(self) -> DecoderPlan: + return self - def encode_editor(self, value: T) -> str: - encoded = FieldCodec.encode(value, self.target) - return json.dumps(encoded) + def encode_editor(self, value: T) -> JsonValue: + return self.format_editor(self.encode(value)) - @boundary_decoder - def decode_editor(self, value): - return None if value is None else json.loads(value) + def decode_editor(self, value: object) -> object: + return self.parse_editor(value) def editor_choices(self) -> tuple[tuple[str, object], ...]: raise TypeError("The declared decoder does not offer finite choices") @@ -1166,6 +1177,20 @@ def sql_checks(self, column: str, quote: Callable[[object], str]) -> tuple[str, def wire_structure(self) -> DecoderPlan: return self.representation.wire_structure(self.structural) + @property + def editor_wire(self) -> DecoderPlan: + return self.wire_structure + + @property + def editor_annotation(self) -> object: + return self.wire_structure.editor_annotation + + def format_editor(self, wire: object) -> JsonValue: + return self.wire_structure.format_editor(wire) + + def parse_editor(self, text: object) -> object: + return self.wire_structure.parse_editor(text) + @property def accepts_null(self) -> bool: return self.wire_structure.accepts_null @@ -2793,11 +2818,11 @@ class _TextDecoder(_ExactScalarDecoder[str]): def encode(self, value: str) -> str: return str(super().encode(value)) - def encode_editor(self, value: str) -> str: - return value + def format_editor(self, wire: object) -> JsonValue: + return cast(JsonValue, wire) - def decode_editor(self, value): - return value + def parse_editor(self, text: object) -> object: + return text class _FlagDecoder(_ExactScalarDecoder[bool]): @@ -3088,6 +3113,24 @@ def decode_candidates( first_error = None +@dataclass(frozen=True, slots=True) +class _TextEditedUnionDecoder[T](UnionDecoder[T]): + """One member's declaration owns typed text; other members remain wire choices.""" + + editor: DecoderPlan + + @property + def editor_annotation(self) -> object: + annotation = self.editor.editor_annotation + return Union[tuple((annotation, type(None)))] if self.accepts_null else annotation + + def format_editor(self, wire: object) -> JsonValue: + return self.editor.format_editor(wire) + + def parse_editor(self, text: object) -> object: + return self.editor.parse_editor(text) + + @dataclass(frozen=True, slots=True) class _ArrayDecoder[T: Iterable[object]](ValueDecoder[T]): container: ClassVar[type] diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index 85e3ea0b73..aa40e9b7a9 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -1,37 +1,46 @@ from __future__ import annotations +from abc import ABC, abstractmethod +from collections.abc import Callable, Mapping +from dataclasses import dataclass, replace + from pydantic import JsonValue -from typing import cast from agent_comms import field_codec -from agent_comms.field_codec import FieldCodec -from collections.abc import Callable -from dataclasses import MISSING, dataclass -from dataclasses import replace +from agent_comms.field_codec import FieldCodec, boundary_decoder from agent_comms.config_options import CatalogChoice, CatalogConfigOption, ConfigOptions from agent_comms.thread_model import CatalogSelection @dataclass(frozen=True) -class TargetField: - """The declared field owns parsing, editor hints and requiredness.""" +class TargetField(ABC): + """The declared field owns parsing, editor hints and input presence.""" plan: field_codec.FieldPlan - value: object @classmethod def of(cls, plan: field_codec.FieldPlan, value: object) -> TargetField: - editor: type[TargetField] = ( + editor: type[SuppliedTargetField] = ( CatalogTargetField if field_codec.Catalog.single(plan.annotation) - else plan.setting(field_codec.Editor, cls) + else plan.setting(field_codec.Editor, SuppliedTargetField) ) return editor(plan, value) + @classmethod + def omitted(cls, plan: field_codec.FieldPlan) -> TargetField: + if plan.constructor_required: + return UnsuppliedTargetField(plan) + return cls.of(plan, plan.default()) + @property def annotation(self) -> object: return self.plan.annotation + @property + def editor_annotation(self) -> object: + return field_codec.compile_decoder(self.annotation).editor_annotation + @property def name(self) -> str: return self.plan.key @@ -45,20 +54,26 @@ def description(self) -> str: return self.plan.description @property - def supplied(self) -> bool: - return self.value is not MISSING + @abstractmethod + def editor_default(self) -> JsonValue: ... - @property - def editor_default(self) -> JsonValue: - return self.plan.editor_default(self.value) if self.supplied else None + @abstractmethod + def default_schema(self) -> dict[str, JsonValue]: ... + + def argument(self, value: object) -> Mapping[str, object]: + return {self.name: value} def editor_hints(self, choices: Callable[[ChoiceTargetField], object]) -> tuple[object, ...]: """An unrestricted field does not declare a finite-choice editor.""" return () - async def prepare(self, catalogs: ConfigOptions, selection: object) -> TargetField: + async def prepare(self, catalogs: ConfigOptions) -> TargetField: return self + async def prepare_catalog(self, catalogs: ConfigOptions, + selection: CatalogSelection) -> TargetField: + return await self.prepare(catalogs) + @property def required(self) -> bool: return self.plan.constructor_required @@ -68,10 +83,44 @@ def value_schema(self) -> dict[str, JsonValue]: def json_schema(self) -> dict[str, JsonValue]: return {**self.value_schema(), 'multiline': self.multiline, - 'description': self.description, 'editor_default': self.editor_default} + 'description': self.description, **self.default_schema()} + + +@dataclass(frozen=True) +class SuppliedTargetField(TargetField): + value: object + + @property + def editor_default(self) -> JsonValue: + return self.plan.editor_default(self.value) + + def default_schema(self) -> dict[str, JsonValue]: + return {'editor_default': self.editor_default} + @dataclass(frozen=True) -class ChoiceTargetField(TargetField): +class UnsuppliedTargetField(TargetField): + """A required answer has no value; the optional editor owns its unset control.""" + + @property + def editor_annotation(self) -> object: + return field_codec.compile_decoder(self.annotation).editor_annotation | type(None) + + @property + def editor_default(self) -> JsonValue: + return None + + def default_schema(self) -> dict[str, JsonValue]: + return {} + + @boundary_decoder + def argument(self, value: object) -> Mapping[str, object]: + """Decode the optional control's raw presence once, not the field's wire value.""" + return {} if value is None else super().argument(value) + + +@dataclass(frozen=True) +class ChoiceTargetField(SuppliedTargetField): """A prepared finite declaration, from its annotation or configuration catalog.""" options: tuple[CatalogChoice, ...] = () @@ -83,8 +132,7 @@ def editor_declarations(self) -> tuple[object, ...]: def choices(self) -> tuple[tuple[str, str], ...]: return tuple((choice.name, choice.editor_value) for choice in self.options) - async def prepare(self, catalogs: ConfigOptions, - selection: object) -> ChoiceTargetField: + async def prepare(self, catalogs: ConfigOptions) -> ChoiceTargetField: decoder = field_codec.compile_decoder(self.annotation) options = (CatalogChoice( name=label, value=decoder.encode_editor(decoder.decode(value))) @@ -94,6 +142,7 @@ async def prepare(self, catalogs: ConfigOptions, def editor_hints(self, choices: Callable[[ChoiceTargetField], object]) -> tuple[object, ...]: return (choices(self),) + class CatalogTargetField(ChoiceTargetField): @property def catalog(self) -> type[CatalogConfigOption]: @@ -103,10 +152,10 @@ def catalog(self) -> type[CatalogConfigOption]: def editor_declarations(self) -> tuple[object, ...]: return (self.catalog,) - async def prepare(self, catalogs: ConfigOptions, - selection: object) -> CatalogTargetField: + async def prepare_catalog(self, catalogs: ConfigOptions, + selection: CatalogSelection) -> CatalogTargetField: catalog = catalogs.catalog_for(self.catalog) - options = await catalog.display_choices(cast(CatalogSelection, selection)) + options = await catalog.display_choices(selection) decoder = field_codec.compile_decoder(self.annotation) return replace(self, options=tuple(CatalogChoice(name=item.name, value=decoder.encode_editor(decoder.decode(item.value))) for item in options)) diff --git a/src/agent_comms/thread_management.py b/src/agent_comms/thread_management.py index cddd5e9f97..5be0b40458 100644 --- a/src/agent_comms/thread_management.py +++ b/src/agent_comms/thread_management.py @@ -38,6 +38,7 @@ from .errors import RelationViolationError from .field_codec import ( DeclaredWireType, + EditsAsText, FieldCodec, NullMember, RecordMember, @@ -77,7 +78,7 @@ def for_child(self, parent: frozenset[str]) -> frozenset[str]: ... @dataclass(frozen=True, slots=True) -class GivenTags(DeclaredWireType, TagChoice): +class GivenTags(EditsAsText, DeclaredWireType, TagChoice): """Tags spelled as comma-separated text on the command line, in the editor and in tools.""" wire_type = str diff --git a/tests/test_thread_creation.py b/tests/test_thread_creation.py index 9f435cead0..e7cf080452 100644 --- a/tests/test_thread_creation.py +++ b/tests/test_thread_creation.py @@ -44,8 +44,11 @@ def test_pending_new_binding_offers_missing_tags_without_constructing_the_comman binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) assert type(binding) is PendingCommandBinding tags = next(item for item in binding.editable_fields if item.name == 'tags') - assert tags.required and not tags.supplied + assert tags.required assert tags.editor_default is None + assert 'editor_default' not in tags.json_schema() + assert tags.argument(None) == {} + assert tags.argument('') == {'tags': ''} with pytest.raises(MalformedRecord): binding.edited(NewThreadCliCommand.editor_arguments({'name': 'child'})) @@ -65,7 +68,9 @@ def test_pending_catalog_context_can_change_before_tags_are_supplied(comms, sour binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) prepared = binding.for_fields({'name': 'child'}) assert prepared.context.name == 'child' - assert not next(item for item in prepared.editable_fields if item.name == 'tags').supplied + tags = next(item for item in prepared.editable_fields if item.name == 'tags') + assert tags.argument(None) == {} + assert tags.argument('') == {'tags': ''} @pytest.mark.parametrize('creation_arguments,forbidden', ( From d44eff33cae41c4e6600513a3fd4543a236cb806 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 17:45:55 -0400 Subject: [PATCH 15/26] Leave child names unsupplied until creation form completion --- src/agent_comms/cli_command.py | 23 +++++++------ src/agent_comms/cli_support/named_thread.py | 3 +- src/agent_comms/cli_support/thread_bound.py | 7 ++-- .../cli_support/thread_creation.py | 6 ++-- src/agent_comms/cli_verbs/fork.py | 21 ++++++------ src/agent_comms/cli_verbs/new.py | 6 ++-- src/agent_comms/field_codec.py | 2 +- tests/test_thread_creation.py | 32 ++++++++++++++++--- 8 files changed, 66 insertions(+), 34 deletions(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index d6f9b42fc7..08e274f0e5 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -110,12 +110,17 @@ def authorize_edits(self, arguments: Mapping[str, object]) -> frozenset[str]: raise ValueError("Parameters not offered by this target command cannot be overridden") return bound + @staticmethod + def decoded_edits(declaration: type, arguments: Mapping[str, object]) -> dict[str, object]: + return {name: value + for entry in FieldCodec.argument_fields(declaration) if entry.key in arguments + for name, value in entry.decoded(arguments).items()} + def edited(self, arguments: Mapping[str, object]) -> CliCommand: self.authorize_edits(arguments) - decoded = {name: value - for entry in FieldCodec.argument_fields(self.declaration) if entry.key in arguments - for name, value in entry.decoded(arguments).items()} - return FieldCodec.construct(self.declaration, {**self.retained_payload(), **decoded}) + return FieldCodec.construct(self.declaration, { + **self.retained_payload(), **self.decoded_edits(self.declaration, arguments), + }) @abstractmethod def confirmation(self) -> str: ... @@ -175,11 +180,11 @@ def field_value(self, entry: FieldPlan) -> TargetField: def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: self.authorize_edits(arguments) - changes = {name: value - for entry in FieldCodec.argument_fields(type(self.context)) if entry.key in arguments - for name, value in entry.decoded({ - entry.key: entry.editor_payload(arguments[entry.key]), - }).items()} + context_fields = {entry.key for entry in FieldCodec.argument_fields(type(self.context))} + wire = FieldCodec.editor_arguments(type(self.context), { + key: value for key, value in arguments.items() if key in context_fields, + }) + changes = self.decoded_edits(type(self.context), wire) return replace(self, context=replace(self.context, **changes)) if changes else self def confirmation(self) -> str: diff --git a/src/agent_comms/cli_support/named_thread.py b/src/agent_comms/cli_support/named_thread.py index badadaaecd..be9f7808d8 100644 --- a/src/agent_comms/cli_support/named_thread.py +++ b/src/agent_comms/cli_support/named_thread.py @@ -1,6 +1,7 @@ from __future__ import annotations from agent_comms.catalog_document import CatalogAccess +from agent_comms.cli_command import CommandBinding from agent_comms.channel_targets import OneChannel, ResolvedChannel from agent_comms.channel_targets import ResolvedThread from agent_comms.cli_support.named import NamedCliCommand @@ -24,7 +25,7 @@ def channel_members(cls, resolved: ResolvedChannel) -> tuple[ResolvedThread, ... return () @classmethod - def channel_bindings(cls, comms: Comms, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess) -> tuple[Self, ...]: + def channel_bindings(cls, comms: Comms, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess) -> tuple[CommandBinding, ...]: return tuple(bound for member in cls.channel_members(resolved) for bound in cls.thread_bindings( comms, member.thread, member.status, OneChannel(resolved.channel.name), catalog=catalog)) diff --git a/src/agent_comms/cli_support/thread_bound.py b/src/agent_comms/cli_support/thread_bound.py index cac53354a5..6b6e2d8129 100644 --- a/src/agent_comms/cli_support/thread_bound.py +++ b/src/agent_comms/cli_support/thread_bound.py @@ -4,13 +4,12 @@ from abc import abstractmethod from agent_comms.catalog_document import CatalogAccess -from agent_comms.cli_command import CliCommand +from agent_comms.cli_command import CliCommand, CommandBinding from agent_comms.comms import Comms from agent_comms.thread_execution import ThreadCommandAvailability from agent_comms.thread_status import ThreadStatus from agent_comms.threads import Thread from dataclasses import dataclass -from typing import Self @dataclass(frozen=True, kw_only=True) @@ -18,9 +17,9 @@ class ThreadBoundCliCommand(ThreadCommandAvailability, CliCommand): @classmethod def thread_bindings(cls, comms: Comms, thread: Thread, status: ThreadStatus, channel: SurroundingChannel, *, - catalog: CatalogAccess) -> tuple[Self, ...]: + catalog: CatalogAccess) -> tuple[CommandBinding, ...]: return (cls.thread_binding(thread),) if cls.available_for_thread(thread, status) else () @classmethod @abstractmethod - def thread_binding(cls, thread: Thread) -> Self: ... + def thread_binding(cls, thread: Thread) -> CommandBinding: ... diff --git a/src/agent_comms/cli_support/thread_creation.py b/src/agent_comms/cli_support/thread_creation.py index 07c617a558..8f8f1aeae4 100644 --- a/src/agent_comms/cli_support/thread_creation.py +++ b/src/agent_comms/cli_support/thread_creation.py @@ -20,8 +20,8 @@ @dataclass(frozen=True, kw_only=True) -class ThreadCreationFields(NameArgument, CommandContext): - """Supplied launch context is also the complete command's field declaration.""" +class ThreadLaunchSettings(CommandContext): + """Acquired launch settings; the creator has not supplied a child name.""" task: Annotated[str, Multiline(), Argument(flags=("--task",))] = "" project: Annotated[Checkout, Argument(flags=("--project",))] model: Annotated[ @@ -38,7 +38,7 @@ def project_path(self) -> Path: @dataclass(frozen=True, kw_only=True) class ThreadCreationCliCommand( - ThreadCreationFields, CatalogBinding, CliCommand[ThreadCreatedResult], declared_name=None, + NameArgument, ThreadLaunchSettings, CatalogBinding, CliCommand[ThreadCreatedResult], declared_name=None, ): @property @abstractmethod diff --git a/src/agent_comms/cli_verbs/fork.py b/src/agent_comms/cli_verbs/fork.py index c39dd652cc..14bdedb8db 100644 --- a/src/agent_comms/cli_verbs/fork.py +++ b/src/agent_comms/cli_verbs/fork.py @@ -1,6 +1,7 @@ from __future__ import annotations -from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand +from agent_comms.cli_command import PendingCommandBinding +from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand, ThreadLaunchSettings from agent_comms.cli_support.thread_bound import ThreadBoundCliCommand from agent_comms.field_codec import Argument from agent_comms.field_codec import TargetBound @@ -10,20 +11,22 @@ from agent_comms.threads import Thread from dataclasses import dataclass from typing import Annotated -from typing import Self @dataclass(frozen=True, kw_only=True) -class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ThreadCreationCliCommand): +class ForkContext(ThreadLaunchSettings): + parent: Annotated[str, TargetBound(), Argument(flags=("--parent",))] + + +@dataclass(frozen=True, kw_only=True) +class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ForkContext, ThreadCreationCliCommand): help = "Fork a child pi thread" spec_type = ForkSpec - parent: Annotated[ - str, TargetBound(), Argument(flags=("--parent",)) - ] tags: Annotated[TagChoice, Argument()] = InheritedTags() @classmethod - def thread_binding(cls, thread: Thread) -> Self: - return cls(name='', parent=thread.name, project=thread.worktree, - model=thread.model, thinking_level=thread.thinking_level) + def thread_binding(cls, thread: Thread) -> PendingCommandBinding: + return PendingCommandBinding(cls, ForkContext( + parent=thread.name, project=thread.worktree, + model=thread.model, thinking_level=thread.thinking_level)) diff --git a/src/agent_comms/cli_verbs/new.py b/src/agent_comms/cli_verbs/new.py index c0ecc5e229..aad00c4ff7 100644 --- a/src/agent_comms/cli_verbs/new.py +++ b/src/agent_comms/cli_verbs/new.py @@ -1,7 +1,7 @@ from __future__ import annotations from agent_comms.cli_command import PendingCommandBinding -from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand, ThreadCreationFields +from agent_comms.cli_support.thread_creation import ThreadCreationCliCommand, ThreadLaunchSettings from agent_comms.comms import Comms from agent_comms.field_codec import Argument from agent_comms.native_settings import NativeThreadDefaults @@ -21,6 +21,6 @@ class NewThreadCliCommand(ThreadCreationCliCommand, declared_name="new"): @classmethod def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[PendingCommandBinding, ...]: defaults = NativeThreadDefaults() - return (PendingCommandBinding(cls, ThreadCreationFields( - name='', project=project, model=defaults.model, + return (PendingCommandBinding(cls, ThreadLaunchSettings( + project=project, model=defaults.model, thinking_level=defaults.thinking_level)),) diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index aa34645a68..776c48255e 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -962,7 +962,7 @@ def format_editor(self, wire: object) -> JsonValue: @boundary_decoder def parse_editor(self, text: object) -> object: - return None if text is None else json.loads(text) + return None if text is None else FieldCodec.parse(FieldCodec.decode(str, text)) def sql_checks(self, column: str, quote: Callable[[object], str]) -> tuple[str, ...]: """Stored-value predicates; the SQL owner quotes its column and constants.""" diff --git a/tests/test_thread_creation.py b/tests/test_thread_creation.py index e7cf080452..cedf3eb331 100644 --- a/tests/test_thread_creation.py +++ b/tests/test_thread_creation.py @@ -3,6 +3,7 @@ Author does not execute these controls; the integration owner selects them once. """ from dataclasses import replace +import json import pytest @@ -10,7 +11,8 @@ from agent_comms.cli_command import PendingCommandBinding from agent_comms.cli_verbs.fork import ForkCliCommand from agent_comms.cli_verbs.new import NewThreadCliCommand -from agent_comms.field_codec import MalformedRecord +from agent_comms.field_codec import MalformedRecord, compile_decoder +from agent_comms.target_fields import UnsuppliedTargetField from agent_comms.launch_runtime import LaunchRuntime from agent_comms.repo import Checkout from agent_comms.thread_management import GivenTags, InheritedTags @@ -43,6 +45,11 @@ def test_new_cli_membership_reaches_registration(comms, source_repository, text, def test_pending_new_binding_offers_missing_tags_without_constructing_the_command(comms, source_repository): binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) assert type(binding) is PendingCommandBinding + name = next(item for item in binding.editable_fields if item.name == 'name') + assert type(name) is UnsuppliedTargetField + assert name.argument(None) == {} + with pytest.raises(MalformedRecord): + binding.edited(NewThreadCliCommand.editor_arguments({'tags': ''})) tags = next(item for item in binding.editable_fields if item.name == 'tags') assert tags.required assert tags.editor_default is None @@ -66,8 +73,9 @@ def test_pending_new_binding_distinguishes_explicit_empty_from_missing(comms, so def test_pending_catalog_context_can_change_before_tags_are_supplied(comms, source_repository): binding, = NewThreadCliCommand.project_bindings(comms, Checkout(source_repository.checkout)) - prepared = binding.for_fields({'name': 'child'}) - assert prepared.context.name == 'child' + prepared = binding.for_fields({'task': 'child task', 'name': 'child'}) + assert prepared.context.task == 'child task' + assert type(next(item for item in prepared.editable_fields if item.name == 'name')) is UnsuppliedTargetField tags = next(item for item in prepared.editable_fields if item.name == 'tags') assert tags.argument(None) == {} assert tags.argument('') == {'tags': ''} @@ -85,6 +93,14 @@ def test_complete_creation_commands_keep_command_edit_authorization(source_repos command.edited(forbidden) +def test_structured_editor_reports_malformed_json_at_the_data_boundary(): + decoder = compile_decoder(tuple[int, ...]) + with pytest.raises(MalformedRecord) as refused: + decoder.decode_editor('[1,') + assert type(refused.value.__cause__) is json.JSONDecodeError + assert decoder.decode(decoder.decode_editor('[1, 2]')) == (1, 2) + + def test_fork_cli_defaults_to_actual_parent_membership(comms, source_repository): parent_command = parsed(['new', '--name', 'parent', '--project', str(source_repository.checkout), '--tags', 'original']) @@ -92,7 +108,15 @@ def test_fork_cli_defaults_to_actual_parent_membership(comms, source_repository) command = parsed(['fork', '--name', 'child', '--parent', parent.name, '--project', str(source_repository.checkout)]) assert command.tags == InheritedTags() - editor = ForkCliCommand.thread_binding(parent) + binding = ForkCliCommand.thread_binding(parent) + assert type(binding) is PendingCommandBinding + assert binding.context.parent == parent.name + assert type(next(item for item in binding.editable_fields if item.name == 'name')) is UnsuppliedTargetField + with pytest.raises(MalformedRecord): + binding.edited({}) + with pytest.raises(ValueError, match='Target-bound'): + binding.edited({'name': 'child', 'parent': 'other'}) + editor = binding.edited(ForkCliCommand.editor_arguments({'name': 'child'})) assert editor.tags == InheritedTags() # A real later parent record, not the stale tags at editor opening, supplies membership. current = comms.registry.declare(replace(parent, tags=frozenset({'current'}))) From a44c624e76f831751755f9ed229b250141bdbdb4 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 17:52:02 -0400 Subject: [PATCH 16/26] Remove invalid trailing comma from pending context edits --- src/agent_comms/cli_command.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index 08e274f0e5..760d788bfe 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -182,7 +182,7 @@ def for_fields(self, arguments: Mapping[str, object]) -> PendingCommandBinding: self.authorize_edits(arguments) context_fields = {entry.key for entry in FieldCodec.argument_fields(type(self.context))} wire = FieldCodec.editor_arguments(type(self.context), { - key: value for key, value in arguments.items() if key in context_fields, + key: value for key, value in arguments.items() if key in context_fields }) changes = self.decoded_edits(type(self.context), wire) return replace(self, context=replace(self.context, **changes)) if changes else self From 6e53464e88b2e7976afbd0f156f51d4d04bc8c05 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 18:21:09 -0400 Subject: [PATCH 17/26] Keep native union members and derive their editor transactions --- src/agent_comms/cli_command.py | 7 +- src/agent_comms/field_codec.py | 195 ++++++++++++++++++++------- src/agent_comms/target_fields.py | 53 +++++++- src/agent_comms/thread_management.py | 3 +- tests/test_editor_defaults.py | 62 ++++++++- 5 files changed, 257 insertions(+), 63 deletions(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index 760d788bfe..d06b70a9c6 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -118,9 +118,10 @@ def decoded_edits(declaration: type, arguments: Mapping[str, object]) -> dict[st def edited(self, arguments: Mapping[str, object]) -> CliCommand: self.authorize_edits(arguments) - return FieldCodec.construct(self.declaration, { - **self.retained_payload(), **self.decoded_edits(self.declaration, arguments), - }) + values = {**self.retained_payload(), **self.decoded_edits(self.declaration, arguments)} + for parameter in self.editable_fields: + parameter.plan.require_constructor_input(values) + return FieldCodec.construct(self.declaration, values) @abstractmethod def confirmation(self) -> str: ... diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index 776c48255e..589d084a8f 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -12,7 +12,7 @@ import types from abc import ABC, abstractmethod from collections.abc import Callable, Iterable, Iterator, Mapping -from dataclasses import MISSING, Field, dataclass, field, fields, is_dataclass +from dataclasses import MISSING, Field, dataclass, field, fields, is_dataclass, make_dataclass from datetime import datetime from enum import Enum, IntFlag from functools import cache, cached_property, lru_cache, partial, reduce @@ -369,10 +369,6 @@ class Normalize(ValueDirective[Callable[[object], object]]): pass -class EditsAsText: - """This member declares the typed editor of its untagged wire family.""" - - class Editor(ValueDirective[type]): pass @@ -625,9 +621,16 @@ def argument_payload(self, value: object) -> object: def editor_payload(self, value: object) -> object: return self.editor_normalizer(value) - def editor_default(self, value: object) -> JsonValue: + def editor_default(self, value: object) -> str: return self.value.encode_editor(value) + def editor_initial(self, value: object) -> object: + return self.value.editor_initial(value) + + def require_constructor_input(self, values: Mapping[str, object]) -> None: + if self.constructor_required and self.name not in values: + raise MalformedRecord(f'Missing required field {self.key}') + @property def description(self) -> str: return Argument.declared(self.annotation).description(self.name) @@ -957,7 +960,34 @@ class DecoderPlan(ABC): def editor_annotation(self) -> object: return str - def format_editor(self, wire: object) -> JsonValue: + @property + def native_editor_annotation(self) -> object: + return self.editor_annotation + + @property + def editor_members(self) -> tuple[EditorMember, ...]: + return () + + def editor_member(self, declared: DecoderPlan) -> EditorMember: + return ScalarEditorMember(declared) + + def selected_editor_member(self, value: object) -> EditorMember: + raise TypeError(f'{self.target} does not declare a union editor') + + def project_editor(self, native: object, wire: Callable[[], object]) -> object: + return self.format_editor(wire()) + + def editor_argument(self, value: object) -> object: + return value + + def accepts_value(self, value: object) -> bool: + """Select a native member at this decoder boundary.""" + return isinstance(value, self.target) + + @abstractmethod + def decode_bound(self, data: object, context: Mapping[str, object]) -> object: ... + + def format_editor(self, wire: object) -> str: return json.dumps(wire) @boundary_decoder @@ -995,6 +1025,73 @@ def external_decoders(self, data: object) -> Iterable[ExternalDecoder]: return () +@dataclass(frozen=True) +class EditorMember(ABC): + """Read-only member metadata; only actual inputs enter a form's ObjectState.""" + + decoder: DecoderPlan + + @property + def declaration(self) -> object: + return self.decoder.target + + @property + def label(self) -> str: + return str(self.declaration) + + @property + @abstractmethod + def fields(self) -> tuple[FieldPlan, ...]: ... + + def current(self, entry: FieldPlan, value: object) -> object: + return value + + @abstractmethod + def wire_payload(self, arguments: Mapping[str, object]) -> object: ... + + def complete(self, arguments: Mapping[str, object]) -> object: + return self.decoder.decode_bound(self.wire_payload(arguments), {}) + + +class ScalarEditorMember(EditorMember): + @cached_property + def fields(self) -> tuple[FieldPlan, ...]: + declaration = make_dataclass('MemberInput', [('value', self.declaration)], + frozen=True, kw_only=True) + return FieldCodec.field_plans(declaration) + + def wire_payload(self, arguments: Mapping[str, object]) -> object: + entry, = self.fields + return entry.editor_payload(entry.raw(dict(arguments))) + + +@dataclass(frozen=True) +class RecordEditorMember(EditorMember): + plans: tuple[FieldPlan, ...] + + @property + def fields(self) -> tuple[FieldPlan, ...]: + return self.plans + + def current(self, entry: FieldPlan, value: object) -> object: + return entry.current(value) + + def wire_payload(self, arguments: Mapping[str, object]) -> object: + return {entry.key: entry.editor_payload(arguments[entry.key]) + for entry in self.fields if entry.key in arguments} + + +class NullEditorMember(EditorMember): + @property + def fields(self) -> tuple[FieldPlan, ...]: + return () + + def wire_payload(self, arguments: Mapping[str, object]) -> object: + if arguments: + raise MalformedRecord('A null member has no editor inputs') + return None + + @dataclass(frozen=True, slots=True) class ValueDecoder[T](DecoderPlan, JsonSchemaOwner): """One annotation owns its wire conversion, draft projection and choice domain.""" @@ -1062,11 +1159,6 @@ def representation_plan( def for_members[M](cls, members: tuple[type[M], ...]) -> UnionDecoder[M]: target = reduce(or_, members) alternatives = tuple(compile_decoder(member) for member in members) - editors = tuple(member for member in members if EditsAsText in member.__mro__) - if len(editors) > 1: - raise TypeError(f'{target} declares more than one typed-text editor: {editors}') - if editors: - return _TextEditedUnionDecoder(target, alternatives, compile_decoder(editors[0]).editor_wire) return UnionDecoder(target, alternatives) @abstractmethod @@ -1107,10 +1199,6 @@ def decode_resolved(self, value: T) -> T: raise MalformedRecord(f"Resolved value does not match {self.target}") return value - def accepts_value(self, value: object) -> bool: - """Select an in-process union case using its compiled declaration.""" - return isinstance(value, self.target) - def accepts_external_value(self, value: object) -> bool: """A structural container match does not validate its contents.""" return False @@ -1138,11 +1226,14 @@ def decode_keys(self, data, keys, selections): def editor_wire(self) -> DecoderPlan: return self - def encode_editor(self, value: T) -> JsonValue: - return self.format_editor(self.encode(value)) + def editor_initial(self, value: T) -> object: + return self.editor_wire.project_editor(value, lambda: self.encode(value)) + + def encode_editor(self, value: T) -> str: + return self.editor_wire.format_editor(self.encode(value)) def decode_editor(self, value: object) -> object: - return self.parse_editor(value) + return self.editor_wire.parse_editor(value) def editor_choices(self) -> tuple[tuple[str, object], ...]: raise TypeError("The declared decoder does not offer finite choices") @@ -1181,16 +1272,6 @@ def wire_structure(self) -> DecoderPlan: def editor_wire(self) -> DecoderPlan: return self.wire_structure - @property - def editor_annotation(self) -> object: - return self.wire_structure.editor_annotation - - def format_editor(self, wire: object) -> JsonValue: - return self.wire_structure.format_editor(wire) - - def parse_editor(self, text: object) -> object: - return self.wire_structure.parse_editor(text) - @property def accepts_null(self) -> bool: return self.wire_structure.accepts_null @@ -2780,7 +2861,13 @@ def fits_wire(self, data: object) -> bool: return self.accepts(data) def accepts_value(self, value: object) -> bool: - return self.accepts(value) + return type(value) is self.target + + def parse_editor(self, text: object) -> object: + value = super().parse_editor(text) + if not self.accepts_value(value): + raise MalformedRecord(f'Expected native {self.target}, received {type(value).__name__}') + return value def accepts_external_value(self, value: object) -> bool: return self.accepts(value) @@ -2806,6 +2893,9 @@ class _NullDecoder(_ExactScalarDecoder[None]): annotations = (type(None),) json_type = "null" + def editor_member(self, declared: DecoderPlan) -> EditorMember: + return NullEditorMember(declared) + def sql_checks(self, column: str, quote: Callable[[object], str]) -> tuple[str, ...]: return (f"{column} IS NULL",) @@ -2818,8 +2908,8 @@ class _TextDecoder(_ExactScalarDecoder[str]): def encode(self, value: str) -> str: return str(super().encode(value)) - def format_editor(self, wire: object) -> JsonValue: - return cast(JsonValue, wire) + def format_editor(self, wire: object) -> str: + return self.decode(wire) def parse_editor(self, text: object) -> object: return text @@ -3050,13 +3140,31 @@ def decode_external(self, data: object, record: Mapping[str, object]) -> T: def editor_annotation(self): return Union[tuple(item.editor_annotation for item in self.alternatives)] + @property + def native_editor_annotation(self) -> object: + return self.target + + @property + def editor_members(self) -> tuple[EditorMember, ...]: + return tuple(member.editor_wire.editor_member(member) for member in self.alternatives) + + def selected_editor_member(self, value: object) -> EditorMember: + member = self.selected(value) + return member.editor_wire.editor_member(member) + + def project_editor(self, native: object, wire: Callable[[], object]) -> object: + return native + + def editor_argument(self, value: object) -> object: + return self.format_editor(FieldCodec.encode(value, self.target)) + def argument_options(self, argument: Argument) -> dict[str, object]: return self.alternatives[0].argument_options(argument) def accepts_value(self, value: object) -> bool: return any(alternative.accepts_value(value) for alternative in self.alternatives) - def selected(self, value: T) -> ValueDecoder[T]: + def selected(self, value: object) -> ValueDecoder[T]: for alternative in self.alternatives: if alternative.accepts_value(value): return alternative @@ -3113,24 +3221,6 @@ def decode_candidates( first_error = None -@dataclass(frozen=True, slots=True) -class _TextEditedUnionDecoder[T](UnionDecoder[T]): - """One member's declaration owns typed text; other members remain wire choices.""" - - editor: DecoderPlan - - @property - def editor_annotation(self) -> object: - annotation = self.editor.editor_annotation - return Union[tuple((annotation, type(None)))] if self.accepts_null else annotation - - def format_editor(self, wire: object) -> JsonValue: - return self.editor.format_editor(wire) - - def parse_editor(self, text: object) -> object: - return self.editor.parse_editor(text) - - @dataclass(frozen=True, slots=True) class _ArrayDecoder[T: Iterable[object]](ValueDecoder[T]): container: ClassVar[type] @@ -3465,6 +3555,9 @@ class _RecordDecoder[T](_ContextDecoder[T]): def plan(self) -> FieldCodec._RecordPlan: return FieldCodec._record_plan(self.target) + def editor_member(self, declared: DecoderPlan) -> EditorMember: + return RecordEditorMember(declared, self.plan.bindings) + def fits_wire(self, data: object) -> TypeGuard[dict[str, object]]: if not isinstance(data, dict): return False diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index aa40e9b7a9..29e4adda58 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -39,7 +39,7 @@ def annotation(self) -> object: @property def editor_annotation(self) -> object: - return field_codec.compile_decoder(self.annotation).editor_annotation + return self.plan.value.editor_wire.native_editor_annotation @property def name(self) -> str: @@ -53,15 +53,29 @@ def multiline(self) -> bool: def description(self) -> str: return self.plan.description + @property + def editor_members(self) -> tuple[field_codec.EditorMember, ...]: + return self.plan.value.editor_wire.editor_members + + def editor_selection(self, value: object) -> object: + return self.plan.value.editor_wire.selected_editor_member(value).declaration + + @property + @abstractmethod + def editor_initial(self) -> object: ... + @property @abstractmethod - def editor_default(self) -> JsonValue: ... + def editor_default(self) -> str | None: ... @abstractmethod def default_schema(self) -> dict[str, JsonValue]: ... def argument(self, value: object) -> Mapping[str, object]: - return {self.name: value} + return {self.name: self.editor_argument(value)} + + def editor_argument(self, value: object) -> object: + return self.plan.value.editor_wire.editor_argument(value) def editor_hints(self, choices: Callable[[ChoiceTargetField], object]) -> tuple[object, ...]: """An unrestricted field does not declare a finite-choice editor.""" @@ -91,7 +105,11 @@ class SuppliedTargetField(TargetField): value: object @property - def editor_default(self) -> JsonValue: + def editor_initial(self) -> object: + return self.plan.editor_initial(self.value) + + @property + def editor_default(self) -> str: return self.plan.editor_default(self.value) def default_schema(self) -> dict[str, JsonValue]: @@ -104,15 +122,23 @@ class UnsuppliedTargetField(TargetField): @property def editor_annotation(self) -> object: - return field_codec.compile_decoder(self.annotation).editor_annotation | type(None) + return self.plan.value.editor_wire.native_editor_annotation | type(None) + + @property + def editor_initial(self) -> object: + return None @property - def editor_default(self) -> JsonValue: + def editor_default(self) -> None: return None def default_schema(self) -> dict[str, JsonValue]: return {} + @boundary_decoder + def editor_selection(self, value: object) -> object: + return None if value is None else super().editor_selection(value) + @boundary_decoder def argument(self, value: object) -> Mapping[str, object]: """Decode the optional control's raw presence once, not the field's wire value.""" @@ -124,6 +150,21 @@ class ChoiceTargetField(SuppliedTargetField): """A prepared finite declaration, from its annotation or configuration catalog.""" options: tuple[CatalogChoice, ...] = () + @property + def editor_annotation(self) -> object: + return self.plan.value.editor_wire.editor_annotation + + @property + def editor_members(self) -> tuple[field_codec.EditorMember, ...]: + return () + + @property + def editor_initial(self) -> object: + return self.editor_default + + def editor_argument(self, value: object) -> object: + return value + @property def editor_declarations(self) -> tuple[object, ...]: return () diff --git a/src/agent_comms/thread_management.py b/src/agent_comms/thread_management.py index 5be0b40458..cddd5e9f97 100644 --- a/src/agent_comms/thread_management.py +++ b/src/agent_comms/thread_management.py @@ -38,7 +38,6 @@ from .errors import RelationViolationError from .field_codec import ( DeclaredWireType, - EditsAsText, FieldCodec, NullMember, RecordMember, @@ -78,7 +77,7 @@ def for_child(self, parent: frozenset[str]) -> frozenset[str]: ... @dataclass(frozen=True, slots=True) -class GivenTags(EditsAsText, DeclaredWireType, TagChoice): +class GivenTags(DeclaredWireType, TagChoice): """Tags spelled as comma-separated text on the command line, in the editor and in tools.""" wire_type = str diff --git a/tests/test_editor_defaults.py b/tests/test_editor_defaults.py index c4b6e38e8a..1ce950cf2e 100644 --- a/tests/test_editor_defaults.py +++ b/tests/test_editor_defaults.py @@ -4,7 +4,9 @@ import pytest from agent_comms.cli_verbs.fork import ForkCliCommand -from agent_comms.field_codec import FieldCodec, compile_decoder +from agent_comms.field_codec import FieldCodec, MalformedRecord, compile_decoder +from agent_comms.target_fields import TargetField +from agent_comms.thread_management import ChangedTags, GivenTags, InheritedTags, TagChoice from agent_comms.pi_vocabulary import HighThinkingLevel, NoThinkingOverride, ThinkingLevelField from agent_comms.repo import LaunchCheckout from agent_comms.thread_execution import ExternalThreadExecution, ThreadExecution @@ -19,6 +21,10 @@ (str | None, None, None), (bool, True, True), (tuple[int, ...], (1, 2), [1, 2]), + (TagChoice, GivenTags(frozenset({'auth'})), 'auth'), + (TagChoice, InheritedTags(), None), + (TagChoice, ChangedTags(frozenset({'auth'}), frozenset({'old'})), + {'add': ['auth'], 'remove': ['old']}), )) def test_declared_default_editor_round_trip(annotation, value, encoded): decoder = compile_decoder(annotation) @@ -41,3 +47,57 @@ def test_fork_default_retains_annotated_thinking_representation(source_repositor assert compile_decoder(str).encode_editor('literal text') == 'literal text' +@pytest.mark.parametrize('annotation,value', ( + (TagChoice, GivenTags(frozenset())), + (TagChoice, GivenTags(frozenset({'auth'}))), + (TagChoice, InheritedTags()), + (TagChoice, ChangedTags(frozenset({'auth'}), frozenset({'old'}))), + (int | float, 3), + (int | float, 3.0), + (int | float | None, None), +)) +def test_native_union_member_and_field_derived_transaction(annotation: object, value: object) -> None: + decoder = compile_decoder(annotation) + assert decoder.editor_initial(value) is value + member = decoder.editor_wire.selected_editor_member(value) + assert member.declaration is type(value) + assert tuple(item.declaration for item in decoder.editor_wire.editor_members).count(type(value)) == 1 + parameters = tuple(TargetField.of(entry, member.current(entry, value)) for entry in member.fields) + arguments = {key: item for parameter in parameters + for key, item in parameter.argument(parameter.editor_initial).items()} + result = member.complete(arguments) + assert type(result) is type(value) + assert result == value + json.dumps(decoder.encode_editor(value)) + assert decoder.decode_editor(decoder.editor_wire.editor_argument(value)) == FieldCodec.encode(value, annotation) + + +def test_overlapping_scalar_wire_does_not_change_selected_native_member() -> None: + decoder = compile_decoder(int | float) + member = decoder.editor_wire.selected_editor_member(3.0) + assert member.declaration is float + assert member.complete({'value': '3.0'}) == 3.0 + with pytest.raises(MalformedRecord): + member.complete({'value': '3'}) + with pytest.raises(MalformedRecord): + FieldCodec.decode(int | float, 3) + + +def test_native_union_default_does_not_leak_into_public_json_schema() -> None: + plan = next(entry for entry in FieldCodec.argument_fields(ForkCliCommand) if entry.name == 'tags') + value = ChangedTags(frozenset({'auth'}), frozenset()) + parameter = TargetField.of(plan, value) + assert parameter.editor_initial is value + assert parameter.editor_selection(value) is ChangedTags + assert isinstance(parameter.editor_default, str) + json.dumps(parameter.json_schema()) + + +def test_record_member_inputs_remain_required_without_invented_values() -> None: + decoder = compile_decoder(TagChoice) + member = decoder.editor_wire.selected_editor_member(ChangedTags(frozenset(), frozenset())) + assert all(entry.constructor_required for entry in member.fields) + with pytest.raises(MalformedRecord): + member.complete({}) + + From d50cc189031fb97a1bcb70464934a605c749c00e Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 18:40:03 -0400 Subject: [PATCH 18/26] Make unchosen a nominal member of editor selection --- src/agent_comms/field_codec.py | 42 +++++++++++++++++++++++++++++++- src/agent_comms/target_fields.py | 23 ++++++++++++++--- tests/test_editor_defaults.py | 23 +++++++++++++++-- 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index 589d084a8f..19f69af185 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -1025,8 +1025,45 @@ def external_decoders(self, data: object) -> Iterable[ExternalDecoder]: return () +class MemberDraftFactory[Draft](ABC): + """An integration constructs drafts; selections own which construction applies.""" + + @abstractmethod + def unchosen(self, member: UnchosenMember) -> Draft: ... + + @abstractmethod + def member(self, member: EditorMember, value: object) -> Draft: ... + + +class MemberSelection(ABC): + @property + @abstractmethod + def declaration(self) -> object: ... + + @property + @abstractmethod + def label(self) -> str: ... + + @abstractmethod + def draft[Draft](self, factory: MemberDraftFactory[Draft], value: object) -> Draft: ... + + +@dataclass(frozen=True) +class UnchosenMember(MemberSelection): + @property + def declaration(self) -> object: + return type(self) + + @property + def label(self) -> str: + return 'Choose a member…' + + def draft[Draft](self, factory: MemberDraftFactory[Draft], value: object) -> Draft: + return factory.unchosen(self) + + @dataclass(frozen=True) -class EditorMember(ABC): +class EditorMember(MemberSelection): """Read-only member metadata; only actual inputs enter a form's ObjectState.""" decoder: DecoderPlan @@ -1039,6 +1076,9 @@ def declaration(self) -> object: def label(self) -> str: return str(self.declaration) + def draft[Draft](self, factory: MemberDraftFactory[Draft], value: object) -> Draft: + return factory.member(self, value) + @property @abstractmethod def fields(self) -> tuple[FieldPlan, ...]: ... diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index 29e4adda58..782edb78d0 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -3,6 +3,7 @@ from abc import ABC, abstractmethod from collections.abc import Callable, Mapping from dataclasses import dataclass, replace +from functools import singledispatchmethod from pydantic import JsonValue @@ -57,8 +58,12 @@ def description(self) -> str: def editor_members(self) -> tuple[field_codec.EditorMember, ...]: return self.plan.value.editor_wire.editor_members - def editor_selection(self, value: object) -> object: - return self.plan.value.editor_wire.selected_editor_member(value).declaration + @property + def editor_selections(self) -> tuple[field_codec.MemberSelection, ...]: + return self.editor_members + + def editor_selection(self, value: object) -> field_codec.MemberSelection: + return self.plan.value.editor_wire.selected_editor_member(value) @property @abstractmethod @@ -135,9 +140,19 @@ def editor_default(self) -> None: def default_schema(self) -> dict[str, JsonValue]: return {} + @property + def editor_selections(self) -> tuple[field_codec.MemberSelection, ...]: + return (field_codec.UnchosenMember(), *super().editor_selections) + + @singledispatchmethod + @boundary_decoder + def editor_selection(self, value: object) -> field_codec.MemberSelection: + return super().editor_selection(value) + + @editor_selection.register @boundary_decoder - def editor_selection(self, value: object) -> object: - return None if value is None else super().editor_selection(value) + def _unchosen_selection(self, value: None) -> field_codec.MemberSelection: + return field_codec.UnchosenMember() @boundary_decoder def argument(self, value: object) -> Mapping[str, object]: diff --git a/tests/test_editor_defaults.py b/tests/test_editor_defaults.py index 1ce950cf2e..9a2a1dd2fa 100644 --- a/tests/test_editor_defaults.py +++ b/tests/test_editor_defaults.py @@ -4,7 +4,9 @@ import pytest from agent_comms.cli_verbs.fork import ForkCliCommand -from agent_comms.field_codec import FieldCodec, MalformedRecord, compile_decoder +from dataclasses import dataclass + +from agent_comms.field_codec import FieldCodec, MalformedRecord, UnchosenMember, compile_decoder from agent_comms.target_fields import TargetField from agent_comms.thread_management import ChangedTags, GivenTags, InheritedTags, TagChoice from agent_comms.pi_vocabulary import HighThinkingLevel, NoThinkingOverride, ThinkingLevelField @@ -88,11 +90,28 @@ def test_native_union_default_does_not_leak_into_public_json_schema() -> None: value = ChangedTags(frozenset({'auth'}), frozenset()) parameter = TargetField.of(plan, value) assert parameter.editor_initial is value - assert parameter.editor_selection(value) is ChangedTags + assert parameter.editor_selection(value).declaration is ChangedTags assert isinstance(parameter.editor_default, str) json.dumps(parameter.json_schema()) +def test_unchosen_selection_is_not_a_supplied_null_member() -> None: + @dataclass(frozen=True) + class RequiredUnion: + value: int | float | None + + plan, = FieldCodec.argument_fields(RequiredUnion) + omitted = TargetField.omitted(plan) + unchosen = omitted.editor_selection(omitted.editor_initial) + assert isinstance(unchosen, UnchosenMember) + assert unchosen.declaration is UnchosenMember + assert unchosen.label == 'Choose a member…' + assert unchosen.declaration in tuple(member.declaration for member in omitted.editor_selections) + assert omitted.editor_selection(3).declaration is int + supplied_null = TargetField.of(plan, None).editor_selection(None) + assert supplied_null.declaration is type(None) + + def test_record_member_inputs_remain_required_without_invented_values() -> None: decoder = compile_decoder(TagChoice) member = decoder.editor_wire.selected_editor_member(ChangedTags(frozenset(), frozenset())) From 7b139e1450083df31fdff9ecf5bdcf6737fc8b81 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 19:04:38 -0400 Subject: [PATCH 19/26] Ask the value decoder directly for editor projections --- src/agent_comms/field_codec.py | 6 ------ src/agent_comms/target_fields.py | 4 ++-- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index 19f69af185..6cf5718230 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -621,12 +621,6 @@ def argument_payload(self, value: object) -> object: def editor_payload(self, value: object) -> object: return self.editor_normalizer(value) - def editor_default(self, value: object) -> str: - return self.value.encode_editor(value) - - def editor_initial(self, value: object) -> object: - return self.value.editor_initial(value) - def require_constructor_input(self, values: Mapping[str, object]) -> None: if self.constructor_required and self.name not in values: raise MalformedRecord(f'Missing required field {self.key}') diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index 782edb78d0..5cd068266e 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -111,11 +111,11 @@ class SuppliedTargetField(TargetField): @property def editor_initial(self) -> object: - return self.plan.editor_initial(self.value) + return self.plan.value.editor_initial(self.value) @property def editor_default(self) -> str: - return self.plan.editor_default(self.value) + return self.plan.value.encode_editor(self.value) def default_schema(self) -> dict[str, JsonValue]: return {'editor_default': self.editor_default} From 3b94c9bd598faa2d4b4eb6c53f577b3d1182ea0e Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Thu, 29 Oct 2026 21:18:34 -0400 Subject: [PATCH 20/26] Remove absent editor defaults and reuse scalar admission --- src/agent_comms/field_codec.py | 2 +- src/agent_comms/target_fields.py | 8 -------- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index 6cf5718230..a852d7e989 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -2895,7 +2895,7 @@ def fits_wire(self, data: object) -> bool: return self.accepts(data) def accepts_value(self, value: object) -> bool: - return type(value) is self.target + return self.accepts(value) def parse_editor(self, text: object) -> object: value = super().parse_editor(text) diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index 5cd068266e..48192859a6 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -69,10 +69,6 @@ def editor_selection(self, value: object) -> field_codec.MemberSelection: @abstractmethod def editor_initial(self) -> object: ... - @property - @abstractmethod - def editor_default(self) -> str | None: ... - @abstractmethod def default_schema(self) -> dict[str, JsonValue]: ... @@ -133,10 +129,6 @@ def editor_annotation(self) -> object: def editor_initial(self) -> object: return None - @property - def editor_default(self) -> None: - return None - def default_schema(self) -> dict[str, JsonValue]: return {} From 5adaa973e2015b02e7441d590d731279b34306df Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 30 Oct 2026 02:28:26 -0400 Subject: [PATCH 21/26] Let native member selection own omitted editor arguments --- src/agent_comms/field_codec.py | 12 +++++++++++- src/agent_comms/target_fields.py | 4 +--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/agent_comms/field_codec.py b/src/agent_comms/field_codec.py index a852d7e989..fb63a9dbb7 100644 --- a/src/agent_comms/field_codec.py +++ b/src/agent_comms/field_codec.py @@ -966,7 +966,7 @@ def editor_member(self, declared: DecoderPlan) -> EditorMember: return ScalarEditorMember(declared) def selected_editor_member(self, value: object) -> EditorMember: - raise TypeError(f'{self.target} does not declare a union editor') + return self.editor_member(self) def project_editor(self, native: object, wire: Callable[[], object]) -> object: return self.format_editor(wire()) @@ -1030,6 +1030,11 @@ def member(self, member: EditorMember, value: object) -> Draft: ... class MemberSelection(ABC): + def argument( + self, key: str, value: object, decode: Callable[[object], object], + ) -> Mapping[str, object]: + return {key: decode(value)} + @property @abstractmethod def declaration(self) -> object: ... @@ -1044,6 +1049,11 @@ def draft[Draft](self, factory: MemberDraftFactory[Draft], value: object) -> Dra @dataclass(frozen=True) class UnchosenMember(MemberSelection): + def argument( + self, key: str, value: object, decode: Callable[[object], object], + ) -> Mapping[str, object]: + return {} + @property def declaration(self) -> object: return type(self) diff --git a/src/agent_comms/target_fields.py b/src/agent_comms/target_fields.py index 48192859a6..64cbb07ee4 100644 --- a/src/agent_comms/target_fields.py +++ b/src/agent_comms/target_fields.py @@ -146,10 +146,8 @@ def editor_selection(self, value: object) -> field_codec.MemberSelection: def _unchosen_selection(self, value: None) -> field_codec.MemberSelection: return field_codec.UnchosenMember() - @boundary_decoder def argument(self, value: object) -> Mapping[str, object]: - """Decode the optional control's raw presence once, not the field's wire value.""" - return {} if value is None else super().argument(value) + return self.editor_selection(value).argument(self.name, value, self.editor_argument) @dataclass(frozen=True) From 2072c0d4bffd146da78f9c1e91af14d4a927655e Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 30 Oct 2026 16:02:59 -0400 Subject: [PATCH 22/26] Derive a pending binding's command from its context's declared command 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 Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu --- src/agent_comms/cli_command.py | 15 ++++++++------- src/agent_comms/cli_verbs/fork.py | 2 +- src/agent_comms/cli_verbs/new.py | 9 +++++++-- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index 43e8059b27..f4f7d11c07 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -147,23 +147,24 @@ async def prepare_field(self, item: TargetField, catalogs: ConfigOptions) -> Tar @dataclass(frozen=True) class CommandContext(CatalogSelection): - """A declaration of supplied fields, not an alternate command editor.""" + """A declaration of supplied fields, not an alternate command editor; the one declared + command inheriting it owns the fields it lacks.""" + + @classmethod + def command(cls) -> type[CliCommand]: + (command,) = CliCommand.members_with(cls) + return command @dataclass(frozen=True) class PendingCommandBinding(CatalogBinding): """Only supplied context, borrowing the concrete command's declared fields.""" - command: type[CliCommand] context: CommandContext - def __post_init__(self) -> None: - if not issubclass(self.command, type(self.context)): - raise TypeError('A pending context must be a declared parent of its command') - @property def declaration(self) -> type[CliCommand]: - return self.command + return type(self.context).command() @property def catalog_context(self) -> CatalogSelection: diff --git a/src/agent_comms/cli_verbs/fork.py b/src/agent_comms/cli_verbs/fork.py index 14bdedb8db..16f4991ab3 100644 --- a/src/agent_comms/cli_verbs/fork.py +++ b/src/agent_comms/cli_verbs/fork.py @@ -27,6 +27,6 @@ class ForkCliCommand(NativeOwnerCommand, ThreadBoundCliCommand, ForkContext, Thr @classmethod def thread_binding(cls, thread: Thread) -> PendingCommandBinding: - return PendingCommandBinding(cls, ForkContext( + return PendingCommandBinding(ForkContext( parent=thread.name, project=thread.worktree, model=thread.model, thinking_level=thread.thinking_level)) diff --git a/src/agent_comms/cli_verbs/new.py b/src/agent_comms/cli_verbs/new.py index aad00c4ff7..2661ab09fe 100644 --- a/src/agent_comms/cli_verbs/new.py +++ b/src/agent_comms/cli_verbs/new.py @@ -13,7 +13,12 @@ @dataclass(frozen=True, kw_only=True) -class NewThreadCliCommand(ThreadCreationCliCommand, declared_name="new"): +class NewThreadContext(ThreadLaunchSettings): + """The launch settings a project offers before the creator names the child.""" + + +@dataclass(frozen=True, kw_only=True) +class NewThreadCliCommand(NewThreadContext, ThreadCreationCliCommand, declared_name="new"): help = "Create a new pi thread" spec_type: ClassVar[type[NewThreadSpec]] = NewThreadSpec tags: Annotated[GivenTags, Argument()] @@ -21,6 +26,6 @@ class NewThreadCliCommand(ThreadCreationCliCommand, declared_name="new"): @classmethod def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[PendingCommandBinding, ...]: defaults = NativeThreadDefaults() - return (PendingCommandBinding(cls, ThreadLaunchSettings( + return (PendingCommandBinding(NewThreadContext( project=project, model=defaults.model, thinking_level=defaults.thinking_level)),) From c194d566a4601b5403789838be0fec2ae7bcc63b Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 30 Oct 2026 16:35:01 -0400 Subject: [PATCH 23/26] Assert the unsupplied tags field by its type, not a missing default 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 ce332013 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 Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu --- tests/test_thread_creation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_thread_creation.py b/tests/test_thread_creation.py index cedf3eb331..3dbb64f2b7 100644 --- a/tests/test_thread_creation.py +++ b/tests/test_thread_creation.py @@ -52,7 +52,7 @@ def test_pending_new_binding_offers_missing_tags_without_constructing_the_comman binding.edited(NewThreadCliCommand.editor_arguments({'tags': ''})) tags = next(item for item in binding.editable_fields if item.name == 'tags') assert tags.required - assert tags.editor_default is None + assert type(tags) is UnsuppliedTargetField assert 'editor_default' not in tags.json_schema() assert tags.argument(None) == {} assert tags.argument('') == {'tags': ''} From efb40c5f7143ec1061d0d250e3834aa6bde7b2f8 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 30 Oct 2026 16:39:08 -0400 Subject: [PATCH 24/26] Let CliCommand's declared default answer type stand in channel_targets 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 Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu --- src/agent_comms/channel_targets.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/agent_comms/channel_targets.py b/src/agent_comms/channel_targets.py index fddd6a3585..f5307a50f9 100644 --- a/src/agent_comms/channel_targets.py +++ b/src/agent_comms/channel_targets.py @@ -24,7 +24,6 @@ from .catalog_document import CatalogAccess, CatalogDocument from .channels import Channel from .cli_command import CliCommand, CommandBinding - from .cli_result import CliResult from .comms import Comms from .goal_management import Goals from .historical_views import ChannelHistory, DMHistory, HistoryView @@ -177,7 +176,7 @@ def history_result(self, require_channel: Callable[[str], Channel], messages: tu raise ValueError("History requires a #channel or DM peer.") def contexts( - self, declaration: type[CliCommand[CliResult]], channel: ChannelSelection, + self, declaration: type[CliCommand], channel: ChannelSelection, ) -> tuple[SurroundingChannel, ...]: return (NoChannel(),) @@ -393,7 +392,7 @@ def history_result(self, require_channel: Callable[[str], Channel], messages: tu return self.history_result_type.for_target(self.name, messages, has_older) def contexts( - self, declaration: type[CliCommand[CliResult]], channel: ChannelSelection, + self, declaration: type[CliCommand], channel: ChannelSelection, ) -> tuple[SurroundingChannel, ...]: return declaration.selection_channels(channel, self.name) From 83469868bfee4d5dc467141104b462f0af78fea7 Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 30 Oct 2026 17:02:42 -0400 Subject: [PATCH 25/26] Dispatch command bindings on the resolved target's type 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 Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu --- src/agent_comms/channel_targets.py | 24 ++----------- src/agent_comms/cli_command.py | 52 ++++++++++++++++++++------- tests/test_cli.py | 4 +-- tests/test_commander_notifications.py | 15 +++++--- 4 files changed, 54 insertions(+), 41 deletions(-) diff --git a/src/agent_comms/channel_targets.py b/src/agent_comms/channel_targets.py index f5307a50f9..e264ec88db 100644 --- a/src/agent_comms/channel_targets.py +++ b/src/agent_comms/channel_targets.py @@ -21,9 +21,9 @@ if TYPE_CHECKING: from .audience_manifest import AudiencedMessage, FrozenRecipient - from .catalog_document import CatalogAccess, CatalogDocument + from .catalog_document import CatalogDocument from .channels import Channel - from .cli_command import CliCommand, CommandBinding + from .cli_command import CliCommand from .comms import Comms from .goal_management import Goals from .historical_views import ChannelHistory, DMHistory, HistoryView @@ -161,11 +161,6 @@ def available(self, snapshot: RegistrySnapshot) -> bool: @abstractmethod def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], Channel]): ... - @abstractmethod - def bindings[C: CliCommand]( - self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved, - *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[CommandBinding, ...]: ... def history_view(self, viewer: DisplayViewer, require_channel: Callable[[str], Channel], channel: type[ChannelHistory], direct: type[DMHistory]) -> HistoryView: @@ -409,11 +404,6 @@ def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], C thread = snapshot.require(self.selected_name(snapshot)) return ResolvedThread(thread, snapshot.status(thread.name)) - def bindings[C: CliCommand]( - self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved: ResolvedThread, - *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[CommandBinding, ...]: - return declaration.thread_bindings(comms, resolved.thread, resolved.status, channel, catalog=catalog) def validate(self, message: Message) -> None: if message.sender == self.name and message.type.conversation_message: @@ -558,11 +548,6 @@ def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], C if channel.matches(thread.tags)) return ResolvedChannel(channel, members) - def bindings[C: CliCommand]( - self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved: ResolvedChannel, - *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[CommandBinding, ...]: - return declaration.channel_bindings(comms, resolved, project=project, snapshot=snapshot, catalog=catalog) def validate(self, sender: Message | None = None): from .channels import BuiltinChannelView @@ -650,11 +635,6 @@ def destination_names(self) -> tuple[str, ...]: def resolve(self, snapshot: RegistrySnapshot, require_channel: Callable[[str], Channel]): return self.project - def bindings[C: CliCommand]( - self, declaration: type[C], comms: Comms, channel: SurroundingChannel, resolved, - *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - ) -> tuple[CommandBinding, ...]: - return declaration.project_bindings(comms, resolved) _TAG_CHARS = set("abcdefghijklmnopqrstuvwxyz0123456789-_") diff --git a/src/agent_comms/cli_command.py b/src/agent_comms/cli_command.py index f4f7d11c07..c7a4013856 100644 --- a/src/agent_comms/cli_command.py +++ b/src/agent_comms/cli_command.py @@ -27,6 +27,7 @@ from agent_comms.declared_family import ModuleMembers from agent_comms.field_codec import FieldPlan, TargetBound from agent_comms.launch_root import LaunchRoot +from functools import singledispatchmethod from inspect import iscoroutinefunction, signature from types import MappingProxyType, get_original_bases from typing import TYPE_CHECKING, TextIO, cast, get_args, get_origin @@ -278,12 +279,36 @@ def project_bindings(cls, comms: Comms, project: Checkout) -> tuple[CommandBindi def channel_bindings(cls, comms: Comms, resolved: ResolvedChannel, *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess) -> tuple[CommandBinding, ...]: return () + @singledispatchmethod @classmethod - def bindings(cls, comms: Comms, target: CommandTarget, channel: SurroundingChannel, - *, project: Checkout, snapshot: RegistrySnapshot, catalog: CatalogAccess, - resolved: ResolvedThread | ResolvedChannel | Checkout) -> tuple[CommandBinding, ...]: - return target.bindings( - cls, comms, channel, resolved, project=project, snapshot=snapshot, catalog=catalog) + def bindings(cls, resolved: object, comms: Comms, channel: SurroundingChannel, + *, project: Checkout, snapshot: RegistrySnapshot, + catalog: CatalogAccess) -> tuple[CommandBinding, ...]: + """The resolved target's type selects which declared bindings answer it.""" + raise TypeError(f"{type(resolved).__qualname__} is not a resolved command target") + + @bindings.register + @classmethod + def _thread_bindings(cls, resolved: ResolvedThread, comms: Comms, channel: SurroundingChannel, + *, project: Checkout, snapshot: RegistrySnapshot, + catalog: CatalogAccess) -> tuple[CommandBinding, ...]: + return cls.thread_bindings( + comms, resolved.thread, resolved.status, channel, catalog=catalog) + + @bindings.register + @classmethod + def _channel_bindings(cls, resolved: ResolvedChannel, comms: Comms, channel: SurroundingChannel, + *, project: Checkout, snapshot: RegistrySnapshot, + catalog: CatalogAccess) -> tuple[CommandBinding, ...]: + return cls.channel_bindings( + comms, resolved, project=project, snapshot=snapshot, catalog=catalog) + + @bindings.register + @classmethod + def _project_bindings(cls, resolved: Checkout, comms: Comms, channel: SurroundingChannel, + *, project: Checkout, snapshot: RegistrySnapshot, + catalog: CatalogAccess) -> tuple[CommandBinding, ...]: + return cls.project_bindings(comms, resolved) @classmethod def target_catalog(cls, comms: Comms, target: TargetSelection, @@ -308,7 +333,8 @@ def target_catalog(cls, comms: Comms, target: TargetSelection, for name, resolved in resolved_targets: for context in name.contexts(member, selected_channels): available = member.bindings( - comms, name, context, project=project, snapshot=snapshot, catalog=catalog, resolved=resolved) + resolved, comms, context, + project=project, snapshot=snapshot, catalog=catalog) if available: declaration = target.listed_declaration(member) grouped.setdefault(declaration, {}).update(dict.fromkeys( @@ -559,8 +585,9 @@ def run(self, declaration: type[CliCommand], comms: Comms, arguments: Mapping[st name, = self.targets resolved = name.resolve(snapshot, comms.bus.require_channel) bindings = tuple(bound for context in name.contexts(declaration, channel) - for bound in declaration.bindings(comms, name, context, project=project, snapshot=snapshot, - catalog=catalog, resolved=resolved)) + for bound in declaration.bindings( + resolved, comms, context, + project=project, snapshot=snapshot, catalog=catalog)) if len(bindings) == 1: bound = bindings[0].for_editor(comms, name.name, project).edited(arguments) return bound.with_confirmation(confirmed).apply(comms) @@ -609,8 +636,9 @@ def run(self, declaration: type[CliCommand], comms: Comms, arguments: Mapping[st for context in name.contexts(declaration, channel): try: bindings = tuple(command for member in declarations - for command in member.bindings(comms, name, context, - project=project, snapshot=snapshot, catalog=catalog, resolved=resolved)) + for command in member.bindings( + resolved, comms, context, + project=project, snapshot=snapshot, catalog=catalog)) if not bindings: raise CommandSelectionChanged( f'This action is no longer available for {name}{context.scope()}') @@ -629,8 +657,8 @@ def run(self, declaration: type[CliCommand], comms: Comms, arguments: Mapping[st current_catalog = (comms.channels.catalog.read() if context.needs_catalog or name.needs_channel_state else NoCatalog()) if bound_command not in bound_command.declaration.bindings( - comms, name, context, project=project, snapshot=current, catalog=current_catalog, - resolved=name.resolve(current, comms.bus.require_channel)): + name.resolve(current, comms.bus.require_channel), comms, context, + project=project, snapshot=current, catalog=current_catalog): raise CommandSelectionChanged( f'The bound command for {name}{context.scope()} changed; refresh the selection') command.schedule(TargetBatchSchedule(position, name, outcomes, together), comms) diff --git a/tests/test_cli.py b/tests/test_cli.py index e4fda55671..b9ced51871 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -523,8 +523,8 @@ def test_channel_start_uses_native_visible_roster_without_launching(self, tmp_pa catalog = comms.channels.catalog.read() target = ChannelTarget("#team") commands = StartCliCommand.bindings( - comms, target, NoChannel(), project=Checkout(tmp_path), snapshot=snapshot, catalog=catalog, - resolved=target.resolve(snapshot, comms.bus.require_channel)) + target.resolve(snapshot, comms.bus.require_channel), comms, NoChannel(), + project=Checkout(tmp_path), snapshot=snapshot, catalog=catalog) assert tuple(command.name for command in commands) == ("alpha",) assert all(comms.registry.require(name).process_identity == NoProcess() for name in ("alpha", "external", "archived", "human")) diff --git a/tests/test_commander_notifications.py b/tests/test_commander_notifications.py index 99c7d78c2e..05ec7f7f94 100644 --- a/tests/test_commander_notifications.py +++ b/tests/test_commander_notifications.py @@ -939,11 +939,16 @@ def test_fork_cli_default_inherits_audience_but_not_commander_role(notification_ comms = notification_bus parent = comms.registry.require('worker') tag = CommandersTag.declared_name - parent = comms.registry.register(replace(parent, tags=parent.tags | {'comms', tag})) - command = ForkCliCommand.thread_binding(parent) - assert command.tags.for_new() == parent.tags - {tag} - explicit = replace(command, tags=GivenTags(parent.tags)) - assert tag in explicit.tags.for_new() + comms.registry.register(replace(parent, tags=parent.tags | {'comms', tag})) + parent = comms.registry.require('worker') + binding = ForkCliCommand.thread_binding(parent) + command = binding.edited(ForkCliCommand.editor_arguments({'name': 'child'})) + + def child_tags(fork: ForkCliCommand) -> frozenset[str]: + return fork.specification().child(parent, parent.runtime, comms.threads).tags + + assert child_tags(command) == parent.tags - {tag} + assert tag in child_tags(replace(command, tags=GivenTags(parent.tags))) def test_a_commander_is_declared_never_inherited(notification_bus, tmp_path): From 77eec13520d96583cbaf6aab6b98e84468b35cbd Mon Sep 17 00:00:00 2001 From: Tristan Simas Date: Fri, 30 Oct 2026 17:13:27 -0400 Subject: [PATCH 26/26] Fold the fork commander-role check into the fork membership test 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 Claude-Session: https://claude.ai/code/session_01XbFMSoWgC3ZwKzjT4SwMDu --- tests/test_commander_notifications.py | 18 ------------------ tests/test_thread_creation.py | 9 +++++++-- 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/tests/test_commander_notifications.py b/tests/test_commander_notifications.py index 05ec7f7f94..96dd5eb6ae 100644 --- a/tests/test_commander_notifications.py +++ b/tests/test_commander_notifications.py @@ -15,8 +15,6 @@ from agent_comms.registry_document import RegisteredCommanderProject from agent_comms.child_process import ProcessIdentity -from agent_comms.cli_verbs.fork import ForkCliCommand -from agent_comms.thread_management import GivenTags from agent_comms.commander_heartbeat import CommanderHeartbeat, CommanderObservation from agent_comms.comms import Comms from commander_heartbeat_fixture import ObservedHeartbeat, capture_frame, current_hud @@ -935,22 +933,6 @@ def test_one_pass_gives_every_commander_its_own_packet(notification_bus): assert CommanderFrame.current(comms, recipient.name).packets == (quiet.task,) -def test_fork_cli_default_inherits_audience_but_not_commander_role(notification_bus): - comms = notification_bus - parent = comms.registry.require('worker') - tag = CommandersTag.declared_name - comms.registry.register(replace(parent, tags=parent.tags | {'comms', tag})) - parent = comms.registry.require('worker') - binding = ForkCliCommand.thread_binding(parent) - command = binding.edited(ForkCliCommand.editor_arguments({'name': 'child'})) - - def child_tags(fork: ForkCliCommand) -> frozenset[str]: - return fork.specification().child(parent, parent.runtime, comms.threads).tags - - assert child_tags(command) == parent.tags - {tag} - assert tag in child_tags(replace(command, tags=GivenTags(parent.tags))) - - def test_a_commander_is_declared_never_inherited(notification_bus, tmp_path): """The hook's role answer reads the one derivation: a thread registered with the tag commands; a fork carries its commander's tags but acts for it, and a bound unit of a diff --git a/tests/test_thread_creation.py b/tests/test_thread_creation.py index 3dbb64f2b7..9a43736043 100644 --- a/tests/test_thread_creation.py +++ b/tests/test_thread_creation.py @@ -8,6 +8,7 @@ import pytest from agent_comms.cli import build_parser +from agent_comms.channel_targets import CommandersTag from agent_comms.cli_command import PendingCommandBinding from agent_comms.cli_verbs.fork import ForkCliCommand from agent_comms.cli_verbs.new import NewThreadCliCommand @@ -119,7 +120,11 @@ def test_fork_cli_defaults_to_actual_parent_membership(comms, source_repository) editor = binding.edited(ForkCliCommand.editor_arguments({'name': 'child'})) assert editor.tags == InheritedTags() # A real later parent record, not the stale tags at editor opening, supplies membership. - current = comms.registry.declare(replace(parent, tags=frozenset({'current'}))) + # The commander role is declared, never inherited; an explicit choice keeps it. + commander = CommandersTag.declared_name + current = comms.registry.declare(replace(parent, tags=frozenset({'current', commander}))) child = command.specification().child(current, current.runtime, comms.threads) - assert child.tags == current.tags == frozenset({'current'}) + assert child.tags == current.tags - {commander} == frozenset({'current'}) + explicit = replace(command, tags=GivenTags(current.tags)) + assert commander in explicit.specification().child(current, current.runtime, comms.threads).tags assert editor.tags.for_child(current.tags) == current.tags