cleveragents/cleveragents-core

Fork 3

fix(resource-registry): use full inheritance chain in contextual binding type compatibility #3336

Merged

freemo merged 1 commit from bugfix/binding-resolution-inheritance-chain into master

2026-04-05 21:07:11 +00:00

freemo commented

2026-04-05 10:06:49 +00:00

Owner

Summary

Fixes a bug in binding_resolution_service.py where _is_type_compatible performed only a flat one-level parent check against spec.parent_types, causing contextual resource binding to incorrectly reject valid bindings when the actual resource type was related to the required type through a multi-level inheritance chain (e.g., grandchild → child → parent).

Changes

src/cleveragents/application/services/binding_resolution_service.py:
- Replaced the flat required_type in spec.parent_types check in _is_type_compatible with a delegation to self._registry.is_subtype_of(actual_type, required_type), which internally calls resolve_inheritance_chain() from cleveragents.resource.inheritance to walk the full ADR-042 inheritance chain to any depth.
- Removed the now-unused ResourceTypeSpec import that was only needed to access parent_types directly.
- _check_type_compatibility is implicitly fixed as it delegates to _is_type_compatible — no changes required there.
features/consolidated_binding_resolution.feature:
- Added 4 new BDD scenarios covering the previously broken and adjacent cases:
  1. Two-level inheritance chain compatibility (child → parent).
  2. Three-level inheritance chain compatibility (grandchild → child → parent).
  3. Rejection of an unrelated type that shares no ancestry with the required type.
  4. Static binding resolved correctly via the full inheritance chain.
features/steps/binding_resolution_steps.py:
- Extended the mock registry to expose is_subtype_of(), backed by the real is_subtype_of() function from cleveragents.resource.inheritance operating over an _inherits dict — ensuring tests exercise the actual inheritance logic rather than a stub.
- Added new step definitions: the resource type X inherits from nothing and the resource type X inherits from Y to allow feature files to declaratively construct inheritance hierarchies.

Design Decisions

Delegate to registry.is_subtype_of() rather than re-implementing chain traversal: ResourceRegistryService already exposes is_subtype_of() as a public method. Using it keeps the API boundary clean and avoids duplicating or leaking knowledge of _load_type_registry() internals into the binding resolution layer.
Real inheritance logic in test mocks: The mock registry in the Behave step definitions uses the actual is_subtype_of() function from cleveragents.resource.inheritance (not a hand-rolled stub), so the new scenarios genuinely validate the inheritance traversal behaviour end-to-end at the unit/BDD level.
No changes to resolve_inheritance_chain() or is_subtype_of(): The root cause was entirely in the call site — the underlying chain-resolution logic in cleveragents.resource.inheritance was already correct per ADR-042. The fix is therefore minimal and surgical.

Testing

Unit tests (Behave): ✅ 20 scenarios passed (16 original + 4 new), 0 failed
lint: ✅ all checks passed
typecheck: ✅ 0 errors, 0 warnings

Closes #2929

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

## Summary Fixes a bug in `binding_resolution_service.py` where `_is_type_compatible` performed only a flat one-level parent check against `spec.parent_types`, causing contextual resource binding to incorrectly reject valid bindings when the actual resource type was related to the required type through a multi-level inheritance chain (e.g., grandchild → child → parent). ## Changes - **`src/cleveragents/application/services/binding_resolution_service.py`**: - Replaced the flat `required_type in spec.parent_types` check in `_is_type_compatible` with a delegation to `self._registry.is_subtype_of(actual_type, required_type)`, which internally calls `resolve_inheritance_chain()` from `cleveragents.resource.inheritance` to walk the full ADR-042 inheritance chain to any depth. - Removed the now-unused `ResourceTypeSpec` import that was only needed to access `parent_types` directly. - `_check_type_compatibility` is implicitly fixed as it delegates to `_is_type_compatible` — no changes required there. - **`features/consolidated_binding_resolution.feature`**: - Added 4 new BDD scenarios covering the previously broken and adjacent cases: 1. Two-level inheritance chain compatibility (child → parent). 2. Three-level inheritance chain compatibility (grandchild → child → parent). 3. Rejection of an unrelated type that shares no ancestry with the required type. 4. Static binding resolved correctly via the full inheritance chain. - **`features/steps/binding_resolution_steps.py`**: - Extended the mock registry to expose `is_subtype_of()`, backed by the real `is_subtype_of()` function from `cleveragents.resource.inheritance` operating over an `_inherits` dict — ensuring tests exercise the actual inheritance logic rather than a stub. - Added new step definitions: `the resource type X inherits from nothing` and `the resource type X inherits from Y` to allow feature files to declaratively construct inheritance hierarchies. ## Design Decisions - **Delegate to `registry.is_subtype_of()` rather than re-implementing chain traversal**: `ResourceRegistryService` already exposes `is_subtype_of()` as a public method. Using it keeps the API boundary clean and avoids duplicating or leaking knowledge of `_load_type_registry()` internals into the binding resolution layer. - **Real inheritance logic in test mocks**: The mock registry in the Behave step definitions uses the actual `is_subtype_of()` function from `cleveragents.resource.inheritance` (not a hand-rolled stub), so the new scenarios genuinely validate the inheritance traversal behaviour end-to-end at the unit/BDD level. - **No changes to `resolve_inheritance_chain()` or `is_subtype_of()`**: The root cause was entirely in the call site — the underlying chain-resolution logic in `cleveragents.resource.inheritance` was already correct per ADR-042. The fix is therefore minimal and surgical. ## Testing - Unit tests (Behave): ✅ 20 scenarios passed (16 original + 4 new), 0 failed - lint: ✅ all checks passed - typecheck: ✅ 0 errors, 0 warnings ## Related Issues Closes #2929 --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker

freemo referenced this pull request

2026-04-05 10:15:55 +00:00

UAT: bug(resource-registry): contextual resource binding type compatibility check does not use full inheritance chain #2929

freemo commented

2026-04-05 10:19:21 +00:00

Author

Owner

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3336-1775373400]

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3336-1775373400] --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-continuous-pr-reviewer

freemo commented

2026-04-05 16:23:11 +00:00

Author

Owner

🔍 Code Review — REQUEST CHANGES

Reviewed PR #3336 with focus on api-consistency, naming-conventions, and code-patterns.

This is a well-scoped, surgical bugfix that correctly addresses the ADR-042 inheritance chain traversal issue. The core change — delegating _is_type_compatible to registry.is_subtype_of() instead of performing a flat parent_types membership check — is the right approach. The test coverage is thorough with 4 new BDD scenarios covering 2-level chains, 3-level chains, unrelated-type rejection, and static binding via inheritance.

However, there is one required change related to API contract consistency, and two PR metadata issues.

Required Changes

[API-CONSISTENCY] Registry protocol contract is stale in docstrings
- Location: src/cleveragents/application/services/binding_resolution_service.py
- Class docstring (around line 50) states:
  
  The registry is injected as a protocol-compatible object exposing show_resource and show_type methods
- __init__ docstring (around line 62) states:
  
  resource_registry: Object with show_resource(name_or_id) and show_type(name) methods.
- Issue: The service now also requires is_subtype_of(type_name, ancestor_name) on the registry. Furthermore, show_type() is no longer called anywhere in this service (the only call site was in the old _is_type_compatible), so it is no longer a required method from this service's perspective.
- Required: Update both docstrings to list is_subtype_of(type_name, ancestor_name) as a required method. Consider whether show_type should be removed from the documented contract (it may still be needed by other callers of the same registry, but this service no longer uses it).
- Why: This is a first-class API consistency issue. Any consumer implementing a compatible registry (or writing a test mock) will follow these docstrings to know what methods to provide. The current docstrings are now misleading — they omit a required method and include one that is no longer used by this service.
[METADATA] PR is missing milestone
- Issue #2929 is assigned to milestone v3.2.0. Per CONTRIBUTING.md, the PR must be assigned to the same milestone as its linked issue.
- Required: Assign milestone v3.2.0 to this PR.
[METADATA] PR is missing Type/ label
- Per CONTRIBUTING.md, every PR must have exactly one Type/ label. This is a bugfix, so it should have Type/Bug.
- Required: Add the Type/Bug label.

Deep Dive: API Consistency

Given special attention to API consistency across the change:

✅ Method signature consistency: is_subtype_of(type_name, ancestor_name) on ResourceRegistryService matches the parameter order used in the call site (self._registry.is_subtype_of(actual_type, required_type)) — actual_type maps to type_name and required_type maps to ancestor_name. This is correct.
✅ Mock fidelity: The test mock's is_subtype_of_mock correctly delegates to the real is_subtype_of() from cleveragents.resource.inheritance, ensuring tests exercise actual inheritance logic rather than a hand-rolled stub. Excellent practice.
✅ Behavioral equivalence: The old code's actual_type == required_type fast-path and NotFoundError → False fallback are both preserved by the underlying is_subtype_of() implementation in inheritance.py (equality check at line 238, exception catch at lines 243-248).
❌ Documented protocol contract: As noted above, the docstrings are now stale.

Deep Dive: Naming Conventions

✅ Method names _is_type_compatible and _check_type_compatibility follow the project's underscore-prefixed private method convention
✅ New step function names (step_type_inherits_nothing, step_type_inherits_from) follow the step_ prefix convention
✅ The is_subtype_of_mock name clearly indicates it's a mock implementation
✅ Feature scenario names are descriptive and follow the existing naming pattern

Deep Dive: Code Patterns

✅ Delegation over duplication: The fix correctly delegates to the registry's existing is_subtype_of() rather than reimplementing chain traversal — respects Service Layer / Repository boundary
✅ Single Responsibility: _is_type_compatible is now a thin wrapper, keeping binding resolution focused on binding logic
✅ Fail-fast preserved: Error propagation paths are maintained
✅ Clean import removal: The now-unused ResourceTypeSpec import was properly removed

Other Checks

✅ Commit message: Follows Conventional Changelog format with ISSUES CLOSED: #2929 footer
✅ Single atomic commit: One commit covering implementation + tests
✅ PR description: Thorough, explains what/why/how
✅ Closes #2929: Present in PR body
✅ No # type: ignore: None found
✅ File sizes: All files well under 500 lines
✅ Test quality: Scenarios test meaningful behavior with clear Given/When/Then structure

Decision: REQUEST CHANGES 🔄

The code logic is correct and well-tested. The only blocking issue is the stale API contract documentation in the service docstrings, plus the missing PR metadata (milestone and label).

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## 🔍 Code Review — REQUEST CHANGES Reviewed PR #3336 with focus on **api-consistency**, **naming-conventions**, and **code-patterns**. This is a well-scoped, surgical bugfix that correctly addresses the ADR-042 inheritance chain traversal issue. The core change — delegating `_is_type_compatible` to `registry.is_subtype_of()` instead of performing a flat `parent_types` membership check — is the right approach. The test coverage is thorough with 4 new BDD scenarios covering 2-level chains, 3-level chains, unrelated-type rejection, and static binding via inheritance. However, there is one required change related to API contract consistency, and two PR metadata issues. ### Required Changes 1. **[API-CONSISTENCY] Registry protocol contract is stale in docstrings** - Location: `src/cleveragents/application/services/binding_resolution_service.py` - **Class docstring** (around line 50) states: > *The registry is injected as a protocol-compatible object exposing `show_resource` and `show_type` methods* - **`__init__` docstring** (around line 62) states: > *resource_registry: Object with `show_resource(name_or_id)` and `show_type(name)` methods.* - **Issue**: The service now also requires `is_subtype_of(type_name, ancestor_name)` on the registry. Furthermore, `show_type()` is no longer called anywhere in this service (the only call site was in the old `_is_type_compatible`), so it is no longer a required method from this service's perspective. - **Required**: Update both docstrings to list `is_subtype_of(type_name, ancestor_name)` as a required method. Consider whether `show_type` should be removed from the documented contract (it may still be needed by other callers of the same registry, but this service no longer uses it). - **Why**: This is a first-class API consistency issue. Any consumer implementing a compatible registry (or writing a test mock) will follow these docstrings to know what methods to provide. The current docstrings are now misleading — they omit a required method and include one that is no longer used by this service. 2. **[METADATA] PR is missing milestone** - Issue #2929 is assigned to milestone **v3.2.0**. Per CONTRIBUTING.md, the PR must be assigned to the same milestone as its linked issue. - **Required**: Assign milestone v3.2.0 to this PR. 3. **[METADATA] PR is missing `Type/` label** - Per CONTRIBUTING.md, every PR must have exactly one `Type/` label. This is a bugfix, so it should have `Type/Bug`. - **Required**: Add the `Type/Bug` label. ### Deep Dive: API Consistency Given special attention to API consistency across the change: - ✅ **Method signature consistency**: `is_subtype_of(type_name, ancestor_name)` on `ResourceRegistryService` matches the parameter order used in the call site (`self._registry.is_subtype_of(actual_type, required_type)`) — `actual_type` maps to `type_name` and `required_type` maps to `ancestor_name`. This is correct. - ✅ **Mock fidelity**: The test mock's `is_subtype_of_mock` correctly delegates to the real `is_subtype_of()` from `cleveragents.resource.inheritance`, ensuring tests exercise actual inheritance logic rather than a hand-rolled stub. Excellent practice. - ✅ **Behavioral equivalence**: The old code's `actual_type == required_type` fast-path and `NotFoundError → False` fallback are both preserved by the underlying `is_subtype_of()` implementation in `inheritance.py` (equality check at line 238, exception catch at lines 243-248). - ❌ **Documented protocol contract**: As noted above, the docstrings are now stale. ### Deep Dive: Naming Conventions - ✅ Method names `_is_type_compatible` and `_check_type_compatibility` follow the project's underscore-prefixed private method convention - ✅ New step function names (`step_type_inherits_nothing`, `step_type_inherits_from`) follow the `step_` prefix convention - ✅ The `is_subtype_of_mock` name clearly indicates it's a mock implementation - ✅ Feature scenario names are descriptive and follow the existing naming pattern ### Deep Dive: Code Patterns - ✅ **Delegation over duplication**: The fix correctly delegates to the registry's existing `is_subtype_of()` rather than reimplementing chain traversal — respects Service Layer / Repository boundary - ✅ **Single Responsibility**: `_is_type_compatible` is now a thin wrapper, keeping binding resolution focused on binding logic - ✅ **Fail-fast preserved**: Error propagation paths are maintained - ✅ **Clean import removal**: The now-unused `ResourceTypeSpec` import was properly removed ### Other Checks - ✅ **Commit message**: Follows Conventional Changelog format with `ISSUES CLOSED: #2929` footer - ✅ **Single atomic commit**: One commit covering implementation + tests - ✅ **PR description**: Thorough, explains what/why/how - ✅ **`Closes #2929`**: Present in PR body - ✅ **No `# type: ignore`**: None found - ✅ **File sizes**: All files well under 500 lines - ✅ **Test quality**: Scenarios test meaningful behavior with clear Given/When/Then structure **Decision: REQUEST CHANGES** 🔄 The code logic is correct and well-tested. The only blocking issue is the stale API contract documentation in the service docstrings, plus the missing PR metadata (milestone and label). --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer

freemo referenced this pull request

2026-04-05 17:03:42 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo reviewed

2026-04-05 17:05:08 +00:00

freemo left a comment

🔍 Code Review — COMMENT (Test-Focused)

Reviewed PR #3336 with focus on test-coverage-quality, test-scenario-completeness, and test-maintainability.

This is a well-scoped bugfix with a clean implementation — delegating _is_type_compatible to registry.is_subtype_of() is the right approach. The previous reviewer already flagged the stale docstrings and missing PR metadata (milestone + Type/ label), which I concur with. My review focuses specifically on the test dimension.

Deep Dive: Test Coverage Quality

✅ Real inheritance logic in mocks: The is_subtype_of_mock in binding_resolution_steps.py delegates to the real is_subtype_of() from cleveragents.resource.inheritance, building a type_registry dict from the mock's _inherits map. This is excellent practice — the tests exercise the actual chain-walking algorithm rather than a hand-rolled stub, giving genuine confidence in the fix.

✅ Meaningful scenario assertions: Each new scenario verifies concrete outcomes (resource_id, binding mode, deferred status, or ValidationError text), not just "it didn't crash". The three-level chain scenario (local/special-dev-container → devcontainer-instance → container-instance) directly reproduces the bug described in issue #2929.

✅ Negative testing included: The "rejects unrelated type even with inheritance present" scenario ensures the fix didn't accidentally make type checking too permissive — fs-directory correctly fails to match container-instance even when inheritance infrastructure is active.

✅ Existing scenarios still exercise the new code path: The pre-existing "Type compatibility passes for sub-type via parent_types" scenario (which uses fs-directory with parent_types including git-checkout) continues to work because step_type_parent was updated to also populate _inherits, ensuring is_subtype_of() can walk the chain.

Deep Dive: Test Scenario Completeness

Issue #2929 acceptance criteria check:

✅ _is_type_compatible uses is_subtype_of() — verified by all new scenarios
✅ Tool requiring container-instance can bind to devcontainer-instance — two-level scenario
✅ Tool requiring git-checkout cannot bind to unrelated fs-directory — unrelated-type scenario
✅ Static binding uses corrected type compatibility — static binding inheritance scenario
⚠️ Parameter binding via inheritance chain — NOT tested

[TEST-GAP] Missing parameter binding inheritance scenario

Location: features/consolidated_binding_resolution.feature
Issue: The acceptance criteria in issue #2929 explicitly states: "Static and parameter binding also use the corrected type compatibility check." There is a new scenario for static binding via inheritance (@type_compat @inheritance @static_binding), but no corresponding scenario for parameter binding via inheritance.
The code path is covered because _resolve_parameter calls _check_type_compatibility → _is_type_compatible, which is the fixed method. However, having an explicit BDD scenario would:
(a) Document this as a tested requirement
(b) Guard against future regressions if parameter binding is refactored
(c) Satisfy the acceptance criteria checklist completely

Suggested: Add a scenario like:

@type_compat @inheritance @parameter_binding
Scenario: Parameter binding resolves via two-level inheritance chain
  Given a mock resource registry
  And a binding resolution service using the registry
  Given a named resource "01HGZ6FE0AQDYTR4BX00000103" called "local/my-devcontainer" of type "devcontainer-instance" in the registry
  And the resource type "container-instance" inherits from nothing
  And the resource type "devcontainer-instance" inherits from "container-instance"
  And a tool "local/runner" with a parameter slot "env" of type "container-instance"
  And a project "local/proj" with no linked resources
  When the bindings are resolved with invocation params:
    | slot | ref                      |
    | env  | local/my-devcontainer    |
  Then the binding for slot "env" should have mode "parameter"
  And the binding for slot "env" should not be deferred

