Skip to content

Dev dependency wiring (1.2.3) - #17

Merged
leynos merged 12 commits into
mainfrom
1-2-3-wire-required-development-dependencies
Jun 21, 2026
Merged

Dev dependency wiring (1.2.3)#17
leynos merged 12 commits into
mainfrom
1-2-3-wire-required-development-dependencies

Conversation

@lodyai

@lodyai lodyai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

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 the
BeatCue 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 whole models set) so uv lock
stays solvable on the project's Python floor, and repairs the
inbound_adapter Hecate group so the next milestone can import
Cyclopts 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

  • Start with
    the Purpose and Constraints sections
    to confirm the intended outcome and the boundaries the implementer
    must not cross.
  • Then read
    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.allowed to permit
    infrastructure imports; and proving the cmdmoxcmd_mox Hecate
    rename with a fixture-based architecture test rather than a runtime
    import.
  • Land on
    Stage B of the Concrete steps
    to review the exact pyproject.toml edits the implementer will apply.
  • Cross-check the
    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.

$ make markdownlint
markdownlint-cli2 '**/*.md'
markdownlint-cli2 v0.22.1 (markdownlint v0.40.0)
Linting: 35 file(s)
Summary: 0 error(s)

Once approved, the implementation PR will run the full
make check-fmt, make lint, make typecheck, and make test gate
set per the plan's Validation and acceptance section.

Notes

The plan went through a logisphere-experts community review before
submission. The crew flagged three blockers — uv lock behaviour on
Python 3.14, the pre-existing inbound_adapter Hecate gap, and the
misleading scope of the originally proposed cmd_mox regression 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

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>

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @LodyAI[bot], you have reached your weekly rate limit of 2500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 9670808d-d33a-4ebb-a533-8661602344c0

📥 Commits

Reviewing files that changed from the base of the PR and between eae8b4b and aa22dd4.

📒 Files selected for processing (1)
  • tests/test_dependency_wiring.py

Overview

This PR implements roadmap item 1.2.3 by wiring runtime dependency extras (core, media, editorial, models) and development tools (syrupy, cmd-mox) as required by the BeatCue technical design (sections 14, 16, and 17). The implementation includes configuration updates, architectural tests, and comprehensive documentation.

Key Changes

Dependency Configuration (pyproject.toml)

  • Added project.optional-dependencies with four runtime extras:
    • core: Cyclopts, Rich, Cuprum, msgspec
    • media: Headless OpenCV + PySceneDetect, plus librosa (gated to python_full_version < '3.14')
    • editorial: OpenTimelineIO (gated to python_full_version < '3.14')
    • models: Transformers, Torch, Accelerate, Timm, Einops, Pillow, SentencePiece, Qwen-VL-Utils (all gated to python_full_version < '3.14')
  • Extended dependency-groups.dev with syrupy and cmd-mox
  • Updated [tool.hecate] infrastructure group prefixes to use importable module names (e.g., PIL instead of pillow, cmd_mox instead of cmdmox)
  • Expanded inbound_adapter group's allowed list to include infrastructure

Architecture and Testing

  • Created tests/test_dependency_wiring.py with three new test functions that verify:
    • Optional dependency groups match the design specification
    • Dev tooling (cmd-mox, syrupy) is properly declared and importable
    • Hecate policy uses correct external import names rather than distribution names
  • Migrated architecture boundary tests to use syrupy snapshot assertions (tests/__snapshots__/test_architecture_checker.ambr) instead of brittle substring matching
  • Added test fixtures for domain-to-external library violations (domain_imports_cmd_mox, domain_imports_pil)
  • Enhanced HecatePolicy in tests/conftest.py with include_external_packages field

