fix(cli): repair the doctor connectivity check

[nadeem4]

The bug

nl2sql doctor — the command a user runs when something is already wrong — crashed instead of diagnosing:

$ nl2sql doctor
File "packages/nl2sql/src/nl2sql/cli/checks.py", line 20, in verify_connectivity
from nl2sql.datasources import load_profiles
ImportError: cannot import name 'load_profiles' from 'nl2sql.datasources'
Please report this bug to the nl2sql team.

verify_connectivity() carried two phantom imports against an API that does not exist:

• from nl2sql.datasources import load_profiles — nl2sql.datasources exports DatasourceRegistry, discover_adapters, the models and the adapter protocol. There is no load_profiles.
• from nl2sql.diagnostics import check_connectivity as core_check — there is no nl2sql.diagnostics module at all.

Both imports sat at lines 19-21, outside the try: that begins at line 23. That is why the ImportError escaped the function's own error handling and surfaced as an unhandled-exception traceback rather than a failed check.

nl2sql setup was affected too: the wizard's connectivity step (cli/commands/setup.py:405) calls the same function, so it died the same way — the second caller alongside cli/commands/doctor.py:48.

The fix

verify_connectivity() is rewritten against the real API, following the construction order already used by nl2sql/context.py and cli/demo/manager.py:

ConfigManager() → load_secrets() / SecretManager.configure() → load_datasources() → DatasourceRegistry(secret_manager) → register_datasource() per config → adapter.test_connection().

Datasources are registered one at a time rather than through register_datasources(), which raises on the first bad entry and would abort the whole check. Every datasource now gets its own row.

doctor must never crash, so each failure mode is reported instead of raised:

Failure	Behaviour
Config file missing or unparseable	Connectivity check failed: ... message, returns False
Adapter driver not installed (mssql without pyodbc)	Failed row with the driver error
Unknown engine type	Failed row
Unresolvable ${env:...} secret	Failed row
test_connection() returns False	Failed row
test_connection() raises	Failed row with the exception text
Zero datasources configured	No datasources configured. message, returns True

test_connection() on BaseSQLAlchemyAdapter returns False on failure, but the protocol only declares -> bool and a third-party adapter may raise, so both are handled.

The return contract is unchanged — True only when every datasource connected — because setup.py branches on it.

Imports moved to module level

The three imports (ConfigManager, DatasourceRegistry, SecretManager) are now at module level. There is no circular-import reason to keep them local: nl2sql.cli.checks is imported only from cli/commands/, and none of nl2sql.configs, nl2sql.datasources or nl2sql.secrets import anything under nl2sql.cli. A phantom import would now fail at import time, loudly, instead of at the moment a user needs the command.

Rich markup

Datasource ids and driver error text are wrapped in Text(...) before they reach the table, and the config-error message is printed as Text rather than a markup string — a driver error containing a bracket sequence such as [/{style}] must not raise MarkupError or be silently swallowed. This is the discipline test_cli_error_markup.py exists to protect.

Tests

New: packages/nl2sql/tests/cli/test_doctor_connectivity.py (10 tests). The absence of any coverage here is why this shipped.

• verify_connectivity returns True when all adapters connect, False when one fails, and does not raise in either case
• a missing config, and an unparseable config, are reported rather than raised
• an adapter raising inside test_connection() is caught and reported — with markup-shaped text in the exception message
• an adapter returning False is reported
• an unresolvable ${env:...} secret is reported
• zero datasources is reported
• nl2sql doctor exits 0 and prints the connectivity table — the CLI smoke test that would have caught the original bug. Against main it fails with assert 1 == 0 and the ImportError traceback in the captured output.

All 10 fail on main; all 10 pass here. SQLite and stubs only — no live database, no network, no API key.

Suites, run twice each (pytest-randomly active):

• pytest -m "not integration" — 241 passed, 1 skipped, 47 deselected (baseline 231 + 10 new)
• pytest -m "integration and not llm" — 28 passed, 261 deselected

