[AUTO-ARCH-1] Spec clarifications: layer boundary DI exception, ULID scope, TUI/ACMS gap-fill #11092

Open
HAL9000 wants to merge 1 commit from auto-arch/spec-pr-10451-test-coverage into master
Owner

BDD test coverage for AUTO-ARCH-1 spec clarifications.

This PR adds automated Behave test scenarios verifying:

  • Application layer DI exception (container.py as sole permitted location for application-to-infrastructure concrete type references)
  • ULID identifier scope (domain entity IDs must be ULIDs, ephemeral internal IDs need not conform to Crockford alphabet)
  • ACMS pipeline protocol contracts with storage tier definitions and budget enforcement
  • TUI component public interfaces (8+ verified widget components)

The feature file features/tdd_spec_clarifications.feature tests the four spec clarifications introduced in this AUTO-ARCH-1 cycle, and corresponding step definitions in features/steps/tdd_spec_clarifications_steps.py implement verification via AST parsing, spec text analysis, and module introspection.

Spec Coverage: Layer boundary DI rules, ULID scope clarification, ACMS pipeline protocol contracts, TUI widget interfaces

BDD test coverage for AUTO-ARCH-1 spec clarifications. This PR adds automated Behave test scenarios verifying: - Application layer DI exception (container.py as sole permitted location for application-to-infrastructure concrete type references) - ULID identifier scope (domain entity IDs must be ULIDs, ephemeral internal IDs need not conform to Crockford alphabet) - ACMS pipeline protocol contracts with storage tier definitions and budget enforcement - TUI component public interfaces (8+ verified widget components) The feature file `features/tdd_spec_clarifications.feature` tests the four spec clarifications introduced in this AUTO-ARCH-1 cycle, and corresponding step definitions in `features/steps/tdd_spec_clarifications_steps.py` implement verification via AST parsing, spec text analysis, and module introspection. **Spec Coverage**: Layer boundary DI rules, ULID scope clarification, ACMS pipeline protocol contracts, TUI widget interfaces
[AUTO-ARCH-1] Spec clarifications: layer boundary DI exception, ULID scope, TUI/ACMS gap-fill
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 36s
CI / lint (pull_request) Failing after 1m20s
CI / build (pull_request) Successful in 59s
CI / helm (pull_request) Successful in 1m6s
CI / quality (pull_request) Successful in 1m34s
CI / benchmark-regression (pull_request) Failing after 1m6s
CI / typecheck (pull_request) Successful in 2m2s
CI / security (pull_request) Successful in 2m1s
CI / integration_tests (pull_request) Successful in 4m23s
CI / unit_tests (pull_request) Failing after 5m15s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / e2e_tests (pull_request) Failing after 5m19s
CI / status-check (pull_request) Failing after 5s
5f403b4f77
Add BDD test coverage for spec clarifications regarding:
- Application layer DI exception (container.py)
- ULID identifier scope (domain vs internal IDs)
- ACMS pipeline protocol contracts
- TUI component public interfaces

ISSUES CLOSED: #10451
HAL9000 force-pushed auto-arch/spec-pr-10451-test-coverage from 5f403b4f77
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / push-validation (pull_request) Successful in 36s
CI / lint (pull_request) Failing after 1m20s
CI / build (pull_request) Successful in 59s
CI / helm (pull_request) Successful in 1m6s
CI / quality (pull_request) Successful in 1m34s
CI / benchmark-regression (pull_request) Failing after 1m6s
CI / typecheck (pull_request) Successful in 2m2s
CI / security (pull_request) Successful in 2m1s
CI / integration_tests (pull_request) Successful in 4m23s
CI / unit_tests (pull_request) Failing after 5m15s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / e2e_tests (pull_request) Failing after 5m19s
CI / status-check (pull_request) Failing after 5s
to c3b684a179
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Failing after 1m2s
CI / helm (pull_request) Successful in 40s
CI / quality (pull_request) Successful in 1m15s
CI / build (pull_request) Successful in 56s
CI / typecheck (pull_request) Successful in 1m38s
CI / security (pull_request) Successful in 1m36s
CI / benchmark-regression (pull_request) Failing after 1m1s
CI / push-validation (pull_request) Successful in 19s
CI / unit_tests (pull_request) Failing after 4m47s
CI / coverage (pull_request) Has been skipped
CI / docker (pull_request) Has been skipped
CI / e2e_tests (pull_request) Successful in 5m1s
CI / integration_tests (pull_request) Successful in 5m39s
CI / status-check (pull_request) Failing after 3s
2026-05-09 13:59:46 +00:00
Compare
HAL9001 left a comment

