Dev dependency wiring (1.2.3) - #17
Conversation
Add an ExecPlan that wires the runtime extras (core, media, editorial, models) and dev-only tools (syrupy, cmd-mox) required by the BeatCue technical design, alongside the Hecate infrastructure prefix updates needed to track the actual import names. Marker-gate the broken-on-3.14 packages (librosa, OpenTimelineIO, torch and the rest of the models set) so uv lock stays solvable on the project's Python floor, and repair the inbound_adapter allowed list so the next milestone can import Cyclopts and Rich. The plan also adds a fixture-based Hecate test for the cmdmox -> cmd_mox prefix rename. Status: DRAFT, awaiting user approval before implementation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
OverviewThis PR implements roadmap item 1.2.3 by wiring runtime dependency extras ( Key ChangesDependency Configuration (pyproject.toml)
Architecture and Testing
Documentation
Repository Maintenance
Implementation Decisions
ValidationAll initially failed checks have been resolved:
WalkthroughRoadmap task 1.2.3 is completed by wiring optional dependency extras ( ChangesDependency extras, Hecate policy, and architecture enforcement
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Declare the runtime dependency extras from the technical design and add the missing development tooling for snapshot and command-runner tests. Keep Python 3.14-incompatible optional stacks marker-gated so `uv lock` and the normal development sync remain usable today. Update Hecate infrastructure prefixes to use importable module names, including `cmd_mox`, and add fixture coverage so third-party prefix mistakes are caught. Record the dependency policy in ADR 009 and mark roadmap item 1.2.3 complete.
Use Pillow's import root, `PIL`, in the infrastructure prefix list so future `from PIL import Image` imports are classified as infrastructure. Add a Hecate fixture for that standard import form and record the correction in the completed dependency execplan.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Record that `docs/users-guide.md` intentionally remains unchanged because this milestone did not alter a user-facing install command or workflow. Capture the per-extra dry-run evidence and link the Python 3.14 marker-removal risk to GitHub issue #18 so roadmap 3.1.3 has a visible prerequisite before it depends on librosa feature loading.
|
@coderabbitai review |
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
✅ Action performedReview finished.
|
Document the temporary Python 3.14 marker strategy, add user and contributor install guidance for runtime extras, and keep the ExecPlan aligned with the implemented architecture policy. Add pyproject configuration tests so optional dependency groups, dev review tooling, and Hecate import-root policy are checked directly.
Exclude the local .memdb runtime directory so Memtrace daemon metadata, absolute paths, and PID files do not appear as repository work.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Replace substring checks for forbidden fixture imports with syrupy snapshots. Capture the full ARCH001 output so diagnostic wording and formatting changes are reviewed explicitly.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/execplans/1-2-3-wire-required-development-dependencies.md (2)
410-415:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRecord the users' guide update here.
The decision log still says to leave
docs/users-guide.mdunchanged, but this milestone now ships install guidance there. Update the note so the retrospective matches the delivered documentation set.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/execplans/1-2-3-wire-required-development-dependencies.md` around lines 410 - 415, The decision log entry for 2026-06-16 states that docs/users-guide.md should remain unchanged, but this milestone actually shipped install guidance in that file. Update the decision log entry to reflect the actual work delivered by removing or revising the rationale that claims the file was left unchanged, and instead document that the users' guide was updated with the new install guidance as part of this milestone.
787-793:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMake the smoke check required.
This block marks
tests/test_dependency_wiring.pyas optional, but the acceptance criteria later treat it as part of the delivered milestone. Promote it to a required Stage B task so the plan stays self-consistent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/execplans/1-2-3-wire-required-development-dependencies.md` around lines 787 - 793, The smoke check test defined in tests/test_dependency_wiring.py is currently marked as optional in the Stage B task list, but the acceptance criteria later reference it as a required deliverable. Promote this smoke check from optional status to a required Stage B task by moving it out of the optional section and into the main required tasks list, ensuring the plan is self-consistent regarding what must be delivered in this milestone.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/users-guide.md`:
- Line 40: The heading "Optional Dependency Groups" uses title case but should
follow the documentation style guide which requires sentence case for headings.
Change the section heading from "## Optional Dependency Groups" to "## Optional
dependency groups" so only the first letter is capitalized, matching the
required documentation style.
---
Outside diff comments:
In `@docs/execplans/1-2-3-wire-required-development-dependencies.md`:
- Around line 410-415: The decision log entry for 2026-06-16 states that
docs/users-guide.md should remain unchanged, but this milestone actually shipped
install guidance in that file. Update the decision log entry to reflect the
actual work delivered by removing or revising the rationale that claims the file
was left unchanged, and instead document that the users' guide was updated with
the new install guidance as part of this milestone.
- Around line 787-793: The smoke check test defined in
tests/test_dependency_wiring.py is currently marked as optional in the Stage B
task list, but the acceptance criteria later reference it as a required
deliverable. Promote this smoke check from optional status to a required Stage B
task by moving it out of the optional section and into the main required tasks
list, ensuring the plan is self-consistent regarding what must be delivered in
this milestone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2fdc5b64-f878-496a-988a-3506dcb6353c
📒 Files selected for processing (9)
.gitignoredocs/adr-009-dependency-extras-and-python-314-markers.mddocs/beatcue-technical-design.mddocs/developers-guide.mddocs/execplans/1-2-3-wire-required-development-dependencies.mddocs/users-guide.mdtests/__snapshots__/test_architecture_checker.ambrtests/test_architecture_checker.pytests/test_dependency_wiring.py
Resolve review feedback on the dependency wiring plan by matching heading case to the documentation style guide, recording the users' guide install guidance, and making the dependency smoke check a required Stage B task.
|
@coderabbitai Have the following now been resolved? docs/execplans/1-2-3-wire-required-development-dependencies.md (2)
🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In
🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== Lines 405-420 ==="
sed -n '405,420p' "docs/execplans/1-2-3-wire-required-development-dependencies.md"
echo ""
echo "=== Lines 782-800 ==="
sed -n '782,800p' "docs/execplans/1-2-3-wire-required-development-dependencies.md"Length of output: 2299
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_dependency_wiring.py`:
- Around line 88-112: The assertions in this test block do not include failure
messages, which makes it harder to diagnose CI failures. Add descriptive failure
messages to each assert statement throughout the test (lines 88-112 and also
lines 120-121, 130-134) using the syntax assert condition, "message" format.
Each message should clearly describe what dependency group or list is being
verified so that when an assertion fails, the error message immediately
indicates which optional_dependencies key or assertion condition failed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2509a6c3-75ea-44dd-a489-fd5b101540f8
📒 Files selected for processing (9)
.gitignoredocs/adr-009-dependency-extras-and-python-314-markers.mddocs/beatcue-technical-design.mddocs/developers-guide.mddocs/execplans/1-2-3-wire-required-development-dependencies.mddocs/users-guide.mdtests/__snapshots__/test_architecture_checker.ambrtests/test_architecture_checker.pytests/test_dependency_wiring.py
Add explicit failure messages to dependency wiring tests so optional dependency, dev tooling, and Hecate import-root failures identify the checked list immediately.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_dependency_wiring.py`:
- Around line 136-150: The test assertions in this block attempt to dereference
nested fields within groups_by_name without first verifying that the required
parent keys exist, which could result in KeyError exceptions masking the
intended test assertions. Add assert statements before the existing assertion
block to verify that both "inbound_adapter" and "infrastructure" keys are
present in the groups_by_name dictionary, then proceed with the existing
assertions that check the nested fields like "allowed" and "prefixes" within
those groups.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fb4c97d7-2581-444e-a6bc-7e1560572e7a
📒 Files selected for processing (1)
tests/test_dependency_wiring.py
Assert the required Hecate groups exist before checking their nested allowed and prefix lists so missing groups fail with clear assertion messages instead of KeyError.
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (2 errors, 3 warnings)
|
This comment was marked as resolved.
This comment was marked as resolved.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Declare the FileNotFoundError and TOMLDecodeError failure modes for the dependency wiring test helper while leaving its logic unchanged.
Format the dependency wiring helper Raises section with the dashed underline expected by the repository Ruff docstring rules.
Summary
This branch carries the pre-implementation ExecPlan for roadmap item
1.2.3 — wiring the runtime extras (
core,media,editorial,models) and development tools (syrupy,cmd-mox) that theBeatCue technical design
§§14, 16, and 17 require. The plan also folds in the Hecate
infrastructure-prefix updates needed to track the real import names of
each third-party package, marker-gates the broken-on-3.14 dependencies
(
librosa,OpenTimelineIO, and the wholemodelsset) souv lockstays solvable on the project's Python floor, and repairs the
inbound_adapterHecate group so the next milestone can importCyclopts and Rich.
No production code is changed in this PR. The next step is reviewer
sign-off on the plan; implementation will follow under a separate PR
once approved.
Roadmap task: (1.2.3) — see
docs/roadmap.md §1.2.3.
Execplan:
docs/execplans/1-2-3-wire-required-development-dependencies.md.
Status: DRAFT, awaiting approval before implementation.
Review walkthrough
the Purpose and Constraints sections
to confirm the intended outcome and the boundaries the implementer
must not cross.
Risks
and the
Decision log
— these capture the three non-obvious calls: marker-gating
Python 3.14-unbuildable extras rather than declaring them
unconditionally; repairing
inbound_adapter.allowedto permitinfrastructure imports; and proving the
cmdmox→cmd_moxHecaterename with a fixture-based architecture test rather than a runtime
import.
Stage B of the Concrete steps
to review the exact
pyproject.tomledits the implementer will apply.Interfaces and dependencies table
for the version pins, distribution-to-import mappings, and per-group
placement.
Validation
This is a documentation-only commit; the relevant gate is the
repository's Markdown lint pass.
Once approved, the implementation PR will run the full
make check-fmt,make lint,make typecheck, andmake testgateset per the plan's
Validation and acceptancesection.Notes
The plan went through a logisphere-experts community review before
submission. The crew flagged three blockers —
uv lockbehaviour onPython 3.14, the pre-existing
inbound_adapterHecate gap, and themisleading scope of the originally proposed
cmd_moxregression test— and all three are folded into the current draft. The Decision log
also records a proposed ADR 009 covering the headless-OpenCV pairing
and the declared-but-not-installable extras policy; that ADR will be
drafted only if reviewers want the policy captured before the next
milestone.
References