GeezerVison is showing github.com — fix: Keep the baseline out of monitoring windows and align its overlay by Nitant28 · Pull Request #7057 · feast-dev/feast
Conversation
What this PR does / why we need it:
Fixes the monitoring UI problems described in #7054 (with screenshots). There is one commit for
each cause.
1. fix: Keep the baseline out of the Monitoring pages' window views
/monitoring/metrics/features returns the baseline row alongside the windows, dated by the day it
was computed. The pages treated it as a window. A new pages/monitoring/metricSelection.ts holds
the shared selection logic:
latestWindowMetric(): the Monitoring tab now shows the newest window, not the baseline compared
with itself. It shows the baseline on its own ("Baseline Distribution", no overlay) only when no
window exists yet.
windowGranularities(): the detail page's View options no longer include the raw "baseline" row
(it keeps its "Baseline (all data)" option), and are ordered shortest window first, so the page
opens on Daily.
seriesForGranularity(): the detail page's Time-Series Analysis plots one granularity (the viewed
one, or the shortest when the baseline is viewed) instead of every row on one line.
2. fix: Draw the Monitoring tab's baseline overlay on the window's bins
The overlay drew baseline bin i behind window bin i, although every stored histogram has its own
edges, and scaled both by raw counts, although the baseline has many more rows. A ×100 unit change
drew the same bars as a clean day.
alignBaselineToBins() (new, components/histogramAlignment.ts) moves each baseline bin's count
onto the window's bins by overlap and scales it to the window's row count. Values are treated as
uniform within a bin; a zero-width bin (the SQL backends' constant-column form) as a point. A
constant window takes the whole baseline bin that contains its value, so a 0/1 flag compares
against the baseline's share of zeros.
- The legend states the share of the baseline outside the window's range.
- A non-numeric baseline (left from before a feature changed type) is no longer overlaid. On master
it throws TypeError: undefined is not iterable.
Design notes
- Scaling instead of shares: scaling to the window's row count keeps the y-axis and tooltips in
this window's rows.
- Why UI-only:
/monitoring/metrics/features and the baseline's metric_date are unchanged.
Changing either would change the REST response or the stored data (metric_date is part of
FEATURE_METRICS_PK), which seems out of scope for a UI fix.
- Untouched: the categorical chart and the time-series component.
If you'd prefer, the two commits split cleanly into two PRs (they share no files).
Relation to #7056. I reported #7054 with this fix ready, and #7056 has since been opened for it
too. Both take the same overall approach. In case it helps you choose or combine them, the
differences I see are:
I'm happy to go with whichever you prefer, or to fold anything useful from #7056 into this one.
Which issue(s) this PR fixes:
Fixes #7054
Checks
- I've made sure the tests are passing.
- My commits are signed off (
git commit -s)
- My PR title follows conventional commits format
Testing Strategy
- Unit tests
- Integration tests
- Manual tests
- Testing is not required for this change
19 new jest tests:
metricSelection.test.ts (6): uses the rows /monitoring/metrics/features returns after a
feast monitor run on the demo. Covers the latest window, baseline only, empty windows,
granularity order, and single-granularity series.
histogramAlignment.test.ts (10): edges, scaling, a shift past the baseline's range, zero-width
bins on either side, a constant 0/1 window, edge values, an empty baseline, a missing last edge.
HistogramChart.test.tsx (3, render): all three fail on master.
- A baseline with 100× the rows draws window bars 1.6 px tall against 160 px baseline bars.
- A window above the baseline's range still gets full-height baseline bars.
- A categorical baseline throws.
Local results (Windows, Node 20; .nvmrc pins 22, which CI uses):
- The new tests pass on this branch, rebased on master at e304a84.
prettier --check, eslint, tsc and the build:lib type check pass, and so does the
production build with CI=true.
- Full suite: 38 of 39 pass. The one failure is in
FeastUISansProviders.test.tsx, which fails 1–3
of its tests per run on this machine: timeouts, and getByRole heading lookups right after
navigation. It fails the same way on unmodified master ("Feature Views" and "Data Sources" headings
not found).
Manual: the steps in #7054 on the feast init demo, served by feast ui with this build. The
screenshots below cover the tab after a fresh run, on a clean day and on a ×100 day, plus the detail
page.
Misc
The overlay is approximate when values aren't uniform within a baseline bin. The stored histograms
keep only bin counts, so some within-bin assumption is unavoidable.
cc @jyejare





Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 50.99%. Comparing base (e304a84) to head (9d0a264).
⚠️ Report is 6 commits behind head on master.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files

@@ Coverage Diff @@
## master #7057 +/- ##
=======================================
Coverage 50.99% 50.99%
=======================================
Files 455 455
Lines 56994 56994
Branches 8321 8321
=======================================
Hits 29063 29063
Misses 25946 25946
Partials 1985 1985
Continue to review full report in Codecov by Harness.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e304a84...9d0a264. Read the comment docs.
🚀 New features to boost your workflow:
- ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This branch has not been deployed
No deployments
Reviewers
shuchu
shuchu approved these changes
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|