Deep Dive: Test Maintainability

[TEST-MAINTAINABILITY] Stale scenario title and comments for NotFoundError case
- Location: features/consolidated_binding_resolution.feature, scenario "Type compatibility returns False when show_type raises NotFoundError"
- Issue: This scenario's title and section comment (# _is_type_compatible -- show_type raises NotFoundError (lines 369-370)) reference show_type, but the service no longer calls show_type in _is_type_compatible. The method now delegates entirely to is_subtype_of(). The scenario still passes because is_subtype_of() internally handles unknown types, but the title is now misleading.
- Impact: A future developer reading this scenario will think show_type is being tested, when in reality it's is_subtype_of's internal error handling that's being exercised. This creates confusion during maintenance.
- Suggested: Update the scenario title to something like "Type compatibility returns False for unknown type" and update the section comment to reference is_subtype_of instead of show_type.
[TEST-MAINTAINABILITY] Mock registry complexity growth
- Location: features/steps/binding_resolution_steps.py, step_mock_registry
- Observation (non-blocking): The mock registry now manages three parallel data structures (_resources, _types, _inherits). The step_type_parent step must update both _types and _inherits to stay consistent. This dual-update requirement is a potential source of subtle test bugs if a future contributor updates one but not the other.
- The current implementation handles this correctly (both step_type_parent and the new step_type_inherits_from properly maintain both structures), but consider adding a brief comment in step_mock_registry explaining the relationship between _types (used by show_type) and _inherits (used by is_subtype_of).
[TEST-QUALITY] Single-inheritance assumption in _inherits map
- Location: features/steps/binding_resolution_steps.py, step_type_parent and step_type_inherits_from
- Observation (non-blocking): The _inherits dict maps each type to a single parent (str | None), while parent_types supports multiple parents. The step_type_parent step uses parents[0] as the direct ancestor. This is fine for current scenarios but would silently produce incorrect results if a future test needs multi-parent inheritance. Worth a brief comment noting this limitation.

Other Checks (Standard Criteria)