Review Summary

This is a first review of PR #11092, adding BDD test coverage for the AUTO-ARCH-1 spec clarifications (layer boundary DI exception, ULID scope, ACMS pipeline contracts, TUI interfaces).

Outcome: REQUEST_CHANGES

The PR has significant defects that prevent it from passing CI. Specifically, unit_tests and lint are both failing, and multiple Behave step definitions are either missing, text-mismatched, or have incorrect function signatures that will cause UndefinedStep or TypeError errors at test runtime.

CI Status (as of head SHA c3b684a)

Check Result
lint FAILING
typecheck PASSING
security PASSING
unit_tests FAILING
coverage ⏭ SKIPPED (unit_tests failed)
benchmark-regression FAILING
status-check FAILING
integration_tests PASSING
e2e_tests PASSING

All five required CI gates must pass before this PR can be approved (lint, typecheck, security, unit_tests, coverage).


Blocking Issues

1. Undefined Step Definitions — Multiple Scenarios Will Fail

Several .feature file step texts have no matching @given/@when/@then decorator in any step file. Behave will mark these scenarios as undefined/failing.

Missing or mismatched steps:

  • Feature line 49: Then the plan model should have a **plan_id** field typed as a ULID string **(26 chars, Crockford alphabet)**
    Step decorator: the {model} model should have **a** field typed as a ULID string — text does not match.

  • Feature line 55: And the decision model should have a **plan_id** field typed as a ULID string
    Same mismatch — the step expects a field not a plan_id field.

  • Feature line 38: Given the file "src/cleveragents/application/container.py" is readable
    Step: @given("the file is readable") — missing the file path parameter.

  • Feature line 112: When I assemble with strategy "tiered" and budget 200 tokens from hot + warm tier fragments
    Nearest step in acms_pipeline_steps.py is @when('I assemble with strategy "{strategy}"') — incompatible text (no budget/tier capture).

  • Feature line 118: Given a pipeline with a custom coordinator that has a per-strategy cost cap
    No matching step found in any step file.

  • Feature line 120: Then the coordination result should list strategies used without raising unhandled exceptions
    Nearest step is the coordination result should list strategies used (without the without raising... suffix) — different text.

  • Feature line 126: Then the coordination fragment count should be less than or equal to total distinct resources
    No matching step found.

  • Feature line 127: And each unique resource_uri should appear at most once in the output
    No matching step found.

  • Feature line 133: Then the payload should include skeleton_fragments derived from parent context
    No matching step found.

2. Function Signature Mismatches — Will Cause TypeError at Runtime

Multiple step functions are decorated with parameterised patterns but their function signatures are missing the corresponding arguments. Behave will raise a TypeError when these steps run.

  • step_prompt_input_modes(context) — decorated {model} but missing model arg.
  • step_prompt_input_emits_event(context) — decorated {model}, {event_type} but missing both args.
  • step_widget_has_set_content_method(context) — decorated {model}, {method} but missing both args.
  • step_model_has_ulid_field(context) — decorated {model} but missing model arg.
  • step_model_has_nullable_ulid_field(context) — decorated {model} but missing model arg.
  • step_throbber_exists(context, dir) — stacked @given("the TUI app module imports widgets") + @when("I verify throbber.py exists in {dir}"). The @given text has no {dir} capture, so calling it as a Given step will pass no argument but the function requires dir. This will raise TypeError.

3. Runtime AttributeError — spec_text.parent

Line 388: f"Spec {spec_text.parent / 'specification.md'} contains..."spec_text is a str (from .read_text()), not a Path. Calling .parent on a str raises AttributeError. Change to _SPEC_PATH() or use a string literal for the diagnostic message.

4. Missing Type Annotations on Helper Functions

_SRC_DIR() and _SPEC_PATH() have no return type annotation. Per project rules (Pyright strict, zero suppressions), all function signatures must be fully annotated. This is likely the cause of the lint/typecheck CI failure.

Fix:

def _SRC_DIR() -> Path:
    return _repo_root() / "src" / "cleveragents"

def _SPEC_PATH() -> Path:
    return _repo_root() / "docs" / "specification.md"

5. No Closes #N Keyword in PR Body

The PR description does not contain a Closes #10451 or Fixes #10451 line. Per CONTRIBUTING.md, every PR must have a closing keyword for its linked issue or it will not be reviewed and cannot be merged.

6. No Forgejo Dependency Direction Set

Per CONTRIBUTING.md, the PR must block the linked issue (PR → blocks → issue). Currently there is no Forgejo dependency link between this PR and issue #10451. Set it: open issue #10451, add this PR under "depends on".

7. No Type/ Label on PR

The PR has no labels. Exactly one Type/ label is required (e.g. Type/Task or Type/Documentation). The linked issue uses Type/Documentation — the PR should match.

8. No Milestone on PR

Issue #10451 is assigned to milestone v3.9.0. The PR must carry the same milestone.

9. No CHANGELOG Entry

The commit does not include a CHANGELOG update. Per project rules, one CHANGELOG entry per commit is required.

10. Branch Naming Convention Violation

Branch name auto-arch/spec-pr-10451-test-coverage does not follow the required feature/mN-<name> or tdd/mN-<name> convention. The linked issue is in milestone v3.9.0 (m9), so the branch should be named something like feature/m9-spec-pr10451-bdd-coverage.


Non-Blocking Observations

  • Weak assertions in several steps — Multiple steps use assert True (line 507) or broad text searches that don't actually verify the thing they claim. For example, step_container_imports_domain_types_and_check_spec_adr_section always asserts True regardless of import reality. While these won't block CI (since they pass trivially), they don't meaningfully test the spec. Consider strengthening in a follow-up.

  • step_container_sole_di_exception logic is fragile (lines 132–137) — The assertion checks candidates_with_infra[0] without first verifying the list is non-empty; if no infrastructure imports are found, this will raise IndexError. Add a length guard.

  • Missing @given step for file is readable with path param — The existing @given("the file is readable") decorator can't receive the file path. The step will either not match or silently not set context._examined_file. The @when('I examine file "{file}"') stacked on the same function is the correct approach for file reading, but the @given variant should either be removed or given the same "{file}" parameter.

  • CONTRIBUTORS.md not updated — If this is from a contributor not already listed, their name should be added. (Minor — if bot, may be acceptable to skip.)


Please fix all blocking items and push a new commit. The review will be re-triggered once CI is green.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

