Skip to content

Feature/custom solver support - #68

Open
adefabian wants to merge 12 commits into
mainfrom
feature/custom_solver_support
Open

Feature/custom solver support#68
adefabian wants to merge 12 commits into
mainfrom
feature/custom_solver_support

Conversation

@adefabian

@adefabian adefabian commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Customers can now ship their own QuerySolver (say, one that reshapes their raw
tables into the Impulse silver schema) without touching Impulse core.

Solvers register themselves by name, the report config picks one by that name through
the existing query_engine.solver field, and each solver can bring its own
SolverConfig subclass whose extra fields get checked at parse time.

Config picks a solver by registered name, not by class path. So a config can't
make Impulse import and run some arbitrary class. What solvers exist depends
entirely on what the driver imports.

DefaultSolver moves onto this mechanism with no behavior change. It registers
under "DefaultSolver" (plus the old "DeltaSolver" / "KeyValueStoreSolver"
aliases), so existing configs keep working.

Changes

  • Registry (solvers/registry.py): @register_solver(name, config_cls, *, aliases, overwrite) plus resolve_registration / is_registered / registered_names. Maps a name to (solver_cls, config_cls), treats re-registering the same class as idempotent, rejects conflicting duplicates, and lists the known names
    when a lookup misses.
  • Uniform construction (QuerySolver.from_config + solvers/solver_context.py): a frozen SolverBuildContext (spark, solver_config, is_raw_data, drop_implausible_data, raw_encoder) is the one place construction inputs live. Base from_config passes just solver_config; DefaultSolver overrides it to also
    wire spark and the raw-data flags. The report factory builds any solver the same way, whatever its constructor looks like.
  • Config (config_parser.py): query_engine.solver is a plain str now (any registered name). Solvers becomes a StrEnum so old == Solvers.DEFAULT_SOLVER comparisons still hold. A mode="before" validator re-validates solver_config through the selected solver's config_cls, so a custom solver's fields are
    enforced at parse time and an unregistered name fails early as a ValidationError.
  • Report factory (core/report.py): create_solver resolves the name through the registry and calls from_config(ctx) instead of a hardcoded match. Solver telemetry now logs the registered name string rather than an enum member.
  • Docs (docs/impulse/docs/data_model/ingestion.md): a "Registering a custom solver" section covering subclassing DefaultSolver, declaring a SolverConfig subclass, selecting it by name, and importing the package so registration runs — plus a worked column-redaction example.
  • Skills (skills/impulse-config, skills/impulse-data-model): updated to describe selecting a solver by registered name.

Customer solvers extend behavior by overriding the supported QuerySolver
pipeline methods (e.g. solve); the report factory and incremental
change-detection paths are unchanged and continue to read silver data through
MeasurementDB.

Test Plan

  • Unit tests added/updated — registry (registry_test.py), from_config hook (from_config_test.py), config/solver-selection validation (solver_registry_config_test.py)
  • Integration tests added — report-factory routing (custom_solver_report_test.py) and an end-to-end custom-solver solve (custom_solver_test.py)
  • Documentation updated

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • No new linter warnings introduced

@codecov

codecov Bot commented Jul 29, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.89%. Comparing base (83838ee) to head (a9a2bfb).

Files with missing lines Patch % Lines
src/impulse_reporting/config/config_parser.py 88.23% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #68      +/-   ##
==========================================
+ Coverage   88.69%   88.89%   +0.20%     
==========================================
  Files          60       62       +2     
  Lines        5015     5081      +66     
  Branches      598      602       +4     
==========================================
+ Hits         4448     4517      +69     
+ Misses        461      459       -2     
+ Partials      106      105       -1     
Flag Coverage Δ
query_engine 84.91% <100.00%> (+0.34%) ⬆️
reporting 94.29% <91.30%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ery_engine/analyze/query/solvers/default_solver.py 97.68% <100.00%> (+0.70%) ⬆️
...query_engine/analyze/query/solvers/query_solver.py 88.40% <100.00%> (+0.52%) ⬆️
...lse_query_engine/analyze/query/solvers/registry.py 100.00% <100.00%> (ø)
...ery_engine/analyze/query/solvers/solver_context.py 100.00% <100.00%> (ø)
src/impulse_reporting/core/report.py 92.64% <100.00%> (+0.97%) ⬆️
src/impulse_reporting/config/config_parser.py 96.29% <88.23%> (-0.99%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@adefabian
adefabian force-pushed the feature/custom_solver_support branch from 1a4a741 to c0b01a0 Compare August 10, 2026 17:26
@adefabian
adefabian marked this pull request as ready for review August 11, 2026 18:40
@adefabian
adefabian requested a review from a team as a code owner August 11, 2026 18:40
@adefabian
adefabian requested a review from tombonfert August 11, 2026 18:40
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.

1 participant