refactor: decompose append.py and effects.py by semantic concern (BTND-07-004) - #2282
Open
sei-ahouseholder wants to merge 2 commits into
Open
refactor: decompose append.py and effects.py by semantic concern (BTND-07-004)#2282sei-ahouseholder wants to merge 2 commits into
sei-ahouseholder wants to merge 2 commits into
Conversation
…D-07-004) Resolves CONCERN-2269. Both modules were at the 500-line ceiling, causing any unrelated edit to require an in-band decomposition. ## status/nodes: append.py → append/ subpackage - Delete append.py (499 lines, 7 classes) - Create append/__init__.py — re-exports all 7 public names - Create append/conditions.py — 4 guard nodes + _has_status_in_participant helper (SkipIfIdempotentNode, CheckStatusNotAlreadyAppendedNode, ValidateRMTransitionNode, CheckParticipantRMNotClosedNode) - Create append/actions.py — 3 mutating action nodes (LoadParticipantNode, ResolveAndPersistStatusObjectNode, AppendStatusAndSaveParticipantNode) - Mirror test structure: test_append.py → append/conftest.py + test_conditions.py + test_actions.py; add TestCheckParticipantRMNotClosedNode (was missing from the original test file) ## sync/nodes: effects.py → per-class files + _helpers.py - Delete effects.py (495 lines, 4 classes + helper) - Create _helpers.py — _extract_id_from_field + _LedgerEffectNode base class (DRY: all 4 effect nodes had identical setup() + _require_log_entry pattern) - Create participant_status_effect.py, note_effect.py, invite_accept_effect.py, close_case_effect.py — one file per class - Update offer_report_effect.py, ownership_effects.py — import _extract_id_from_field from _helpers instead of deleted effects.py - Update sync/nodes/__init__.py — 4 separate per-class imports - Mirror test structure: test_effects.py → test_participant_status_effect.py + test_note_effect.py + test_invite_accept_effect.py + test_close_case_effect.py; adds tests for the 3 previously untested effect nodes - Update notes/structured-logging.md — reference updated to close_case_effect.py Note: 5 other modules are approaching the 500-line ceiling (replay.py 498, suggest_actor/emit.py 498, deploy_fix.py 497, embargo/nodes/lifecycle.py 494, conditions.py 488) — tracked as future work. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…position eliminates BTND-07-004 churn Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
sei-ahouseholder
left a comment
Contributor
Author
There was a problem hiding this comment.
PR Triage: #2282 — refactor: decompose append.py and effects.py by semantic concern (BTND-07-004)
Linked issues: #2269 (BTND-07-004's 500-line ceiling is forcing unrelated decomposition churn — append.py and effects.py both at the limit)
Changed files: 22 files — core/behaviors/status, core/behaviors/sync
CI status:
Merge state: ❌ CONFLICTING (DIRTY)
Base branch: main
Needs integration tests: yes
Findings
| # | Phase | Severity | Description | Outcome |
|---|---|---|---|---|
| phase12-merge-conflict-0 | merge-state | ❌ FAIL | Branch has conflicts with main — cannot merge until resolved |
fix-now |
| phase3-closes-not-at-top-0 | pr-body-format | Closes #2269 is at the bottom of the PR body; must be the first line before any ## header (pr-body-guide.md) |
fix-now | |
| phase9-stale-docstring-ref-0 | notes-docs-currency | leave.py:22 docstring still references deleted sync.nodes.effects.ApplyCloseCaseFromLedgerNode; correct path is sync.nodes.close_case_effect.ApplyCloseCaseFromLedgerNode |
fix-now |
Total: 1 FAIL · 2 IMPROVE · 0 NEW-ISSUE
Notes
- BTND-07 structure test: 93/93 passed. All new modules are ≤ 329 lines. ✓
__init__.pyre-exports all 7 public names fromappend/subpackage. ✓sync/nodes/__init__.pyre-exports all effect node names from per-class files. ✓- No backward-compatibility shims (per doctrine). ✓
notes/structured-logging.mdupdated to referenceclose_case_effect.py. ✓- No ADR warranted — pure structural decomposition, no architectural change. ✓
- Demo CI failures (
engage_caseevent type missing in fvcv-handoff, fcvcv, fcv-reject, fv) are pre-existing on all recentmainruns and are unrelated to this PR.
Triage artifact: .claude/pr-2282-triage.json
Next step: /pr-execute will resolve the merge conflict, move the Closes line, and fix the stale docstring.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
append/subpackage:conditions.py(4 guard nodes +_has_status_in_participanthelper),actions.py(3 mutating nodes),__init__.py(re-exports all 7 public names)participant_status_effect.py,note_effect.py,invite_accept_effect.py,close_case_effect.py) +_helpers.py(_LedgerEffectNodebase +_extract_id_from_field) extracting shared DRY patternTestCheckParticipantRMNotClosedNode(previously untested) and tests for the 3 previously untestedeffects.pynodesWhat changed
status/nodes/append.py(499 lines)status/nodes/append/conditions.py+actions.py+__init__.pysync/nodes/effects.py(495 lines)_helpers.py+ 4 per-class filestest_append.pyappend/conftest.py+test_conditions.py+test_actions.pytest_effects.pyDRY extractions
_LedgerEffectNodebase class extracted from the 4effects.pynodes — all had identicalsetup()(registeractivityblackboard key) and_require_log_entrycall pattern_has_status_in_participant()helper inappend/conditions.py_extract_id_from_field()moved fromeffects.pyto_helpers.pyOut of scope — 5 other near-limit modules
replay.py(498),suggest_actor/emit.py(498),deploy_fix.py(497),embargo/nodes/lifecycle.py(494),conditions.py(488).Test plan
uv run pytest test/core/behaviors/status/nodes/append/— 18 passeduv run pytest test/core/behaviors/sync/nodes/test_*_effect.py— 13 passeduv run pytest test/core/ test/wire/ test/adapters/— all passeduv run flake8— cleanuv run pyrighton all new/modified files — 0 errorsCloses #2269
🤖 Generated with Claude Code