## Review Summary This is a first review of PR #11092, adding BDD test coverage for the AUTO-ARCH-1 spec clarifications (layer boundary DI exception, ULID scope, ACMS pipeline contracts, TUI interfaces). **Outcome: REQUEST_CHANGES** The PR has significant defects that prevent it from passing CI. Specifically, `unit_tests` and `lint` are both failing, and multiple Behave step definitions are either missing, text-mismatched, or have incorrect function signatures that will cause `UndefinedStep` or `TypeError` errors at test runtime. ### CI Status (as of head SHA `c3b684a`) | Check | Result | |---|---| | lint | ❌ FAILING | | typecheck | ✅ PASSING | | security | ✅ PASSING | | unit_tests | ❌ FAILING | | coverage | ⏭ SKIPPED (unit_tests failed) | | benchmark-regression | ❌ FAILING | | status-check | ❌ FAILING | | integration_tests | ✅ PASSING | | e2e_tests | ✅ PASSING | **All five required CI gates must pass before this PR can be approved (lint, typecheck, security, unit_tests, coverage).** --- ### Blocking Issues #### 1. Undefined Step Definitions — Multiple Scenarios Will Fail Several `.feature` file step texts have no matching `@given`/`@when`/`@then` decorator in *any* step file. Behave will mark these scenarios as **undefined/failing**. Missing or mismatched steps: - **Feature line 49**: `Then the plan model should have a **plan_id** field typed as a ULID string **(26 chars, Crockford alphabet)**` Step decorator: `the {model} model should have **a** field typed as a ULID string` — text does not match. - **Feature line 55**: `And the decision model should have a **plan_id** field typed as a ULID string` Same mismatch — the step expects `a field` not `a plan_id field`. - **Feature line 38**: `Given the file "src/cleveragents/application/container.py" is readable` Step: `@given("the file is readable")` — missing the file path parameter. - **Feature line 112**: `When I assemble with strategy "tiered" and budget 200 tokens from hot + warm tier fragments` Nearest step in `acms_pipeline_steps.py` is `@when('I assemble with strategy "{strategy}"')` — incompatible text (no budget/tier capture). - **Feature line 118**: `Given a pipeline with a custom coordinator that has a per-strategy cost cap` No matching step found in any step file. - **Feature line 120**: `Then the coordination result should list strategies used without raising unhandled exceptions` Nearest step is `the coordination result should list strategies used` (without the `without raising...` suffix) — different text. - **Feature line 126**: `Then the coordination fragment count should be less than or equal to total distinct resources` No matching step found. - **Feature line 127**: `And each unique resource_uri should appear at most once in the output` No matching step found. - **Feature line 133**: `Then the payload should include skeleton_fragments derived from parent context` No matching step found. #### 2. Function Signature Mismatches — Will Cause TypeError at Runtime Multiple step functions are decorated with parameterised patterns but their function signatures are missing the corresponding arguments. Behave will raise a `TypeError` when these steps run. - `step_prompt_input_modes(context)` — decorated `{model}` but missing `model` arg. - `step_prompt_input_emits_event(context)` — decorated `{model}`, `{event_type}` but missing both args. - `step_widget_has_set_content_method(context)` — decorated `{model}`, `{method}` but missing both args. - `step_model_has_ulid_field(context)` — decorated `{model}` but missing `model` arg. - `step_model_has_nullable_ulid_field(context)` — decorated `{model}` but missing `model` arg. - `step_throbber_exists(context, dir)` — stacked `@given("the TUI app module imports widgets")` + `@when("I verify throbber.py exists in {dir}")`. The `@given` text has no `{dir}` capture, so calling it as a Given step will pass no argument but the function requires `dir`. This will raise `TypeError`. #### 3. Runtime AttributeError — `spec_text.parent` Line 388: `f"Spec {spec_text.parent / 'specification.md'} contains..."` — `spec_text` is a `str` (from `.read_text()`), not a `Path`. Calling `.parent` on a `str` raises `AttributeError`. Change to `_SPEC_PATH()` or use a string literal for the diagnostic message. #### 4. Missing Type Annotations on Helper Functions `_SRC_DIR()` and `_SPEC_PATH()` have no return type annotation. Per project rules (Pyright strict, zero suppressions), all function signatures must be fully annotated. This is likely the cause of the `lint`/`typecheck` CI failure. Fix: ```python def _SRC_DIR() -> Path: return _repo_root() / "src" / "cleveragents" def _SPEC_PATH() -> Path: return _repo_root() / "docs" / "specification.md" ``` #### 5. No `Closes #N` Keyword in PR Body The PR description does not contain a `Closes #10451` or `Fixes #10451` line. Per CONTRIBUTING.md, every PR must have a closing keyword for its linked issue or it will not be reviewed and cannot be merged. #### 6. No Forgejo Dependency Direction Set Per CONTRIBUTING.md, the PR must *block* the linked issue (`PR → blocks → issue`). Currently there is no Forgejo dependency link between this PR and issue #10451. Set it: open issue #10451, add this PR under "depends on". #### 7. No `Type/` Label on PR The PR has no labels. Exactly one `Type/` label is required (e.g. `Type/Task` or `Type/Documentation`). The linked issue uses `Type/Documentation` — the PR should match. #### 8. No Milestone on PR Issue #10451 is assigned to milestone `v3.9.0`. The PR must carry the same milestone. #### 9. No CHANGELOG Entry The commit does not include a CHANGELOG update. Per project rules, one CHANGELOG entry per commit is required. #### 10. Branch Naming Convention Violation Branch name `auto-arch/spec-pr-10451-test-coverage` does not follow the required `feature/mN-<name>` or `tdd/mN-<name>` convention. The linked issue is in milestone v3.9.0 (m9), so the branch should be named something like `feature/m9-spec-pr10451-bdd-coverage`. --- ### Non-Blocking Observations - **Weak assertions in several steps** — Multiple steps use `assert True` (line 507) or broad text searches that don't actually verify the thing they claim. For example, `step_container_imports_domain_types_and_check_spec_adr_section` always asserts `True` regardless of import reality. While these won't block CI (since they pass trivially), they don't meaningfully test the spec. Consider strengthening in a follow-up. - **`step_container_sole_di_exception` logic is fragile** (lines 132–137) — The assertion checks `candidates_with_infra[0]` without first verifying the list is non-empty; if no infrastructure imports are found, this will raise `IndexError`. Add a length guard. - **Missing `@given` step for `file is readable` with path param** — The existing `@given("the file is readable")` decorator can't receive the file path. The step will either not match or silently not set `context._examined_file`. The `@when('I examine file "{file}"')` stacked on the same function is the correct approach for file reading, but the `@given` variant should either be removed or given the same `"{file}"` parameter. - **CONTRIBUTORS.md not updated** — If this is from a contributor not already listed, their name should be added. (Minor — if bot, may be acceptable to skip.) --- Please fix all blocking items and push a new commit. The review will be re-triggered once CI is green. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +42,4 @@
def _SRC_DIR():
return _repo_root() / "src" / "cleveragents"
Owner