[nadeem4]

The bug

nl2sql doctor — the command a user runs when something is already wrong — crashed instead of diagnosing:

$ nl2sql doctor
File "packages/nl2sql/src/nl2sql/cli/checks.py", line 20, in verify_connectivity
from nl2sql.datasources import load_profiles
ImportError: cannot import name 'load_profiles' from 'nl2sql.datasources'
Please report this bug to the nl2sql team.

verify_connectivity() carried two phantom imports against an API that does not exist:

• from nl2sql.datasources import load_profiles — nl2sql.datasources exports DatasourceRegistry, discover_adapters, the models and the adapter protocol. There is no load_profiles.
• from nl2sql.diagnostics import check_connectivity as core_check — there is no nl2sql.diagnostics module at all.

Both imports sat at lines 19-21, outside the try: that begins at line 23. That is why the ImportError escaped the function's own error handling and surfaced as an unhandled-exception traceback rather than a failed check.

nl2sql setup was affected too: the wizard's connectivity step (cli/commands/setup.py:405) calls the same function, so it died the same way — the second caller alongside cli/commands/doctor.py:48.

The fix

verify_connectivity() is rewritten against the real API, following the construction order already used by nl2sql/context.py and cli/demo/manager.py:

ConfigManager() → load_secrets() / SecretManager.configure() → load_datasources() → DatasourceRegistry(secret_manager) → register_datasource() per config → adapter.test_connection().

Datasources are registered one at a time rather than through register_datasources(), which raises on the first bad entry and would abort the whole check. Every datasource now gets its own row.

doctor must never crash, so each failure mode is reported instead of raised:

Failure	Behaviour
Config file missing or unparseable	Connectivity check failed: ... message, returns False
Adapter driver not installed (mssql without pyodbc)	Failed row with the driver error
Unknown engine type	Failed row
Unresolvable ${env:...} secret	Failed row
test_connection() returns False	Failed row
test_connection() raises	Failed row with the exception text
Zero datasources configured	No datasources configured. message, returns True

test_connection() on BaseSQLAlchemyAdapter returns False on failure, but the protocol only declares -> bool and a third-party adapter may raise, so both are handled.

The return contract is unchanged — True only when every datasource connected — because setup.py branches on it.

Imports moved to module level

The three imports (ConfigManager, DatasourceRegistry, SecretManager) are now at module level. There is no circular-import reason to keep them local: nl2sql.cli.checks is imported only from cli/commands/, and none of nl2sql.configs, nl2sql.datasources or nl2sql.secrets import anything under nl2sql.cli. A phantom import would now fail at import time, loudly, instead of at the moment a user needs the command.

Rich markup

Datasource ids and driver error text are wrapped in Text(...) before they reach the table, and the config-error message is printed as Text rather than a markup string — a driver error containing a bracket sequence such as [/{style}] must not raise MarkupError or be silently swallowed. This is the discipline test_cli_error_markup.py exists to protect.

Tests

New: packages/nl2sql/tests/cli/test_doctor_connectivity.py (10 tests). The absence of any coverage here is why this shipped.

• verify_connectivity returns True when all adapters connect, False when one fails, and does not raise in either case
• a missing config, and an unparseable config, are reported rather than raised
• an adapter raising inside test_connection() is caught and reported — with markup-shaped text in the exception message
• an adapter returning False is reported
• an unresolvable ${env:...} secret is reported
• zero datasources is reported
• nl2sql doctor exits 0 and prints the connectivity table — the CLI smoke test that would have caught the original bug. Against main it fails with assert 1 == 0 and the ImportError traceback in the captured output.

All 10 fail on main; all 10 pass here. SQLite and stubs only — no live database, no network, no API key.

Suites, run twice each (pytest-randomly active):

• pytest -m "not integration" — 241 passed, 1 skipped, 47 deselected (baseline 231 + 10 new)
• pytest -m "integration and not llm" — 28 passed, 261 deselected

[nadeem4]

The bug

nl2sql doctor — the command a user runs when something is already wrong — crashed instead of diagnosing:

$ nl2sql doctor
File "packages/nl2sql/src/nl2sql/cli/checks.py", line 20, in verify_connectivity
from nl2sql.datasources import load_profiles
ImportError: cannot import name 'load_profiles' from 'nl2sql.datasources'
Please report this bug to the nl2sql team.

verify_connectivity() carried two phantom imports against an API that does not exist:

• from nl2sql.datasources import load_profiles — nl2sql.datasources exports DatasourceRegistry, discover_adapters, the models and the adapter protocol. There is no load_profiles.
• from nl2sql.diagnostics import check_connectivity as core_check — there is no nl2sql.diagnostics module at all.

Both imports sat at lines 19-21, outside the try: that begins at line 23. That is why the ImportError escaped the function's own error handling and surfaced as an unhandled-exception traceback rather than a failed check.

nl2sql setup was affected too: the wizard's connectivity step (cli/commands/setup.py:405) calls the same function, so it died the same way — the second caller alongside cli/commands/doctor.py:48.

The fix

verify_connectivity() is rewritten against the real API, following the construction order already used by nl2sql/context.py and cli/demo/manager.py:

ConfigManager() → load_secrets() / SecretManager.configure() → load_datasources() → DatasourceRegistry(secret_manager) → register_datasource() per config → adapter.test_connection().

Datasources are registered one at a time rather than through register_datasources(), which raises on the first bad entry and would abort the whole check. Every datasource now gets its own row.

doctor must never crash, so each failure mode is reported instead of raised:

Failure	Behaviour
Config file missing or unparseable	Connectivity check failed: ... message, returns False
Adapter driver not installed (mssql without pyodbc)	Failed row with the driver error
Unknown engine type	Failed row
Unresolvable ${env:...} secret	Failed row
test_connection() returns False	Failed row
test_connection() raises	Failed row with the exception text
Zero datasources configured	No datasources configured. message, returns True

test_connection() on BaseSQLAlchemyAdapter returns False on failure, but the protocol only declares -> bool and a third-party adapter may raise, so both are handled.

The return contract is unchanged — True only when every datasource connected — because setup.py branches on it.

Imports moved to module level

The three imports (ConfigManager, DatasourceRegistry, SecretManager) are now at module level. There is no circular-import reason to keep them local: nl2sql.cli.checks is imported only from cli/commands/, and none of nl2sql.configs, nl2sql.datasources or nl2sql.secrets import anything under nl2sql.cli. A phantom import would now fail at import time, loudly, instead of at the moment a user needs the command.

Rich markup

Datasource ids and driver error text are wrapped in Text(...) before they reach the table, and the config-error message is printed as Text rather than a markup string — a driver error containing a bracket sequence such as [/{style}] must not raise MarkupError or be silently swallowed. This is the discipline test_cli_error_markup.py exists to protect.

Tests

New: packages/nl2sql/tests/cli/test_doctor_connectivity.py (10 tests). The absence of any coverage here is why this shipped.

• verify_connectivity returns True when all adapters connect, False when one fails, and does not raise in either case
• a missing config, and an unparseable config, are reported rather than raised
• an adapter raising inside test_connection() is caught and reported — with markup-shaped text in the exception message
• an adapter returning False is reported
• an unresolvable ${env:...} secret is reported
• zero datasources is reported
• nl2sql doctor exits 0 and prints the connectivity table — the CLI smoke test that would have caught the original bug. Against main it fails with assert 1 == 0 and the ImportError traceback in the captured output.

All 10 fail on main; all 10 pass here. SQLite and stubs only — no live database, no network, no API key.

Suites, run twice each (pytest-randomly active):

• pytest -m "not integration" — 241 passed, 1 skipped, 47 deselected (baseline 231 + 10 new)
• pytest -m "integration and not llm" — 28 passed, 261 deselected

[nadeem4]

The bug

nl2sql doctor — the command a user runs when something is already wrong — crashed instead of diagnosing:

$ nl2sql doctor
File "packages/nl2sql/src/nl2sql/cli/checks.py", line 20, in verify_connectivity
from nl2sql.datasources import load_profiles
ImportError: cannot import name 'load_profiles' from 'nl2sql.datasources'
Please report this bug to the nl2sql team.

verify_connectivity() carried two phantom imports against an API that does not exist:

• from nl2sql.datasources import load_profiles — nl2sql.datasources exports DatasourceRegistry, discover_adapters, the models and the adapter protocol. There is no load_profiles.
• from nl2sql.diagnostics import check_connectivity as core_check — there is no nl2sql.diagnostics module at all.

Both imports sat at lines 19-21, outside the try: that begins at line 23. That is why the ImportError escaped the function's own error handling and surfaced as an unhandled-exception traceback rather than a failed check.

nl2sql setup was affected too: the wizard's connectivity step (cli/commands/setup.py:405) calls the same function, so it died the same way — the second caller alongside cli/commands/doctor.py:48.

The fix

verify_connectivity() is rewritten against the real API, following the construction order already used by nl2sql/context.py and cli/demo/manager.py:

ConfigManager() → load_secrets() / SecretManager.configure() → load_datasources() → DatasourceRegistry(secret_manager) → register_datasource() per config → adapter.test_connection().

Datasources are registered one at a time rather than through register_datasources(), which raises on the first bad entry and would abort the whole check. Every datasource now gets its own row.

doctor must never crash, so each failure mode is reported instead of raised:

Failure	Behaviour
Config file missing or unparseable	Connectivity check failed: ... message, returns False
Adapter driver not installed (mssql without pyodbc)	Failed row with the driver error
Unknown engine type	Failed row
Unresolvable ${env:...} secret	Failed row
test_connection() returns False	Failed row
test_connection() raises	Failed row with the exception text
Zero datasources configured	No datasources configured. message, returns True

test_connection() on BaseSQLAlchemyAdapter returns False on failure, but the protocol only declares -> bool and a third-party adapter may raise, so both are handled.

The return contract is unchanged — True only when every datasource connected — because setup.py branches on it.

Imports moved to module level

The three imports (ConfigManager, DatasourceRegistry, SecretManager) are now at module level. There is no circular-import reason to keep them local: nl2sql.cli.checks is imported only from cli/commands/, and none of nl2sql.configs, nl2sql.datasources or nl2sql.secrets import anything under nl2sql.cli. A phantom import would now fail at import time, loudly, instead of at the moment a user needs the command.

Rich markup

Datasource ids and driver error text are wrapped in Text(...) before they reach the table, and the config-error message is printed as Text rather than a markup string — a driver error containing a bracket sequence such as [/{style}] must not raise MarkupError or be silently swallowed. This is the discipline test_cli_error_markup.py exists to protect.

Tests

New: packages/nl2sql/tests/cli/test_doctor_connectivity.py (10 tests). The absence of any coverage here is why this shipped.

• verify_connectivity returns True when all adapters connect, False when one fails, and does not raise in either case
• a missing config, and an unparseable config, are reported rather than raised
• an adapter raising inside test_connection() is caught and reported — with markup-shaped text in the exception message
• an adapter returning False is reported
• an unresolvable ${env:...} secret is reported
• zero datasources is reported
• nl2sql doctor exits 0 and prints the connectivity table — the CLI smoke test that would have caught the original bug. Against main it fails with assert 1 == 0 and the ImportError traceback in the captured output.

All 10 fail on main; all 10 pass here. SQLite and stubs only — no live database, no network, no API key.

Suites, run twice each (pytest-randomly active):

• pytest -m "not integration" — 241 passed, 1 skipped, 47 deselected (baseline 231 + 10 new)
• pytest -m "integration and not llm" — 28 passed, 261 deselected

[nadeem4]

The bug

nl2sql doctor — the command a user runs when something is already wrong — crashed instead of diagnosing:

$ nl2sql doctor
File "packages/nl2sql/src/nl2sql/cli/checks.py", line 20, in verify_connectivity
from nl2sql.datasources import load_profiles
ImportError: cannot import name 'load_profiles' from 'nl2sql.datasources'
Please report this bug to the nl2sql team.

verify_connectivity() carried two phantom imports against an API that does not exist:

• from nl2sql.datasources import load_profiles — nl2sql.datasources exports DatasourceRegistry, discover_adapters, the models and the adapter protocol. There is no load_profiles.
• from nl2sql.diagnostics import check_connectivity as core_check — there is no nl2sql.diagnostics module at all.

Both imports sat at lines 19-21, outside the try: that begins at line 23. That is why the ImportError escaped the function's own error handling and surfaced as an unhandled-exception traceback rather than a failed check.

nl2sql setup was affected too: the wizard's connectivity step (cli/commands/setup.py:405) calls the same function, so it died the same way — the second caller alongside cli/commands/doctor.py:48.

The fix

verify_connectivity() is rewritten against the real API, following the construction order already used by nl2sql/context.py and cli/demo/manager.py:

ConfigManager() → load_secrets() / SecretManager.configure() → load_datasources() → DatasourceRegistry(secret_manager) → register_datasource() per config → adapter.test_connection().

Datasources are registered one at a time rather than through register_datasources(), which raises on the first bad entry and would abort the whole check. Every datasource now gets its own row.

doctor must never crash, so each failure mode is reported instead of raised:

Failure	Behaviour
Config file missing or unparseable	Connectivity check failed: ... message, returns False
Adapter driver not installed (mssql without pyodbc)	Failed row with the driver error
Unknown engine type	Failed row
Unresolvable ${env:...} secret	Failed row
test_connection() returns False	Failed row
test_connection() raises	Failed row with the exception text
Zero datasources configured	No datasources configured. message, returns True

test_connection() on BaseSQLAlchemyAdapter returns False on failure, but the protocol only declares -> bool and a third-party adapter may raise, so both are handled.

The return contract is unchanged — True only when every datasource connected — because setup.py branches on it.

Imports moved to module level

The three imports (ConfigManager, DatasourceRegistry, SecretManager) are now at module level. There is no circular-import reason to keep them local: nl2sql.cli.checks is imported only from cli/commands/, and none of nl2sql.configs, nl2sql.datasources or nl2sql.secrets import anything under nl2sql.cli. A phantom import would now fail at import time, loudly, instead of at the moment a user needs the command.

Rich markup

Datasource ids and driver error text are wrapped in Text(...) before they reach the table, and the config-error message is printed as Text rather than a markup string — a driver error containing a bracket sequence such as [/{style}] must not raise MarkupError or be silently swallowed. This is the discipline test_cli_error_markup.py exists to protect.

Tests

New: packages/nl2sql/tests/cli/test_doctor_connectivity.py (10 tests). The absence of any coverage here is why this shipped.

• verify_connectivity returns True when all adapters connect, False when one fails, and does not raise in either case
• a missing config, and an unparseable config, are reported rather than raised
• an adapter raising inside test_connection() is caught and reported — with markup-shaped text in the exception message
• an adapter returning False is reported
• an unresolvable ${env:...} secret is reported
• zero datasources is reported
• nl2sql doctor exits 0 and prints the connectivity table — the CLI smoke test that would have caught the original bug. Against main it fails with assert 1 == 0 and the ImportError traceback in the captured output.

All 10 fail on main; all 10 pass here. SQLite and stubs only — no live database, no network, no API key.

Suites, run twice each (pytest-randomly active):

• pytest -m "not integration" — 241 passed, 1 skipped, 47 deselected (baseline 231 + 10 new)
• pytest -m "integration and not llm" — 28 passed, 261 deselected

[nadeem4]

The bug

nl2sql doctor — the command a user runs when something is already wrong — crashed instead of diagnosing:

$ nl2sql doctor
File "packages/nl2sql/src/nl2sql/cli/checks.py", line 20, in verify_connectivity
from nl2sql.datasources import load_profiles
ImportError: cannot import name 'load_profiles' from 'nl2sql.datasources'
Please report this bug to the nl2sql team.

verify_connectivity() carried two phantom imports against an API that does not exist:

• from nl2sql.datasources import load_profiles — nl2sql.datasources exports DatasourceRegistry, discover_adapters, the models and the adapter protocol. There is no load_profiles.
• from nl2sql.diagnostics import check_connectivity as core_check — there is no nl2sql.diagnostics module at all.

Both imports sat at lines 19-21, outside the try: that begins at line 23. That is why the ImportError escaped the function's own error handling and surfaced as an unhandled-exception traceback rather than a failed check.

nl2sql setup was affected too: the wizard's connectivity step (cli/commands/setup.py:405) calls the same function, so it died the same way — the second caller alongside cli/commands/doctor.py:48.

The fix

verify_connectivity() is rewritten against the real API, following the construction order already used by nl2sql/context.py and cli/demo/manager.py:

ConfigManager() → load_secrets() / SecretManager.configure() → load_datasources() → DatasourceRegistry(secret_manager) → register_datasource() per config → adapter.test_connection().

Datasources are registered one at a time rather than through register_datasources(), which raises on the first bad entry and would abort the whole check. Every datasource now gets its own row.

doctor must never crash, so each failure mode is reported instead of raised:

Failure	Behaviour
Config file missing or unparseable	Connectivity check failed: ... message, returns False
Adapter driver not installed (mssql without pyodbc)	Failed row with the driver error
Unknown engine type	Failed row
Unresolvable ${env:...} secret	Failed row
test_connection() returns False	Failed row
test_connection() raises	Failed row with the exception text
Zero datasources configured	No datasources configured. message, returns True

test_connection() on BaseSQLAlchemyAdapter returns False on failure, but the protocol only declares -> bool and a third-party adapter may raise, so both are handled.

The return contract is unchanged — True only when every datasource connected — because setup.py branches on it.

Imports moved to module level

The three imports (ConfigManager, DatasourceRegistry, SecretManager) are now at module level. There is no circular-import reason to keep them local: nl2sql.cli.checks is imported only from cli/commands/, and none of nl2sql.configs, nl2sql.datasources or nl2sql.secrets import anything under nl2sql.cli. A phantom import would now fail at import time, loudly, instead of at the moment a user needs the command.

Rich markup

Datasource ids and driver error text are wrapped in Text(...) before they reach the table, and the config-error message is printed as Text rather than a markup string — a driver error containing a bracket sequence such as [/{style}] must not raise MarkupError or be silently swallowed. This is the discipline test_cli_error_markup.py exists to protect.

Tests

New: packages/nl2sql/tests/cli/test_doctor_connectivity.py (10 tests). The absence of any coverage here is why this shipped.

• verify_connectivity returns True when all adapters connect, False when one fails, and does not raise in either case
• a missing config, and an unparseable config, are reported rather than raised
• an adapter raising inside test_connection() is caught and reported — with markup-shaped text in the exception message
• an adapter returning False is reported
• an unresolvable ${env:...} secret is reported
• zero datasources is reported
• nl2sql doctor exits 0 and prints the connectivity table — the CLI smoke test that would have caught the original bug. Against main it fails with assert 1 == 0 and the ImportError traceback in the captured output.

All 10 fail on main; all 10 pass here. SQLite and stubs only — no live database, no network, no API key.

Suites, run twice each (pytest-randomly active):

• pytest -m "not integration" — 241 passed, 1 skipped, 47 deselected (baseline 231 + 10 new)
• pytest -m "integration and not llm" — 28 passed, 261 deselected

[nadeem4]

The bug

nl2sql doctor — the command a user runs when something is already wrong — crashed instead of diagnosing:

$ nl2sql doctor
File "packages/nl2sql/src/nl2sql/cli/checks.py", line 20, in verify_connectivity
from nl2sql.datasources import load_profiles
ImportError: cannot import name 'load_profiles' from 'nl2sql.datasources'
Please report this bug to the nl2sql team.

verify_connectivity() carried two phantom imports against an API that does not exist:

• from nl2sql.datasources import load_profiles — nl2sql.datasources exports DatasourceRegistry, discover_adapters, the models and the adapter protocol. There is no load_profiles.
• from nl2sql.diagnostics import check_connectivity as core_check — there is no nl2sql.diagnostics module at all.

Both imports sat at lines 19-21, outside the try: that begins at line 23. That is why the ImportError escaped the function's own error handling and surfaced as an unhandled-exception traceback rather than a failed check.

nl2sql setup was affected too: the wizard's connectivity step (cli/commands/setup.py:405) calls the same function, so it died the same way — the second caller alongside cli/commands/doctor.py:48.

The fix

verify_connectivity() is rewritten against the real API, following the construction order already used by nl2sql/context.py and cli/demo/manager.py:

ConfigManager() → load_secrets() / SecretManager.configure() → load_datasources() → DatasourceRegistry(secret_manager) → register_datasource() per config → adapter.test_connection().

Datasources are registered one at a time rather than through register_datasources(), which raises on the first bad entry and would abort the whole check. Every datasource now gets its own row.

doctor must never crash, so each failure mode is reported instead of raised:

Failure	Behaviour
Config file missing or unparseable	Connectivity check failed: ... message, returns False
Adapter driver not installed (mssql without pyodbc)	Failed row with the driver error
Unknown engine type	Failed row
Unresolvable ${env:...} secret	Failed row
test_connection() returns False	Failed row
test_connection() raises	Failed row with the exception text
Zero datasources configured	No datasources configured. message, returns True

test_connection() on BaseSQLAlchemyAdapter returns False on failure, but the protocol only declares -> bool and a third-party adapter may raise, so both are handled.

The return contract is unchanged — True only when every datasource connected — because setup.py branches on it.

Imports moved to module level

The three imports (ConfigManager, DatasourceRegistry, SecretManager) are now at module level. There is no circular-import reason to keep them local: nl2sql.cli.checks is imported only from cli/commands/, and none of nl2sql.configs, nl2sql.datasources or nl2sql.secrets import anything under nl2sql.cli. A phantom import would now fail at import time, loudly, instead of at the moment a user needs the command.

Rich markup

Datasource ids and driver error text are wrapped in Text(...) before they reach the table, and the config-error message is printed as Text rather than a markup string — a driver error containing a bracket sequence such as [/{style}] must not raise MarkupError or be silently swallowed. This is the discipline test_cli_error_markup.py exists to protect.

Tests

New: packages/nl2sql/tests/cli/test_doctor_connectivity.py (10 tests). The absence of any coverage here is why this shipped.

• verify_connectivity returns True when all adapters connect, False when one fails, and does not raise in either case
• a missing config, and an unparseable config, are reported rather than raised
• an adapter raising inside test_connection() is caught and reported — with markup-shaped text in the exception message
• an adapter returning False is reported
• an unresolvable ${env:...} secret is reported
• zero datasources is reported
• nl2sql doctor exits 0 and prints the connectivity table — the CLI smoke test that would have caught the original bug. Against main it fails with assert 1 == 0 and the ImportError traceback in the captured output.

All 10 fail on main; all 10 pass here. SQLite and stubs only — no live database, no network, no API key.

Suites, run twice each (pytest-randomly active):

• pytest -m "not integration" — 241 passed, 1 skipped, 47 deselected (baseline 231 + 10 new)
• pytest -m "integration and not llm" — 28 passed, 261 deselected