Documentation

  • ADR 009: New architectural decision record (docs/adr-009-dependency-extras-and-python-314-markers.md) documenting the Python 3.14 marker strategy and marking it as accepted (2026-06-14)
  • ExecPlan 1.2.3: New comprehensive plan document (docs/execplans/1-2-3-wire-required-development-dependencies.md, 965 lines) detailing constraints, escalation triggers, risks, and a stage-gated procedure with explicit progress/decision log and outcomes narrative
  • Technical Design: Updated docs/beatcue-technical-design.md with ADR 009 reference and Python 3.14 marker gating approach
  • Users' Guide: Added "Optional dependency groups" section with uv sync --extra <group> installation instructions for each capability group
  • Developers' Guide: Extended with "Dependency extras" section explaining per-extra installation via uv sync --group dev --extra <name> and clarifying Hecate tracks importable module names, not PyPI distribution names
  • Roadmap: Marked 1.2.3 as completed with implementation notes referencing ADR 009

Repository Maintenance

  • Updated .gitignore to exclude .memdb/ directory (local MemDB state)

Implementation Decisions

  1. Marker-gating for Python 3.14 compatibility: librosa, OpenTimelineIO, and model stack packages are declared with python_full_version < '3.14' markers to maintain uv lock solvability on supported Python versions (issue #18 tracks removal)
  2. Infrastructure-prefix correctness: Hecate mappings now reflect actual importable module names (e.g., cmd_mox, PIL) rather than distribution names
  3. Inbound adapter repair: Updated allowed imports to permit Cyclopts and Rich, unblocking subsequent milestones

Validation

All initially failed checks have been resolved:

  • Configuration tests added verifying optional groups and markers
  • User-facing documentation updated with installation commands
  • Developer documentation enriched with per-group installation guidance
  • Hecate snapshot tests implemented, replacing substring assertions
  • Defensive assertions added to guard group dictionary access
  • _pyproject() helper fallibility documented in docstring Raises: section
  • ADR 009 and ExecPlan 1.2.3 created to formalise decisions and planning

Walkthrough

Roadmap task 1.2.3 is completed by wiring optional dependency extras (core, media, editorial, models) and dev tooling (syrupy, cmd-mox) in pyproject.toml with Python 3.14 PEP 508 marker-gating, correcting Hecate infrastructure prefixes to use importable module names, adding architecture-violation test fixtures and dependency-wiring tests, and recording the decisions in ADR 009 and ExecPlan 1.2.3.

Changes

Dependency extras, Hecate policy, and architecture enforcement

Layer / File(s) Summary
ADR 009 and pyproject.toml optional-dependencies
docs/adr-009-dependency-extras-and-python-314-markers.md, pyproject.toml
ADR 009 defines the optional-extra layout, Python 3.14 marker policy, headless media-stack distribution strategy, and the import-name rule for architecture validation. pyproject.toml implements [project.optional-dependencies] (core, media, editorial, models) with PEP 508 markers gating librosa, OpenTimelineIO, and the model stack to python_full_version < '3.14', and extends [dependency-groups].dev with syrupy and cmd-mox.
Hecate infrastructure prefixes and inbound_adapter allowed list
pyproject.toml, tests/conftest.py
pyproject.toml expands the infrastructure group's prefixes to include correct underscore import names (cmd_mox, qwen_vl_utils) and additional third-party module prefixes, and adds infrastructure to inbound_adapter.allowed. HecatePolicy in conftest.py gains an include_external_packages: bool field.
Architecture-violation test fixtures for cmd_mox and PIL
tests/fixtures/architecture/domain_imports_cmd_mox/*, tests/fixtures/architecture/domain_imports_pil/*
Two new fixture packages: domain_imports_cmd_mox (imports cmd_mox to trigger ARCH001) and domain_imports_pil (imports PIL.Image with marker-gated lint and type-checking suppressions to trigger ARCH001).
Architecture checker and dependency wiring tests
tests/test_architecture_checker.py, tests/test_dependency_wiring.py, tests/__snapshots__/test_architecture_checker.ambr
_fixture_policy() in test_architecture_checker.py emits include_external_packages in the generated [tool.hecate] TOML and parametrises two new ARCH001 violation expectations for cmd_mox and PIL fixtures, refactoring assertions to use syrupy snapshots. test_dependency_wiring.py adds parametrised tests: test_dev_dependency_import_names_are_resolvable validates installed dev distributions, test_runtime_optional_dependency_groups_match_design validates optional-dependency structure and markers, test_dev_dependency_group_includes_review_tooling checks dev group content, test_hecate_policy_models_external_import_names asserts infrastructure prefixes use import names.
ADR 009 and technical design integration
docs/adr-009-dependency-extras-and-python-314-markers.md, docs/beatcue-technical-design.md, docs/developers-guide.md, docs/contents.md
ADR 009 is indexed in contents.md. beatcue-technical-design.md integrates ADR 009 into the dependency-groups section, explaining runtime vs tooling declaration locations and temporary Python 3.14 marker-gating. developers-guide.md adds the "Dependency extras" section documenting install commands and import-name mapping, and expands the Hecate forbidden-imports list to include msgspec, opentimelineio, and torch.
Roadmap completion and user documentation
docs/roadmap.md, docs/contents.md, docs/users-guide.md
roadmap.md marks task 1.2.3 complete with implementation notes linking to ADR 009 and Hecate import-name tracking. contents.md adds a new "Plans" item for the ExecPlan. users-guide.md adds the "Optional Dependency Groups" section explaining installation commands and temporary Python 3.14 marker-gating behaviour.
ExecPlan 1.2.3: Wire required development dependencies
docs/execplans/1-2-3-wire-required-development-dependencies.md
New living document for roadmap item 1.2.3, spanning purpose (closing dependency-manifest gap), repository constraints, tolerance/exception triggers, execution progress and surprises, key policy decisions (headless OpenCV, marker-gating, infrastructure prefix alignment), baseline context and research, four-stage execution plan (A–D), concrete staged commands and pyproject.toml wiring details, Stage D acceptance criteria, dependency interface mapping table, and documentation/ADR impact instructions.
Gitignore housekeeping
.gitignore
Adds .memdb/ to the ignore list.

Possibly related issues

Possibly related PRs

  • leynos/beatcue#12: Both PRs modify the Hecate-based architecture enforcement wiring in [tool.hecate] and pyproject.toml, so the import-name and prefix updates build on the earlier Hecate infrastructure migration.
  • leynos/beatcue#13: Both PRs extend the HecatePolicy TypedDict in tests/conftest.py and build on the same architecture-test fixture infrastructure, so the include_external_packages addition and fixture-based violation expectations are directly related.

Poem

🎸 A lock file resolves, the markers hold tight,
cmd_mox and PIL caught red in the night.
With extras declared and the dev group aligned,
No wheel shall break what the markers have signed.
ADR 009 stands: import names, not dist — ✅

🚥 Pre-merge checks | ✅ 20
✅ Passed checks (20 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the core change as dependency wiring and correctly includes the roadmap reference (1.2.3) as required.
Description check ✅ Passed The description comprehensively covers the changeset, explaining the wiring of runtime extras and development tools, marker-gating strategy, Hecate updates, and references the ExecPlan and roadmap alignment.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Testing (Overall) ✅ Passed Tests are substantive and rigorous. test_dependency_wiring.py uses exact equality checks on pyproject.toml configuration (not vacuous subset/presence checks), test_dev_dependency_import_names_are_r...
User-Facing Documentation ✅ Passed docs/users-guide.md adds comprehensive "Optional dependency groups" section documenting all four runtime extras (core, media, editorial, models) with clear installation instructions, group descript...
Developer Documentation ✅ Passed Developer documentation comprehensive: ADR 009 (Accepted), ExecPlan 1.2.3 (COMPLETE), roadmap item 1.2.3 checked off, developers-guide.md enriched with "Dependency extras" section, technical design...
Module-Level Documentation ✅ Passed All new and modified Python modules in the pull request carry appropriate module-level docstrings that clearly explain purpose, utility, and relationships to other components per the check requirem...
Testing (Unit And Behavioural) ✅ Passed PR adds 4 unit tests validating configuration invariants (optional-dependencies, dev tooling, Hecate import-name tracking) and 2 behavioural end-to-end tests verifying Hecate CLI correctly detects...
Testing (Property / Proof) ✅ Passed Tests assert fixed configuration invariants (exact sets, lists, and membership). No unbounded ranges, multiple states, orderings, transitions, or lemmas requiring property-based or proof-based test...
Testing (Compile-Time / Ui) ✅ Passed PR implements snapshot testing with Syrupy for Hecate architecture-checker text-based output: 7 parametrised test cases with focused AMBR snapshots capturing specific boundary violations, stable de...
Unit Architecture ✅ Passed Pull request maintains clear separation between queries, commands, fallible operations, and side-effects. Query functions (_pyproject, hecate_policy) explicitly document fallibility; test functions...
Domain Architecture ✅ Passed This PR maintains proper domain architecture segregation. The domain group (beatcue.domain) remains isolated with only domain-to-domain imports allowed via Hecate policy. New infrastructure depende...
Observability ✅ Passed PR introduces no operational behaviour changes—only documentation, configuration declarations, and test infrastructure. No runtime code modifications detected in beatcue/ package.
Security And Privacy ✅ Passed PR introduces no secrets, authentication bypasses, injection risks, unsafe deserialization, over-broad permissions, or sensitive data exposure. Documentation carefully discusses privacy policy; dep...
Performance And Resource Use ✅ Passed All new code follows linear algorithmic complexity with bounded collections; file I/O occurs once per test rather than in loops; snapshot assertions avoid string matching overhead; no unbounded mem...
Concurrency And State ✅ Passed This PR introduces no concurrency, async, threading, locking, or problematic shared mutable state. It is purely documentation, configuration, and test infrastructure; no production code changes. Te...
Architectural Complexity And Maintainability ✅ Passed Complexity proportional to problem: snapshot testing (syrupy) reduces brittle assertions; test fixtures are minimal; test_dependency_wiring automates verification without speculative code. Minor co...
Rust Compiler Lint Integrity ✅ Passed This is a Python project with no Rust code. The custom check (Rust compiler lint integrity) does not apply to Python-only changes in documentation, tests, configuration, and .gitignore.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 1-2-3-wire-required-development-dependencies

Comment @coderabbitai help to get the list of available commands and usage tips.

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.
@leynos
leynos marked this pull request as ready for review June 14, 2026 04:48

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @leynos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

chatgpt-codex-connector[bot]

This comment was marked as resolved.

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.
@leynos

leynos commented Jun 16, 2026

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.
@leynos

leynos commented Jun 17, 2026

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot added the Roadmap label Jun 17, 2026
coderabbitai[bot]

This comment was marked as resolved.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot]

This comment was marked as resolved.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

leynos added 2 commits June 17, 2026 20:55
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.
@leynos

This comment was marked as resolved.

@coderabbitai

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.
@leynos

leynos commented Jun 18, 2026

Copy link
Copy Markdown
Owner

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews resumed.

@coderabbitai

This comment was marked as resolved.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 win

Record the users' guide update here.

The decision log still says to leave docs/users-guide.md unchanged, 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 win

Make the smoke check required.

This block marks tests/test_dependency_wiring.py as 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

📥 Commits

Reviewing files that changed from the base of the PR and between 30ab89c and 80a9bee.

📒 Files selected for processing (9)
  • .gitignore
  • docs/adr-009-dependency-extras-and-python-314-markers.md
  • docs/beatcue-technical-design.md
  • docs/developers-guide.md
  • docs/execplans/1-2-3-wire-required-development-dependencies.md
  • docs/users-guide.md
  • tests/__snapshots__/test_architecture_checker.ambr
  • tests/test_architecture_checker.py
  • tests/test_dependency_wiring.py

Comment thread docs/users-guide.md Outdated
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.
@leynos leynos changed the title Plan dev-dependency wiring (1.2.3) Dev dependency wiring (1.2.3) Jun 20, 2026
@leynos

leynos commented Jun 20, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Have the following now been resolved?

docs/execplans/1-2-3-wire-required-development-dependencies.md (2)

410-415: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
Record the users' guide update here.
The decision log still says to leave docs/users-guide.md unchanged, but this milestone now ships install guidance there. Update the note so the retrospective matches the delivered documentation set.

🤖 Detailed instructions

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 win
Make the smoke check required.
This block marks tests/test_dependency_wiring.py as 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.

🤖 Detailed instructions

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.

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

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



@leynos Both issues are resolved.

Finding Status Evidence
Lines 410–415: decision log omits users-guide update ✅ Resolved The 2026-06-16 decision log entry now reads "Update docs/users-guide.md with optional dependency install guidance", documenting the correct delivered state.
Lines 787–793: smoke check marked optional ✅ Resolved The smoke check block ("Add a runtime smoke check (tests/test_dependency_wiring.py)…") carries no "optional" qualifier; it is a plain required task in the prose at those lines.

@leynos

leynos commented Jun 20, 2026

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 30ab89c and eae8b4b.

📒 Files selected for processing (9)
  • .gitignore
  • docs/adr-009-dependency-extras-and-python-314-markers.md
  • docs/beatcue-technical-design.md
  • docs/developers-guide.md
  • docs/execplans/1-2-3-wire-required-development-dependencies.md
  • docs/users-guide.md
  • tests/__snapshots__/test_architecture_checker.ambr
  • tests/test_architecture_checker.py
  • tests/test_dependency_wiring.py

Comment thread tests/test_dependency_wiring.py Outdated
Add explicit failure messages to dependency wiring tests so optional dependency, dev tooling, and Hecate import-root failures identify the checked list immediately.
@leynos

leynos commented Jun 21, 2026

Copy link
Copy Markdown
Owner

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between eae8b4b and 8391b7c.

📒 Files selected for processing (1)
  • tests/test_dependency_wiring.py

Comment thread 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.
@leynos

leynos commented Jun 21, 2026

Copy link
Copy Markdown
Owner

@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)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error Test test_hecate_policy_models_external_import_names accesses dict keys without asserting group existence first, risking KeyError instead of clear assertion failures if groups missing. Add assertions for group presence: assert "inbound_adapter" in groups_by_name and assert "infrastructure" in groups_by_name before accessing their nested fields.
Unit Architecture ❌ Error PR violates Unit Architecture check: test_dependency_wiring.py fails to declare fallibility (tomllib parsing, file I/O can raise); test at line 136 accesses dictionary keys without prior existence... Apply suggested review patch adding key existence assertions before indexing. Make _pyproject() fallibility explicit in signature or catch exceptions at boundary. Separate import side-effects from query logic or document this is intentio...
Testing (Unit And Behavioural) ⚠️ Warning test_hecate_policy_models_external_import_names() accesses groups_by_name dict keys without asserting their presence first, allowing KeyError to mask assertion failures. Add assertions to guard against KeyError: assert 'inbound_adapter' in groups_by_name and assert 'infrastructure' in groups_by_name before dereferencing nested keys.
Testing (Compile-Time / Ui) ⚠️ Warning Snapshot tests properly capture architecture diagnostic output with meaningful assertions, but test_hecate_policy_models_external_import_names lacks group-existence assertions before dereferencing,... Add assertions "assert "inbound_adapter" in groups_by_name" and "assert "infrastructure" in groups_by_name" before dereferencing them in test_hecate_policy_models_external_import_names at lines 136–139.
Architectural Complexity And Maintainability ⚠️ Warning Test function test_hecate_policy_models_external_import_names lacks defensive assertions before indexing—KeyError could mask real configuration errors instead of surfacing AssertionError. Add assertions checking "inbound_adapter" and "infrastructure" keys exist in groups_by_name before indexing into them, as flagged in review comment.

@coderabbitai

This comment was marked as resolved.

@leynos

leynos commented Jun 21, 2026

Copy link
Copy Markdown
Owner

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews resumed.

leynos added 2 commits June 21, 2026 15:35
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant