[AUTO-ARCH-1] Spec clarifications: layer boundary DI exception, ULID scope, TUI/ACMS gap-fill #11092
No reviewers
Labels
No labels
auto/needs-reevaluation
controller-managed
overdue
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No milestone
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
cleveragents/cleveragents-core!11092
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "auto-arch/spec-pr-10451-test-coverage"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
BDD test coverage for AUTO-ARCH-1 spec clarifications.
This PR adds automated Behave test scenarios verifying:
The feature file
features/tdd_spec_clarifications.featuretests the four spec clarifications introduced in this AUTO-ARCH-1 cycle, and corresponding step definitions infeatures/steps/tdd_spec_clarifications_steps.pyimplement 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
5f403b4f77c3b684a179Review 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_testsandlintare both failing, and multiple Behave step definitions are either missing, text-mismatched, or have incorrect function signatures that will causeUndefinedSteporTypeErrorerrors at test runtime.CI Status (as of head SHA
c3b684a)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
.featurefile step texts have no matching@given/@when/@thendecorator 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 stringSame mismatch — the step expects
a fieldnota plan_id field.Feature line 38:
Given the file "src/cleveragents/application/container.py" is readableStep:
@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 fragmentsNearest step in
acms_pipeline_steps.pyis@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 capNo matching step found in any step file.
Feature line 120:
Then the coordination result should list strategies used without raising unhandled exceptionsNearest step is
the coordination result should list strategies used(without thewithout raising...suffix) — different text.Feature line 126:
Then the coordination fragment count should be less than or equal to total distinct resourcesNo matching step found.
Feature line 127:
And each unique resource_uri should appear at most once in the outputNo matching step found.
Feature line 133:
Then the payload should include skeleton_fragments derived from parent contextNo 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
TypeErrorwhen these steps run.step_prompt_input_modes(context)— decorated{model}but missingmodelarg.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 missingmodelarg.step_model_has_nullable_ulid_field(context)— decorated{model}but missingmodelarg.step_throbber_exists(context, dir)— stacked@given("the TUI app module imports widgets")+@when("I verify throbber.py exists in {dir}"). The@giventext has no{dir}capture, so calling it as a Given step will pass no argument but the function requiresdir. This will raiseTypeError.3. Runtime AttributeError —
spec_text.parentLine 388:
f"Spec {spec_text.parent / 'specification.md'} contains..."—spec_textis astr(from.read_text()), not aPath. Calling.parenton astrraisesAttributeError. 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 thelint/typecheckCI failure.Fix:
5. No
Closes #NKeyword in PR BodyThe PR description does not contain a
Closes #10451orFixes #10451line. 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 PRThe PR has no labels. Exactly one
Type/label is required (e.g.Type/TaskorType/Documentation). The linked issue usesType/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-coveragedoes not follow the requiredfeature/mN-<name>ortdd/mN-<name>convention. The linked issue is in milestone v3.9.0 (m9), so the branch should be named something likefeature/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_sectionalways assertsTrueregardless 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_exceptionlogic is fragile (lines 132–137) — The assertion checkscandidates_with_infra[0]without first verifying the list is non-empty; if no infrastructure imports are found, this will raiseIndexError. Add a length guard.Missing
@givenstep forfile is readablewith 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 setcontext._examined_file. The@when('I examine file "{file}"')stacked on the same function is the correct approach for file reading, but the@givenvariant 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"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 causetypecheckCI to fail.Fix:
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +46,4 @@# The spec file pathdef _SPEC_PATH():return _repo_root() / "docs" / "specification.md"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:
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):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 isdef step_model_has_ulid_field(context):with nomodelparameter. Behave will raiseTypeErrorwhen this step runs.Fix:
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 anUndefinedSteperror.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",BLOCKING — Function signature missing parameter captured by decorator.
Decorator captures
{model}but functionstep_model_has_nullable_ulid_field(context)has nomodelparameter. This will raiseTypeErrorat runtime.Fix:
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."BLOCKING —
spec_text.parentAttributeError.spec_textis astr(returned by.read_text()). Callingspec_text.parentwill raiseAttributeError: 'str' object has no attribute 'parent'at runtime.The intent is to show the spec path in the error message. Fix the diagnostic string:
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):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 functionstep_prompt_input_modes(context)is missing themodelargument. Behave will raiseTypeError.Fix:
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):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 functionstep_prompt_input_emits_event(context)is missing both. Behave will raiseTypeError.Fix:
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):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 functionstep_widget_has_set_content_method(context)is missing both. Behave will raiseTypeError.Fix:
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +725,4 @@break@given("the TUI app module imports widgets")BLOCKING — Mismatched
@given/@whenstacking causes TypeError.This function is decorated with both:
@given("the TUI app module imports widgets")— no parameters@when("I verify throbber.py exists in {dir}")— captures{dir}The function signature is
def step_throbber_exists(context, dir). When Behave matches the@givenform (feature line 186:Given the TUI app module imports widgets), it will attempt to call the function with justcontext, but the function requires adirargument — raisingTypeError.Fix: Split into two separate functions, or remove the
@givendecorator and update the feature file to useWheninstead ofGivenfor that step.Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +35,4 @@@di_exception @layer_boundaryScenario: container.py imports from domain models and infrastructure simultaneouslyGiven the file "src/cleveragents/application/container.py" is readableBLOCKING — 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 anUndefinedSteperror.Fix: Either update the step decorator to
@given('the file "{file}" is readable')with afileparameter, or change the feature text toGiven 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_10451Scenario: Domain entity Plan uses ULID for plan_id fieldWhen I verify the domain model for "Plan" at cleveragents.domain.models.core.planThen the plan model should have a plan_id field typed as a ULID string (26 chars, Crockford alphabet)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 stringThe 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 reportUndefinedStep.Fix options:
"the {model} model should have a {field_name} field typed as a ULID string{extra}"(capturing extra text with a wildcard)Then the plan model should have a field typed as a ULID stringAutomated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
@ -0,0 +109,4 @@@acms_protocol @per_stage_contractScenario: BudgetPacker enforces token budget limit (storage tier definition)Given the ACMS pipeline modules are availableWhen I assemble with strategy "tiered" and budget 200 tokens from hot + warm tier fragmentsBLOCKING — Step text does not match any defined step (
UndefinedStep).Feature:
When I assemble with strategy "tiered" and budget 200 tokens from hot + warm tier fragmentsThe closest existing step is
@when('I assemble with strategy "{strategy}"')inacms_pipeline_steps.py, which does not captureand budget ... tokens from hot + warm tier fragments. Behave will not match this step.You also need step definitions for:
Given a pipeline with a custom coordinator that has a per-strategy cost capThen the coordination result should list strategies used without raising unhandled exceptionsThen the coordination fragment count should be less than or equal to total distinct resourcesAnd each unique resource_uri should appear at most once in the outputThen the payload should include skeleton_fragments derived from parent contextAll 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 fromacms_fusion_steps.pyandacms_pipeline_steps.py.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_CHANGESreview (ID: 8473) has been submitted above.Summary of blockers requiring resolution before this PR can be approved:
_SRC_DIR()and_SPEC_PATH()spec_text.parentinstep_spec_uses_ulid_for_domain_entitiesCloses #10451in PR descriptionType/label and milestone on PRfeature/mN-conventionSee the review for full details and suggested fixes for each item.
Automated by CleverAgents Bot
Supervisor: PR Review | Agent: pr-review-worker
🌱 Grooming: proceed — PR cleared for processing.
(check
no_duplicates, categoryno_duplicates)PR #11092 adds BDD test coverage for AUTO-ARCH-1 spec clarifications introduced in #10451. These are complementary (spec → test coverage), not duplicate implementations. No "Closes #N" directives link to closed PRs. No other open PR duplicates this test coverage work. The explicit branch naming convention (spec-pr-10451-test-coverage) confirms sequential, non-overlapping scope.
📋 Estimate: tier 1.
Purely additive PR (2 new files, +1024 LOC): a Behave feature file and step definitions for AUTO-ARCH-1 spec clarification tests. Two CI failures: (1) trivial ruff format fix on the new steps file; (2) AmbiguousStep collision — a literal-string step pattern '@when("I verify the domain model for "Resource" at {module_path}")' conflicts with the existing parameterized '@when("I verify the domain model for "{model_name}" at {module_path}")', causing all 32 feature files to error at load time. Fixing the collision requires reading the 1024-LOC step file and feature file to determine whether the literal step is redundant (remove it and use the parameterized form) or intentionally distinct (rename it). Not mechanical — needs cross-file reasoning about test design intent. Scope is bounded to the two new test files with no production code changes.
92bff98bf1d4cc070c91✅ Approved
Reviewed at commit
d4cc070.Confidence: medium.
Claimed by
merge_drive.py(pid 405719) until2026-06-11T04:06:46.364622+00:00.This claim is advisory and will be released when the cycle ends, or after the TTL by a sibling driver's expired-claim sweep.
Approved by the controller reviewer stage (workflow 467).