BLOCKING — Missing return type annotation.

_SRC_DIR() has no return type annotation. Per project rules, Pyright strict mode is required and all functions must be fully annotated. This will cause typecheck CI to fail.

Fix:

def _SRC_DIR() -> Path:
    return _repo_root() / "src" / "cleveragents"

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Missing return type annotation.** `_SRC_DIR()` has no return type annotation. Per project rules, Pyright strict mode is required and all functions must be fully annotated. This will cause `typecheck` CI to fail. Fix: ```python def _SRC_DIR() -> Path: return _repo_root() / "src" / "cleveragents" ``` --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +46,4 @@
# The spec file path
def _SPEC_PATH():
return _repo_root() / "docs" / "specification.md"
Owner

BLOCKING — Missing return type annotation.

_SPEC_PATH() has no return type annotation. Same issue as _SRC_DIR() above — Pyright strict requires all function signatures to be fully annotated.

Fix:

def _SPEC_PATH() -> Path:
    return _repo_root() / "docs" / "specification.md"

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Missing return type annotation.** `_SPEC_PATH()` has no return type annotation. Same issue as `_SRC_DIR()` above — Pyright strict requires all function signatures to be fully annotated. Fix: ```python def _SPEC_PATH() -> Path: return _repo_root() / "docs" / "specification.md" ``` --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +281,4 @@
@then(
"the {model} model should have a field typed as a ULID string",
)
def step_model_has_ulid_field(context):
Owner

BLOCKING — Function signature missing parameter captured by decorator.

The decorator pattern is @then("the {model} model should have a field typed as a ULID string") which captures {model}, but the function signature is def step_model_has_ulid_field(context): with no model parameter. Behave will raise TypeError when this step runs.

Fix:

def step_model_has_ulid_field(context, model: str) -> None:

