diff --git a/docs/reference/devcontainer_resources.md b/docs/reference/devcontainer_resources.md index 12cf50b57..75524bdbe 100644 --- a/docs/reference/devcontainer_resources.md +++ b/docs/reference/devcontainer_resources.md @@ -73,17 +73,47 @@ root): 1. `.devcontainer/devcontainer.json` 2. `.devcontainer.json` (root-level) +3. `.devcontainer//devcontainer.json` (named configuration, v3.6.0+) + +### Named Configuration Scanning (v3.6.0) + +As of **v3.6.0**, the discovery module supports **named configurations** +for monorepo projects. A named configuration is a +`devcontainer.json` file located at +`.devcontainer//devcontainer.json`, where `` is the +subdirectory name (e.g. `api`, `frontend`, `worker`). + +Each named configuration produces a separate `devcontainer-instance` +resource. The `config_name` attribute on `DevcontainerDiscoveryResult` +carries the subdirectory name, allowing the resource registry to +distinguish between multiple devcontainer environments in the same +repository. + +```python +from cleveragents.resource.handlers.discovery import discover_devcontainers + +results = discover_devcontainers( + resource_location="/path/to/repo", + resource_type="git-checkout", +) + +for result in results: + print(result.config_name) # None for root configs, "api" / "frontend" etc. for named + print(result.config_path) # Absolute path to devcontainer.json +``` ### Discovery Process (Planned) 1. **Trigger**: Linking a `git-checkout` or `fs-directory` resource. 2. **Scan**: The discovery module checks for configuration files at the - well-known paths listed above. + well-known paths listed above, including named configurations under + `.devcontainer//devcontainer.json`. 3. **Validate**: Each discovered file is parsed as JSON. Invalid files are skipped with a warning. 4. **Register**: For each valid configuration: - A `devcontainer-instance` child resource is created under the - parent resource. + parent resource. Named configurations carry the `config_name` + attribute set to the subdirectory name. - A `devcontainer-file` child resource is created under the devcontainer instance, pointing to the JSON file. @@ -298,10 +328,20 @@ when the resource is in a non-running state. It handles both The `cleveragents.resource.handlers.discovery` module provides: - `discover_devcontainers(location, type)`: Scans a path for - devcontainer configs. + devcontainer configs, including named configurations (v3.6.0+). - `is_trigger_type(type)`: Checks if a resource type triggers auto-discovery. -- `DevcontainerDiscoveryResult`: Data class holding discovery results. +- `DevcontainerDiscoveryResult`: Data class holding discovery results, + including the `config_name` attribute for named configurations. + +#### `DevcontainerDiscoveryResult` attributes + +| Attribute | Type | Description | +|--------------------|-----------------|-----------------------------------------------------------------------------| +| `config_path` | `Path` | Absolute path to the `devcontainer.json` file. | +| `config_data` | `dict` | Parsed JSON content of the devcontainer config. | +| `parent_location` | `str` | Location of the parent resource. | +| `config_name` | `str \| None` | Name of the named configuration (e.g. `"api"`), or `None` for root configs. | ## See Also diff --git a/docs/reference/di.md b/docs/reference/di.md index 52bfc5fdd..f5ad27681 100644 --- a/docs/reference/di.md +++ b/docs/reference/di.md @@ -132,3 +132,53 @@ decision = decision_svc.record_decision( # Retrieve decisions decisions = decision_svc.list_decisions("01HV...") ``` + +## Skeleton Compressor Resolution + +The ACMS skeleton compressor is resolved outside the main DI container +via `resolve_configured_skeleton_compressor()` in +`cleveragents.application.services.acms_skeleton_compressor`. + +The default implementation is `DepthReductionCompressor`, which +re-renders parent context fragments at overview depths 0–1 to fit a +token budget. It was introduced in **v3.5.0**. + +```python +from cleveragents.application.services.acms_skeleton_compressor import ( + DepthReductionCompressor, + resolve_configured_skeleton_compressor, +) + +# Resolve the configured compressor (returns DepthReductionCompressor by default) +compressor = resolve_configured_skeleton_compressor() + +# Compress fragments to fit a budget +compressed: tuple = compressor.compress(fragments, skeleton_budget=2000) +``` + +The `compress()` method signature: + +```python +def compress( + self, + fragments: tuple[ContextFragment, ...], + skeleton_budget: int, +) -> tuple[ContextFragment, ...]: + ... +``` + +It returns a `tuple[ContextFragment, ...]` — the compressed subset of +fragments whose total token count fits within `skeleton_budget`. + +### Custom Compressor + +A custom compressor can be registered via the config key +`context.pipeline.skeleton-compressor`: + +```toml +[context.pipeline] +skeleton-compressor = "mypackage.mymodule.MyCompressor" +``` + +The class must expose a `compress(fragments, skeleton_budget)` method +with the same signature as `DepthReductionCompressor.compress`. diff --git a/docs/reference/skeleton_compressor.md b/docs/reference/skeleton_compressor.md index 55120d4ea..a3d6474e6 100644 --- a/docs/reference/skeleton_compressor.md +++ b/docs/reference/skeleton_compressor.md @@ -7,11 +7,54 @@ plan's accumulated context for propagation to child plans as inherited context. Compression is governed by the `skeleton_ratio` budget parameter set on a project's context policy. -The compressor lives in +The default compressor is `DepthReductionCompressor`, introduced in +**v3.5.0** as part of the ACMS (Adaptive Context Management System) +pipeline. It lives in +`cleveragents.application.services.acms_skeleton_compressor.DepthReductionCompressor` +and is registered in the DI container via +`resolve_configured_skeleton_compressor()`. + +The legacy ratio-based compressor lives in `cleveragents.application.services.skeleton_compressor.SkeletonCompressorService` and is registered in the DI container as `skeleton_compressor_service`. -## Skeleton Ratio +## DepthReductionCompressor (v3.5.0+) + +`DepthReductionCompressor` compresses parent fragments by re-rendering +them at overview depths 0–1 using the UKO ontology detail-level maps. +It is the default compressor used by the ACMS skeleton pipeline. + +### Algorithm + +1. Normalise each fragment's `detail_level_domain` metadata to the + most specific UKO domain available. +2. Sort fragments by `(-relevance_score, detail_depth, fragment_id)`. +3. For each fragment, attempt to re-render at depth 1 (summary) then + depth 0 (overview). Use the first rendering that fits the remaining + budget. +4. If no rendering fits, clip the overview rendering to the remaining + budget. +5. Return the compressed tuple and log a summary. + +### Configuration + +The compressor is resolved via `resolve_configured_skeleton_compressor()` +in `cleveragents.application.services.acms_skeleton_compressor`. The +default is `builtin:DepthReductionCompressor`. A custom compressor can +be configured via: + +```toml +[context.pipeline] +skeleton-compressor = "mypackage.mymodule.MyCompressor" +``` + +The configured class must expose a `compress(fragments, skeleton_budget)` +method. + +## Skeleton Ratio (Legacy SkeletonCompressorService) + +The legacy `SkeletonCompressorService` uses a `skeleton_ratio` budget +parameter: | Ratio | Meaning | Behaviour | |------:|:--------|:----------| @@ -37,7 +80,7 @@ deterministic output: identical inputs always produce identical compressed payloads regardless of the order in which fragments arrive. -## Compression Algorithm +## Compression Algorithm (Legacy) 1. Validate all inputs (ratio, fragment fields). 2. Compute `original_tokens` — sum of `token_count` across all diff --git a/features/align_documentation.feature b/features/align_documentation.feature new file mode 100644 index 000000000..5327f9d6d --- /dev/null +++ b/features/align_documentation.feature @@ -0,0 +1,50 @@ +Feature: Documentation alignment for DepthReductionCompressor and devcontainer discovery + As a developer reading the CleverAgents documentation + I need the reference docs to accurately reflect the current implementation + So that I can understand and use the system correctly + + # ── skeleton_compressor.md ────────────────────────────────────────── + + Scenario: skeleton_compressor.md references DepthReductionCompressor + Given the skeleton_compressor.md documentation file exists + When I read the skeleton_compressor.md documentation file + Then the documentation should contain "DepthReductionCompressor" + + Scenario: skeleton_compressor.md references the correct module path + Given the skeleton_compressor.md documentation file exists + When I read the skeleton_compressor.md documentation file + Then the documentation should contain "acms_skeleton_compressor" + + Scenario: skeleton_compressor.md references v3.5.0 milestone + Given the skeleton_compressor.md documentation file exists + When I read the skeleton_compressor.md documentation file + Then the documentation should contain "v3.5.0" + + # ── di.md ─────────────────────────────────────────────────────────── + + Scenario: di.md documents the DI resolution path + Given the di.md documentation file exists + When I read the di.md documentation file + Then the documentation should contain "resolve_configured_skeleton_compressor" + + Scenario: di.md usage example matches tuple return type + Given the di.md documentation file exists + When I read the di.md documentation file + Then the documentation should contain "DepthReductionCompressor" + + # ── devcontainer_resources.md ─────────────────────────────────────── + + Scenario: devcontainer_resources.md covers named configuration scanning + Given the devcontainer_resources.md documentation file exists + When I read the devcontainer_resources.md documentation file + Then the documentation should contain "named configuration" + + Scenario: devcontainer_resources.md references v3.6.0 + Given the devcontainer_resources.md documentation file exists + When I read the devcontainer_resources.md documentation file + Then the documentation should contain "v3.6.0" + + Scenario: devcontainer_resources.md documents config_name attribute + Given the devcontainer_resources.md documentation file exists + When I read the devcontainer_resources.md documentation file + Then the documentation should contain "config_name" diff --git a/features/execution_environment.feature b/features/execution_environment.feature index c439136cc..49b16fcdf 100644 --- a/features/execution_environment.feature +++ b/features/execution_environment.feature @@ -129,10 +129,10 @@ Feature: Execution environment routing Scenario: CONTAINER_RESOURCE_TYPES includes expected types Given I import CONTAINER_RESOURCE_TYPES - Then it should contain "container-instance" - And it should contain "devcontainer-instance" - And it should contain "devcontainer-file" - And it should not contain "git-checkout" + Then the container types should contain "container-instance" + And the container types should contain "devcontainer-instance" + And the container types should contain "devcontainer-file" + And the container types should not contain "git-checkout" # ── ContainerUnavailableError ─────────────────────────────────────── @@ -145,4 +145,3 @@ Feature: Execution environment routing When I create a ContainerUnavailableError without project name Then exec-env the error message should contain "Container resource unavailable" And exec-env the error message should not contain "for project" - diff --git a/features/steps/documentation_steps.py b/features/steps/documentation_steps.py new file mode 100644 index 000000000..d91242d0d --- /dev/null +++ b/features/steps/documentation_steps.py @@ -0,0 +1,39 @@ +"""Step definitions for documentation alignment tests.""" + +from pathlib import Path + +from behave import given, then, when + + +@given("the {filename} documentation file exists") +def step_doc_file_exists(context: object, filename: str) -> None: + """Check that a documentation file exists.""" + context.doc_path = Path(__file__).parent.parent.parent / "docs" / "reference" / filename + assert context.doc_path.exists(), f"Documentation file not found: {context.doc_path}" + + +@when("I read the {filename} documentation file") +def step_read_doc_file(context: object, filename: str) -> None: + """Read a documentation file.""" + context.doc_path = Path(__file__).parent.parent.parent / "docs" / "reference" / filename + context.doc_content = context.doc_path.read_text(encoding="utf-8") + + +@then("the documentation should contain {text}") +def step_should_contain(context: object, text: str) -> None: + """Check that documentation contains specific text.""" + # Remove quotes from the text parameter + search_text = text.strip('"\'') + assert search_text in context.doc_content, ( + f"Expected text not found in documentation: {search_text}" + ) + + +@then("the documentation should not contain {text}") +def step_should_not_contain(context: object, text: str) -> None: + """Check that documentation does not contain specific text.""" + # Remove quotes from the text parameter + search_text = text.strip('"\'') + assert search_text not in context.doc_content, ( + f"Unexpected text found in documentation: {search_text}" + ) diff --git a/features/steps/execution_environment_steps.py b/features/steps/execution_environment_steps.py index 1379ffb40..54349e1c0 100644 --- a/features/steps/execution_environment_steps.py +++ b/features/steps/execution_environment_steps.py @@ -342,12 +342,12 @@ def step_import_types(context: Context) -> None: context.container_types = CONTAINER_RESOURCE_TYPES -@then('it should contain "{value}"') +@then('the container types should contain "{value}"') def step_types_contain(context: Context, value: str) -> None: assert value in context.container_types -@then('it should not contain "{value}"') +@then('the container types should not contain "{value}"') def step_types_not_contain(context: Context, value: str) -> None: assert value not in context.container_types diff --git a/features/steps/tui_permissions_screen_steps.py b/features/steps/tui_permissions_screen_steps.py index f6e4b9160..5cfe991d9 100644 --- a/features/steps/tui_permissions_screen_steps.py +++ b/features/steps/tui_permissions_screen_steps.py @@ -616,3 +616,71 @@ def step_left_side_empty_for_inserted(context): @then("the clear result should be True") def step_clear_result_true(context): assert context._clear_result is True + + +# --------------------------------------------------------------------------- +# Bug #10488: PermissionsScreen base class checks +# --------------------------------------------------------------------------- + + +@when("I check the base class of PermissionsScreen") +def step_check_base_class(context): + from cleveragents.tui.permissions.screen import PermissionsScreen + + context._permissions_screen_cls = PermissionsScreen + + +@then("PermissionsScreen should be a subclass of textual.app.Screen") +def step_permissions_screen_is_screen_subclass(context): + import importlib + + from cleveragents.tui.permissions import screen as screen_module + + # Check the _ScreenBase variable in the module — if textual is available, + # it should be textual.app.Screen; if not, the fallback is used. + screen_base = getattr(screen_module, "_ScreenBase", None) + assert screen_base is not None, ( + "Expected screen module to have _ScreenBase variable" + ) + + try: + Screen = importlib.import_module("textual.app").Screen + # Textual is available — verify PermissionsScreen inherits from Screen + assert issubclass(context._permissions_screen_cls, Screen), ( + f"Expected PermissionsScreen to be a subclass of textual.app.Screen, " + f"but its MRO is: {[c.__name__ for c in context._permissions_screen_cls.__mro__]}" + ) + assert screen_base is Screen, ( + f"Expected _ScreenBase to be textual.app.Screen, got {screen_base!r}" + ) + except ImportError: + # Textual not installed — verify the module is designed to use Screen + # by checking that _load_screen_base is defined (not _load_static_base). + load_fn = getattr(screen_module, "_load_screen_base", None) + assert load_fn is not None, ( + "Expected screen module to have _load_screen_base function " + "(not _load_static_base). The module must be designed to load " + "textual.app.Screen as the base class." + ) + + +@then("PermissionsScreen should have a BINDINGS class variable") +def step_permissions_screen_has_bindings(context): + cls = context._permissions_screen_cls + assert hasattr(cls, "BINDINGS"), ( + "Expected PermissionsScreen to have a BINDINGS class variable" + ) + assert cls.BINDINGS, ( + "Expected PermissionsScreen.BINDINGS to be non-empty" + ) + + +@then('PermissionsScreen should have action method "{method_name}"') +def step_permissions_screen_has_action_method(context, method_name): + cls = context._permissions_screen_cls + assert hasattr(cls, method_name), ( + f"Expected PermissionsScreen to have action method '{method_name}'" + ) + assert callable(getattr(cls, method_name)), ( + f"Expected PermissionsScreen.{method_name} to be callable" + ) diff --git a/features/tui_permissions_screen.feature b/features/tui_permissions_screen.feature index d96f920b8..8fb741269 100644 --- a/features/tui_permissions_screen.feature +++ b/features/tui_permissions_screen.feature @@ -373,3 +373,26 @@ Feature: TUI PermissionsScreen And I record decision "allow_always" for request "req-1" And I clear the session decision for "local/file-write" Then the clear result should be True + + # ── Bug #10488: PermissionsScreen must inherit from Screen ──── + + @tdd_issue @tdd_issue_10488 + Scenario: Bug #10488 - PermissionsScreen inherits from textual.app.Screen + When I check the base class of PermissionsScreen + Then PermissionsScreen should be a subclass of textual.app.Screen + + @tdd_issue @tdd_issue_10488 + Scenario: Bug #10488 - PermissionsScreen has BINDINGS class variable + When I check the base class of PermissionsScreen + Then PermissionsScreen should have a BINDINGS class variable + + @tdd_issue @tdd_issue_10488 + Scenario: Bug #10488 - PermissionsScreen has action methods for keyboard bindings + When I check the base class of PermissionsScreen + Then PermissionsScreen should have action method "action_allow_once" + And PermissionsScreen should have action method "action_allow_always" + And PermissionsScreen should have action method "action_reject_once" + And PermissionsScreen should have action method "action_reject_always" + And PermissionsScreen should have action method "action_nav_next" + And PermissionsScreen should have action method "action_nav_prev" + And PermissionsScreen should have action method "action_cycle_diff" diff --git a/src/cleveragents/tui/permissions/screen.py b/src/cleveragents/tui/permissions/screen.py index b56b05df5..a303f6500 100644 --- a/src/cleveragents/tui/permissions/screen.py +++ b/src/cleveragents/tui/permissions/screen.py @@ -1,4 +1,4 @@ -"""PermissionsScreen widget for displaying tool permission requests with diff views. +"""PermissionsScreen for displaying tool permission requests with diff views. Shows a split-pane layout: a file list on the left and a diff view on the right. Supports three diff display modes (unified, side-by-side, context) toggled with ``d``. @@ -8,8 +8,9 @@ Allow/reject keyboard bindings: ``a`` allow-once, ``A`` allow-always, from __future__ import annotations +import contextlib import importlib -from typing import Any +from typing import Any, ClassVar from cleveragents.tui.permissions.models import ( DiffDisplayMode, @@ -23,22 +24,22 @@ __all__ = ["PermissionsScreen"] # ── Optional Textual import gate ───────────────────────────────── -def _load_static_base() -> type[Any]: +def _load_screen_base() -> type[Any]: try: - return importlib.import_module("textual.widgets").Static + return importlib.import_module("textual.app").Screen except Exception: # pragma: no cover - class _FallbackStatic: + class _FallbackScreen: def __init__(self, *args: object, **kwargs: object) -> None: self._text = "" def update(self, text: str) -> None: self._text = text - return _FallbackStatic + return _FallbackScreen -_StaticBase = _load_static_base() +_ScreenBase = _load_screen_base() # ── Diff display mode cycle ─────────────────────────────────────── @@ -110,11 +111,16 @@ def _render_screen( return f"Permission Request\n\n{file_list}\n\n{diff_panel}\n\n{status_bar}" -# ── PermissionsScreen widget ────────────────────────────────────── +# ── PermissionsScreen ───────────────────────────────────────────── -class PermissionsScreen(_StaticBase): - """TUI widget that displays a tool permission request with a diff view. +class PermissionsScreen(_ScreenBase): + """TUI screen that displays a tool permission request with a diff view. + + Inherits from ``textual.app.Screen`` so it can be pushed onto the + Textual screen stack via ``app.push_screen()``, receive keyboard events + natively through ``BINDINGS``, and participate in the full Textual + screen lifecycle (``compose``, ``on_mount``, ``dismiss``, etc.). Layout: - Title: "Permission Request" @@ -129,9 +135,20 @@ class PermissionsScreen(_StaticBase): - ``R``: reject always (session) - ``j`` / ``k``: navigate file list (next / previous) - ``d``: cycle diff display mode (unified → side-by-side → context) - - ``escape``: dismiss (caller responsibility) + - ``escape``: dismiss screen """ + BINDINGS: ClassVar[list[tuple[str, str, str]]] = [ + ("a", "allow_once", "Allow Once"), + ("A", "allow_always", "Allow Always"), + ("r", "reject_once", "Reject Once"), + ("R", "reject_always", "Reject Always"), + ("j", "nav_next", "Next File"), + ("k", "nav_prev", "Previous File"), + ("d", "cycle_diff", "Cycle Diff Mode"), + ("escape", "dismiss_screen", "Dismiss"), + ] + def __init__(self, *args: object, **kwargs: object) -> None: super().__init__(*args, **kwargs) self._request: ToolPermissionRequest | None = None @@ -139,7 +156,22 @@ class PermissionsScreen(_StaticBase): self._diff_mode: DiffDisplayMode = DiffDisplayMode.UNIFIED self._decision: PermissionDecision | None = None self._text: str = "(no permission request)" - self.update(self._text) + + def update(self, text: str) -> None: + """Update the internal text representation. + + Stores the rendered text for programmatic access and testing. + In a live Textual app, the ``compose()`` method renders the + content via widgets; this method keeps the ``_text`` attribute + in sync for non-Textual usage and testing. + """ + self._text = text + + def compose(self) -> Any: + """Compose the screen layout with a Static widget showing the content.""" + with contextlib.suppress(Exception): # pragma: no cover + Static = importlib.import_module("textual.widgets").Static + yield Static(self._text, id="permissions-content") # ── Public API ──────────────────────────────────────────────── @@ -238,6 +270,41 @@ class PermissionsScreen(_StaticBase): self._refresh() return decision + # ── Textual action methods ──────────────────────────────────── + + def action_allow_once(self) -> None: + """Textual action: allow once (``a`` key binding).""" + self.allow_once() + + def action_allow_always(self) -> None: + """Textual action: allow always (``A`` key binding).""" + self.allow_always() + + def action_reject_once(self) -> None: + """Textual action: reject once (``r`` key binding).""" + self.reject_once() + + def action_reject_always(self) -> None: + """Textual action: reject always (``R`` key binding).""" + self.reject_always() + + def action_nav_next(self) -> None: + """Textual action: navigate to next file (``j`` key binding).""" + self.navigate_next() + + def action_nav_prev(self) -> None: + """Textual action: navigate to previous file (``k`` key binding).""" + self.navigate_prev() + + def action_cycle_diff(self) -> None: + """Textual action: cycle diff display mode (``d`` key binding).""" + self.cycle_diff_mode() + + def action_dismiss_screen(self) -> None: + """Textual action: dismiss the screen (``escape`` key binding).""" + with contextlib.suppress(Exception): # pragma: no cover + self.dismiss() + # ── Rendering ───────────────────────────────────────────────── def _refresh(self) -> None: