Skip to content

[FEAT] Add domain layout support to addroute - #83

Merged
bnbong merged 2 commits into
bnbong:mainfrom
vastimofeev:codex/addroute-domain-layout
Oct 7, 2026
Merged

bnbong merged 2 commits into
bnbong:mainfrom
vastimofeev:codex/addroute-domain-layout

Conversation

@vastimofeev

@vastimofeev vastimofeev commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

fastkit addroute always generated api/routes, crud, and schemas modules, including in domain-starter projects. This adds --layout=classic-layer|domain; without an override, domain-starter selects domain generation and other presets retain classic generation. When preset metadata is absent, the recorded template determines the fallback, covering startdemo and interactive init projects.

The domain layout generates models, schemas, repository, service, and router files with names derived from the route (for example, HealthChecksService). It provides a typed CRUD example backed by process-local memory, with reset support and package re-exports. The starter test fixture resets generated domains between tests. Both layouts set the domain prefix and tags when registering the router in the shared API router; the shipped items domain follows the same convention.

Existing files and router registrations are preserved on repeat runs, including keyword-form include_router calls. Custom registration options are retained with an info message. Invalid aggregator syntax falls back to text insertion. Import aliases allow same-named modules in different layouts to coexist. HTTP path conflicts and migration of existing modules remain the application developer's responsibility. English CLI and domain documentation is updated.

Validation

  • Black and isort checks pass for the repository.
  • Full-source mypy passes with the Linux target used by CI (--platform linux).
  • 227 relevant tests pass: route generators, wiring, backend, CLI configuration, preset layout, and scaffolding. Three unrelated path tests were excluded.
  • Added executable lines have 100% local coverage (170/170).
  • Runtime tests cover CRUD, validation, 404 responses, independent repositories, concurrent IDs, retry behavior, aliases, the shipped items endpoints and OpenAPI tags, and generated-project test isolation across successive tests.

The initial PR revision passed the complete upstream test workflow on Ubuntu with Python 3.12, 3.13, and 3.14. CI for the review fixes is pending. The complete local Windows suite is not confirmed green; the baseline contains existing Windows/environment failures.

@github-actions github-actions Bot added the template Add or editing a FastAPI template label Oct 6, 2026
@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.57143% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/fastapi_fastkit/backend/main.py 68.75% 5 Missing ⚠️
src/fastapi_fastkit/backend/route_wiring.py 98.59% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@bnbong

bnbong commented Oct 7, 2026

Copy link
Copy Markdown
Owner

Thanks for this. The direction is right and the code is clean. I pulled the branch, ran the full suite and the runtime tests with FastAPI installed, and generated a project end-to-end. All green locally.

Status: one blocker, one design question, then merge.

🔴 Blocker: startdemo projects still get the classic layout

resolve_route_layout only reads [tool.fastapi-fastkit].preset.
That key is written only by fastkit init --config.

The two common paths write template = "fastapi-domain-starter" and no preset:

  • fastkit startdemo fastapi-domain-starter
  • interactive fastkit init

On those projects:

$ fastkit addroute users
Layout: classic-layer        # creates src/app/api/routes, crud, schemas

That is the bug this PR targets, and the updated tutorial promises the opposite.

Fix: when preset is missing, fall back to template.
preset_layout._PRESET_PROFILES already maps preset → base_template; reverse-lookup it.
Please add a test that scaffolds with ProjectScaffolder and no preset_id.

❓ Design question: generated repo vs. the starter's db/memory.py

shipped items domain generated domain
storage db.memory.InMemoryStore own dict + Lock
reset() yes (used by autouse fixture in conftest.py) no
__init__.py re-exports submodules empty

So in a domain-starter project the new domain is not reset between tests and does not match what the tutorial teaches.

Pick one:

  1. Use InMemoryStore when db/memory.py exists; keep the self-contained store as the fallback.
  2. Keep the self-contained store, but add reset() and the __init__.py re-exports.

🟡 Smaller fixes

  • Existing registration with a different prefix is silently kept. Right call, but print an info line so a re-run does not end in a bare "success".
  • include_router(router=...) (keyword-only) is not detected. It gets inserted twice.
  • ast.parse in route_wiring.py is unguarded. A syntax error in the aggregator writes the domain files first, then raises. Fall back to text insertion like insert_import_line does.
  • Codecov patch: 90.4% vs. 95.4% target. Uncovered: alias-collision and empty-aggregator branches.
  • Optional: route_generators.py uses private helpers from backend.main and avoids the cycle with a lazy import. Mergeable as is; moving classic generation into the generator module would remove the cycle.

✅ Not on you

  • The red inspect-changed-templates check is our workflow. Inspection passed; only the PR-comment step got a 403 from the fork's read-only token.
  • Moving prefix/tags on the items router is fine. It goes into the release notes.
  • Non-English docs are regenerated with make translate, so editing docs/en only is correct.

Next: land the template fallback and tell me which repository option you prefer. Then I merge.

@vastimofeev

Copy link
Copy Markdown
Contributor Author

Addressed in d371fee.

The template fallback now uses the registered domain-starter base template when preset is absent. An explicit --layout or recorded preset still takes priority. The regression test scaffolds through ProjectScaffolder without preset_id and verifies that addroute generates a domain.

I chose option 2: keep the self-contained repository, add reset() and package re-exports. The shared default repository has a service.reset_store() hook, and the starter's autouse fixture calls it for generated domains. A generated-project pytest run verifies that two successive tests each start empty with IDs beginning at 1.

Also fixed keyword-form router registration, added an info message when preserving custom registration options, and added the syntax-error fallback and coverage for alias collisions and empty aggregators.

Validation: 227 relevant tests passed locally, with three unrelated path tests excluded; Black/isort and full-source mypy targeting Linux pass. Added executable lines are locally covered at 100% (170/170). The new GitHub CI/Codecov results are pending. The optional classic-generator refactor and the fork workflow issue were left unchanged.

@bnbong

bnbong commented Oct 7, 2026

Copy link
Copy Markdown
Owner

Thanks for the quick follow-up. I re-checked d371fee: full suite, lint/mypy, and a fresh startdemo project end-to-end. Everything from the first round is addressed and works as described.

The red inspect-changed-templates job is still just our PR-comment step failing on the fork token; the inspection itself passed. I'll handle that on my side.

Approving and merging. Thank you very much for the contribution. It will ship in the next FastAPI-fastkit release, with credit to you in the changelog.

@bnbong
bnbong merged commit 09d590c into bnbong:main Oct 7, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

template Add or editing a FastAPI template

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants