cleveragents/cleveragents-core

Fork 3

Proposal: update specification — resolve default enabled strategies contradiction and document checkpoint trigger names #3692

New issue

Open

opened 2026-04-05 22:12:07 +00:00 by freemo · 1 comment

freemo commented

2026-04-05 22:12:07 +00:00

Owner

Proposal: Specification Update

This proposal was triggered by the following recently merged PRs:

#3635 — fix(acms): implement real retrieval logic in all 6 spec-required context strategies
#3474 — fix(executor): implement automatic per-tool-write and event-based checkpoint triggers
#3676 — fix(acms): invoke SkeletonCompressor in ContextAssembler.assemble() to propagate skeleton context to child plans
#3619 — fix(plan-executor): wire SubplanService and SubplanExecutionService into Execute phase

Discrepancies Found

1. Internal Contradiction: Default Enabled Strategies (CRITICAL)

Spec section: docs/specification.md lines 25659 and 30732

Current spec text (line 25659):

[context.strategies]
enabled = ["simple-keyword", "semantic-embedding", "arce", "breadth-depth-navigator"]

This shows 4 strategies including arce.

Current spec text (line 30732 — config key table):

| `context.strategies.enabled` | list | `["simple-keyword", "semantic-embedding", "breadth-depth-navigator"]` | ...

This shows 3 strategies without arce.

Implementation (strategy_stubs.py line 965):

DEFAULT_ENABLED_STRATEGIES: tuple[str, ...] = (
    "simple-keyword",
    "semantic-embedding",
    "breadth-depth-navigator",
)

The implementation follows the 3-strategy list (line 30732), which is the more specific/operational reference (the config key table).

Proposed fix: Update line 25659 to match line 30732 — remove arce from the example enabled list. The arce strategy is available but not enabled by default because it requires all three backends (text + vector + graph) which may not be available in all deployments.

Proposed text for line 25659:

[context.strategies]
enabled = ["simple-keyword", "semantic-embedding", "breadth-depth-navigator"]

2. Missing Documentation: Automatic Checkpoint Trigger Names

Spec section: docs/specification.md §19387 (Checkpointing in Execute)

Current spec text: The spec describes automatic checkpointing at Execute-phase decision points but does not define the named trigger events used to configure which automatic checkpoints fire.

Implementation (tool/runner.py lines 53-60):

DEFAULT_AUTO_TRIGGERS: frozenset[str] = frozenset({
    "before_tool_execute",
    "after_tool_execute",
    "on_subplan_spawn",
    "on_error",
})

The implementation defines 4 named triggers:

before_tool_execute — checkpoint before any tool with writes=True executes
after_tool_execute — checkpoint after any tool with writes=True completes
on_subplan_spawn — checkpoint when a child plan is spawned
on_error — checkpoint on execution error (for forensic rollback)

These are configurable via auto_checkpoint_triggers on ToolRunner. The PlanExecutor delegates to ToolRunner.is_trigger_active().

Proposed fix: Add a subsection to §19387 documenting the 4 trigger names, their semantics, and the core.checkpoints.auto_create_on configuration key (which maps to auto_checkpoint_triggers).

Proposed text to add after line 19436:

##### Automatic Checkpoint Triggers

The runtime supports four named automatic checkpoint triggers, configurable via `core.checkpoints.auto_create_on` in `config.toml`:

| Trigger | Default | Description |
|---------|---------|-------------|
| `before_tool_execute` | enabled | Checkpoint created before any tool with `writes=True` executes |
| `after_tool_execute` | enabled | Checkpoint created after any tool with `writes=True` completes successfully |
| `on_subplan_spawn` | enabled | Checkpoint created when a child plan is spawned |
| `on_error` | enabled | Checkpoint created on execution error for forensic rollback |

All four triggers are active by default. Individual triggers can be disabled:

```toml
[core.checkpoints]
auto_create_on = ["before_tool_execute", "after_tool_execute", "on_subplan_spawn"]
# on_error omitted to disable error checkpoints

The ToolRunner evaluates trigger activation; the PlanExecutor and SubplanExecutionService delegate to ToolRunner.is_trigger_active().


---

### 3. SkeletonCompressor Interface Discrepancy

**Spec section:** `docs/specification.md` line 45346

**Current spec pseudocode:**
```python
skeleton = self.pipeline.skeleton_compressor.compress(
    parent_context=parent_context,
    child_focus=child_focus,
    skeleton_budget=skeleton_budget,
)

Implementation (acms_service.py line 969):

skeleton_frags = self._skeleton_compressor.compress(
    parent_fragments,
    skeleton_budget,
)

The implementation uses a simpler interface: compress(fragments: list[ContextFragment], skeleton_budget: int) rather than the spec's compress(parent_context, child_focus, skeleton_budget). The implementation is cleaner — child_focus is not needed because the compressor sorts by relevance score (which already encodes focus proximity), and parent_context is replaced by the more concrete parent_fragments.

