fix(resource-registry): implement get_parents() on ResourceDagMixin #3252

Merged

freemo merged 1 commits from fix/v3.7.0-resource-dag-mixin-get-parents into master 2026-04-05 21:11:48 +00:00

Conversation 5 Commits 1 Files Changed 1

+35

freemo commented 2026-04-05 08:31:44 +00:00

Owner

Copy Link

Copy Source

Summary

Adds the missing get_parents() method to ResourceDagMixin in _resource_registry_dag.py, resolving the failing BDD scenario "Get parents returns all direct parents" in resource_dag.feature.

Problem

ResourceDagMixin (and therefore ResourceRegistryService) had get_children() but was missing the symmetric get_parents() method required by the specification. The BDD scenario @dag_traversal "Get parents returns all direct parents" could not pass because the method did not exist.

Solution

Added get_parents(name_or_id: str) -> list[Resource] to ResourceDagMixin in src/cleveragents/application/services/_resource_registry_dag.py. The implementation mirrors get_children() exactly but queries ResourceLinkModel by child_id instead of parent_id:

• Calls show_resource() first → raises NotFoundError for non-existent resources
• Queries ResourceLinkModel.filter_by(child_id=resource.resource_id) to find all parent links
• Returns a deterministically sorted list[Resource] (by name, then resource_id)
• Exposed via ResourceRegistryService through the existing mixin inheritance chain

Test Results

• ✅ All 15 scenarios in resource_dag.feature pass (including the new "Get parents returns all direct parents" scenario)
• ✅ nox -s typecheck — 0 errors, 0 warnings
• ✅ nox -s lint — all checks passed

Files Changed

• src/cleveragents/application/services/_resource_registry_dag.py — added get_parents() method (35 lines)

Closes #2844

---

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

## Summary Adds the missing `get_parents()` method to `ResourceDagMixin` in `_resource_registry_dag.py`, resolving the failing BDD scenario "Get parents returns all direct parents" in `resource_dag.feature`. ## Problem `ResourceDagMixin` (and therefore `ResourceRegistryService`) had `get_children()` but was missing the symmetric `get_parents()` method required by the specification. The BDD scenario `@dag_traversal` "Get parents returns all direct parents" could not pass because the method did not exist. ## Solution Added `get_parents(name_or_id: str) -> list[Resource]` to `ResourceDagMixin` in `src/cleveragents/application/services/_resource_registry_dag.py`. The implementation mirrors `get_children()` exactly but queries `ResourceLinkModel` by `child_id` instead of `parent_id`: - Calls `show_resource()` first → raises `NotFoundError` for non-existent resources - Queries `ResourceLinkModel.filter_by(child_id=resource.resource_id)` to find all parent links - Returns a deterministically sorted `list[Resource]` (by name, then resource_id) - Exposed via `ResourceRegistryService` through the existing mixin inheritance chain ## Test Results - ✅ All 15 scenarios in `resource_dag.feature` pass (including the new "Get parents returns all direct parents" scenario) - ✅ `nox -s typecheck` — 0 errors, 0 warnings - ✅ `nox -s lint` — all checks passed ## Files Changed - `src/cleveragents/application/services/_resource_registry_dag.py` — added `get_parents()` method (35 lines) Closes #2844 --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker

freemo added 1 commit 2026-04-05 08:31:44 +00:00

fix(resource-registry): implement get_parents() on ResourceDagMixin ... cd162488ec

Add get_parents(name_or_id: str) -> list[Resource] method to ResourceDagMixin
in _resource_registry_dag.py. The method is symmetric to the existing
get_children() method but queries ResourceLinkModel by child_id instead of
parent_id to return all direct parent resources.

Implementation details:
- Calls show_resource() first to raise NotFoundError for non-existent resources
- Queries ResourceLinkModel filtered by child_id=resource.resource_id
- Returns deterministically sorted list (by name, then resource_id)
- Exposed via ResourceRegistryService through the mixin inheritance chain

The BDD scenario 'Get parents returns all direct parents' in
resource_dag.feature now passes. All 15 existing @dag_traversal and other
DAG scenarios continue to pass with no regression.

ISSUES CLOSED: #2844

freemo referenced this pull request 2026-04-05 08:36:41 +00:00

UAT: `ResourceDagMixin` missing `get_parents()` method — `resource_dag.feature` scenario "Get parents returns all direct parents" has no implementation #2844

freemo commented 2026-04-05 09:18:54 +00:00

Author

Owner

Copy Link

Copy Source

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3252-1775374200]

---

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

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

freemo commented 2026-04-05 09:24:33 +00:00

Author

Owner

Copy Link

Copy Source

🔒 Review claimed by reviewer-pool-1 [claim-token: reviewer-pool-1-3252-1775373000]

---

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

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

freemo referenced this pull request 2026-04-05 10:00:39 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3124

freemo added this to the v3.7.0 milestone 2026-04-05 10:05:35 +00:00

freemo reviewed 2026-04-05 10:10:31 +00:00

freemo left a comment

Author

Owner

Copy Link

Copy Source

✅ Independent Code Review — Approved

Summary

This PR adds the missing get_parents() method to ResourceDagMixin, completing the symmetric DAG traversal API alongside the existing get_children(). The implementation is correct, well-structured, and aligns with the specification.

Note: Self-approval is blocked by Forgejo. This review documents the independent code review findings. The PR is approved from a code quality perspective.

Review Checklist

Specification Alignment ✅

• get_parents(name_or_id: str) -> list[Resource] matches the spec-required signature from issue #2844
• Method mirrors get_children() exactly, querying ResourceLinkModel by child_id instead of parent_id
• Raises NotFoundError for non-existent resources via show_resource() — fail-fast pattern ✅
• Returns empty list for root nodes (no parents) — correct behavior ✅
• Exposed via ResourceRegistryService through the mixin inheritance chain ✅

Code Quality ✅

• Clean 35-line addition — minimal, focused change
• Follows existing patterns in the file exactly (session management, sorting, error handling)
• File remains at 293 lines — well under the 500-line CONTRIBUTING limit
• Docstring is accurate and includes Raises section (actually more complete than get_children()'s docstring)
• No # type: ignore suppressions in new code ✅
• Deterministic ordering (by name, then resource_id) matches the convention used throughout

Correctness ✅

• Query logic is correct: filter_by(child_id=resource.resource_id) finds all parent links
• str(link.parent_id) cast matches the pattern in get_children()
• Session is properly closed in finally block — no resource leaks
• show_resource() call validates input before opening session — fail-fast ✅

Test Quality ✅

• BDD scenario "Get parents returns all direct parents" exercises the multi-parent case (C1 linked to P1 and P2)
• Step definitions step_dag_get_parents and step_dag_n_parents are properly implemented
• All 15 existing @dag_traversal and DAG scenarios continue to pass (no regression)

Security ✅

• No secrets or credentials
• Input validation via show_resource()
• ORM filter_by() prevents SQL injection

PR Metadata ✅

• Title follows Conventional Changelog format
• Commit message has ISSUES CLOSED: #2844 footer
• Closes #2844 in PR body
• Type/Bug label present
• Milestone v3.7.0 assigned (corrected during review)

Minor Observation (Non-blocking)

The N+1 query pattern (one query per link to fetch the parent ResourceModel) is inherited from get_children(). This is a pre-existing pattern and not introduced by this PR. A future optimization could use a single IN query, but that's out of scope here.

---

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

## ✅ Independent Code Review — Approved ### Summary This PR adds the missing `get_parents()` method to `ResourceDagMixin`, completing the symmetric DAG traversal API alongside the existing `get_children()`. The implementation is correct, well-structured, and aligns with the specification. **Note:** Self-approval is blocked by Forgejo. This review documents the independent code review findings. The PR is approved from a code quality perspective. ### Review Checklist #### Specification Alignment ✅ - `get_parents(name_or_id: str) -> list[Resource]` matches the spec-required signature from issue #2844 - Method mirrors `get_children()` exactly, querying `ResourceLinkModel` by `child_id` instead of `parent_id` - Raises `NotFoundError` for non-existent resources via `show_resource()` — fail-fast pattern ✅ - Returns empty list for root nodes (no parents) — correct behavior ✅ - Exposed via `ResourceRegistryService` through the mixin inheritance chain ✅ #### Code Quality ✅ - Clean 35-line addition — minimal, focused change - Follows existing patterns in the file exactly (session management, sorting, error handling) - File remains at 293 lines — well under the 500-line CONTRIBUTING limit - Docstring is accurate and includes `Raises` section (actually more complete than `get_children()`'s docstring) - No `# type: ignore` suppressions in new code ✅ - Deterministic ordering (by name, then resource_id) matches the convention used throughout #### Correctness ✅ - Query logic is correct: `filter_by(child_id=resource.resource_id)` finds all parent links - `str(link.parent_id)` cast matches the pattern in `get_children()` - Session is properly closed in `finally` block — no resource leaks - `show_resource()` call validates input before opening session — fail-fast ✅ #### Test Quality ✅ - BDD scenario "Get parents returns all direct parents" exercises the multi-parent case (C1 linked to P1 and P2) - Step definitions `step_dag_get_parents` and `step_dag_n_parents` are properly implemented - All 15 existing `@dag_traversal` and DAG scenarios continue to pass (no regression) #### Security ✅ - No secrets or credentials - Input validation via `show_resource()` - ORM `filter_by()` prevents SQL injection #### PR Metadata ✅ - Title follows Conventional Changelog format - Commit message has `ISSUES CLOSED: #2844` footer - `Closes #2844` in PR body - `Type/Bug` label present - Milestone v3.7.0 assigned (corrected during review) ### Minor Observation (Non-blocking) The N+1 query pattern (one query per link to fetch the parent `ResourceModel`) is inherited from `get_children()`. This is a pre-existing pattern and not introduced by this PR. A future optimization could use a single `IN` query, but that's out of scope here. --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer

freemo scheduled this pull request to auto merge when all checks succeed 2026-04-05 10:15:31 +00:00

freemo referenced this pull request 2026-04-05 10:17:46 +00:00

UAT: `ResourceDagMixin` missing `get_parents()` method — `resource_dag.feature` scenario "Get parents returns all direct parents" has no implementation #2844

freemo referenced this pull request 2026-04-05 17:03:42 +00:00

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

freemo referenced this pull request 2026-04-05 17:11:37 +00:00

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

freemo reviewed 2026-04-05 17:17:38 +00:00

freemo left a comment

Author

Owner

Copy Link

Copy Source

Independent Code Review — PR #3252

Focus Areas: architecture-alignment, module-boundaries, interface-contracts

Summary

This PR adds the missing get_parents() method to ResourceDagMixin, completing the symmetric DAG traversal API alongside the existing get_children(). The change is a single 35-line method addition to _resource_registry_dag.py, resolving issue #2844.

I reviewed the full file on the branch (293 lines), the master baseline (262 lines), the RegistryHost protocol in _resource_registry_ops.py, the commit message, PR metadata, and the linked issue's acceptance criteria.

---

Architecture Alignment ✅

• Correct placement: get_parents() is added to ResourceDagMixin in _resource_registry_dag.py, which is the designated module for DAG traversal operations. This respects the architectural split between instance operations (_resource_registry_ops.py) and DAG operations (_resource_registry_dag.py).
• Layer separation: The method returns domain objects (Resource) via db_resource_to_domain(), not raw DB models. Correct abstraction level.
• Fail-fast validation: show_resource(name_or_id) is called before opening a session, raising NotFoundError immediately for invalid inputs. This follows the project's fail-fast argument validation pattern.
• Symmetric design: The implementation mirrors get_children() exactly — same session management, same sorting convention, same guard for missing rows. This is the correct approach for a symmetric operation.

Module Boundaries ✅

• No new imports: The method uses only types and functions already imported in the file (ResourceLinkModel, ResourceModel, db_resource_to_domain, Resource).
• No new cross-module dependencies: No architectural boundary violations.
• File size: 293 lines — well under the 500-line CONTRIBUTING limit.
• Internal module convention: File is _-prefixed, correctly indicating it's internal. External code accesses get_parents() through ResourceRegistryService via the mixin inheritance chain.

Interface Contracts ✅

• Signature matches spec: get_parents(name_or_id: str) -> list[Resource] satisfies the contract defined in issue #2844.
• RegistryHost protocol: The method uses self.show_resource() and self._session(), both already declared in the RegistryHost protocol. No protocol update needed.
• Error contract: Raises NotFoundError for non-existent resources (via show_resource()). Documented in the docstring's Raises section.
• Return contract: Returns list[Resource] with deterministic ordering (by name, then resource_id). Returns empty list for root nodes with no parents.
• Mixin exposure: ResourceRegistryService inherits from ResourceDagMixin, so get_parents() is automatically exposed without additional wiring.

Code Correctness ✅

• Query logic: filter_by(child_id=resource.resource_id) correctly finds all parent links for the given resource. This is the symmetric inverse of get_children()'s filter_by(parent_id=...).
• Type cast: str(link.parent_id) matches the pattern in get_children() (str(link.child_id)).
• Session cleanup: finally: session.close() ensures no resource leaks.
• No # type: ignore in new code ✅

Commit & PR Metadata ✅

• Commit message: fix(resource-registry): implement get_parents() on ResourceDagMixin — Conventional Changelog format ✅
• Commit footer: ISSUES CLOSED: #2844 ✅
• PR body: Closes #2844 ✅
• Single atomic commit: One commit, one file changed ✅
• Label: Type/Bug ✅
• Milestone: v3.7.0 (matches issue) ✅

Test Quality ✅

• The BDD scenario "Get parents returns all direct parents" was pre-written by the UAT tester (TDD approach) and exercises the multi-parent case (C1 linked to both P1 and P2).
• All 15 existing DAG scenarios continue to pass — no regression.
• The PR claims nox -s typecheck and nox -s lint pass cleanly.

---

Observations (Non-blocking)

1. Pre-existing # type: ignore[override] on link_child (line ~42) and unlink_child (line ~113): These violate CONTRIBUTING.md's "no type: ignore" rule but are not introduced by this PR. The new get_parents() method correctly avoids this pattern. Consider filing a separate issue to address the pre-existing suppressions.

2. N+1 query pattern: The method queries each parent ResourceModel individually in a loop, rather than using a single IN query. This is inherited from the get_children() pattern and is not introduced by this PR. For DAGs with many parents this could be a performance concern, but it's out of scope for this bug fix.

3. Silent skip on orphaned links: The if parent_row is not None guard silently skips links where the parent resource no longer exists (data integrity issue). This matches get_children()'s behavior and is a pre-existing design decision. In a strict fail-fast world, a warning log or error might be appropriate, but changing this pattern is out of scope.

4. Docstring improvement: get_parents() includes a Raises section that get_children() lacks. This is actually an improvement — consider backfilling the Raises section on get_children() in a future cleanup.

Decision

No blocking issues found. The implementation is correct, well-structured, follows existing patterns, and satisfies all acceptance criteria from issue #2844. The code aligns with the specification's DAG traversal requirements and respects all module boundaries and interface contracts.

---

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

## Independent Code Review — PR #3252 **Focus Areas:** architecture-alignment, module-boundaries, interface-contracts ### Summary This PR adds the missing `get_parents()` method to `ResourceDagMixin`, completing the symmetric DAG traversal API alongside the existing `get_children()`. The change is a single 35-line method addition to `_resource_registry_dag.py`, resolving issue #2844. I reviewed the full file on the branch (293 lines), the master baseline (262 lines), the `RegistryHost` protocol in `_resource_registry_ops.py`, the commit message, PR metadata, and the linked issue's acceptance criteria. --- ### Architecture Alignment ✅ - **Correct placement**: `get_parents()` is added to `ResourceDagMixin` in `_resource_registry_dag.py`, which is the designated module for DAG traversal operations. This respects the architectural split between instance operations (`_resource_registry_ops.py`) and DAG operations (`_resource_registry_dag.py`). - **Layer separation**: The method returns domain objects (`Resource`) via `db_resource_to_domain()`, not raw DB models. Correct abstraction level. - **Fail-fast validation**: `show_resource(name_or_id)` is called before opening a session, raising `NotFoundError` immediately for invalid inputs. This follows the project's fail-fast argument validation pattern. - **Symmetric design**: The implementation mirrors `get_children()` exactly — same session management, same sorting convention, same guard for missing rows. This is the correct approach for a symmetric operation. ### Module Boundaries ✅ - **No new imports**: The method uses only types and functions already imported in the file (`ResourceLinkModel`, `ResourceModel`, `db_resource_to_domain`, `Resource`). - **No new cross-module dependencies**: No architectural boundary violations. - **File size**: 293 lines — well under the 500-line CONTRIBUTING limit. - **Internal module convention**: File is `_`-prefixed, correctly indicating it's internal. External code accesses `get_parents()` through `ResourceRegistryService` via the mixin inheritance chain. ### Interface Contracts ✅ - **Signature matches spec**: `get_parents(name_or_id: str) -> list[Resource]` satisfies the contract defined in issue #2844. - **`RegistryHost` protocol**: The method uses `self.show_resource()` and `self._session()`, both already declared in the `RegistryHost` protocol. No protocol update needed. - **Error contract**: Raises `NotFoundError` for non-existent resources (via `show_resource()`). Documented in the docstring's `Raises` section. - **Return contract**: Returns `list[Resource]` with deterministic ordering (by name, then resource_id). Returns empty list for root nodes with no parents. - **Mixin exposure**: `ResourceRegistryService` inherits from `ResourceDagMixin`, so `get_parents()` is automatically exposed without additional wiring. ### Code Correctness ✅ - **Query logic**: `filter_by(child_id=resource.resource_id)` correctly finds all parent links for the given resource. This is the symmetric inverse of `get_children()`'s `filter_by(parent_id=...)`. - **Type cast**: `str(link.parent_id)` matches the pattern in `get_children()` (`str(link.child_id)`). - **Session cleanup**: `finally: session.close()` ensures no resource leaks. - **No `# type: ignore`** in new code ✅ ### Commit & PR Metadata ✅ - **Commit message**: `fix(resource-registry): implement get_parents() on ResourceDagMixin` — Conventional Changelog format ✅ - **Commit footer**: `ISSUES CLOSED: #2844` ✅ - **PR body**: `Closes #2844` ✅ - **Single atomic commit**: One commit, one file changed ✅ - **Label**: `Type/Bug` ✅ - **Milestone**: v3.7.0 (matches issue) ✅ ### Test Quality ✅ - The BDD scenario "Get parents returns all direct parents" was pre-written by the UAT tester (TDD approach) and exercises the multi-parent case (C1 linked to both P1 and P2). - All 15 existing DAG scenarios continue to pass — no regression. - The PR claims `nox -s typecheck` and `nox -s lint` pass cleanly. --- ### Observations (Non-blocking) 1. **Pre-existing `# type: ignore[override]` on `link_child` (line ~42) and `unlink_child` (line ~113)**: These violate CONTRIBUTING.md's "no type: ignore" rule but are **not introduced by this PR**. The new `get_parents()` method correctly avoids this pattern. Consider filing a separate issue to address the pre-existing suppressions. 2. **N+1 query pattern**: The method queries each parent `ResourceModel` individually in a loop, rather than using a single `IN` query. This is inherited from the `get_children()` pattern and is **not introduced by this PR**. For DAGs with many parents this could be a performance concern, but it's out of scope for this bug fix. 3. **Silent skip on orphaned links**: The `if parent_row is not None` guard silently skips links where the parent resource no longer exists (data integrity issue). This matches `get_children()`'s behavior and is a pre-existing design decision. In a strict fail-fast world, a warning log or error might be appropriate, but changing this pattern is out of scope. 4. **Docstring improvement**: `get_parents()` includes a `Raises` section that `get_children()` lacks. This is actually an improvement — consider backfilling the `Raises` section on `get_children()` in a future cleanup. ### Decision No blocking issues found. The implementation is correct, well-structured, follows existing patterns, and satisfies all acceptance criteria from issue #2844. The code aligns with the specification's DAG traversal requirements and respects all module boundaries and interface contracts. --- **Automated by CleverAgents Bot** Reviewer: Code Quality | Agent: ca-pr-self-reviewer

freemo referenced this pull request 2026-04-05 19:48:58 +00:00

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

freemo reviewed 2026-04-05 19:49:59 +00:00

freemo left a comment

Author

Owner

Copy Link

Copy Source

Independent Code Review — PR #3252 (APPROVED)

Focus Areas: architecture-alignment, module-boundaries, interface-contracts

Note: Self-approval is blocked by Forgejo since the bot user is the PR author. This review documents the independent code review findings. The PR is approved from a code quality perspective.

Summary

This PR adds the missing get_parents() method to ResourceDagMixin in _resource_registry_dag.py, completing the symmetric DAG traversal API alongside the existing get_children(). The change is a single ~31-line method addition resolving issue #2844.

I independently reviewed the full file on the branch (293 lines, 11486 bytes), the master baseline (262 lines, 10274 bytes), the RegistryHost protocol in _resource_registry_ops.py, the ResourceRegistryService mixin inheritance chain, the BDD feature file resource_dag.feature, the commit message, and all PR metadata.

---

Architecture Alignment ✅

• Correct module placement: get_parents() is added to ResourceDagMixin in _resource_registry_dag.py — the designated module for DAG traversal operations. This respects the architectural split between instance operations (_resource_registry_ops.py), DAG operations (_resource_registry_dag.py), and the public service facade (resource_registry_service.py).
• Layer separation: The method returns domain objects (Resource) via db_resource_to_domain(), not raw DB models. Correct abstraction level for the application service layer.
• Fail-fast validation: show_resource(name_or_id) is called before opening a session, raising NotFoundError immediately for invalid inputs. This follows the project's fail-fast argument validation pattern per CONTRIBUTING.md.
• Symmetric design: The implementation mirrors get_children() exactly — same session management pattern, same deterministic sorting convention, same guard for missing rows. This is the correct approach for a symmetric DAG operation.

Module Boundaries ✅

• No new imports: The method uses only types and functions already imported in the file (ResourceLinkModel, ResourceModel, db_resource_to_domain, Resource).
• No new cross-module dependencies: Zero architectural boundary violations introduced.
• File size: 293 lines — well under the 500-line CONTRIBUTING limit.
• Internal module convention: File is _-prefixed, correctly indicating it's internal. External code accesses get_parents() through ResourceRegistryService via the mixin inheritance chain.
• Mixin exposure verified: ResourceRegistryService(ResourceInstanceMixin, ResourceDagMixin) inherits from ResourceDagMixin, so get_parents() is automatically exposed without additional wiring. Confirmed by reading resource_registry_service.py.

Interface Contracts ✅

• Signature matches spec: get_parents(name_or_id: str) -> list[Resource] satisfies the contract defined in issue #2844's acceptance criteria.
• RegistryHost protocol compliance: The method uses self.show_resource() and self._session(), both already declared in the RegistryHost protocol. No protocol update needed.
• Error contract: Raises NotFoundError for non-existent resources (via show_resource()). Documented in the docstring's Raises section.
• Return contract: Returns list[Resource] with deterministic ordering (by name, then resource_id). Returns empty list for root nodes with no parents.

Code Correctness ✅

• Query logic: filter_by(child_id=resource.resource_id) correctly finds all parent links for the given resource. This is the symmetric inverse of get_children()'s filter_by(parent_id=...).
• Type cast: str(link.parent_id) matches the pattern in get_children() (str(link.child_id)).
• Session cleanup: finally: session.close() ensures no resource leaks.
• No # type: ignore in new code ✅

Commit & PR Metadata ✅

• Commit message: fix(resource-registry): implement get_parents() on ResourceDagMixin — Conventional Changelog format ✅
• Commit footer: ISSUES CLOSED: #2844 ✅
• PR body: Closes #2844 ✅
• Single atomic commit: One commit, one file changed ✅
• Branch name: fix/v3.7.0-resource-dag-mixin-get-parents matches issue metadata ✅
• Label: Type/Bug ✅
• Milestone: v3.7.0 (matches issue) ✅

Test Quality ✅

• The BDD scenario "Get parents returns all direct parents" was pre-written by the UAT tester (TDD approach) — confirmed by identical SHA (a1e50453) on both master and branch.
• The scenario exercises the multi-parent case (C1 linked to both P1 and P2) with a count assertion.
• All 15 existing DAG scenarios continue to pass — no regression.
• The PR reports nox -s typecheck and nox -s lint pass cleanly.

---

Observations (Non-blocking)

1. Pre-existing # type: ignore[override] on link_child (~line 42) and unlink_child (~line 113): These violate CONTRIBUTING.md's "no type: ignore" rule but are not introduced by this PR. The new get_parents() method correctly avoids this pattern. Consider filing a separate issue to address the pre-existing suppressions.

2. N+1 query pattern: The method queries each parent ResourceModel individually in a loop, rather than using a single IN query. This is inherited from the get_children() pattern and is not introduced by this PR. For DAGs with many parents this could be a performance concern, but it's out of scope for this bug fix.

3. Silent skip on orphaned links: The if parent_row is not None guard silently skips links where the parent resource no longer exists (data integrity issue). This matches get_children()'s behavior and is a pre-existing design decision, not introduced here.

4. Docstring improvement: get_parents() includes a Raises section that get_children() lacks. This is actually an improvement over the existing pattern — consider backfilling the Raises section on get_children() in a future cleanup.

Decision

No blocking issues found. The implementation is correct, well-structured, follows existing patterns precisely, and satisfies all acceptance criteria from issue #2844. The code aligns with the specification's DAG traversal requirements and respects all module boundaries and interface contracts.

Decision: APPROVED ✅

---

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

## Independent Code Review — PR #3252 (APPROVED) **Focus Areas:** architecture-alignment, module-boundaries, interface-contracts > **Note:** Self-approval is blocked by Forgejo since the bot user is the PR author. This review documents the independent code review findings. The PR is **approved** from a code quality perspective. ### Summary This PR adds the missing `get_parents()` method to `ResourceDagMixin` in `_resource_registry_dag.py`, completing the symmetric DAG traversal API alongside the existing `get_children()`. The change is a single ~31-line method addition resolving issue #2844. I independently reviewed the full file on the branch (293 lines, 11486 bytes), the master baseline (262 lines, 10274 bytes), the `RegistryHost` protocol in `_resource_registry_ops.py`, the `ResourceRegistryService` mixin inheritance chain, the BDD feature file `resource_dag.feature`, the commit message, and all PR metadata. --- ### Architecture Alignment ✅ - **Correct module placement**: `get_parents()` is added to `ResourceDagMixin` in `_resource_registry_dag.py` — the designated module for DAG traversal operations. This respects the architectural split between instance operations (`_resource_registry_ops.py`), DAG operations (`_resource_registry_dag.py`), and the public service facade (`resource_registry_service.py`). - **Layer separation**: The method returns domain objects (`Resource`) via `db_resource_to_domain()`, not raw DB models. Correct abstraction level for the application service layer. - **Fail-fast validation**: `show_resource(name_or_id)` is called before opening a session, raising `NotFoundError` immediately for invalid inputs. This follows the project's fail-fast argument validation pattern per CONTRIBUTING.md. - **Symmetric design**: The implementation mirrors `get_children()` exactly — same session management pattern, same deterministic sorting convention, same guard for missing rows. This is the correct approach for a symmetric DAG operation. ### Module Boundaries ✅ - **No new imports**: The method uses only types and functions already imported in the file (`ResourceLinkModel`, `ResourceModel`, `db_resource_to_domain`, `Resource`). - **No new cross-module dependencies**: Zero architectural boundary violations introduced. - **File size**: 293 lines — well under the 500-line CONTRIBUTING limit. - **Internal module convention**: File is `_`-prefixed, correctly indicating it's internal. External code accesses `get_parents()` through `ResourceRegistryService` via the mixin inheritance chain. - **Mixin exposure verified**: `ResourceRegistryService(ResourceInstanceMixin, ResourceDagMixin)` inherits from `ResourceDagMixin`, so `get_parents()` is automatically exposed without additional wiring. Confirmed by reading `resource_registry_service.py`. ### Interface Contracts ✅ - **Signature matches spec**: `get_parents(name_or_id: str) -> list[Resource]` satisfies the contract defined in issue #2844's acceptance criteria. - **`RegistryHost` protocol compliance**: The method uses `self.show_resource()` and `self._session()`, both already declared in the `RegistryHost` protocol. No protocol update needed. - **Error contract**: Raises `NotFoundError` for non-existent resources (via `show_resource()`). Documented in the docstring's `Raises` section. - **Return contract**: Returns `list[Resource]` with deterministic ordering (by name, then resource_id). Returns empty list for root nodes with no parents. ### Code Correctness ✅ - **Query logic**: `filter_by(child_id=resource.resource_id)` correctly finds all parent links for the given resource. This is the symmetric inverse of `get_children()`'s `filter_by(parent_id=...)`. - **Type cast**: `str(link.parent_id)` matches the pattern in `get_children()` (`str(link.child_id)`). - **Session cleanup**: `finally: session.close()` ensures no resource leaks. - **No `# type: ignore`** in new code ✅ ### Commit & PR Metadata ✅ - **Commit message**: `fix(resource-registry): implement get_parents() on ResourceDagMixin` — Conventional Changelog format ✅ - **Commit footer**: `ISSUES CLOSED: #2844` ✅ - **PR body**: `Closes #2844` ✅ - **Single atomic commit**: One commit, one file changed ✅ - **Branch name**: `fix/v3.7.0-resource-dag-mixin-get-parents` matches issue metadata ✅ - **Label**: `Type/Bug` ✅ - **Milestone**: v3.7.0 (matches issue) ✅ ### Test Quality ✅ - The BDD scenario "Get parents returns all direct parents" was pre-written by the UAT tester (TDD approach) — confirmed by identical SHA (`a1e50453`) on both master and branch. - The scenario exercises the multi-parent case (C1 linked to both P1 and P2) with a count assertion. - All 15 existing DAG scenarios continue to pass — no regression. - The PR reports `nox -s typecheck` and `nox -s lint` pass cleanly. --- ### Observations (Non-blocking) 1. **Pre-existing `# type: ignore[override]` on `link_child` (~line 42) and `unlink_child` (~line 113)**: These violate CONTRIBUTING.md's "no type: ignore" rule but are **not introduced by this PR**. The new `get_parents()` method correctly avoids this pattern. Consider filing a separate issue to address the pre-existing suppressions. 2. **N+1 query pattern**: The method queries each parent `ResourceModel` individually in a loop, rather than using a single `IN` query. This is inherited from the `get_children()` pattern and is **not introduced by this PR**. For DAGs with many parents this could be a performance concern, but it's out of scope for this bug fix. 3. **Silent skip on orphaned links**: The `if parent_row is not None` guard silently skips links where the parent resource no longer exists (data integrity issue). This matches `get_children()`'s behavior and is a pre-existing design decision, not introduced here. 4. **Docstring improvement**: `get_parents()` includes a `Raises` section that `get_children()` lacks. This is actually an improvement over the existing pattern — consider backfilling the `Raises` section on `get_children()` in a future cleanup. ### Decision No blocking issues found. The implementation is correct, well-structured, follows existing patterns precisely, and satisfies all acceptance criteria from issue #2844. The code aligns with the specification's DAG traversal requirements and respects all module boundaries and interface contracts. **Decision: APPROVED** ✅ --- **Automated by CleverAgents Bot** Supervisor: PR Review | Agent: ca-pr-self-reviewer

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 21:00:52 +00:00

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

freemo merged commit cab263394c into master 2026-04-05 21:11:48 +00:00

freemo referenced this pull request 2026-04-05 22:16:28 +00:00

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

freemo removed this from the v3.7.0 milestone 2026-04-07 00:12:28 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

freemo

Labels

Clear labels

auto/needs-reevaluation

Controller deferred this PR; awaiting Phase 6+ scope-evaluator or operator re-enablement.

controller-managed

Auto-agents controller manages this PR/issue (see tools/controller/deploy/RUNBOOK.md). Remove this label to abandon controller management.

overdue

auto/blocked-by-deps

PR blocked by an open issue dependency. Operator must close the dep (or remove the dependency link) before the merge driver can act. Auto-cleared by merge_drive when no open deps remain.

auto/ci-timeout

Most recent merge cycle hit CI timeout. Driver excludes this PR while last merge_cycle row is < 30 min old; label persists thereafter as visible history.

auto/claimed-implementer

Currently being processed by an implementer worker.

auto/claimed-merge

Currently being processed by the merge driver.

auto/claimed-reviewer

Currently being processed by a reviewer worker.

auto/driver-down

Merge driver heartbeat stale; pipeline halted. Closed automatically on next clean tick.

auto/invariant-violation

Detected master commit violating the strict merge invariant. Tracked as an issue (not a PR label); kept here for label completeness.

auto/last-attempt-tier-0

In-cycle escalation: most recent attempt ran at the Tier 0 slot (`tier-0`). Slot's model defined in .opencode/models/tiers.yaml.

auto/last-attempt-tier-1

In-cycle escalation: most recent attempt ran at the Tier 1 slot (`tier-1`). Slot's model defined in .opencode/models/tiers.yaml.

auto/last-attempt-tier-2

In-cycle escalation: most recent attempt ran at the Tier 2 slot (`tier-2`). Slot's model defined in .opencode/models/tiers.yaml. Gated behind IMPLEMENTER_ESCALATION_TIER2_ENABLED.

auto/last-attempt-tier-min

In-cycle escalation: most recent attempt ran at the Tier -1 slot (`tier-min`). Slot's model defined in .opencode/models/tiers.yaml. Suffix is ``-min`` (not ``--1``) so the Forgejo UI reads naturally.

Automation Tracking

Tracking issues used by the AI Automation system for agents to communicate and report.

auto/needs-conflict-resolution

Rebase conflict needs LLM conflict-resolver.

auto/needs-implementer

Failing CI needs implementer attention.

auto/postmortem

Documenting a driver incident or rollback.

auto/ready-to-merge

Reviewer has APPROVED this PR and no later REQUEST_CHANGES is outstanding. The merge driver requires this label to even consider a PR for merging. Set by the reviewer worker on APPROVE; cleared on REQUEST_CHANGES.

auto/restart-throttled

Train repeatedly lost master-tempo races. Driver excludes via merge_cycle until cooldown elapses; label persists as visible history.

auto/revert

Revert PR backing out an invariant violation. Fast-tracked through the merge driver.

auto/sentinel

Sentinel PR duplicated from upstream into a personal fork by tools/duplicate_prs_to_fork.py for pipeline testing. Lives only in the fork; the canonical pipeline never sees it.

auto/stale-inactivity

No implementer activity for N days. Flagged for human review. Auto-cleared on next push to head branch.

auto/unstable

Repeatedly fails on current master (>= 3 ci-fail-on-rebased-sha releases in 12 h). Excluded from driver until human triage.

Blocked

A ticket in a blocked state and unable to complete until some other task is completed first.

Bounty

$100

A bounty of $100 for any open-source contributor who provides a MR that solves this issue

Bounty

$1000

A bounty of $1000 for any open-source contributor who provides a MR that solves this issue

Bounty

$10000

A bounty of $10000 for any open-source contributor who provides a MR that solves this issue

Bounty

$20

A bounty of $20 for any open-source contributor who provides a MR that solves this issue

Bounty

$2000

A bounty of $2000 for any open-source contributor who provides a MR that solves this issue

Bounty

$250

A bounty of $250 for any open-source contributor who provides a MR that solves this issue

Bounty

$50

A bounty of $50 for any open-source contributor who provides a MR that solves this issue

Bounty

$500

A bounty of $500 for any open-source contributor who provides a MR that solves this issue

Bounty

$5000

A bounty of $5000 for any open-source contributor who provides a MR that solves this issue

Bounty

$750

A bounty of $750 for any open-source contributor who provides a MR that solves this issue

MoSCoW

Could have

Could have feature in order to satisfy the epic/legendary.

MoSCoW

Must have

Must have feature in order to satisfy the epic/legendary.

MoSCoW

Should have

Should have feature in order to satisfy the epic/legendary.

Needs Feedback

There are questions in the ticket that can not be completed until the project owner provides clarity.

Points

1

1 man-hours worth of work for an expert with no learning curve.

Points

13

13 man-hours worth of work for an expert with no learning curve.

Points

2

2 man-hours worth of work for an expert with no learning curve.

Points

21

21 man-hours worth of work for an expert with no learning curve.

Points

3

3 man-hours worth of work for an expert with no learning curve.

Points

34

34 man-hours worth of work for an expert with no learning curve.

Points

5

5 man-hours worth of work for an expert with no learning curve.

Points

55

55 man-hours worth of work for an expert with no learning curve.

Points

8

8 man-hours worth of work for an expert with no learning curve.

Points

88

88 man-hours worth of work for an expert with no learning curve.

Priority

Backlog

This ticket has backlogged priority and is not to be worked on yet

Priority

CI Blocker

Critical priority issue that blocks CI/CD pipeline and prevents PR merges

Priority

Critical

The priority is critical

Priority

High

The priority is high

Priority

Low

The priority is low

Priority

Medium

The priority is medium

Signed-off: Owner

When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.

Signed-off: Scrum Master

When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.

Signed-off: Tech Lead

When an epic or legendary is in review it must be signed off by owner, tech lead, and scrum master before being marked as completed.

Spike

A ticket for learning a tool or technology that is needed to be able to do future planning and design.

State

Completed

The ticket has been fully implemented, completed, and merged with the source code. This label should only be applied once a ticket is closed.

State

Duplicate

A ticket that represents the same content as an existing ticket.

State

In Progress

A ticket that is actively being developed.

State

In Review

A ticket that has had some code completed to implement but is waiting to pass peer review and is not yet merged in.

State

Paused

This ticket's work started but wasn't finished. It's on hold (likely in a feature branch) and will be resumed later, either due to a blocker or a delay.

State

Unverified

All new tickets start in this state. A developer may set it to show the ticket is unverified. This means we haven't agreed to work on it. It will either move to a verified state or be closed as wontdo.

State

Verified

The issue has been verified by a developer as legitimate. It will be worked on and verified tickets are now considered part of the backlog.

State

Wont Do

This ticket has been decided it wont be done. This may mean the bug has been determined to not be real (cant verify) or the feature is one we have decided we dont want to adopt.

Type

Automation

Any edits or discussion about the AI automated coding system.

Type

Bug

Something that doesnt work as intended.

Type

Discussion

Anytime a ticket represents a discussion about a subject and doesnt fall into one of the other categories.

Type

Documentation

An error or improvement needed in the documentation.

Type

Epic

Any first tier epic. That is, an epic which contains only issues as children and will not have sub-epics.

Type

Feature

Some new functionality not present.

Type

Legendary

A type of Epic which will contain other Epics.

Type

Refactor

A code change that restructures existing code without changing its external behavior.

Type

Support

Someone needs help using the project.

Type

Task

A generic task that doesnt fit into the other type categories.

Type

Testing

Work exclusively focusing on fixing or expanding testing.

No Label

Milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

aditya (Aditya Chhabra) aleenaumair (Aleena Umair) brent.edwards (Brent Edwards) CoreRasurae (Luis Mendes) drew (Drew Morris) eugen.thaci (Eugen Thaci) freemo (Jeffrey Phillips Freeman) HAL9000 (HAL 9000) HAL9001 (HAL9001) hamza.khyari (Hamza Khyari) hurui200320 (Rui Hu) justin.morris khird (Kyle Hird) org.cleveragents

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: cleveragents/cleveragents-core#3252