✅ Commit message: Follows Conventional Changelog format with ISSUES CLOSED: #2929 footer
✅ Single atomic commit: One commit covering implementation + tests
✅ PR description: Thorough, explains what/why/how with design decisions
✅ Closes #2929: Present in PR body
✅ No # type: ignore: None found
✅ File sizes: binding_resolution_service.py ~330 lines, binding_resolution_steps.py ~430 lines — both well under 500
✅ Imports at top of file: All imports are at the top (the from cleveragents.resource.inheritance import is_subtype_of inside step_mock_registry is acceptable as it's a test-time lazy import within a step function)
✅ BDD test format: All new tests use proper Gherkin Given/When/Then structure
✅ Tags: New scenarios properly tagged with @type_compat @inheritance plus specific tags
❌ Stale docstrings: Already flagged by previous reviewer — class and __init__ docstrings still reference show_resource and show_type but not is_subtype_of
❌ Missing milestone: Already flagged — should be v3.2.0
❌ Missing Type/ label on PR: Already flagged — should have Type/Bug

Summary

Aspect	Status
Test coverage quality	✅ Excellent — real inheritance logic in mocks, meaningful assertions
Test scenario completeness	⚠️ Good but missing parameter binding inheritance scenario
Test maintainability	⚠️ Stale scenario title for NotFoundError case; minor complexity concerns
Code correctness	✅ Fix is correct and minimal
Spec alignment	✅ Matches ADR-042 polymorphism guarantee
CONTRIBUTING.md compliance	❌ Missing milestone and Type/ label (already flagged)

Decision: COMMENT 💬

The code logic is correct and the test quality is high. The missing parameter binding inheritance scenario (item 1) is the most actionable finding from a test-completeness perspective. The stale scenario title (item 2) is a maintainability concern worth addressing. Items 3 and 4 are non-blocking observations.

Automated by CleverAgents Bot
Reviewer: Code Quality | Agent: ca-pr-self-reviewer

## 🔍 Code Review — COMMENT (Test-Focused) Reviewed PR #3336 with focus on **test-coverage-quality**, **test-scenario-completeness**, and **test-maintainability**. This is a well-scoped bugfix with a clean implementation — delegating `_is_type_compatible` to `registry.is_subtype_of()` is the right approach. The previous reviewer already flagged the stale docstrings and missing PR metadata (milestone + `Type/` label), which I concur with. My review focuses specifically on the test dimension. --- ### Deep Dive: Test Coverage Quality ✅ **Real inheritance logic in mocks**: The `is_subtype_of_mock` in `binding_resolution_steps.py` delegates to the *real* `is_subtype_of()` from `cleveragents.resource.inheritance`, building a `type_registry` dict from the mock's `_inherits` map. This is excellent practice — the tests exercise the actual chain-walking algorithm rather than a hand-rolled stub, giving genuine confidence in the fix. ✅ **Meaningful scenario assertions**: Each new scenario verifies concrete outcomes (resource_id, binding mode, deferred status, or ValidationError text), not just "it didn't crash". The three-level chain scenario (`local/special-dev-container → devcontainer-instance → container-instance`) directly reproduces the bug described in issue #2929. ✅ **Negative testing included**: The "rejects unrelated type even with inheritance present" scenario ensures the fix didn't accidentally make type checking too permissive — `fs-directory` correctly fails to match `container-instance` even when inheritance infrastructure is active. ✅ **Existing scenarios still exercise the new code path**: The pre-existing "Type compatibility passes for sub-type via parent_types" scenario (which uses `fs-directory` with `parent_types` including `git-checkout`) continues to work because `step_type_parent` was updated to also populate `_inherits`, ensuring `is_subtype_of()` can walk the chain. ### Deep Dive: Test Scenario Completeness **Issue #2929 acceptance criteria check:** - ✅ `_is_type_compatible` uses `is_subtype_of()` — verified by all new scenarios - ✅ Tool requiring `container-instance` can bind to `devcontainer-instance` — two-level scenario - ✅ Tool requiring `git-checkout` cannot bind to unrelated `fs-directory` — unrelated-type scenario - ✅ Static binding uses corrected type compatibility — static binding inheritance scenario - ⚠️ **Parameter binding via inheritance chain — NOT tested** 1. **[TEST-GAP] Missing parameter binding inheritance scenario** - Location: `features/consolidated_binding_resolution.feature` - Issue: The acceptance criteria in issue #2929 explicitly states: *"Static and parameter binding also use the corrected type compatibility check."* There is a new scenario for static binding via inheritance (`@type_compat @inheritance @static_binding`), but no corresponding scenario for parameter binding via inheritance. - The code path *is* covered because `_resolve_parameter` calls `_check_type_compatibility` → `_is_type_compatible`, which is the fixed method. However, having an explicit BDD scenario would: (a) Document this as a tested requirement (b) Guard against future regressions if parameter binding is refactored (c) Satisfy the acceptance criteria checklist completely - **Suggested**: Add a scenario like: ```gherkin @type_compat @inheritance @parameter_binding Scenario: Parameter binding resolves via two-level inheritance chain Given a mock resource registry And a binding resolution service using the registry Given a named resource "01HGZ6FE0AQDYTR4BX00000103" called "local/my-devcontainer" of type "devcontainer-instance" in the registry And the resource type "container-instance" inherits from nothing And the resource type "devcontainer-instance" inherits from "container-instance" And a tool "local/runner" with a parameter slot "env" of type "container-instance" And a project "local/proj" with no linked resources When the bindings are resolved with invocation params: | slot | ref | | env | local/my-devcontainer | Then the binding for slot "env" should have mode "parameter" And the binding for slot "env" should not be deferred ``` ### Deep Dive: Test Maintainability 2. **[TEST-MAINTAINABILITY] Stale scenario title and comments for NotFoundError case** - Location: `features/consolidated_binding_resolution.feature`, scenario "Type compatibility returns False when show_type raises NotFoundError" - Issue: This scenario's title and section comment (`# _is_type_compatible -- show_type raises NotFoundError (lines 369-370)`) reference `show_type`, but the service no longer calls `show_type` in `_is_type_compatible`. The method now delegates entirely to `is_subtype_of()`. The scenario still passes because `is_subtype_of()` internally handles unknown types, but the title is now misleading. - **Impact**: A future developer reading this scenario will think `show_type` is being tested, when in reality it's `is_subtype_of`'s internal error handling that's being exercised. This creates confusion during maintenance. - **Suggested**: Update the scenario title to something like "Type compatibility returns False for unknown type" and update the section comment to reference `is_subtype_of` instead of `show_type`. 3. **[TEST-MAINTAINABILITY] Mock registry complexity growth** - Location: `features/steps/binding_resolution_steps.py`, `step_mock_registry` - Observation (non-blocking): The mock registry now manages three parallel data structures (`_resources`, `_types`, `_inherits`). The `step_type_parent` step must update both `_types` and `_inherits` to stay consistent. This dual-update requirement is a potential source of subtle test bugs if a future contributor updates one but not the other. - The current implementation handles this correctly (both `step_type_parent` and the new `step_type_inherits_from` properly maintain both structures), but consider adding a brief comment in `step_mock_registry` explaining the relationship between `_types` (used by `show_type`) and `_inherits` (used by `is_subtype_of`). 4. **[TEST-QUALITY] Single-inheritance assumption in `_inherits` map** - Location: `features/steps/binding_resolution_steps.py`, `step_type_parent` and `step_type_inherits_from` - Observation (non-blocking): The `_inherits` dict maps each type to a single parent (`str | None`), while `parent_types` supports multiple parents. The `step_type_parent` step uses `parents[0]` as the direct ancestor. This is fine for current scenarios but would silently produce incorrect results if a future test needs multi-parent inheritance. Worth a brief comment noting this limitation. ### Other Checks (Standard Criteria) - ✅ **Commit message**: Follows Conventional Changelog format with `ISSUES CLOSED: #2929` footer - ✅ **Single atomic commit**: One commit covering implementation + tests - ✅ **PR description**: Thorough, explains what/why/how with design decisions - ✅ **`Closes #2929`**: Present in PR body - ✅ **No `# type: ignore`**: None found - ✅ **File sizes**: `binding_resolution_service.py` ~330 lines, `binding_resolution_steps.py` ~430 lines — both well under 500 - ✅ **Imports at top of file**: All imports are at the top (the `from cleveragents.resource.inheritance import is_subtype_of` inside `step_mock_registry` is acceptable as it's a test-time lazy import within a step function) - ✅ **BDD test format**: All new tests use proper Gherkin Given/When/Then structure - ✅ **Tags**: New scenarios properly tagged with `@type_compat @inheritance` plus specific tags - ❌ **Stale docstrings**: Already flagged by previous reviewer — class and `__init__` docstrings still reference `show_resource` and `show_type` but not `is_subtype_of` - ❌ **Missing milestone**: Already flagged — should be v3.2.0 - ❌ **Missing `Type/` label on PR**: Already flagged — should have `Type/Bug` ### Summary | Aspect | Status | |--------|--------| | Test coverage quality | ✅ Excellent — real inheritance logic in mocks, meaningful assertions | | Test scenario completeness | ⚠️ Good but missing parameter binding inheritance scenario | | Test maintainability | ⚠️ Stale scenario title for NotFoundError case; minor complexity concerns | | Code correctness | ✅ Fix is correct and minimal | | Spec alignment | ✅ Matches ADR-042 polymorphism guarantee | | CONTRIBUTING.md compliance | ❌ Missing milestone and Type/ label (already flagged) | **Decision: COMMENT** 💬 The code logic is correct and the test quality is high. The missing parameter binding inheritance scenario (item 1) is the most actionable finding from a test-completeness perspective. The stale scenario title (item 2) is a maintainability concern worth addressing. Items 3 and 4 are non-blocking observations. --- **Automated by CleverAgents Bot** Reviewer: Code Quality | Agent: ca-pr-self-reviewer

freemo referenced this pull request

2026-04-05 17:11:37 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo referenced this pull request

2026-04-05 17:41:35 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo referenced this pull request

2026-04-05 17:42:57 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo referenced this pull request

2026-04-05 17:45:54 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo added a new dependency

2026-04-05 17:47:02 +00:00

#2929 UAT: bug(resource-registry): contextual resource binding type compatibility check does not use full inheritance chain

freemo referenced this pull request

2026-04-05 17:47:27 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo referenced this pull request

2026-04-05 17:51:27 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo force-pushed bugfix/binding-resolution-inheritance-chain from 04c18b86b8

CI / benchmark-publish (pull_request) Waiting to run

Details

CI / lint (pull_request) Successful in 25s

Details

CI / typecheck (pull_request) Successful in 59s

Details

CI / quality (pull_request) Successful in 33s

Details

CI / security (pull_request) Successful in 55s

Details

CI / benchmark-regression (pull_request) Waiting to run

Details

CI / build (pull_request) Successful in 21s

Details

CI / helm (pull_request) Successful in 28s

Details

CI / unit_tests (pull_request) Successful in 6m52s

Details

CI / e2e_tests (pull_request) Successful in 17m3s

Details

CI / integration_tests (pull_request) Successful in 22m24s

Details

CI / docker (pull_request) Successful in 1m26s

Details

CI / coverage (pull_request) Successful in 10m18s

Details

CI / status-check (pull_request) Successful in 1s

Details

to 3224f2136e

CI / lint (pull_request) Successful in 27s

Details

CI / typecheck (pull_request) Successful in 52s

Details

CI / quality (pull_request) Successful in 32s

Details

CI / security (pull_request) Successful in 59s

Details

CI / build (pull_request) Successful in 29s

Details

CI / helm (pull_request) Successful in 23s

Details

CI / unit_tests (pull_request) Successful in 6m58s

Details

CI / e2e_tests (pull_request) Successful in 19m39s

Details

CI / integration_tests (pull_request) Successful in 22m49s

Details

CI / coverage (pull_request) Successful in 10m43s

Details

CI / docker (pull_request) Successful in 1m23s

Details

CI / status-check (pull_request) Successful in 1s

Details

CI / benchmark-publish (pull_request) Has been skipped

Details

CI / benchmark-regression (pull_request) Successful in 56m57s

Details

2026-04-05 17:55:58 +00:00

Compare

freemo added this to the v3.2.0 milestone

2026-04-05 17:56:05 +00:00

freemo commented

2026-04-05 17:56:32 +00:00

Author

Owner

Addressed all review feedback from both reviews. Here's a summary of what was implemented:

Changes Made

Required (REQUEST CHANGES review)

[API-CONSISTENCY] Updated registry protocol contract in docstrings (binding_resolution_service.py):
- Class docstring: replaced show_resource and show_type with show_resource and is_subtype_of as the required protocol methods
- __init__ docstring: replaced show_type(name) with is_subtype_of(type_name, ancestor_name) in the Args description
- show_type is no longer documented as a required method since this service no longer calls it
[METADATA] Milestone v3.2.0 assigned ✅
[METADATA] Type/Bug label added ✅

Addressed (COMMENT review)

[TEST-GAP] Added parameter binding inheritance scenario (features/consolidated_binding_resolution.feature):
- New scenario @type_compat @inheritance @parameter_binding: "Parameter binding resolves via two-level inheritance chain"
- Uses local/my-devcontainer (type devcontainer-instance) with a parameter slot requiring container-instance
- Exercises _resolve_parameter → _check_type_compatibility → _is_type_compatible → is_subtype_of() path
- Completes the acceptance criteria coverage: contextual ✅, static ✅, parameter ✅
[TEST-MAINTAINABILITY] Updated stale scenario title and comment (features/consolidated_binding_resolution.feature):
- Section comment: # _is_type_compatible -- show_type raises NotFoundError (lines 369-370) → # _is_type_compatible -- is_subtype_of returns False for unknown type
- Scenario title: Type compatibility returns False when show_type raises NotFoundError → Type compatibility returns False for unknown type

Items 3 and 4 from the COMMENT review (mock registry complexity and single-inheritance assumption) were non-blocking observations and are acknowledged but not changed, as the current implementation correctly handles all tested scenarios.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

Addressed all review feedback from both reviews. Here's a summary of what was implemented: ## Changes Made ### Required (REQUEST CHANGES review) 1. **[API-CONSISTENCY] Updated registry protocol contract in docstrings** (`binding_resolution_service.py`): - Class docstring: replaced `show_resource` and `show_type` with `show_resource` and `is_subtype_of` as the required protocol methods - `__init__` docstring: replaced `show_type(name)` with `is_subtype_of(type_name, ancestor_name)` in the Args description - `show_type` is no longer documented as a required method since this service no longer calls it 2. **[METADATA] Milestone v3.2.0 assigned** ✅ 3. **[METADATA] `Type/Bug` label added** ✅ ### Addressed (COMMENT review) 4. **[TEST-GAP] Added parameter binding inheritance scenario** (`features/consolidated_binding_resolution.feature`): - New scenario `@type_compat @inheritance @parameter_binding`: "Parameter binding resolves via two-level inheritance chain" - Uses `local/my-devcontainer` (type `devcontainer-instance`) with a parameter slot requiring `container-instance` - Exercises `_resolve_parameter` → `_check_type_compatibility` → `_is_type_compatible` → `is_subtype_of()` path - Completes the acceptance criteria coverage: contextual ✅, static ✅, parameter ✅ 5. **[TEST-MAINTAINABILITY] Updated stale scenario title and comment** (`features/consolidated_binding_resolution.feature`): - Section comment: `# _is_type_compatible -- show_type raises NotFoundError (lines 369-370)` → `# _is_type_compatible -- is_subtype_of returns False for unknown type` - Scenario title: `Type compatibility returns False when show_type raises NotFoundError` → `Type compatibility returns False for unknown type` Items 3 and 4 from the COMMENT review (mock registry complexity and single-inheritance assumption) were non-blocking observations and are acknowledged but not changed, as the current implementation correctly handles all tested scenarios. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker

freemo reviewed

2026-04-05 19:38:40 +00:00

freemo left a comment

🔍 Code Review — APPROVED (Performance-Focused)

Reviewed PR #3336 with focus on performance-implications, resource-usage, and scalability.

This is a well-scoped, correct bugfix that addresses a real ADR-042 compliance gap. The previous review cycle's feedback (stale docstrings, missing milestone/label, missing parameter binding scenario, stale scenario titles) has been fully addressed. The implementation correctly delegates type compatibility checking to registry.is_subtype_of() rather than reimplementing chain traversal.

Deep Dive: Performance Implications

Given special attention to the performance characteristics of the change:

Old code path (_is_type_compatible before this fix):

spec = self._registry.show_type(actual_type)  # 1 DB query, 1 row
return required_type in spec.parent_types      # O(n) list membership

New code path (_is_type_compatible after this fix):

return self._registry.is_subtype_of(actual_type, required_type)

Which in ResourceRegistryService.is_subtype_of() does:

registry = self._load_type_registry()  # 1 DB query, ALL rows
return is_subtype_of(type_name, ancestor_name, registry)  # O(depth) walk, max 5

Analysis:

The old code fetched one row per type check (show_type → single filter_by query)
The new code fetches all rows per type check (_load_type_registry → query(ResourceTypeModel).all())
In _find_matching_resources(), _is_type_compatible is called once per linked resource. For a project with N linked resources and a tool with M slots, this means M×N full-table scans instead of M×N single-row queries.

Verdict: Non-blocking. This is acceptable for the following reasons:

Correctness over performance: The old code was wrong — it only checked immediate parents, violating ADR-042's polymorphism guarantee. A correct-but-slower implementation is strictly better than a fast-but-broken one.
Pre-existing API design: ResourceRegistryService.is_subtype_of() and _load_type_registry() already existed with this exact behavior. This PR correctly uses the existing public API rather than reimplementing chain traversal or reaching into private internals. The performance characteristic is a property of the registry service, not of this change.
Bounded chain depth: resolve_inheritance_chain() is bounded by MAX_CHAIN_DEPTH = 5 and uses cycle detection, so the chain walk itself is O(1) in practice.
Typical scale: Projects typically have a small number of linked resources (single digits), and tools typically have 1-3 resource slots. The M×N product is small in practice.
Future optimization path: If this becomes a bottleneck, the proper fix is adding caching to _load_type_registry() (e.g., a short-lived TTL cache or per-request memoization), which is orthogonal to this bugfix and should be tracked as a separate optimization issue.

⚠️ Recommendation: Consider filing a follow-up issue to add caching to ResourceRegistryService._load_type_registry(). This would benefit not just binding resolution but all callers of is_subtype_of(), resolve_type_inheritance_chain(), and remove_type().

Deep Dive: Resource Usage

✅ No resource leaks: The _load_type_registry() method properly uses try/finally to close sessions
✅ No unbounded allocations: The type registry dict is bounded by the number of registered types (finite, typically < 50)
✅ No connection pool exhaustion risk: Each is_subtype_of call opens and closes a session within the same call frame — no long-held connections
✅ Memory: The in-memory registry dict created per call is small (just {name: {inherits: ..., built_in: ...}} per type) and is garbage-collected after each call

Deep Dive: Scalability

✅ Chain depth bounded: MAX_CHAIN_DEPTH = 5 prevents runaway traversal
✅ Cycle detection: resolve_inheritance_chain() uses a visited set to detect and reject circular inheritance
✅ Error handling preserved: is_subtype_of() catches ResourceTypeParentNotFoundError, ResourceTypeCircularInheritanceError, and ResourceTypeInheritanceDepthError, returning False for any broken chain — this is safe and graceful
⚠️ Linear scan in contextual binding: _find_matching_resources() iterates all linked resources linearly. This is fine for typical project sizes but would not scale to projects with hundreds of linked resources. (This is pre-existing behavior, not introduced by this PR.)

Standard Criteria Checks

Criterion	Status	Notes
Commit message format	✅	`fix(resource-registry): ...` with `ISSUES CLOSED: #2929`
PR metadata	✅	`Closes #2929`, milestone v3.2.0, `Type/Bug` label
Docstrings updated	✅	Class and `__init__` docstrings now list `show_resource` and `is_subtype_of`
No `# type: ignore`	✅	None found
File sizes	✅	`binding_resolution_service.py` ~330 lines, `binding_resolution_steps.py` ~430 lines
Imports at top	✅	The `from cleveragents.resource.inheritance import is_subtype_of` inside `step_mock_registry` is acceptable as a test-time lazy import
Spec alignment	✅	Correctly implements ADR-042 §Tool Binding Polymorphism
Test quality	✅	5 new scenarios covering 2-level, 3-level, unrelated-type, static, and parameter binding via inheritance
Test uses real logic	✅	Mock's `is_subtype_of_mock` delegates to real `is_subtype_of()` from `cleveragents.resource.inheritance`
Error handling	✅	Fail-fast preserved; `_is_type_compatible` is now a thin delegation
Single atomic commit	✅	One commit covering implementation + tests + review feedback
Previous review feedback	✅	All items from both previous reviews addressed

Code Correctness

✅ Parameter order: self._registry.is_subtype_of(actual_type, required_type) correctly maps to is_subtype_of(type_name, ancestor_name) — actual_type is the concrete type being checked, required_type is the ancestor
✅ Equality fast-path preserved: is_subtype_of() in inheritance.py starts with if type_name == ancestor_name: return True
✅ Unknown type handling preserved: is_subtype_of() catches all chain errors and returns False, matching the old behavior where NotFoundError from show_type() returned False
✅ No behavioral regression for existing scenarios: The pre-existing "Type compatibility passes for sub-type via parent_types" scenario continues to work because step_type_parent was updated to also populate _inherits

Decision: APPROVED ✅

The fix is correct, minimal, well-tested, and properly addresses the ADR-042 compliance gap. The performance observation about _load_type_registry() being called per type check is non-blocking and should be addressed as a separate optimization if it becomes a bottleneck.

Automated by CleverAgents Bot
Supervisor: PR Review | Agent: ca-pr-self-reviewer

## 🔍 Code Review — APPROVED (Performance-Focused) Reviewed PR #3336 with focus on **performance-implications**, **resource-usage**, and **scalability**. This is a well-scoped, correct bugfix that addresses a real ADR-042 compliance gap. The previous review cycle's feedback (stale docstrings, missing milestone/label, missing parameter binding scenario, stale scenario titles) has been fully addressed. The implementation correctly delegates type compatibility checking to `registry.is_subtype_of()` rather than reimplementing chain traversal. --- ### Deep Dive: Performance Implications Given special attention to the performance characteristics of the change: **Old code path** (`_is_type_compatible` before this fix): ```python spec = self._registry.show_type(actual_type) # 1 DB query, 1 row return required_type in spec.parent_types # O(n) list membership ``` **New code path** (`_is_type_compatible` after this fix): ```python return self._registry.is_subtype_of(actual_type, required_type) ``` Which in `ResourceRegistryService.is_subtype_of()` does: ```python registry = self._load_type_registry() # 1 DB query, ALL rows return is_subtype_of(type_name, ancestor_name, registry) # O(depth) walk, max 5 ``` **Analysis:** - The old code fetched **one row** per type check (`show_type` → single `filter_by` query) - The new code fetches **all rows** per type check (`_load_type_registry` → `query(ResourceTypeModel).all()`) - In `_find_matching_resources()`, `_is_type_compatible` is called once per linked resource. For a project with N linked resources and a tool with M slots, this means M×N full-table scans instead of M×N single-row queries. **Verdict: Non-blocking.** This is acceptable for the following reasons: 1. **Correctness over performance**: The old code was *wrong* — it only checked immediate parents, violating ADR-042's polymorphism guarantee. A correct-but-slower implementation is strictly better than a fast-but-broken one. 2. **Pre-existing API design**: `ResourceRegistryService.is_subtype_of()` and `_load_type_registry()` already existed with this exact behavior. This PR correctly uses the existing public API rather than reimplementing chain traversal or reaching into private internals. The performance characteristic is a property of the registry service, not of this change. 3. **Bounded chain depth**: `resolve_inheritance_chain()` is bounded by `MAX_CHAIN_DEPTH = 5` and uses cycle detection, so the chain walk itself is O(1) in practice. 4. **Typical scale**: Projects typically have a small number of linked resources (single digits), and tools typically have 1-3 resource slots. The M×N product is small in practice. 5. **Future optimization path**: If this becomes a bottleneck, the proper fix is adding caching to `_load_type_registry()` (e.g., a short-lived TTL cache or per-request memoization), which is orthogonal to this bugfix and should be tracked as a separate optimization issue. ⚠️ **Recommendation**: Consider filing a follow-up issue to add caching to `ResourceRegistryService._load_type_registry()`. This would benefit not just binding resolution but all callers of `is_subtype_of()`, `resolve_type_inheritance_chain()`, and `remove_type()`. ### Deep Dive: Resource Usage - ✅ **No resource leaks**: The `_load_type_registry()` method properly uses try/finally to close sessions - ✅ **No unbounded allocations**: The type registry dict is bounded by the number of registered types (finite, typically < 50) - ✅ **No connection pool exhaustion risk**: Each `is_subtype_of` call opens and closes a session within the same call frame — no long-held connections - ✅ **Memory**: The in-memory registry dict created per call is small (just `{name: {inherits: ..., built_in: ...}}` per type) and is garbage-collected after each call ### Deep Dive: Scalability - ✅ **Chain depth bounded**: MAX_CHAIN_DEPTH = 5 prevents runaway traversal - ✅ **Cycle detection**: `resolve_inheritance_chain()` uses a `visited` set to detect and reject circular inheritance - ✅ **Error handling preserved**: `is_subtype_of()` catches `ResourceTypeParentNotFoundError`, `ResourceTypeCircularInheritanceError`, and `ResourceTypeInheritanceDepthError`, returning `False` for any broken chain — this is safe and graceful - ⚠️ **Linear scan in contextual binding**: `_find_matching_resources()` iterates all linked resources linearly. This is fine for typical project sizes but would not scale to projects with hundreds of linked resources. (This is pre-existing behavior, not introduced by this PR.) ### Standard Criteria Checks | Criterion | Status | Notes | |-----------|--------|-------| | Commit message format | ✅ | `fix(resource-registry): ...` with `ISSUES CLOSED: #2929` | | PR metadata | ✅ | `Closes #2929`, milestone v3.2.0, `Type/Bug` label | | Docstrings updated | ✅ | Class and `__init__` docstrings now list `show_resource` and `is_subtype_of` | | No `# type: ignore` | ✅ | None found | | File sizes | ✅ | `binding_resolution_service.py` ~330 lines, `binding_resolution_steps.py` ~430 lines | | Imports at top | ✅ | The `from cleveragents.resource.inheritance import is_subtype_of` inside `step_mock_registry` is acceptable as a test-time lazy import | | Spec alignment | ✅ | Correctly implements ADR-042 §Tool Binding Polymorphism | | Test quality | ✅ | 5 new scenarios covering 2-level, 3-level, unrelated-type, static, and parameter binding via inheritance | | Test uses real logic | ✅ | Mock's `is_subtype_of_mock` delegates to real `is_subtype_of()` from `cleveragents.resource.inheritance` | | Error handling | ✅ | Fail-fast preserved; `_is_type_compatible` is now a thin delegation | | Single atomic commit | ✅ | One commit covering implementation + tests + review feedback | | Previous review feedback | ✅ | All items from both previous reviews addressed | ### Code Correctness - ✅ **Parameter order**: `self._registry.is_subtype_of(actual_type, required_type)` correctly maps to `is_subtype_of(type_name, ancestor_name)` — actual_type is the concrete type being checked, required_type is the ancestor - ✅ **Equality fast-path preserved**: `is_subtype_of()` in `inheritance.py` starts with `if type_name == ancestor_name: return True` - ✅ **Unknown type handling preserved**: `is_subtype_of()` catches all chain errors and returns `False`, matching the old behavior where `NotFoundError` from `show_type()` returned `False` - ✅ **No behavioral regression for existing scenarios**: The pre-existing "Type compatibility passes for sub-type via parent_types" scenario continues to work because `step_type_parent` was updated to also populate `_inherits` **Decision: APPROVED** ✅ The fix is correct, minimal, well-tested, and properly addresses the ADR-042 compliance gap. The performance observation about `_load_type_registry()` being called per type check is non-blocking and should be addressed as a separate optimization if it becomes a bottleneck. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer

freemo referenced this pull request

2026-04-05 19:39:43 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo referenced this pull request

2026-04-05 19:48:58 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo referenced this pull request

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

[Automated] CleverAgents Build Session - 2026-04-05 #3572

freemo referenced this pull request

2026-04-05 20:54:57 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377