[claude-tmgbedu]

Follow-up to #323 (merged). Fixes the findings from the review and renames the command to match the db:* convention. ADR 041 and its index row are updated in place.

Bugs

• Import dedupe by name only. An existing import is now compared by its full path, or by object identity when both paths are already loaded (so re-exports still count as the same class). If the name is bound to something else, for example the base fastapi_startkit.masoniteorm.config.database.DatabaseConfig, a relative import, or an alias, the command stops. It prints DatabaseConfig is already imported as <path>, not config.database.DatabaseConfig. plus the manual steps, and exits 1 without touching the file. Aliasing was considered and rejected in the ADR.
• Attribute references. orm.DatabaseProvider and fastapi_startkit.masoniteorm.DatabaseProvider now count as already registered.
• Positional providers. Application(base, env, [...]) now has the entry inserted into that list. A positional or keyword value that is not a list literal, or a call with *args, stops with a clear reason.

register_provider now raises RegistrationError(reason) instead of returning None, and the command prints that reason before the manual steps.

Usability

• After a successful run, next steps are printed. They include uv add "fastapi-startkit[database,<driver>]" when find_spec("sqlalchemy") finds nothing, the DB_* .env entries taken from the installer's define_database section (the same data as init --database), mkdir -p databases/migrations when the directory is missing (db:make:migration does not create it), and the db:make:migration / db:migrate commands.

Rename

• database:install is now db:install. The class is DbInstallCommand in console/db_install_command.py, and the tests, AppProvider registration, ADR 041 title/text and docs/adr/AGENTS.md row are updated to match. No other docs or installer code referenced the old name.

Tests

Each fix has a regression test. I confirmed that bugs 3–5 reproduce with the register_provider on main.

• uv run pytest --ignore=tests/masoniteorm/postgres --cov: 3464 passed, 10 skipped, total coverage 92.09%. install_command.py and db_install_command.py are at 100%.
• ruff check and ruff format --check pass. basedpyright on the changed sources and tests: 0 errors, no new warnings.

🤖 Generated with Claude Code

[claude-tmgbedu]

Verdict: approve (no blockers, posted as COMMENT because the author account is the reviewer account)

Follow-up to #323. I read the full diff, checked each item from the #323 review, ran the bug scenarios against main's register_provider, and ran the command on scratch apps.

Blockers

None.

Item-by-item

#	Item	Status	Evidence
3	Import dedupe by path / identity	Fixed	On main, from fastapi_startkit.masoniteorm.config import CacheConfig, from other import Something as CacheProvider and from .cache import CacheConfig are all kept, and the provider is registered against the wrong class. On this branch each raises RegistrationError, and db:install exits 1 with the file byte-identical (test_does_not_register_when_database_config_is_imported_from_elsewhere, test_conflicting_import_prints_manual_steps_and_leaves_bootstrap).
4	Attribute refs count as registered	Fixed	On main, [(cache.CacheProvider, X)] and [fastapi_startkit.cache.CacheProvider] get a second entry. Now unchanged.
5	Positional providers / *args / RegistrationError	Fixed	On main, Application('.', None, [A]) became ..., [A], providers=[...] (TypeError at boot). It is now edited in place. A non-list positional value and *ARGS stop with a reason. The only caller of register_provider is InstallCommand.register_provider, which catches RegistrationError. The installer (installer/) does not call it.
U1	uv add hint	Fixed	Real run on a fresh fastapi-startkit init --database none app with no SQLAlchemy printed uv add "fastapi-startkit[database,postgres]". The extras database/sqlite/postgres/mysql all exist in pyproject.toml.
U2	Next steps	Accurate	I followed them on a scratch copy of example/fastapi-app (sqlite): mkdir -p databases/migrations, the DB_* lines into .env, db:make:migration create_users_table, then db:migrate, which migrated OK. The uv run python artisan ... form matches init's own output.
6	Rename	Complete	git grep finds no database:install / DatabaseInstallCommand / database_install_command references. The ADR 041 title and text and the index row are updated.

Non-blocking notes

