Skip to content

Apply channels.filters at read time (#103) - #104

Merged
tombonfert merged 8 commits into
mainfrom
feature/channel_table_filters
Sep 24, 2026
Merged

tombonfert merged 8 commits into
mainfrom
feature/channel_table_filters

Conversation

@tombonfert

@tombonfert tombonfert commented Sep 18, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

SolverConfig.channels accepted a filters dict but the engine never applied it, so channel-table filters were silently dropped. This wires them up and adds a guardrail against the one way the feature can be misused.

Changes

  • Apply channels.filters in DefaultSolver._prepare_channels_join, after column_name_mapping and before the UDF-column projection, using the same equality-filter mechanism as container_tags, container_metrics, and channel_mapping. Applying it before the projection lets a filter reference any channels column, including dimension columns the solve step drops.
  • Reject implausibility-via-filter in RAW mode. A new config validator raises when data_type = RAW and channels.filters targets is_plausible. Such a filter runs before raw encoding and would bridge intervals across dropped samples; drop_implausible_data is the correct tool (it drops inside the encoder, so boundaries stay correct). The check keys off the internal is_plausible name, so it also catches a physical column mapped to it.
  • Docs. Documented channels-table filter support, noted that filter values are coerced to the target column type (booleans take "true"/"false"), added the previously undocumented is_plausible and timestamp internal names to the mapping table, and added a RAW-mode caveat. Also corrected a stale "RAW to RLE" error string to "RAW to interval" (the drop works for both raw encoders).
  • Solves Apply solver_config.channels.filters when reading the channels table #103

Test Plan

  • Unit tests added/updated
  • Manual testing completed
  • Documentation updated (if applicable)

Checklist

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

Wire per-table equality filters into `_prepare_channels_join` so the channels table is filtered after `column_name_mapping` and before the UDF-column projection. Update configuration docs to list `channels` as a filters-supported table and clarify filter value coercion. Add end-to-end unit tests covering string, non-matching, and boolean channel filters.
Add a QueryEngine validator that blocks `channels.filters` entries targeting the `is_plausible` column when `data_type=RAW`, since those filters run before raw encoding and bridge intervals across dropped implausible samples. Direct users to `drop_implausible_data=True` instead. Update configuration docs with the new internal columns and a caution callout. Add unit tests covering mapped plausibility columns, non-plausibility filters, and RLE mode.
@tombonfert
tombonfert requested a review from a team as a code owner September 18, 2026 15:09
@codecov

codecov Bot commented Sep 18, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.80%. Comparing base (88af4a7) to head (0eca05e).

Files with missing lines Patch % Lines
src/impulse_reporting/config/config_parser.py 93.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #104      +/-   ##
==========================================
+ Coverage   89.78%   89.80%   +0.02%     
==========================================
  Files          62       62              
  Lines        5717     5740      +23     
  Branches      716      722       +6     
==========================================
+ Hits         5133     5155      +22     
  Misses        463      463              
- Partials      121      122       +1     
Flag Coverage Δ
query_engine 86.15% <100.00%> (+0.03%) ⬆️
reporting 94.65% <93.33%> (-0.01%) ⬇️

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 94.87% <100.00%> (+0.04%) ⬆️
...uery_engine/analyze/query/solvers/solver_config.py 100.00% <100.00%> (ø)
src/impulse_reporting/config/config_parser.py 97.07% <93.33%> (-0.30%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…mode

Add a QueryEngine validator that emits a `UserWarning` when `channels.filters`
contains any non-plausibility column in RAW mode, since such filters run before
raw encoding and bridge intervals across dropped samples. Only whole-channel
scoping is safe; per-sample cleaning should use `drop_implausible_data`.
Update configuration docs to clarify intended scope and adjust tests to assert
the warning.
Document that RLE mode does not share RAW mode's restriction on
`channels.filters`: because the channels table is already encoded,
filters drop matching `[tstart, tend)` interval rows without bridging
across samples and may target any column.
…ter rejection in SolverConfig

Move the invariant that rejects `channels.filters` on the plausibility column in RAW mode into `SolverConfig.reject_implausible_channels_filter_in_raw()` and call it from both the reporting `QueryEngine` validator and `DefaultSolver` construction. This ensures direct query-engine use cannot bypass the guard. Add unit tests verifying the rejection in RAW mode and allowance outside it.
…lidator warning stacklevel

Document that `channels.filters` in RAW mode run before raw encoding and should only be used for whole-channel scoping; `is_plausible` filters are rejected and others warn. Fix the `UserWarning` stacklevel inside the Pydantic model_validator so the warning points at the validator call rather than Pydantic internals.
@tombonfert
tombonfert merged commit 1e850db into main Sep 24, 2026
6 checks passed
@tombonfert
tombonfert deleted the feature/channel_table_filters branch September 24, 2026 09:35
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