Note also: the feature file uses "the plan model should have a **plan_id** field typed as a ULID string (26 chars, Crockford alphabet)" which does NOT match the step decorator "the {model} model should have **a** field typed as a ULID string". The step text must match the feature file text exactly (or the feature file must be updated to match the step). This will cause an UndefinedStep error.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Function signature missing parameter captured by decorator.** The decorator pattern is `@then("the {model} model should have a field typed as a ULID string")` which captures `{model}`, but the function signature is `def step_model_has_ulid_field(context):` with no `model` parameter. Behave will raise `TypeError` when this step runs. Fix: ```python def step_model_has_ulid_field(context, model: str) -> None: ``` Note also: the feature file uses `"the plan model should have a **plan_id** field typed as a ULID string (26 chars, Crockford alphabet)"` which does NOT match the step decorator `"the {model} model should have **a** field typed as a ULID string"`. The step text must match the feature file text exactly (or the feature file must be updated to match the step). This will cause an `UndefinedStep` error. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +296,4 @@
@then(
"the {model} model should have a parent_plan_id field typed as a nullable ULID",
Owner

BLOCKING — Function signature missing parameter captured by decorator.

Decorator captures {model} but function step_model_has_nullable_ulid_field(context) has no model parameter. This will raise TypeError at runtime.

Fix:

def step_model_has_nullable_ulid_field(context, model: str) -> None:

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Function signature missing parameter captured by decorator.** Decorator captures `{model}` but function `step_model_has_nullable_ulid_field(context)` has no `model` parameter. This will raise `TypeError` at runtime. Fix: ```python def step_model_has_nullable_ulid_field(context, model: str) -> None: ``` --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +386,4 @@
ulid_count = len(re.findall(r"\bULID\b", spec_text))
assert ulid_count >= 10, (
f"Spec {spec_text.parent / 'specification.md'} contains only {ulid_count} ULID references. "
f"Domain entity ID clarification should have many more."
Owner

BLOCKING — spec_text.parent AttributeError.

spec_text is a str (returned by .read_text()). Calling spec_text.parent will raise AttributeError: 'str' object has no attribute 'parent' at runtime.

The intent is to show the spec path in the error message. Fix the diagnostic string:

assert ulid_count >= 10, (
    f"Spec {_SPEC_PATH()} contains only {ulid_count} ULID references. "
    f"Domain entity ID clarification should have many more."
)

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — `spec_text.parent` AttributeError.** `spec_text` is a `str` (returned by `.read_text()`). Calling `spec_text.parent` will raise `AttributeError: 'str' object has no attribute 'parent'` at runtime. The intent is to show the spec path in the error message. Fix the diagnostic string: ```python assert ulid_count >= 10, ( f"Spec {_SPEC_PATH()} contains only {ulid_count} ULID references. " f"Domain entity ID clarification should have many more." ) ``` --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +625,4 @@
@then(
"{model} should support InputMode enum values for mode-dependent symbol rendering",
)
def step_prompt_input_modes(context):
Owner

BLOCKING — Function signature missing parameters captured by decorator.

Decorator: @then("{model} should support InputMode enum values for mode-dependent symbol rendering") captures {model}, but the function step_prompt_input_modes(context) is missing the model argument. Behave will raise TypeError.

Fix:

def step_prompt_input_modes(context, model: str) -> None:

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Function signature missing parameters captured by decorator.** Decorator: `@then("{model} should support InputMode enum values for mode-dependent symbol rendering")` captures `{model}`, but the function `step_prompt_input_modes(context)` is missing the `model` argument. Behave will raise `TypeError`. Fix: ```python def step_prompt_input_modes(context, model: str) -> None: ``` --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +640,4 @@
@then(
"{model} should emit a {event_type} event on submission",
)
def step_prompt_input_emits_event(context):
Owner

BLOCKING — Function signature missing parameters captured by decorator.

Decorator: @then("{model} should emit a {event_type} event on submission") captures {model} and {event_type}, but the function step_prompt_input_emits_event(context) is missing both. Behave will raise TypeError.

Fix:

def step_prompt_input_emits_event(context, model: str, event_type: str) -> None:

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Function signature missing parameters captured by decorator.** Decorator: `@then("{model} should emit a {event_type} event on submission")` captures `{model}` and `{event_type}`, but the function `step_prompt_input_emits_event(context)` is missing both. Behave will raise `TypeError`. Fix: ```python def step_prompt_input_emits_event(context, model: str, event_type: str) -> None: ``` --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +674,4 @@
@then(
"{model} should have a {method} method for updating display text",
)
def step_widget_has_set_content_method(context):
Owner

BLOCKING — Function signature missing parameters captured by decorator.

Decorator: @then("{model} should have a {method} method for updating display text") captures {model} and {method}, but the function step_widget_has_set_content_method(context) is missing both. Behave will raise TypeError.

Fix:

def step_widget_has_set_content_method(context, model: str, method: str) -> None:

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Function signature missing parameters captured by decorator.** Decorator: `@then("{model} should have a {method} method for updating display text")` captures `{model}` and `{method}`, but the function `step_widget_has_set_content_method(context)` is missing both. Behave will raise `TypeError`. Fix: ```python def step_widget_has_set_content_method(context, model: str, method: str) -> None: ``` --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +725,4 @@
break
@given("the TUI app module imports widgets")
Owner

BLOCKING — Mismatched @given/@when stacking causes TypeError.

This function is decorated with both:

  1. @given("the TUI app module imports widgets") — no parameters
  2. @when("I verify throbber.py exists in {dir}") — captures {dir}

The function signature is def step_throbber_exists(context, dir). When Behave matches the @given form (feature line 186: Given the TUI app module imports widgets), it will attempt to call the function with just context, but the function requires a dir argument — raising TypeError.

Fix: Split into two separate functions, or remove the @given decorator and update the feature file to use When instead of Given for that step.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Mismatched `@given`/`@when` stacking causes TypeError.** This function is decorated with both: 1. `@given("the TUI app module imports widgets")` — no parameters 2. `@when("I verify throbber.py exists in {dir}")` — captures `{dir}` The function signature is `def step_throbber_exists(context, dir)`. When Behave matches the `@given` form (feature line 186: `Given the TUI app module imports widgets`), it will attempt to call the function with just `context`, but the function requires a `dir` argument — raising `TypeError`. Fix: Split into two separate functions, or remove the `@given` decorator and update the feature file to use `When` instead of `Given` for that step. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +35,4 @@
@di_exception @layer_boundary
Scenario: container.py imports from domain models and infrastructure simultaneously
Given the file "src/cleveragents/application/container.py" is readable
Owner

BLOCKING — Step text does not match any defined step.

Given the file "src/cleveragents/application/container.py" is readable — the only matching step decorator is @given("the file is readable") which has no file path parameter. These texts do not match; Behave will report an UndefinedStep error.

Fix: Either update the step decorator to @given('the file "{file}" is readable') with a file parameter, or change the feature text to Given the file is readable (and set the path in the @given("the source directory is at ...") Given step instead).


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Step text does not match any defined step.** `Given the file "src/cleveragents/application/container.py" is readable` — the only matching step decorator is `@given("the file is readable")` which has no file path parameter. These texts do not match; Behave will report an `UndefinedStep` error. Fix: Either update the step decorator to `@given('the file "{file}" is readable')` with a `file` parameter, or change the feature text to `Given the file is readable` (and set the path in the `@given("the source directory is at ...")` Given step instead). --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +46,4 @@
@ulid_scope @spec_clarification_10451
Scenario: Domain entity Plan uses ULID for plan_id field
When I verify the domain model for "Plan" at cleveragents.domain.models.core.plan
Then the plan model should have a plan_id field typed as a ULID string (26 chars, Crockford alphabet)
Owner

BLOCKING — Step text does not match any defined step (UndefinedStep).

Feature text: Then the plan model should have a plan_id field typed as a ULID string (26 chars, Crockford alphabet)

Step decorator: the {model} model should have a field typed as a ULID string

The step pattern expects "a field" but the feature says "a plan_id field". The trailing (26 chars, Crockford alphabet) also has no equivalent in the step text. These do not match; Behave will report UndefinedStep.

Fix options:

  1. Update the step decorator to "the {model} model should have a {field_name} field typed as a ULID string{extra}" (capturing extra text with a wildcard)
  2. Or simplify the feature text to match the step: Then the plan model should have a field typed as a ULID string

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Step text does not match any defined step (`UndefinedStep`).** Feature text: `Then the plan model should have a plan_id field typed as a ULID string (26 chars, Crockford alphabet)` Step decorator: `the {model} model should have a field typed as a ULID string` The step pattern expects `"a field"` but the feature says `"a plan_id field"`. The trailing `(26 chars, Crockford alphabet)` also has no equivalent in the step text. These do not match; Behave will report `UndefinedStep`. Fix options: 1. Update the step decorator to `"the {model} model should have a {field_name} field typed as a ULID string{extra}"` (capturing extra text with a wildcard) 2. Or simplify the feature text to match the step: `Then the plan model should have a field typed as a ULID string` --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +109,4 @@
@acms_protocol @per_stage_contract
Scenario: BudgetPacker enforces token budget limit (storage tier definition)
Given the ACMS pipeline modules are available
When I assemble with strategy "tiered" and budget 200 tokens from hot + warm tier fragments
Owner

BLOCKING — Step text does not match any defined step (UndefinedStep).

Feature: When I assemble with strategy "tiered" and budget 200 tokens from hot + warm tier fragments

The closest existing step is @when('I assemble with strategy "{strategy}"') in acms_pipeline_steps.py, which does not capture and budget ... tokens from hot + warm tier fragments. Behave will not match this step.

You also need step definitions for:

  • Line 118: Given a pipeline with a custom coordinator that has a per-strategy cost cap
  • Line 120: Then the coordination result should list strategies used without raising unhandled exceptions
  • Line 126: Then the coordination fragment count should be less than or equal to total distinct resources
  • Line 127: And each unique resource_uri should appear at most once in the output
  • Line 133: Then the payload should include skeleton_fragments derived from parent context

All of these are missing step definitions. Either add the missing steps to tdd_spec_clarifications_steps.py, or rewrite the scenarios to reuse exactly-matching existing steps from acms_fusion_steps.py and acms_pipeline_steps.py.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

**BLOCKING — Step text does not match any defined step (`UndefinedStep`).** Feature: `When I assemble with strategy "tiered" and budget 200 tokens from hot + warm tier fragments` The closest existing step is `@when('I assemble with strategy "{strategy}"')` in `acms_pipeline_steps.py`, which does not capture `and budget ... tokens from hot + warm tier fragments`. Behave will not match this step. You also need step definitions for: - Line 118: `Given a pipeline with a custom coordinator that has a per-strategy cost cap` - Line 120: `Then the coordination result should list strategies used without raising unhandled exceptions` - Line 126: `Then the coordination fragment count should be less than or equal to total distinct resources` - Line 127: `And each unique resource_uri should appear at most once in the output` - Line 133: `Then the payload should include skeleton_fragments derived from parent context` All of these are missing step definitions. Either add the missing steps to `tdd_spec_clarifications_steps.py`, or rewrite the scenarios to reuse exactly-matching existing steps from `acms_fusion_steps.py` and `acms_pipeline_steps.py`. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Owner

This PR has been reviewed by the automated peer review bot. A formal REQUEST_CHANGES review (ID: 8473) has been submitted above.

Summary of blockers requiring resolution before this PR can be approved:

  1. lint CI failing — missing return type annotations on _SRC_DIR() and _SPEC_PATH()
  2. unit_tests CI failing — multiple undefined steps (feature/step text mismatches) and function signature bugs (missing captured parameters)
  3. Runtime AttributeErrorspec_text.parent in step_spec_uses_ulid_for_domain_entities
  4. Missing Closes #10451 in PR description
  5. Missing Forgejo dependency — PR must block issue #10451
  6. Missing Type/ label and milestone on PR
  7. Missing CHANGELOG entry
  8. Branch naming does not follow feature/mN- convention

See the review for full details and suggested fixes for each item.


Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker

This PR has been reviewed by the automated peer review bot. A formal `REQUEST_CHANGES` review (ID: 8473) has been submitted above. Summary of blockers requiring resolution before this PR can be approved: 1. **lint CI failing** — missing return type annotations on `_SRC_DIR()` and `_SPEC_PATH()` 2. **unit_tests CI failing** — multiple undefined steps (feature/step text mismatches) and function signature bugs (missing captured parameters) 3. **Runtime AttributeError** — `spec_text.parent` in `step_spec_uses_ulid_for_domain_entities` 4. **Missing `Closes #10451`** in PR description 5. **Missing Forgejo dependency** — PR must block issue #10451 6. **Missing `Type/` label and milestone** on PR 7. **Missing CHANGELOG entry** 8. **Branch naming** does not follow `feature/mN-` convention See the review for full details and suggested fixes for each item. --- Automated by CleverAgents Bot Supervisor: PR Review | Agent: pr-review-worker
Some checks failed
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Failing after 1m2s
Required
Details
CI / helm (pull_request) Successful in 40s
CI / quality (pull_request) Successful in 1m15s
Required
Details
CI / build (pull_request) Successful in 56s
Required
Details
CI / typecheck (pull_request) Successful in 1m38s
Required
Details
CI / security (pull_request) Successful in 1m36s
Required
Details
CI / benchmark-regression (pull_request) Failing after 1m1s
CI / push-validation (pull_request) Successful in 19s
CI / unit_tests (pull_request) Failing after 4m47s
Required
Details
CI / coverage (pull_request) Has been skipped
Required
Details
CI / docker (pull_request) Has been skipped
Required
Details
CI / e2e_tests (pull_request) Successful in 5m1s
CI / integration_tests (pull_request) Successful in 5m39s
Required
Details
CI / status-check (pull_request) Failing after 3s
This pull request doesn't have enough approvals yet. 0 of 1 approvals granted.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin auto-arch/spec-pr-10451-test-coverage:auto-arch/spec-pr-10451-test-coverage
git switch auto-arch/spec-pr-10451-test-coverage
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core!11092
No description provided.