1. **kwargs is not treated like *args (inline). Application(**KW) gets providers=[...] appended. That is valid syntax, but it raises TypeError at boot if KW already has providers.
2. A bootstrap with a syntax error gives a raw traceback (inline, carried over from #323). ast.parse raises SyntaxError, which is not converted into a RegistrationError with manual steps.
3. Only top-level import statements are checked for conflicts (inline, carried over from #323). A class DatabaseConfig: or DatabaseConfig = ... at module level, or an import inside if TYPE_CHECKING: or try:, is not seen. In the class case the new import is inserted and then silently shadowed by the class.
4. is_registered matches anywhere inside the list (inline). [(A, settings.DatabaseProvider)] (config position) counts as registered. This is contrived and only mentioned for completeness.
5. Application imported under an alias (from fastapi_startkit import Application as App) stops with "Could not find an Application(...) call.". That is safe, but a hint about the alias would help.
6. same_object only resolves modules that are already in sys.modules. In a real artisan run the bootstrap is imported first, so a submodule import such as from fastapi_startkit.masoniteorm.<submodule> import DatabaseProvider resolves correctly. Calling the pure function standalone would report a false conflict. Behaviour is fine; just be aware.
7. Nit: test_db_install_command.py has a stray blank line between two fastapi_startkit imports (lines 8–10).

Break-it attempts (all produce valid, compiling output or a clean stop)

• These were all correct: a multi-line list with a trailing # comment and no comma, a commented-out item after the last one, ] on the last item's line, CRLF line endings, non-ASCII text before the insertion point, a multi-line parenthesised import as the last import, an import with a trailing # noqa, semicolon imports, from __future__ only, [*BASE, A], Application('.', None) (gets providers=), and a second Application(...) inside a function (the module-level call is edited).
• A clean stop for providers=P (variable), Application(*ARGS) and the aliased Application.

Real runs (scratch copies, run twice)

• example/fastapi-app: the first run created the config, registered the provider and exited 0. The second run said "already exists, skipped" and "already registered", exited 0, and both files kept identical md5s.
• application/ does not exist in this repo, so I used a fresh fastapi-startkit init starter --database none app (bootstrap with config=AppConfig), with --driver postgres and SQLAlchemy missing. The first run printed the uv add hint. The second run failed at artisan import with ModuleNotFoundError: sqlalchemy, which is exactly what the hint warns about. After installing the extra deps, the rerun was idempotent (md5 unchanged).

Results

• uv run pytest --ignore=tests/masoniteorm/postgres --cov --cov-report=term-missing: 3465 passed, 10 skipped. Total coverage 92.10%. install_command.py and db_install_command.py are at 100%.
• ruff check passes and ruff format --check is clean on the changed files. basedpyright on console/ and the tests: 0 errors. The 4 reportMissingTypeArgument warnings are in files this PR does not touch.

[claude-tmgbedu]

Non-blocking: **kwargs isn't covered. Application(**KW) falls through to providers is None and gets providers=[...] appended. That raises TypeError: got multiple values for keyword argument 'providers' at boot when KW contains it. To be consistent with *args, consider also stopping on any(keyword.arg is None for keyword in call.keywords) when there is no explicit providers=.

[claude-tmgbedu]

Non-blocking (carried over from #323): a bootstrap with a syntax error raises SyntaxError here and the user gets a traceback instead of the reason and manual steps. Wrapping it as RegistrationError(f"{BOOTSTRAP_PATH} has a syntax error on line {error.lineno}.") would fit the new error path.

[claude-tmgbedu]

Non-blocking: only top-level import nodes are inspected. A module-level class DatabaseConfig: / DatabaseConfig = ..., or an import under if TYPE_CHECKING: / try:, is not seen. Reproduced: with class DatabaseConfig: pass in the bootstrap, the new from config.database import DatabaseConfig is inserted above it and then silently shadowed. This is rare in a bootstrap file, so it's fine to leave, but it is a gap in the "name bound to something else" check.

[claude-tmgbedu]

Nit: ast.walk matches the name anywhere in the list, including the config position of a tuple: [(A, settings.DatabaseProvider)] counts as registered. Checking only each element (or elt.elts[0] for tuples) would be stricter. This is contrived, so it's optional.

[claude-tmgbedu]

Nit: stray blank line splitting the fastapi_startkit import group.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!