Proposed fix: Update the spec pseudocode at line 45346 to match the implemented interface:

skeleton = self.pipeline.skeleton_compressor.compress(
    parent_fragments,
    skeleton_budget,
)

And update the SkeletonCompressorProtocol definition (line 45069) to reflect the actual signature:

class SkeletonCompressorProtocol(Protocol):
    def compress(
        self,
        fragments: list[ContextFragment],
        skeleton_budget: int,
    ) -> tuple[ContextFragment, ...]: ...

Scope of Changes

Spec Section	Change Type	Lines Affected
§25659 (Strategy Registration example)	Fix contradiction	~1 line
§19387 (Checkpointing in Execute)	Add trigger documentation	~20 lines added
§45346 (Context Inheritance Mechanism)	Update interface signature	~5 lines
§45069 (SkeletonCompressorProtocol)	Update protocol signature	~5 lines

Rationale

Change 1 resolves a spec-internal contradiction. The implementation correctly follows the config key table (line 30732) which is the authoritative operational reference. The example at line 25659 was inconsistent.
Change 2 documents implementation behavior that was added in PR #3474 but not reflected in the spec. The trigger names are part of the public configuration API.
Change 3 updates pseudocode to match the implemented interface. The implementation is better — it avoids passing child_focus (which the compressor doesn't need since relevance scores encode focus) and uses concrete ContextFragment objects rather than an opaque parent_context object.

Automated by CleverAgents Bot
Supervisor: Spec Evolution | Agent: ca-spec-updater

## Proposal: Specification Update This proposal was triggered by the following recently merged PRs: - **#3635** — `fix(acms): implement real retrieval logic in all 6 spec-required context strategies` - **#3474** — `fix(executor): implement automatic per-tool-write and event-based checkpoint triggers` - **#3676** — `fix(acms): invoke SkeletonCompressor in ContextAssembler.assemble() to propagate skeleton context to child plans` - **#3619** — `fix(plan-executor): wire SubplanService and SubplanExecutionService into Execute phase` --- ## Discrepancies Found ### 1. Internal Contradiction: Default Enabled Strategies (CRITICAL) **Spec section:** `docs/specification.md` lines 25659 and 30732 **Current spec text (line 25659):** ```toml [context.strategies] enabled = ["simple-keyword", "semantic-embedding", "arce", "breadth-depth-navigator"] ``` This shows **4 strategies** including `arce`. **Current spec text (line 30732 — config key table):** ``` | `context.strategies.enabled` | list | `["simple-keyword", "semantic-embedding", "breadth-depth-navigator"]` | ... ``` This shows **3 strategies** without `arce`. **Implementation (`strategy_stubs.py` line 965):** ```python DEFAULT_ENABLED_STRATEGIES: tuple[str, ...] = ( "simple-keyword", "semantic-embedding", "breadth-depth-navigator", ) ``` The implementation follows the **3-strategy** list (line 30732), which is the more specific/operational reference (the config key table). **Proposed fix:** Update line 25659 to match line 30732 — remove `arce` from the example `enabled` list. The `arce` strategy is available but not enabled by default because it requires all three backends (text + vector + graph) which may not be available in all deployments. **Proposed text for line 25659:** ```toml [context.strategies] enabled = ["simple-keyword", "semantic-embedding", "breadth-depth-navigator"] ``` --- ### 2. Missing Documentation: Automatic Checkpoint Trigger Names **Spec section:** `docs/specification.md` §19387 (Checkpointing in Execute) **Current spec text:** The spec describes automatic checkpointing at Execute-phase decision points but does not define the named trigger events used to configure which automatic checkpoints fire. **Implementation (`tool/runner.py` lines 53-60):** ```python DEFAULT_AUTO_TRIGGERS: frozenset[str] = frozenset({ "before_tool_execute", "after_tool_execute", "on_subplan_spawn", "on_error", }) ``` The implementation defines 4 named triggers: - `before_tool_execute` — checkpoint before any tool with `writes=True` executes - `after_tool_execute` — checkpoint after any tool with `writes=True` completes - `on_subplan_spawn` — checkpoint when a child plan is spawned - `on_error` — checkpoint on execution error (for forensic rollback) These are configurable via `auto_checkpoint_triggers` on `ToolRunner`. The `PlanExecutor` delegates to `ToolRunner.is_trigger_active()`. **Proposed fix:** Add a subsection to §19387 documenting the 4 trigger names, their semantics, and the `core.checkpoints.auto_create_on` configuration key (which maps to `auto_checkpoint_triggers`). **Proposed text to add after line 19436:** ```markdown ##### Automatic Checkpoint Triggers The runtime supports four named automatic checkpoint triggers, configurable via `core.checkpoints.auto_create_on` in `config.toml`: | Trigger | Default | Description | |---------|---------|-------------| | `before_tool_execute` | enabled | Checkpoint created before any tool with `writes=True` executes | | `after_tool_execute` | enabled | Checkpoint created after any tool with `writes=True` completes successfully | | `on_subplan_spawn` | enabled | Checkpoint created when a child plan is spawned | | `on_error` | enabled | Checkpoint created on execution error for forensic rollback | All four triggers are active by default. Individual triggers can be disabled: ```toml [core.checkpoints] auto_create_on = ["before_tool_execute", "after_tool_execute", "on_subplan_spawn"] # on_error omitted to disable error checkpoints ``` The `ToolRunner` evaluates trigger activation; the `PlanExecutor` and `SubplanExecutionService` delegate to `ToolRunner.is_trigger_active()`. ``` --- ### 3. SkeletonCompressor Interface Discrepancy **Spec section:** `docs/specification.md` line 45346 **Current spec pseudocode:** ```python skeleton = self.pipeline.skeleton_compressor.compress( parent_context=parent_context, child_focus=child_focus, skeleton_budget=skeleton_budget, ) ``` **Implementation (`acms_service.py` line 969):** ```python skeleton_frags = self._skeleton_compressor.compress( parent_fragments, skeleton_budget, ) ``` The implementation uses a simpler interface: `compress(fragments: list[ContextFragment], skeleton_budget: int)` rather than the spec's `compress(parent_context, child_focus, skeleton_budget)`. The implementation is cleaner — `child_focus` is not needed because the compressor sorts by relevance score (which already encodes focus proximity), and `parent_context` is replaced by the more concrete `parent_fragments`. **Proposed fix:** Update the spec pseudocode at line 45346 to match the implemented interface: ```python skeleton = self.pipeline.skeleton_compressor.compress( parent_fragments, skeleton_budget, ) ``` And update the `SkeletonCompressorProtocol` definition (line 45069) to reflect the actual signature: ```python class SkeletonCompressorProtocol(Protocol): def compress( self, fragments: list[ContextFragment], skeleton_budget: int, ) -> tuple[ContextFragment, ...]: ... ``` --- ## Scope of Changes | Spec Section | Change Type | Lines Affected | |---|---|---| | §25659 (Strategy Registration example) | Fix contradiction | ~1 line | | §19387 (Checkpointing in Execute) | Add trigger documentation | ~20 lines added | | §45346 (Context Inheritance Mechanism) | Update interface signature | ~5 lines | | §45069 (SkeletonCompressorProtocol) | Update protocol signature | ~5 lines | ## Rationale - **Change 1** resolves a spec-internal contradiction. The implementation correctly follows the config key table (line 30732) which is the authoritative operational reference. The example at line 25659 was inconsistent. - **Change 2** documents implementation behavior that was added in PR #3474 but not reflected in the spec. The trigger names are part of the public configuration API. - **Change 3** updates pseudocode to match the implemented interface. The implementation is better — it avoids passing `child_focus` (which the compressor doesn't need since relevance scores encode focus) and uses concrete `ContextFragment` objects rather than an opaque `parent_context` object. --- **Automated by CleverAgents Bot** Supervisor: Spec Evolution | Agent: ca-spec-updater

freemo referenced this issue

2026-04-05 22:13:11 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:13:12 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo commented

2026-04-05 22:20:24 +00:00

Author

Owner

Label compliance fix applied:

Removed: State/Completed, State/In Progress, State/Verified (3 conflicting State labels)
Removed: Type/Automation (incorrect type for a spec proposal)
Kept: Priority/Medium
Added: State/Unverified, Type/Task
Reason: Issue had 4 conflicting State labels and an incorrect Type label. Per CONTRIBUTING.md, every issue must have exactly ONE State/* label and ONE Type/* label. This is a spec update proposal, so Type/Task is correct. State/Unverified is the appropriate starting state.

Automated by CleverAgents Bot
Supervisor: Backlog Grooming | Agent: ca-backlog-groomer

Label compliance fix applied: - **Removed**: `State/Completed`, `State/In Progress`, `State/Verified` (3 conflicting State labels) - **Removed**: `Type/Automation` (incorrect type for a spec proposal) - **Kept**: `Priority/Medium` - **Added**: `State/Unverified`, `Type/Task` - Reason: Issue had 4 conflicting State labels and an incorrect Type label. Per CONTRIBUTING.md, every issue must have exactly ONE `State/*` label and ONE `Type/*` label. This is a spec update proposal, so `Type/Task` is correct. `State/Unverified` is the appropriate starting state. --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: ca-backlog-groomer

freemo referenced this issue

2026-04-05 22:20:38 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:22:14 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654