feat: migrate core/behaviors/ Ports (1/5) — trivial base-only reparent - #2309
Conversation
Re-implementation of #1883 on current main (closed PR #2125 which had drifted 280 commits and was cheaper to redo than rebase). Migrates 41 Type-A BT nodes from DataLayerCondition/DataLayerAction to DataLayerConditionWithPorts/DataLayerActionWithPorts across case/, note/, and status/ domains. Type-A nodes are those whose only blackboard access is inherited from the base class — no domain-specific register_key() calls — so the change is a pure base-class swap with no method-body edits. Also removes two trivial no-op setup() overrides from ownership_transfer.py (AC-2 equivalent) and migrates CheckCaseStatusIdempotencyNode (SilentIdempotencyGuardMixin + base) since the mixin is base-agnostic. Adds 25 typed-ports isolation tests (AC-4) across three new test files: test/core/behaviors/{case,note,status}/nodes/test_typed_ports.py Two nodes left on legacy bases per agent assessment (Type-B, not trivial): - EmitOfferCaseParticipantToOwnerNode — uses self.blackboard in setup() - EmitCloseCaseNode — uses self.blackboard in setup() and update() Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
sei-ahouseholder
left a comment
There was a problem hiding this comment.
PR Triage — #2309
Overall: PENDING-CI (CI still running) · 1 FAIL · 2 IMPROVE · 0 NEW-ISSUE
FAIL
[phase3-pr-body-closes-top-0] Closes #1883 buried inside ## Summary body — not at top before any ## header. Per pr-body-guide.md closing references must be the first lines of the PR body. GitHub sidebar expansion breaks without this.
IMPROVE
[phase3-pr-body-missing-verification-0] PR body uses non-standard ## Test plan with unchecked boxes instead of the required ## Verification section. Needs: actual total/new test counts, ticked AC-1–AC-4 checkboxes from #1883, and a ## Changes section with file-level descriptions.
[phase8-dead-code-guard-emit-0] RecordRecommendationRecommenderNode.update() (suggest_actor/emit.py ~line 86) has a dead guard:
if self.datalayer is None:
return Status.SUCCESSUnder DataLayerActionWithPorts, initialise() calls get_input("datalayer") which raises NoDataAvailable before update() runs — so self.datalayer is never None in update(). Guard is unreachable dead code and should be removed (BTND-03-011: missing required port → NoDataAvailable is the correct contract).
Passing phases
- ✅ Issue #1883 linked and scope matches (Type-A base-only reparent for case/status/note)
- ✅ ADR-0044 implemented; no contradiction
- ✅ All retained legacy nodes (
CheckIsCaseManagerNode,BroadcastCaseUpdateNode,EmitOfferCaseParticipantToOwnerNode,EmitCloseCaseNode, etc.) are legitimately Type-B — confirmed domainregister_key()calls - ✅ No migrated node has old-style
self.datalayer = self.blackboard.datalayerin body - ✅ 25 typed-ports tests: constructor signatures correct,
setup_ports()/get_input("datalayer")/NoDataAvailablepattern correct per BTND-03-011 - ✅
notes/py-trees-ports-adoption.mdupdated with current migration state; frontmatter valid - ✅ CI pending (lint passed, tests running)
- phase3-pr-body-closes-top-0 — moved Closes #1883 to top of PR body (body-only fix, no code change) - phase3-pr-body-missing-verification-0 — rewrote PR body with Changes + Verification sections and AC checkboxes (body-only fix) - phase8-dead-code-guard-emit-0 — replaced dead `if self.datalayer is None: return SUCCESS` guard in RecordRecommendationRecommenderNode.update() with standard _require_datalayer() + assert pattern per BTND-03-011 Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
The import expansion (DataLayerAction + DataLayerActionWithPorts) pushed emit.py to 501 lines, 1 over the BTND-07-004 500-line limit. Replace the 3-line _require_datalayer() + assert pattern in RecordRecommendationRecommenderNode.update() with a single assert — the guard is unreachable under DataLayerActionWithPorts since NoDataAvailable fires in initialise() before update() runs. File now at 499 lines. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
sei-ahouseholder
left a comment
There was a problem hiding this comment.
PR Execute — #2309
All 3 triage findings resolved + 1 bonus CI fix. 6943 tests pass.
Fixed
[phase3-pr-body-closes-top-0] (FAIL) — PR body rewritten via gh pr edit: - Closes #1883 now at the top as a standalone bullet before any ## header. ✅
[phase3-pr-body-missing-verification-0] (IMPROVE) — Added ## Changes (per-file descriptions) and ## Verification (ticked AC-1–AC-4, total/new test counts). ✅
[phase8-dead-code-guard-emit-0] (IMPROVE) — Replaced dead if self.datalayer is None: return Status.SUCCESS guard with assert self.datalayer is not None in RecordRecommendationRecommenderNode.update(). Under DataLayerActionWithPorts the guard was unreachable; the assert satisfies the type checker without adding unreachable control flow. ✅ (commit 4c297ade)
[BTND-07-004 compliance — bonus CI fix] The import expansion pushed emit.py to 501 lines (1 over the 500-line limit). Using assert instead of the full _require_datalayer() + assert pattern keeps the file at 499 lines. ✅ (commit 2cf9c681)
Test results
6943 passed, 13 skipped, 5 xfailed — all pre-existing xfails reference open issues
No new failures. Pre-existing CI invariant failures (test_fv_invariants) are on main.
sei-ahouseholder
left a comment
There was a problem hiding this comment.
PR Verify — #2309 — PENDING-CI
All findings confirmed at HEAD. CI still running (pyright, black, mypy, pytest, demo integrations pending).
Finding verdicts
| ID | Severity | Outcome | Verdict |
|---|---|---|---|
| phase3-pr-body-closes-top-0 | FAIL | fixed | ✅ CONFIRMED — body starts with - Closes #1883 |
| phase3-pr-body-missing-verification-0 | IMPROVE | fixed | ✅ CONFIRMED — ## Changes + ## Verification with ticked AC checkboxes present |
| phase8-dead-code-guard-emit-0 | IMPROVE | fixed | ✅ CONFIRMED — assert self.datalayer is not None at emit.py:86, dead guard gone (4c297ade) |
| phase-ci-btnd07-line-count-0 | FAIL (bonus) | fixed | ✅ CONFIRMED — emit.py at 499 lines (2cf9c681) |
Note: execute artifact has 4 findings vs 3 in triage — the 4th (phase-ci-btnd07-line-count-0) was a BTND-07-004 compliance issue discovered during CI remediation, not a missed triage finding. All 3 triage findings are covered.
Overall: PENDING-CI — all findings confirmed; waiting on CI (pyright, black, mypy, pytest, demo integrations). Re-run /pr-verify after CI completes to get final verdict and clean up artifacts.
sei-ahouseholder
left a comment
There was a problem hiding this comment.
PR Verify — #2309 — ✅ READY-TO-MERGE
All findings confirmed at HEAD. CI fully green. PR is mergeable.
Finding verdicts
| ID | Severity | Outcome | Verdict |
|---|---|---|---|
| phase3-pr-body-closes-top-0 | FAIL | fixed | ✅ CONFIRMED — - Closes #1883 is first line of PR body |
| phase3-pr-body-missing-verification-0 | IMPROVE | fixed | ✅ CONFIRMED — ## Changes + ## Verification with ticked AC-1–AC-4 present |
| phase8-dead-code-guard-emit-0 | IMPROVE | fixed | ✅ CONFIRMED — assert self.datalayer is not None at emit.py:86 (4c297ade) |
| phase-ci-btnd07-line-count-0 | FAIL (CI bonus) | fixed | ✅ CONFIRMED — emit.py at 499 lines (2cf9c681) |
Note: triage artifact missing (expected — cleaned up after prior PENDING-CI pass). Execute artifact was recreated from session history for this final verify pass.
Merge state
mergeable: MERGEABLE
merge_state_status: CLEAN
base: main
No conflict markers in tree.
CI
19/19 checks SUCCESS — all linters, pytest, all 4 demo integrations + invariant harnesses.
Overall: READY-TO-MERGE
Summary
Re-implementation of #1883 on current main. The original PR #2125 had drifted 280 commits behind (status nodes were restructured, announce.py grew significantly) and was cheaper to reimplement from scratch than rebase. Migrates 41 Type-A BT nodes to typed Ports across `case/`, `note/`, and `status/` domains.
Changes
Two nodes intentionally left on legacy bases (Type-B, scope of later parts):
Part 1/5 of the #1809 full-migration chain.
Verification
🤖 Generated with Claude Code