Skip to content

refactor(presets): split domain internals into private modules - #4747

Merged
mnriem merged 2 commits into
github:mainfrom
mnriem:mnriem-preset-split-review
Sep 25, 2026
Merged

mnriem merged 2 commits into
github:mainfrom
mnriem:mnriem-preset-split-review

Conversation

@mnriem

@mnriem mnriem commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator

Description

Split the preset domain's large __init__.py into private manifest, registry,
catalog, resolver, manager, command-artifact, and skill-artifact modules. Keep
the established package exports intact, so existing callers retain their import
paths. This makes preset internals easier to locate and change without altering
the CLI surface.

Move the existing domain tests into corresponding test_<module>.py suites at
the parent preset package level, reusing shared setup helpers and leaving
command-focused suites in place. Keep cross-domain and bundled-content tests
in tests/test_presets.py. Document the one-to-one source/test layout in
design/cli.md.

The migration preserves all 8,566 collected cases, including all 708 preset
cases; every original parametrized test identity is represented after relocation.
The 463 test functions moved from tests/test_presets.py have unchanged bodies.
This is a structural change, not a change to preset behavior.

Testing

  • Tested locally with specify --help from the worktree's virtual environment.
  • Ran the full suite from isolated worktree environments on Python 3.13 and
    Python 3.14: 8,549 passed, 17 skipped on each.
  • Tested with a sample project (not applicable to this structural refactor).

Also passed uvx ruff@0.15.0 check src tests, markdownlint, ShellCheck and
the Bash portability guard, the pinned dependency audit, the extension version
guard, pre-commit hooks, and local CodeQL Python and Actions analysis (zero
findings in both).

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (details below)

AI disclosure: Posted on behalf of @mnriem by GitHub Copilot (models:
GPT-6 Sol and GPT-6 Astra; default reasoning settings, autonomous). GitHub
Copilot generated the preset-domain refactor, test-file reorganization, related
documentation, validations, and this PR description. GPT-6 Sol authored the
initial domain change; GPT-6 Astra transferred and committed it and reorganized
the tests; GPT-6 Sol prepared this PR. The human contributor requested the
work and test alignment; this is not a claim of line-by-line human review.

mnriem and others added 2 commits September 25, 2026 08:39
Code authored by the parent GitHub Copilot agent using GPT-6 Sol; exact transfer and commit performed by GitHub Copilot using GPT-6 Astra.

Assisted-by: GitHub Copilot (model: GPT-6 Sol, autonomous)
Assisted-by: GitHub Copilot (model: GPT-6 Astra, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Move existing preset domain tests into suites matching their implementation modules, share artifact setup helpers, and retain cross-domain contracts. Preserve all 8566 collected tests, including all 708 preset cases, without changing production behavior.

Assisted-by: GitHub Copilot (model: GPT-6 Astra, autonomous)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings September 25, 2026 14:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🔵 Needs a closer look

The large structural relocation preserves inspected definitions and coverage, but its scale warrants final human verification.

Review effort: Balanced
Findings: None

What changed in this PR

Refactors preset internals into focused private modules while preserving package-level imports and aligning tests with source boundaries.

Changes:

  • Split manifest, registry, catalog, resolver, manager, command, and skill logic.
  • Reorganized preset tests and shared helpers.
  • Documented the mirrored source/test layout.
File(s) Description
src/​specify_cli/​presets/​__init__.py Preserves existing exports.
src/​specify_cli/​presets/​_manifest.py, _registry.py, _catalog.py, _resolver.py Houses extracted domain logic.
src/​specify_cli/​presets/​_manager.py, _manager_commands.py, _manager_skills.py Separates lifecycle and artifact management.
tests/​test_presets.py Retains cross-domain coverage.
tests/​specify_cli/​presets/​test_*.py Mirrors private domain modules and verifies exports.
tests/​specify_cli/​presets/​_helpers.py, conftest.py Shares relocated test setup.
tests/​test_extension_skills.py, tests/​integration/​test_preset_update_workflow.py Updates relocated test references.
design/​cli.md Documents the source/test convention.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@mnriem
mnriem merged commit ab80b14 into github:main Sep 25, 2026
15 checks passed
@mnriem
mnriem deleted the mnriem-preset-split-review branch September 25, 2026 14:22
Ashfaqbs added a commit to Ashfaqbs/spec-kit that referenced this pull request Sep 25, 2026
Port the script continuation dispatcher onto the private presets modules
introduced by the domain split (github#4747), and move its tests into the
mirrored tests/specify_cli/presets layout.
chelsealong added a commit to chelsealong/spec-kit that referenced this pull request Sep 25, 2026
…-removal

Resolves conflicts from upstream's preset domain split (PR github#4747): the
ExtensionManager/PresetManager.remove() security fix from b6dc0a6 is
ported from the old monolithic presets/__init__.py into the new
PresetManager.remove() in presets/_manager.py, and its regression
tests are ported into tests/specify_cli/presets/test_manager.py.

Assisted-by: Claude Code (model: Claude Sonnet 5, autonomous)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants