release: prepare v0.2.17 — MLZOO-D + dogfood automation + cross-slice import pattern#266
Merged
Conversation
chore(repo): back-merge main into dev after v0.2.16 release (#252)
…rror (#256) Surface the v0.2.16 advanced-model metadata (LightGBM / XGBoost / regression / prophet_like) via two new read-only forecasting endpoints — the registry- keyed twin of PRP-28's /explain/runs/{run_id} and the job-keyed twin of /explain/jobs/{job_id}. Adds the computed model_family field on RunResponse, the FeatureImportanceUnavailableError + UnprocessableEntityError exception classes, and matching unit + route tests. Backend additions: - app/features/forecasting/feature_metadata.py — model_family_for, importance_type_for, extract_feature_importance. Pure-function; lightgbm / xgboost never imported at module scope (optional extras). Detects sklearn's HistGradientBoostingRegressor.feature_importances_ gap and raises FeatureImportanceUnavailableError → 422. Detects the SimpleImputer-drops-all-NaN-columns case for prophet_like and pads ridge.coef_ back to the canonical 14-column width with 0.0 (regression caught during dogfood; days_since_launch on a product with no launch date triggers it). - app/features/forecasting/schemas.py — ModelFamily str enum, FeatureImportanceItem (signed importance for linear_coef kind), FeatureMetadataResponse. - app/features/forecasting/service.py — get_feature_metadata_for_run + for_job. The two cross-slice imports (RegistryService, JobService) are LAZY (inside the methods) to break the registry ↔ forecasting cycle the computed field would otherwise close. - app/features/forecasting/routes.py — GET /forecasting/runs/{run_id}/ feature-metadata + GET /forecasting/jobs/{job_id}/feature-metadata. Mirror PRP-28's exception-flow contract: ForecastLabError subclasses flow through to forecastlab_exception_handler, SQLAlchemyError becomes DatabaseError. Also fixes the /train docstring drift — now lists xgboost, regression, prophet_like alongside lightgbm. - app/core/exceptions.py + problem_details.py — UnprocessableEntityError (status_code=422, code='UNPROCESSABLE_ENTITY') and matching ERROR_TYPES entry. Distinct from ValidationError (code='VALIDATION_ERROR') so consumers can disambiguate state-prevented operations from input failures via the RFC 7807 type URI. - app/features/registry/schemas.py — model_family computed_field on RunResponse (no DB column, no migration). Tests (37 new): test_feature_metadata.py (17, including the imputer-drop regression), test_routes_feature_metadata.py (20, full matrix for both endpoints — 200 / 400 / 404 / 422 with mocked RegistryService / JobService / load_model_bundle), and 10 new RunResponse model_family cases in test_schemas.py. Closes the MLZOO sequence backend gap.
Surface the new /forecasting/{runs,jobs}/{id}/feature-metadata endpoints
and the computed model_family field on every ModelRun across the
explorer + visualize pages. One panel, two display modes (tree: positive
bars; linear_coef: signed bars with direction colour + icon).
Frontend additions:
- types/api.ts — ModelFamily ('baseline' | 'tree' | 'additive');
ModelRun.model_family; FeatureImportanceItem (signed importance);
FeatureMetadataResponse.
- hooks/use-feature-metadata.ts — useRunFeatureMetadata +
useJobFeatureMetadata sibling hooks. Mirror useRunExplanation /
useJobExplanation exactly (retry: false; query-key shape).
- components/common/model-family-badge.tsx + test — pure derivation,
no hooks: baseline → secondary + Activity, tree → default + TreePine,
additive → outline + LineChart. data-family attr + testid.
- components/explainability/feature-importance-panel.tsx + test —
one card, branches on FeatureImportanceItem.kind. Linear coef rows
render with sign-coloured bars + TrendingUp / TrendingDown icons.
Verbatim correlation-vs-causation caveat in the card footer.
Neutral muted message for ApiError 400 (baseline family) and 422
(no artifact / missing extra / HistGBR-no-importance). Destructive
ErrorDisplay for unexpected.
Page wiring (additive, in-place edits only):
- explorer/runs.tsx — new 'Family' column with ModelFamilyBadge,
MODEL_TYPES allow-list extended to regression / lightgbm / xgboost /
prophet_like, csvColumns + Model filter dropdown extended.
- explorer/run-detail.tsx — ModelFamilyBadge in profile header + row,
'Feature Metadata' card listing the 14 canonical feature columns,
FeatureImportancePanel — all gated on model_family !== 'baseline'
with enabled:false on the TanStack Query so baseline runs never
trigger a 400 burst.
- explorer/run-compare.tsx — Family row in the profile table, new
collapsible 'Feature Importance (Run A vs Run B)' card. Side-by-side
panels only when both runs share a non-baseline family; cross-family
pairs render a muted explanatory message and DO NOT fetch (enabled:
false on both hooks).
- visualize/forecast.tsx — CRITICAL: uses useJobFeatureMetadata
(NOT useRunFeatureMetadata). trainJob.result.run_id is the FORECAST
ARTIFACT KEY (uuid.uuid4().hex[:12], see forecasting/service.py:270),
NOT a registry UUID; calling the run-keyed hook would 404. Inline
CRITICAL comment + memory cross-link to scenario-run-id-vs-registry-
run-id. Collapsible defaultOpen=false to preserve scan flow.
- visualize/backtest.tsx — MODEL_OPTIONS extended to all seven types
(B.2's feature-aware backtest is what makes the four advanced
families reachable from the UI).
Tests (10 new): model-family-badge.test.tsx (3 cases),
feature-importance-panel.test.tsx (7 cases). Both use afterEach
(cleanup) because vitest is not configured for auto-cleanup.
…y-explainability feat(forecast,ui): MLZOO-D frontend, registry, and explainability polish (#256)
feat(repo): add scripts/dogfood-browser.sh wrapper for snap-chromium playwright (#262)
docs(repo): name cross-slice read-only import pattern (#264)
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @w7-mgfcode, your pull request is larger than the review limit of 150000 diff characters
📝 WalkthroughWalkthroughThis PR implements PRP-31 to surface advanced ML model metadata: feature-importance extraction by model family (baseline/tree/additive), two new feature-metadata API endpoints (run- and job-keyed), computed ChangesBackend Feature Metadata API
Frontend Feature Metadata UI
Documentation and Configuration
Sequence DiagramsequenceDiagram
participant Browser
participant FrontendRoute
participant APIEndpoint
participant ForecastingService
participant RegistryService
participant ModelArtifact
participant FeatureExtractor
Browser->>FrontendRoute: GET /forecasting/runs/{run_id}/feature-metadata
FrontendRoute->>APIEndpoint: HTTP Request (useRunFeatureMetadata)
APIEndpoint->>ForecastingService: get_feature_metadata_for_run(db, run_id)
ForecastingService->>RegistryService: get_run(run_id)
RegistryService-->>ForecastingService: RunResponse (model_type, artifact_uri)
ForecastingService->>ModelArtifact: load_model_bundle(artifact_uri)
ModelArtifact-->>ForecastingService: ModelBundle
ForecastingService->>FeatureExtractor: extract_feature_importance(bundle)
FeatureExtractor-->>ForecastingService: FeatureImportanceItem[]
ForecastingService-->>APIEndpoint: FeatureMetadataResponse
APIEndpoint-->>FrontendRoute: 200 JSON
FrontendRoute->>Browser: Render FeatureImportancePanel
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/features/forecasting/routes.py (1)
120-131:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReplace remaining bare
HTTPExceptionbranches with ForecastLab errors.
train_modelandpredictstill raise rawHTTPExceptionfor user-facing failures, which breaks your RFC 7807 consistency inapp/**/*.py. Please map these toBadRequestError/NotFoundError(or otherForecastLabErrorsubclasses) so responses always flow throughproblem_detailshandling.As per coding guidelines, "Use RFC 7807
application/problem+jsonerror responses viaapp/core/problem_details.py. Never use bareHTTPExceptionwith raw strings or ad-hoc error shapes."Also applies to: 222-237
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/features/forecasting/routes.py` around lines 120 - 131, The code currently raises bare HTTPException in the forecasting handlers (notably in train_model, predict, and the ValueError except block shown) which bypasses the RFC7807 problem_details pipeline; replace those HTTPException usages with the appropriate ForecastLabError subclasses (e.g., raise BadRequestError for input/validation issues and NotFoundError for missing resources) so responses are emitted via app/core/problem_details.py; update the except ValueError block to raise BadRequestError (including the original message and preserving exception chaining) and scan the other HTTPException occurrences around the predict/train_model code (lines ~222-237) to map them to ForecastLabError subclasses accordingly.
🧹 Nitpick comments (2)
frontend/src/components/explainability/feature-importance-panel.tsx (1)
30-35: ⚡ Quick winStrengthen
FAMILY_DESCRIPTIONtype constraint toModelFamily.Currently
Record<string, string>loses compile-time exhaustiveness checks. Typing asRecord<ModelFamily, string>ensures the map remains in sync if model families are added or renamed. Thefamilyparameter is already constrained toModelFamily, so this tightens the type consistency.Proposed diff
-import type { FeatureImportanceItem, FeatureMetadataResponse } from '`@/types/api`' +import type { + FeatureImportanceItem, + FeatureMetadataResponse, + ModelFamily, +} from '`@/types/api`' @@ -const FAMILY_DESCRIPTION: Record<string, string> = { +const FAMILY_DESCRIPTION: Record<ModelFamily, string> = {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/explainability/feature-importance-panel.tsx` around lines 30 - 35, FAMILY_DESCRIPTION is currently typed as Record<string, string>, losing exhaustiveness checks; change its type to Record<ModelFamily, string> and ensure ModelFamily is imported or referenced where this file uses it (the same type used for the family parameter). Update the declaration of FAMILY_DESCRIPTION to use ModelFamily so the compiler will catch missing or renamed model families and keep the map in sync with the family parameter usage.frontend/src/hooks/use-feature-metadata.ts (1)
15-21: ⚡ Quick winAlign both
queryFnimplementations with the async/await guideline.Both hooks return promises implicitly; update them to use explicit async/await syntax as required by the coding guidelines.
Proposed diff
export function useRunFeatureMetadata(runId: string, enabled = true) { return useQuery({ queryKey: ['feature-metadata', 'run', runId], - queryFn: () => - api<FeatureMetadataResponse>(`/forecasting/runs/${runId}/feature-metadata`), + queryFn: async () => { + return await api<FeatureMetadataResponse>( + `/forecasting/runs/${runId}/feature-metadata`, + ) + }, enabled: enabled && !!runId, retry: false, }) } export function useJobFeatureMetadata(jobId: string, enabled = true) { return useQuery({ queryKey: ['feature-metadata', 'job', jobId], - queryFn: () => - api<FeatureMetadataResponse>(`/forecasting/jobs/${jobId}/feature-metadata`), + queryFn: async () => { + return await api<FeatureMetadataResponse>( + `/forecasting/jobs/${jobId}/feature-metadata`, + ) + }, enabled: enabled && !!jobId, retry: false, }) }Both lines 17-18 and 37-38 require this change per the guideline: "Use async/await for all promise handling in TypeScript/JavaScript code."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/hooks/use-feature-metadata.ts` around lines 15 - 21, The queryFn currently returns the api(...) promise directly; change it to an async function that awaits the API call to follow the project's async/await guideline. Update each useQuery's queryFn (the handlers that call api<FeatureMetadataResponse>(`/forecasting/runs/${runId}/feature-metadata`) and the other similar queryFn later in the file) to be async () => { return await api<FeatureMetadataResponse>(...) } (or simply const result = await api(...); return result) so the promise is handled with explicit async/await rather than an implicit returned promise.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/features/forecasting/service.py`:
- Around line 809-812: The code only checks truthiness of bundle_path_str (from
job.result["model_path"]) and can accept non-string truthy values; update the
validation around bundle_path_str in the function that handles job.result so it
enforces isinstance(bundle_path_str, str) and bundle_path_str.strip() and raise
UnprocessableEntityError when the check fails; specifically modify the block
using result = job.result or {} and bundle_path_str = result.get("model_path")
to validate type and non-empty string before proceeding to any bundle loading
logic.
In `@app/features/registry/schemas.py`:
- Around line 20-27: Current direct import of ModelFamily (and the related
model_family_for usage) in app/features/registry/schemas.py creates a
registry↔forecasting slice dependency; extract the ModelFamily type and the
model_family_for helper into a neutral shared module (e.g., app/core/models or
app/shared/models), update both slices to import ModelFamily and
model_family_for from that new module instead of
app.features.forecasting.schemas, and remove any direct forecasting imports from
registry.schemas (and from the other referenced usages around lines 155–166) so
both registry and forecasting depend only on the shared/core module.
In `@PRPs/PRP-31-mlzoo-d-frontend-registry-explainability.md`:
- Around line 17-18: The scope statement currently says “one new backend
endpoint” but the PRP actually defines two REST endpoints
(/runs/{run_id}/feature-metadata and /jobs/{job_id}/feature-metadata); update
the top-level scope sentence to accurately state two backend endpoints (or
rephrase to “one or more backend endpoints” and explicitly list the two paths)
so the scope matches the detailed specification and removes ambiguity.
- Around line 1206-1211: The error mapping under "Error semantics (RFC 7807
application/problem+json)" incorrectly lists missing bundle file as 500; update
that bullet so the missing/deleted artifact case uses 422 UNPROCESSABLE_ENTITY
to match the rest of the PRP and backend tests (change the line that reads "500
— bundle file missing on disk (storage corruption)" to "422 — bundle file
missing on disk (storage corruption)"); ensure any references to this mapping in
the document or tests that expect 422 remain consistent with the symbol "Error
semantics (RFC 7807 application/problem+json)" and the "bundle file missing"
bullet.
---
Outside diff comments:
In `@app/features/forecasting/routes.py`:
- Around line 120-131: The code currently raises bare HTTPException in the
forecasting handlers (notably in train_model, predict, and the ValueError except
block shown) which bypasses the RFC7807 problem_details pipeline; replace those
HTTPException usages with the appropriate ForecastLabError subclasses (e.g.,
raise BadRequestError for input/validation issues and NotFoundError for missing
resources) so responses are emitted via app/core/problem_details.py; update the
except ValueError block to raise BadRequestError (including the original message
and preserving exception chaining) and scan the other HTTPException occurrences
around the predict/train_model code (lines ~222-237) to map them to
ForecastLabError subclasses accordingly.
---
Nitpick comments:
In `@frontend/src/components/explainability/feature-importance-panel.tsx`:
- Around line 30-35: FAMILY_DESCRIPTION is currently typed as Record<string,
string>, losing exhaustiveness checks; change its type to Record<ModelFamily,
string> and ensure ModelFamily is imported or referenced where this file uses it
(the same type used for the family parameter). Update the declaration of
FAMILY_DESCRIPTION to use ModelFamily so the compiler will catch missing or
renamed model families and keep the map in sync with the family parameter usage.
In `@frontend/src/hooks/use-feature-metadata.ts`:
- Around line 15-21: The queryFn currently returns the api(...) promise
directly; change it to an async function that awaits the API call to follow the
project's async/await guideline. Update each useQuery's queryFn (the handlers
that call
api<FeatureMetadataResponse>(`/forecasting/runs/${runId}/feature-metadata`) and
the other similar queryFn later in the file) to be async () => { return await
api<FeatureMetadataResponse>(...) } (or simply const result = await api(...);
return result) so the promise is handled with explicit async/await rather than
an implicit returned promise.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9d4a8e9-7df6-4ce9-8bf6-b6dac6a4379b
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (38)
.gitignorePRPs/PRP-31-mlzoo-d-frontend-registry-explainability.mdPRPs/PRP-MLZOO-B.2-feature-aware-backtesting.mdPRPs/PRP-MLZOO-C2-prophet-like-additive-model.mdREADME.mdapp/core/exceptions.pyapp/core/problem_details.pyapp/features/backtesting/routes.pyapp/features/backtesting/tests/test_routes_integration.pyapp/features/forecasting/feature_metadata.pyapp/features/forecasting/models.pyapp/features/forecasting/routes.pyapp/features/forecasting/schemas.pyapp/features/forecasting/service.pyapp/features/forecasting/tests/test_feature_metadata.pyapp/features/forecasting/tests/test_routes_feature_metadata.pyapp/features/jobs/schemas.pyapp/features/registry/schemas.pyapp/features/registry/tests/test_schemas.pyapp/shared/feature_frames/rows.pydocs/_base/ARCHITECTURE.mddocs/user-guide/dashboard-guide.mddocs/user-guide/feature-reference.mddocs/user-guide/getting-started.mdexamples/models/feature_frame_contract.mdexamples/models/model_interface.mdfrontend/src/components/common/model-family-badge.test.tsxfrontend/src/components/common/model-family-badge.tsxfrontend/src/components/explainability/feature-importance-panel.test.tsxfrontend/src/components/explainability/feature-importance-panel.tsxfrontend/src/hooks/use-feature-metadata.tsfrontend/src/pages/explorer/run-compare.tsxfrontend/src/pages/explorer/run-detail.tsxfrontend/src/pages/explorer/runs.tsxfrontend/src/pages/visualize/backtest.tsxfrontend/src/pages/visualize/forecast.tsxfrontend/src/types/api.tsscripts/dogfood-browser.sh
This was referenced May 20, 2026
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Cuts v0.2.17 for the 22-commit train queued on `dev` ahead of `main`.
Notable changes since v0.2.16
Browser dogfood evidence
PRP-31 Task 21 (scenarios 1-7) ran post-merge on `dev` — all 8 paths (S1–S7 incl. S6b) green. Verdict comment with per-scenario assertions and network-call evidence: #256 (comment)
Validation
Test plan
Summary by CodeRabbit
New Features
Documentation
Bug Fixes