[nadeem4]

The bug

nl2sql doctor — the command a user runs when something is already wrong — crashed instead of diagnosing:

$ nl2sql doctor
File "packages/nl2sql/src/nl2sql/cli/checks.py", line 20, in verify_connectivity
from nl2sql.datasources import load_profiles
ImportError: cannot import name 'load_profiles' from 'nl2sql.datasources'
Please report this bug to the nl2sql team.

verify_connectivity() carried two phantom imports against an API that does not exist:

• from nl2sql.datasources import load_profiles — nl2sql.datasources exports DatasourceRegistry, discover_adapters, the models and the adapter protocol. There is no load_profiles.
• from nl2sql.diagnostics import check_connectivity as core_check — there is no nl2sql.diagnostics module at all.

Both imports sat at lines 19-21, outside the try: that begins at line 23. That is why the ImportError escaped the function's own error handling and surfaced as an unhandled-exception traceback rather than a failed check.

nl2sql setup was affected too: the wizard's connectivity step (cli/commands/setup.py:405) calls the same function, so it died the same way — the second caller alongside cli/commands/doctor.py:48.

The fix

verify_connectivity() is rewritten against the real API, following the construction order already used by nl2sql/context.py and cli/demo/manager.py:

ConfigManager() → load_secrets() / SecretManager.configure() → load_datasources() → DatasourceRegistry(secret_manager) → register_datasource() per config → adapter.test_connection().

Datasources are registered one at a time rather than through register_datasources(), which raises on the first bad entry and would abort the whole check. Every datasource now gets its own row.

doctor must never crash, so each failure mode is reported instead of raised:

Failure	Behaviour
Config file missing or unparseable	Connectivity check failed: ... message, returns False
Adapter driver not installed (mssql without pyodbc)	Failed row with the driver error
Unknown engine type	Failed row
Unresolvable ${env:...} secret	Failed row
test_connection() returns False	Failed row
test_connection() raises	Failed row with the exception text
Zero datasources configured	No datasources configured. message, returns True

test_connection() on BaseSQLAlchemyAdapter returns False on failure, but the protocol only declares -> bool and a third-party adapter may raise, so both are handled.

The return contract is unchanged — True only when every datasource connected — because setup.py branches on it.

Imports moved to module level

The three imports (ConfigManager, DatasourceRegistry, SecretManager) are now at module level. There is no circular-import reason to keep them local: nl2sql.cli.checks is imported only from cli/commands/, and none of nl2sql.configs, nl2sql.datasources or nl2sql.secrets import anything under nl2sql.cli. A phantom import would now fail at import time, loudly, instead of at the moment a user needs the command.

Rich markup

Datasource ids and driver error text are wrapped in Text(...) before they reach the table, and the config-error message is printed as Text rather than a markup string — a driver error containing a bracket sequence such as [/{style}] must not raise MarkupError or be silently swallowed. This is the discipline test_cli_error_markup.py exists to protect.

Tests

New: packages/nl2sql/tests/cli/test_doctor_connectivity.py (10 tests). The absence of any coverage here is why this shipped.

• verify_connectivity returns True when all adapters connect, False when one fails, and does not raise in either case
• a missing config, and an unparseable config, are reported rather than raised
• an adapter raising inside test_connection() is caught and reported — with markup-shaped text in the exception message
• an adapter returning False is reported
• an unresolvable ${env:...} secret is reported
• zero datasources is reported
• nl2sql doctor exits 0 and prints the connectivity table — the CLI smoke test that would have caught the original bug. Against main it fails with assert 1 == 0 and the ImportError traceback in the captured output.

All 10 fail on main; all 10 pass here. SQLite and stubs only — no live database, no network, no API key.

Suites, run twice each (pytest-randomly active):

• pytest -m "not integration" — 241 passed, 1 skipped, 47 deselected (baseline 231 + 10 new)
• pytest -m "integration and not llm" — 28 passed, 261 deselected