Skip to content

fix(execution): make ERA collapse ordering deterministic#36

Merged
egillax merged 1 commit into
developfrom
fix/era-collapse-deterministic
May 6, 2026
Merged

fix(execution): make ERA collapse ordering deterministic#36
egillax merged 1 commit into
developfrom
fix/era-collapse-deterministic

Conversation

@egillax

@egillax egillax commented May 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes nondeterministic final ERA collapse in the Ibis execution engine.

The collapse window previously ordered only by start_date. When multiple intervals had the same start
date, the backend could process the shorter interval before the longer one. That could leave a later
interval, fully contained by the longer interval, as an extra cohort row.

This change makes the collapse ordering deterministic by processing longer tied-start intervals first.

Minimal Reprex

Input intervals:

2020-01-01 -> 2020-01-02
2020-01-01 -> 2020-02-01
2020-01-10 -> 2020-01-15

Before:

2020-01-01 -> 2020-02-01
2020-01-10 -> 2020-01-15

After:

2020-01-01 -> 2020-02-01

The second Before row is contained in the first row and should not survive ERA collapse.

## Validation

uv run --extra dev pytest tests/execution/
test_end_strategy_censoring.py::test_collapse_settings_era_merges_contained_intervals_after_tied_start_d
ates -q
uv run --extra dev pytest tests/execution -q

Results:

1 passed
271 passed

Private real-data validation against the first 100 PhenotypeLibrary cohorts on real data reduced
CirceR row count mismatches from 19/100 to 0/100.

@egillax egillax requested a review from azimov May 6, 2026 13:46
@codecov

codecov Bot commented May 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.79%. Comparing base (76466c5) to head (b4d9419).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop      #36   +/-   ##
========================================
  Coverage    85.79%   85.79%           
========================================
  Files          169      169           
  Lines        12514    12514           
========================================
  Hits         10737    10737           
  Misses        1777     1777           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@azimov

azimov commented May 6, 2026

Copy link
Copy Markdown
Collaborator

No issue with the fix - I've tested and confirmed this.

I will create an issue for better testing to have higher visibility for more real cohorts between ibis and java implementations.

@egillax egillax merged commit c2e7067 into develop May 6, 2026
10 checks passed
@egillax egillax deleted the fix/era-collapse-deterministic branch May 6, 2026 14:29
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.

2 participants