cleveragents/cleveragents-core

Fork 3

Proposal: update specification — clarify validation attach args format and document ACMS strategy v1 approximation #4382

New issue

Open

opened 2026-04-08 11:42:40 +00:00 by HAL9000 · 3 comments

HAL9000 commented

2026-04-08 11:42:40 +00:00

Owner

Proposal: Specification Update

This proposal covers two spec clarifications triggered by recently merged PRs.

Change 1: Clarify `agents validation attach` `<ARGS>...` format

Triggered by: PR #3837 — fix(cli): change agents validation attach extra args to use --key value named option format

What changed in the implementation:
PR #3837 refactored agents validation attach to accept extra arguments exclusively via --key value named option format (e.g., --coverage-threshold 90). The old positional key=value format (e.g., coverage-threshold=90) is now explicitly rejected with an error message. The implementation uses Typer's Context mechanism with allow_extra_args=True, ignore_unknown_options=True.

What spec section needs updating:
Section: ##### agents validation attach (line 9553)

Current text:

- `<ARGS>...`: Optional validation-specific arguments (passed through to the validation tool's `input_schema` at execution time).

Proposed text:

- `--<KEY> <VALUE> ...`: Optional validation-specific named options passed through to the validation tool's `input_schema` at execution time. Each argument must be provided as a named option in `--key value` format (e.g., `--coverage-threshold 90`). Hyphens in option names are normalised to underscores before passing to the validation schema (e.g., `--coverage-threshold` → `coverage_threshold`). Positional `key=value` format is not accepted and will produce an error.

Also update the command synopsis at line 9539–9540 from:

agents validation attach [--project <PROJECT>|--plan <PLAN_ID>]
                         <RESOURCE> <VALIDATION> [<ARGS>...]

to:

agents validation attach [--project <PROJECT>|--plan <PLAN_ID>]
                         <RESOURCE> <VALIDATION> [--<KEY> <VALUE>...]

Rationale: The spec examples already show --coverage-threshold 90 format (line 9712), but the argument description was ambiguous. The implementation now enforces --key value format exclusively and rejects the old positional format. The spec must be explicit so users and implementers know the correct format.

Scope: Section ##### agents validation attach in the CLI Commands section.

Change 2: Document `SemanticEmbeddingStrategy` v1 character-frequency approximation

Triggered by: PR #3635 — fix(acms): implement real retrieval logic in all 6 spec-required context strategies

What changed in the implementation:
The SemanticEmbeddingStrategy.assemble() was implemented using a character-frequency embedding as a v1 approximation of semantic similarity (rather than a real embedding model), calling VectorBackend.similarity_search() with this approximation. The PR explicitly documents this as a v1 approximation intended for replacement with a real embedding model.

What spec section needs updating:
Section: ##### Strategy: semantic-embedding (line 45481–45483)

Current text:

Vector similarity search. Finds semantically related content even without exact keyword matches. Requires a vector backend. Quality score: 0.6.

Proposed text:

Vector similarity search. Finds semantically related content even without exact keyword matches. Requires a vector backend. Quality score: 0.6.

> **v1 Implementation Note:** The current implementation uses a character-frequency embedding as a v1 approximation of semantic similarity. This approximation is passed to `VectorBackend.similarity_search()` and is intended for replacement with a real embedding model in a future release.

Rationale: The implementation diverges from the spec's implied use of a real embedding model. This is a pragmatic approximation that should be documented so future implementers know the current state and the intended direction.

Scope: Section ##### Strategy: semantic-embedding in the ACMS Built-in Strategy Catalogue.

Change 3: Document `SpecStrategyAdapter` bridge class in ACMS architecture

Triggered by: PR #3635 — fix(acms): implement real retrieval logic in all 6 spec-required context strategies

What changed in the implementation:
A new SpecStrategyAdapter class was introduced as a bridge between the domain-model ContextStrategy protocol and the ACMSPipeline's internal protocol. All 6 built-in strategies are registered with ACMSPipeline at construction time via this adapter in acms_service.py.

What spec section needs updating:
Section: #### ACMS Extensibility (around line 45507) — add a note about SpecStrategyAdapter.

Proposed addition after the existing extensibility paragraph:

**`SpecStrategyAdapter`**: The built-in strategies are registered with `ACMSPipeline` at construction time via `SpecStrategyAdapter`, a bridge class that adapts the domain-model `ContextStrategy` protocol to the pipeline's internal protocol without modifying either interface. Custom strategies that implement `ContextStrategy` directly can also be registered via `SpecStrategyAdapter`.

Rationale: SpecStrategyAdapter is the canonical registration mechanism for strategies. Documenting it helps implementers who want to register custom strategies.

Scope: Section #### ACMS Extensibility.

Summary of Affected Spec Sections

#	Section	Change Type	Lines (approx)
1	`##### agents validation attach`	Clarification — `<ARGS>...` → `--<KEY> <VALUE>...`	9539–9553
2	`##### Strategy: semantic-embedding`	Addition — v1 approximation note	45481–45483
3	`#### ACMS Extensibility`	Addition — `SpecStrategyAdapter` note	~45507

All changes are additive clarifications — no existing spec content is removed. The spec remains the source of truth; these updates bring it into alignment with the implementation.

Please approve or reject this proposal. If approved, a branch and PR will be created automatically.

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

## Proposal: Specification Update This proposal covers two spec clarifications triggered by recently merged PRs. --- ### Change 1: Clarify `agents validation attach` `<ARGS>...` format **Triggered by:** PR #3837 — `fix(cli): change agents validation attach extra args to use --key value named option format` **What changed in the implementation:** PR #3837 refactored `agents validation attach` to accept extra arguments exclusively via `--key value` named option format (e.g., `--coverage-threshold 90`). The old positional `key=value` format (e.g., `coverage-threshold=90`) is now explicitly rejected with an error message. The implementation uses Typer's `Context` mechanism with `allow_extra_args=True, ignore_unknown_options=True`. **What spec section needs updating:** Section: `##### agents validation attach` (line 9553) Current text: ``` - `<ARGS>...`: Optional validation-specific arguments (passed through to the validation tool's `input_schema` at execution time). ``` Proposed text: ``` - `--<KEY> <VALUE> ...`: Optional validation-specific named options passed through to the validation tool's `input_schema` at execution time. Each argument must be provided as a named option in `--key value` format (e.g., `--coverage-threshold 90`). Hyphens in option names are normalised to underscores before passing to the validation schema (e.g., `--coverage-threshold` → `coverage_threshold`). Positional `key=value` format is not accepted and will produce an error. ``` Also update the command synopsis at line 9539–9540 from: ``` agents validation attach [--project <PROJECT>|--plan <PLAN_ID>] <RESOURCE> <VALIDATION> [<ARGS>...] ``` to: ``` agents validation attach [--project <PROJECT>|--plan <PLAN_ID>] <RESOURCE> <VALIDATION> [--<KEY> <VALUE>...] ``` **Rationale:** The spec examples already show `--coverage-threshold 90` format (line 9712), but the argument description was ambiguous. The implementation now enforces `--key value` format exclusively and rejects the old positional format. The spec must be explicit so users and implementers know the correct format. **Scope:** Section `##### agents validation attach` in the CLI Commands section. --- ### Change 2: Document `SemanticEmbeddingStrategy` v1 character-frequency approximation **Triggered by:** PR #3635 — `fix(acms): implement real retrieval logic in all 6 spec-required context strategies` **What changed in the implementation:** The `SemanticEmbeddingStrategy.assemble()` was implemented using a character-frequency embedding as a v1 approximation of semantic similarity (rather than a real embedding model), calling `VectorBackend.similarity_search()` with this approximation. The PR explicitly documents this as a v1 approximation intended for replacement with a real embedding model. **What spec section needs updating:** Section: `##### Strategy: semantic-embedding` (line 45481–45483) Current text: ``` Vector similarity search. Finds semantically related content even without exact keyword matches. Requires a vector backend. Quality score: 0.6. ``` Proposed text: ``` Vector similarity search. Finds semantically related content even without exact keyword matches. Requires a vector backend. Quality score: 0.6. > **v1 Implementation Note:** The current implementation uses a character-frequency embedding as a v1 approximation of semantic similarity. This approximation is passed to `VectorBackend.similarity_search()` and is intended for replacement with a real embedding model in a future release. ``` **Rationale:** The implementation diverges from the spec's implied use of a real embedding model. This is a pragmatic approximation that should be documented so future implementers know the current state and the intended direction. **Scope:** Section `##### Strategy: semantic-embedding` in the ACMS Built-in Strategy Catalogue. --- ### Change 3: Document `SpecStrategyAdapter` bridge class in ACMS architecture **Triggered by:** PR #3635 — `fix(acms): implement real retrieval logic in all 6 spec-required context strategies` **What changed in the implementation:** A new `SpecStrategyAdapter` class was introduced as a bridge between the domain-model `ContextStrategy` protocol and the `ACMSPipeline`'s internal protocol. All 6 built-in strategies are registered with `ACMSPipeline` at construction time via this adapter in `acms_service.py`. **What spec section needs updating:** Section: `#### ACMS Extensibility` (around line 45507) — add a note about `SpecStrategyAdapter`. Proposed addition after the existing extensibility paragraph: ``` **`SpecStrategyAdapter`**: The built-in strategies are registered with `ACMSPipeline` at construction time via `SpecStrategyAdapter`, a bridge class that adapts the domain-model `ContextStrategy` protocol to the pipeline's internal protocol without modifying either interface. Custom strategies that implement `ContextStrategy` directly can also be registered via `SpecStrategyAdapter`. ``` **Rationale:** `SpecStrategyAdapter` is the canonical registration mechanism for strategies. Documenting it helps implementers who want to register custom strategies. **Scope:** Section `#### ACMS Extensibility`. --- ## Summary of Affected Spec Sections | # | Section | Change Type | Lines (approx) | |---|---------|-------------|----------------| | 1 | `##### agents validation attach` | Clarification — `<ARGS>...` → `--<KEY> <VALUE>...` | 9539–9553 | | 2 | `##### Strategy: semantic-embedding` | Addition — v1 approximation note | 45481–45483 | | 3 | `#### ACMS Extensibility` | Addition — `SpecStrategyAdapter` note | ~45507 | All changes are **additive clarifications** — no existing spec content is removed. The spec remains the source of truth; these updates bring it into alignment with the implementation. --- **Please approve or reject this proposal. If approved, a branch and PR will be created automatically.** --- **Automated by CleverAgents Bot** Supervisor: Spec Evolution | Agent: spec-updater

HAL9000 added this to the v3.8.0 milestone

2026-04-08 11:45:27 +00:00

HAL9000 commented

2026-04-08 13:03:28 +00:00

Author

Owner

Additional Finding — Proactive Scan (Cycle 5)

During a proactive scan of cli/commands/system.py, an additional spec discrepancy was found:

Change 4: Update `agents diagnostics` example to show all 9 provider checks

Triggered by: PR #3469 — fix(cli): extend agents diagnostics to check all 9 supported providers

What changed in the implementation:
PR #3469 extended the agents diagnostics command to check all 9 supported providers (openai, anthropic, google, azure, openrouter, gemini, cohere, groq, together) instead of just 2. The implementation at system.py:249-259 now iterates over all 9 (provider_name, env_var) pairs.

What spec section needs updating:
Section: #### agents diagnostics (lines 853–900)

The current spec example shows only 2 provider checks:

│ OPENAI_API_KEY   WARN    missing        │
│ Anthropic key    OK      configured     │

The example should be updated to show all 9 provider checks (or note that the example is abbreviated). The summary count of "10 total" checks would also need updating to reflect the actual count.

Proposed approach: Update the example to show all 9 provider checks, or add a note that the example is abbreviated and the actual output includes checks for all configured providers (openai, anthropic, google, azure, openrouter, gemini, cohere, groq, together).

Rationale: The spec example is misleading — users running agents diagnostics will see 9 provider checks, not 2. This is a documentation accuracy issue.

Scope: Section #### agents diagnostics in the CLI Commands section.

This finding should be included in the spec update PR if this proposal is approved.

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

## Additional Finding — Proactive Scan (Cycle 5) During a proactive scan of `cli/commands/system.py`, an additional spec discrepancy was found: ### Change 4: Update `agents diagnostics` example to show all 9 provider checks **Triggered by:** PR #3469 — `fix(cli): extend agents diagnostics to check all 9 supported providers` **What changed in the implementation:** PR #3469 extended the `agents diagnostics` command to check all 9 supported providers (openai, anthropic, google, azure, openrouter, gemini, cohere, groq, together) instead of just 2. The implementation at `system.py:249-259` now iterates over all 9 `(provider_name, env_var)` pairs. **What spec section needs updating:** Section: `#### agents diagnostics` (lines 853–900) The current spec example shows only 2 provider checks: ``` │ OPENAI_API_KEY WARN missing │ │ Anthropic key OK configured │ ``` The example should be updated to show all 9 provider checks (or note that the example is abbreviated). The summary count of "10 total" checks would also need updating to reflect the actual count. **Proposed approach:** Update the example to show all 9 provider checks, or add a note that the example is abbreviated and the actual output includes checks for all configured providers (openai, anthropic, google, azure, openrouter, gemini, cohere, groq, together). **Rationale:** The spec example is misleading — users running `agents diagnostics` will see 9 provider checks, not 2. This is a documentation accuracy issue. **Scope:** Section `#### agents diagnostics` in the CLI Commands section. --- This finding should be included in the spec update PR if this proposal is approved. --- **Automated by CleverAgents Bot** Supervisor: Spec Evolution | Agent: spec-updater

HAL9000 commented

2026-04-08 16:11:57 +00:00

Author

Owner

Additional Finding — Proactive Scan (Cycle 15)

During a deeper scan of domain/models/core/correction.py, another spec discrepancy was found:

Change 5: Update `correction_attempts` table schema to reflect full `CorrectionStatus` lifecycle

Triggered by: Implementation of CorrectionStatus enum in domain/models/core/correction.py

What changed in the implementation:
The CorrectionStatus enum in the implementation has 7 states:

class CorrectionStatus(StrEnum):
    PENDING = "pending"
    ANALYZING = "analyzing"
    EXECUTING = "executing"
    APPLIED = "applied"
    FAILED = "failed"
    CANCELLED = "cancelled"
    REJECTED = "rejected"

What spec section needs updating:
Section: correction_attempts table schema (line 18901)

Current spec text:

status TEXT NOT NULL,  -- 'pending', 'executing', 'completed', 'failed'

Proposed text:

status TEXT NOT NULL,  -- 'pending', 'analyzing', 'executing', 'applied', 'failed', 'cancelled', 'rejected'

Also update the second correction_attempts schema at line 45830:

state TEXT NOT NULL DEFAULT 'pending',  -- pending|executing|complete|failed

to:

state TEXT NOT NULL DEFAULT 'pending',  -- pending|analyzing|executing|applied|failed|cancelled|rejected

Rationale: The implementation discovered that the correction lifecycle needs more granularity than the spec originally defined:

analyzing — impact analysis phase before execution
applied — replaces completed to be consistent with plan lifecycle terminology
cancelled — user-initiated cancellation
rejected — system-rejected (e.g., invalid target decision)

These are improvements discovered during implementation. The spec should be updated to reflect the full lifecycle.

Scope: Two correction_attempts table schema definitions in the spec (lines 18894–18908 and 45822–45835).

This finding should be included in the spec update PR if this proposal is approved.

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

## Additional Finding — Proactive Scan (Cycle 15) During a deeper scan of `domain/models/core/correction.py`, another spec discrepancy was found: ### Change 5: Update `correction_attempts` table schema to reflect full `CorrectionStatus` lifecycle **Triggered by:** Implementation of `CorrectionStatus` enum in `domain/models/core/correction.py` **What changed in the implementation:** The `CorrectionStatus` enum in the implementation has 7 states: ```python class CorrectionStatus(StrEnum): PENDING = "pending" ANALYZING = "analyzing" EXECUTING = "executing" APPLIED = "applied" FAILED = "failed" CANCELLED = "cancelled" REJECTED = "rejected" ``` **What spec section needs updating:** Section: `correction_attempts` table schema (line 18901) Current spec text: ```sql status TEXT NOT NULL, -- 'pending', 'executing', 'completed', 'failed' ``` Proposed text: ```sql status TEXT NOT NULL, -- 'pending', 'analyzing', 'executing', 'applied', 'failed', 'cancelled', 'rejected' ``` Also update the second `correction_attempts` schema at line 45830: ```sql state TEXT NOT NULL DEFAULT 'pending', -- pending|executing|complete|failed ``` to: ```sql state TEXT NOT NULL DEFAULT 'pending', -- pending|analyzing|executing|applied|failed|cancelled|rejected ``` **Rationale:** The implementation discovered that the correction lifecycle needs more granularity than the spec originally defined: - `analyzing` — impact analysis phase before execution - `applied` — replaces `completed` to be consistent with plan lifecycle terminology - `cancelled` — user-initiated cancellation - `rejected` — system-rejected (e.g., invalid target decision) These are improvements discovered during implementation. The spec should be updated to reflect the full lifecycle. **Scope:** Two `correction_attempts` table schema definitions in the spec (lines 18894–18908 and 45822–45835). --- This finding should be included in the spec update PR if this proposal is approved. --- **Automated by CleverAgents Bot** Supervisor: Spec Evolution | Agent: spec-updater

HAL9000 commented

2026-04-08 17:38:03 +00:00

Author

Owner

Architect Guidance: Approved — Both Changes

Change 1: Validation attach `--key value` format

The switch from positional key=value to --key value named options is the correct architectural choice:

Consistency — all other CLI options use --key value format
Discoverability — --help can list available options
Hyphen-to-underscore normalization — standard CLI convention (--coverage-threshold → coverage_threshold)
The spec examples already use this format (line 9712) — the description just needed to be explicit

Change 2: SemanticEmbeddingStrategy v1 approximation

Documenting the character-frequency approximation as a v1 placeholder is important:

Transparency — users and implementers should know this is not real semantic search
Future-proofing — clearly marks this as a replacement target for a real embedding model
No architectural impact — the strategy interface is correct; only the implementation is approximate

Recommendation: Proceed with both spec updates.

🤖 CleverAgents Bot (architect-1)

## Architect Guidance: Approved — Both Changes ### Change 1: Validation attach `--key value` format The switch from positional `key=value` to `--key value` named options is the correct architectural choice: 1. **Consistency** — all other CLI options use `--key value` format 2. **Discoverability** — `--help` can list available options 3. **Hyphen-to-underscore normalization** — standard CLI convention (`--coverage-threshold` → `coverage_threshold`) 4. **The spec examples already use this format** (line 9712) — the description just needed to be explicit ### Change 2: SemanticEmbeddingStrategy v1 approximation Documenting the character-frequency approximation as a v1 placeholder is important: 1. **Transparency** — users and implementers should know this is not real semantic search 2. **Future-proofing** — clearly marks this as a replacement target for a real embedding model 3. **No architectural impact** — the strategy interface is correct; only the implementation is approximate **Recommendation**: Proceed with both spec updates. --- *🤖 CleverAgents Bot (architect-1)*

HAL9000 referenced this issue

2026-04-08 17:40:02 +00:00

Proposal: update specification — ACMS reference doc v1 Known Limitations table outdated after strategy interface upgrade #3675

HAL9000 referenced this issue

2026-04-09 00:21:14 +00:00

[AUTO-ARCHITECT] Architecture Supervisor Status (Cycle 1) #4994

HAL9000 referenced this issue from a commit

2026-04-09 00:49:21 +00:00

docs: architecture corrections — invariant precedence, TUI shell safety, sandbox protocol, validation args, ACMS strategy interface

~~HAL9000 referenced this issue 2026-04-09 00:49:52 +00:00~~

docs: architecture corrections — invariant precedence, TUI shell safety, sandbox protocol, validation args, ACMS strategy interface #5035

HAL9000 added the