Dataface Tasks

refactor: Move database/source detection from playground to core

IDISSUE-298
Statuscompleted
Priorityp1
Milestonem1-ft-analytics-analyst-pilot
Ownerhead-of-engineering

Problem

Database and source detection logic currently lives inside the playground application rather than in the core library, which means the hosted Cloud app and any other consumer must either duplicate that logic or depend on playground internals. This architectural misplacement has already caused divergent behavior between playground and Cloud when detecting adapter types and connection parameters. Moving detection into core would establish a single canonical code path that all surfaces — playground, Cloud, CLI, and future integrations — share, eliminating duplication and behavior drift.

Context

Existing detection code lives in three places:

  1. dataface/core/compile/sources.py — Already has detect_dbt_database_type(), detect_dbt_connections(), DB_INFO_MAP, get_database_info(). This is the canonical home.
  2. apps/playground/routes.py (lines 881–998) — Two wrapper functions: - _get_database_type_from_profiles(): Thin wrapper around core's detect_dbt_database_type() adding DuckDB fallback when profiles exist but detection fails. - _get_database_type_info(registry): Introspects AdapterRegistry to detect database type from registered adapters (DbtAdapter/SqlAdapter).
  3. dataface/cli/commands/inspect.py (lines 109–209)_detect_dbt_connection(cwd): Independent reimplementation that walks up directories to find dbt_project.yml, reads profiles.yml, and builds connection strings. Does not use core at all.

Callers: - Playground: get_database_schema() endpoint uses both wrapper functions. - CLI: inspect_command() and inspect_all_command() call _detect_dbt_connection() at lines 253 and 414. - Cloud: ConnectionService already imports detect_dbt_connections from core — no change needed. - Tests: tests/core/test_inspect_adapters.py imports _detect_dbt_connection from CLI (6 test classes).

Constraints: - Core must not import from playground or CLI (dependency direction: surfaces → core). - AdapterRegistry is in dataface.core.execute.adapters — same package, so core-internal references are fine. - detect_dbt_database_type already returns profile_name/target_name in result dict.

Possible Solutions

A. Add fallback_type to detect_dbt_database_type + move functions to core

Enhance existing core function with an optional fallback_type parameter. Move _get_database_type_info and _detect_dbt_connection to sources.py. Callers import from core. - Pro: Minimal new API surface; playground and CLI become thin import wrappers. - Con: detect_database_type_from_registry couples sources.py to adapter internals.

B. New dataface/core/detect.py module

Create a dedicated detection module separate from sources.py. - Pro: Clean separation. - Con: Splits closely related functions across two files for little benefit.

C. Add detection methods to AdapterRegistry itself

Teach AdapterRegistry to report its database type. - Pro: Encapsulates adapter introspection. - Con: Registry shouldn't own detection policy; larger refactor.

Recommended: A — extend sources.py with three additions, keeping all detection in one place. The adapter-introspection function is small and the coupling is acceptable since both live in core.

Plan

  • [x] Inventory existing detection logic and call sites in playground vs core.
  • [x] Write failing tests for: detect_dbt_database_type with fallback_type, detect_database_type_from_registry, detect_dbt_connection_string.
  • [x] Add fallback_type param to detect_dbt_database_type in sources.py.
  • [x] Add detect_database_type_from_registry(registry) to sources.py.
  • [x] Add detect_dbt_connection_string(cwd) to sources.py (moved from CLI).
  • [x] Export new functions from dataface/core/compile/__init__.py.
  • [x] Migrate apps/playground/routes.py — replace _get_database_type_from_profiles and _get_database_type_info with thin wrappers to core.
  • [x] Migrate dataface/cli/commands/inspect.py — replace _detect_dbt_connection with thin wrapper to core.
  • [x] Existing test imports in tests/core/test_inspect_adapters.py still work (CLI wrapper delegates to core).
  • [x] Run focused tests and validate — 2610 passed, 0 failures.

Implementation Progress

Changes Made

  1. dataface/core/compile/sources.py — Added three features: - fallback_type keyword parameter on detect_dbt_database_type(): when profile lookup fails but the file exists, returns info for the fallback database type. Subsumes playground's DuckDB-fallback wrapper. - detect_database_type_from_registry(registry): introspects an AdapterRegistry's SQL adapters to detect database type. Moved from playground's _get_database_type_info. - detect_dbt_connection_string(cwd): walks up directories to find dbt_project.yml, reads profiles, builds (connection_string, dialect). Moved from CLI's _detect_dbt_connection.

  2. dataface/core/compile/__init__.py — Exports detect_database_type_from_registry and detect_dbt_connection_string.

  3. apps/playground/routes.py_get_database_type_from_profiles and _get_database_type_info replaced with thin 2-line wrappers that delegate to core. All call sites unchanged.

  4. dataface/cli/commands/inspect.py_detect_dbt_connection replaced with thin wrapper importing detect_dbt_connection_string from core. Existing callers and test imports unchanged.

  5. tests/core/test_source_detection.py — 18 new focused tests covering all three core additions (fallback_type, registry detection, connection string detection for DuckDB/Postgres/BigQuery/Snowflake/Databricks/edge cases).

Migration Notes

  • For playground consumers: _get_database_type_from_profiles and _get_database_type_info still exist as thin wrappers. No import changes needed.
  • For CLI consumers: _detect_dbt_connection still exists as a thin wrapper. No import changes needed. New code should prefer from dataface.core.compile.sources import detect_dbt_connection_string.
  • For new consumers: Import directly from dataface.core.compile.sources or dataface.core.compile.

Review Feedback

  • [x] Review cleared — self-review: all tests pass (149 focused, 2610 full suite), lint/format clean, pre-commit hooks pass, rebase on main clean.