Release: Feature Engineering + Forecasting Module (PRP-4 & PRP-5)#29
Merged
Conversation
* docs: update INITIAL-4 and INITIAL-5 with additional references - Add scikit-learn, mlforecast, and sktime documentation links - Add considerations for imputation logic, agent tooling, and computation overhead - Add model persistence documentation references Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat(featuresets): implement time-safe feature engineering layer Add complete feature engineering module with: - Pydantic schemas for feature configuration (lag, rolling, calendar, exogenous, imputation) - FeatureEngineeringService with CRITICAL leakage prevention: - Lag features use positive shift() only - Rolling features use shift(1) BEFORE rolling to exclude current observation - Group-aware operations prevent cross-series leakage - Cutoff date filtering before any computation - FastAPI endpoints: POST /featuresets/compute and /featuresets/preview - Comprehensive test suite (55 tests) including leakage prevention tests - Example demo script Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: update INITIAL-5.md * docs: update documentation for Phase 3 Feature Engineering - README.md: Add featuresets module to project structure and API endpoints - docs/ARCHITECTURE.md: Add Feature Engineering section (section 6) - docs/PHASE-index.md: Mark Phase 3 as completed with summary - docs/PHASE/3-FEATURE_ENGINEERING.md: Create detailed phase documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix(featuresets): address code review feedback and prevent data leakage Routes: - Validate store_id/product_id presence (no silent defaults to 0) - Convert ValueError for unsupported date types to HTTP 400 Service: - Add expanding_mean imputation strategy (time-safe alternative) - Add warnings when bfill/mean strategies are used (leakage risk) - Fix price_pct_change_7d to use shift(1) before pct_change Schemas: - Add expanding_mean to ImputationConfig Literal type - Document time-safety of each imputation strategy - Fix PreviewFeaturesRequest docstring: GET → POST Documentation: - Convert bare URLs to markdown links in INITIAL-4.md, INITIAL-5.md - Fix PRP-4 to show POST for preview endpoint Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * style: format schemas.py Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Gabe@w7dev <gabor@w7-7.net> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Service: - Fix min_periods falsy check to explicit None check (preserves 0) Tests: - Add expanding_mean to test_valid_strategies, expect 6 strategies Documentation: - Update PR reference from #24 to #25 - Fix all GET /featuresets/preview to POST in PRP-4 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* docs: update DAILY-FLOW.md for Phase 4 Forecasting Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: add PRP-5 for Forecasting module Comprehensive PRP including: - Model zoo (naive, seasonal naive, moving average) - Unified BaseForecaster interface (fit/predict/serialize) - ModelBundle persistence with joblib - 15 ordered implementation tasks - 40+ test cases specified - Integration with FeatureEngineeringService Confidence: 8/10 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Gabe@w7dev <gabor@w7-7.net> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
…#28) * feat(forecasting): implement baseline model zoo and unified interface Add forecasting module (PRP-5) with: - BaseForecaster ABC with scikit-learn-style interface (fit/predict) - NaiveForecaster, SeasonalNaiveForecaster, MovingAverageForecaster - ModelBundle persistence with joblib serialization - POST /forecasting/train and /forecasting/predict endpoints - ForecastingService for orchestration - 81 unit tests covering schemas, models, persistence, and service - Example scripts demonstrating each baseline model - LightGBM placeholder (feature-flagged, not yet implemented) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * docs: Update documentation for forecasting module (PRP-5) - Add forecasting API endpoints to README.md with examples - Update ARCHITECTURE.md with forecasting implementation details - Add scikit-learn and joblib to dependencies list - Add forecasting config variables to .env.example - Mark forecasting module as IMPLEMENTED in architecture docs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: address CI lint and type check failures - Add type: ignore for intentional type mismatch in frozen config test - Add S101 ignore for examples/ to allow assert statements Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Gabe@w7dev <gabor@w7-7.net> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
![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, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
📝 WalkthroughWalkthroughAdds a complete forecasting feature: baseline forecasters, Pydantic schemas, Joblib persistence with metadata and deterministic hashing, a ForecastingService (DB integration), FastAPI train/predict endpoints, settings/env additions, tests, docs, and examples. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Routes as "FastAPI Routes"
participant Service as "ForecastingService"
participant DB as "Database"
participant Factory as "ModelFactory"
participant Forecaster as "BaseForecaster"
participant FS as "FileSystem/ModelBundle"
Client->>Routes: POST /forecasting/train (store, product, dates, config)
Routes->>Service: train_model(request)
Service->>DB: query training data
DB-->>Service: TrainingData
Service->>Factory: create forecaster from config
Factory-->>Service: Forecaster instance
Service->>Forecaster: fit(y)
Forecaster-->>Service: FitResult
Service->>FS: save_model_bundle(bundle)
FS-->>Service: model_path
Service-->>Routes: TrainResponse
Routes-->>Client: 200 + model_path, config_hash
sequenceDiagram
participant Client
participant Routes as "FastAPI Routes"
participant Service as "ForecastingService"
participant FS as "FileSystem/ModelBundle"
participant Forecaster as "BaseForecaster"
Client->>Routes: POST /forecasting/predict (store, product, horizon, model_path)
Routes->>Service: predict(request)
Service->>FS: load_model_bundle(model_path)
FS-->>Service: ModelBundle (model + metadata)
Service->>Forecaster: predict(horizon)
Forecaster-->>Service: forecast values
Service-->>Routes: PredictResponse (forecasts[], config_hash)
Routes-->>Client: 200 + forecasts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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: 10
🤖 Fix all issues with AI agents
In `@app/features/forecasting/models.py`:
- Around line 329-338: The constructor (__init__) for the moving average
forecaster must validate that window_size is an integer >= 1 to prevent invalid
states used by fit() and predict(); update the __init__ (the method that assigns
self.window_size and self._forecast_value) to check the incoming window_size and
raise a ValueError with a clear message if window_size < 1 (optionally also
coerce/validate type to int), so downstream logic in fit() and predict() can
assume a valid positive window size.
- Around line 234-243: The constructor __init__ currently accepts any
season_length which can lead to a modulo-by-zero in predict(); add a validation
in the class initializer (the __init__ that sets self.season_length) to require
season_length >= 1 and raise a ValueError with a clear message if violated, so
callers get an immediate, descriptive error instead of a runtime failure in
predict().
In `@app/features/forecasting/persistence.py`:
- Around line 114-133: The load_model_bundle function currently calls
joblib.load on a user-supplied path (via the /forecasting/predict flow), which
allows deserialization of arbitrary objects; restrict loading to trusted
artifacts by validating the resolved path is inside the server's
forecast_model_artifacts_dir (or replace direct loading with a server-side
lookup mapping model identifiers to approved files) before calling joblib.load;
update load_model_bundle to resolve and check Path(...).resolve() against the
base directory (forecast_model_artifacts_dir) and raise an error if outside the
allowlist, ensuring only approved model artifacts are deserialized.
In `@app/features/forecasting/service.py`:
- Around line 218-239: The code currently passes user-controlled model_path from
PredictRequest into load_model_bundle (which uses joblib.load); to prevent path
traversal/unsafe deserialization, validate model_path before loading: resolve
model_path to an absolute path (use pathlib.Path.resolve or
os.path.abspath+normpath), ensure it has a .joblib extension, and ensure the
resolved path is inside your configured artifacts base directory (compare prefix
against the configured ARTIFACTS_DIR Path). If any check fails, raise a
ValueError (or appropriate error) instead of calling load_model_bundle; only
call load_model_bundle once the path is validated. Also log the rejection reason
for auditing and keep references to model_path, load_model_bundle, and
PredictRequest in the change notes.
In `@app/features/forecasting/tests/test_service.py`:
- Around line 229-263: Patch get_settings before constructing
ForecastingService: move the with
patch("app.features.forecasting.service.get_settings") block so it wraps the
ForecastingService() instantiation, return a MagicMock settings with
forecast_random_seed and forecast_model_artifacts_dir set to the tmpdir, then
call service.train_model(...) as before; this ensures ForecastingService
initialization reads the patched get_settings and uses the temporary artifacts
dir.
In `@docs/ARCHITECTURE.md`:
- Around line 239-248: The doc table uses outdated config names; update the
Model Zoo table entries for the seasonal_naive and moving_average models to
match code: replace `seasonal_period` with `season_length` for `seasonal_naive`
and replace `window` with `window_size` for `moving_average` (ensure the Config
Parameters column reflects `season_length` and `window_size` and any default
values match the implementation).
In `@PRPs/PRP-5-forecasting.md`:
- Around line 216-244: The markdown tables in this file violate MD060 (compact
table pipe spacing); locate the tables near the ModelConfig /
LightGBMModelConfig / TrainRequest sections (use those identifiers to find the
surrounding text) and update each table row to have a single space on both sides
of every pipe delimiter (e.g., change "a|b" or "|b" to "a | b" and "| b|" to "|
b |") so all table pipe spacing is consistent with the compact style rule.
- Around line 166-177: Replace the bold-only warning lines (e.g., "CRITICAL:
Recursive forecasting propagates errors", "CRITICAL: All forecasters must be
deterministic with fixed random_state", "CRITICAL: Multi-horizon forecasting
updates lags recursively", "CRITICAL: Feature engineering must use cutoff_date =
last training date") with proper ATX headings (use `##` or `###`) so they are
recognized as headings by markdownlint (MD036); ensure each critical statement
becomes an explicit heading line (e.g., "## CRITICAL: Recursive forecasting
propagates errors") and not just bold text, and keep the existing text content
unchanged under each new heading.
- Around line 134-146: The markdown contains fenced code-tree blocks without a
language tag (MD040); update each fenced block in PRPs/PRP-5-forecasting.md that
shows the project tree (the blocks showing "├── routes.py …" and
"examples/models/ …") to include a language identifier such as text (change ```
to ```text) so linters stop flagging them; search for the fence around the tree
listings and add the identifier to each opening fence.
In `@README.md`:
- Around line 189-229: The training example for the POST /forecasting/train
request uses the wrong config key "seasonal_period"; update the JSON example's
config object to use "season_length" instead (i.e., change "seasonal_period": 7
to "season_length": 7) so it matches the schema/validation for the forecasting
training endpoint and the config parsing logic.
🧹 Nitpick comments (5)
.env.example (1)
18-23: Reorder FORECAST_ keys to satisfy dotenv-linter.*This avoids UnorderedKey warnings and keeps the block consistent.
♻️ Suggested reorder
-FORECAST_RANDOM_SEED=42 -FORECAST_DEFAULT_HORIZON=14 -FORECAST_MAX_HORIZON=90 -FORECAST_MODEL_ARTIFACTS_DIR=./artifacts/models -FORECAST_ENABLE_LIGHTGBM=false +FORECAST_DEFAULT_HORIZON=14 +FORECAST_ENABLE_LIGHTGBM=false +FORECAST_MAX_HORIZON=90 +FORECAST_MODEL_ARTIFACTS_DIR=./artifacts/models +FORECAST_RANDOM_SEED=42examples/models/model_interface.md (1)
128-148: Add language identifiers to fenced formula blocks.This clears MD040 warnings in markdownlint.
📝 Suggested tweak
-``` +```text ŷ[t+h] = y[t] for all h ∈ [1, horizon]-
+text
ŷ[t+h] = y[t + h - m] where m = season_lengthapp/features/forecasting/tests/test_models.py (1)
17-24: Avoid asserting private state in unit tests.Asserting
_last_valuecouples the test to implementation details; prefer asserting via the publicpredictbehavior.♻️ Suggested tweak
- assert model._last_value == 60.0 # Last value in 1-60 sequence + assert model.predict(horizon=1)[0] == 60.0 # Last value in 1-60 sequenceexamples/models/baseline_seasonal.py (1)
49-54: Prefer explicit validation overassertin runnable examples.
assertis stripped withpython -O; an explicit check keeps the verification active.♻️ Suggested tweak
- assert np.array_equal(forecasts[:7], forecasts[7:]), "Seasonal pattern should repeat!" - print(" ✓ Pattern repeats correctly") + if not np.array_equal(forecasts[:7], forecasts[7:]): + raise ValueError("Seasonal pattern should repeat!") + print(" ✓ Pattern repeats correctly")app/features/forecasting/tests/test_persistence.py (1)
159-217: Avoid assuming.joblibsuffix in tests.
Use the path returned bysave_model_bundleto make tests robust if the fixture path already includes an extension.♻️ Proposed refactor
- save_model_bundle(bundle, tmp_model_path) - loaded_bundle = load_model_bundle(tmp_model_path + ".joblib") + saved_path = save_model_bundle(bundle, tmp_model_path) + loaded_bundle = load_model_bundle(saved_path) @@ - save_model_bundle(bundle, tmp_model_path) - loaded_bundle = load_model_bundle(tmp_model_path + ".joblib") + saved_path = save_model_bundle(bundle, tmp_model_path) + loaded_bundle = load_model_bundle(saved_path) @@ - save_model_bundle(bundle, tmp_model_path) - original_hash = bundle.bundle_hash - - loaded_bundle = load_model_bundle(tmp_model_path + ".joblib") + saved_path = save_model_bundle(bundle, tmp_model_path) + original_hash = bundle.bundle_hash + + loaded_bundle = load_model_bundle(saved_path)
Comment on lines
+216
to
+244
| model_type: Literal["moving_average"] = "moving_average" | ||
| window_size: int = Field(default=7, ge=1, le=90, description="Window size for averaging") | ||
|
|
||
|
|
||
| class LightGBMModelConfig(ModelConfigBase): | ||
| """Config for LightGBM regressor (feature-flagged).""" | ||
| model_type: Literal["lightgbm"] = "lightgbm" | ||
| n_estimators: int = Field(default=100, ge=10, le=1000) | ||
| max_depth: int = Field(default=6, ge=1, le=20) | ||
| learning_rate: float = Field(default=0.1, ge=0.001, le=1.0) | ||
| feature_config_hash: str | None = Field(default=None, description="Hash of FeatureSetConfig used") | ||
|
|
||
|
|
||
| # Union type for all configs | ||
| ModelConfig = NaiveModelConfig | SeasonalNaiveModelConfig | MovingAverageModelConfig | LightGBMModelConfig | ||
|
|
||
|
|
||
| class TrainRequest(BaseModel): | ||
| """Request body for POST /forecasting/train.""" | ||
| model_config = ConfigDict(strict=True) | ||
|
|
||
| store_id: int = Field(..., ge=1) | ||
| product_id: int = Field(..., ge=1) | ||
| train_start_date: date | ||
| train_end_date: date | ||
| config: ModelConfig | ||
|
|
||
| @field_validator("train_end_date") | ||
| @classmethod |
There was a problem hiding this comment.
Fix table pipe spacing for compact style (MD060).
Add spaces around | in the affected tables to satisfy the configured table style.
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 216-216: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 216-216: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 216-216: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 216-216: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 216-216: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 216-216: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
🤖 Prompt for AI Agents
In `@PRPs/PRP-5-forecasting.md` around lines 216 - 244, The markdown tables in
this file violate MD060 (compact table pipe spacing); locate the tables near the
ModelConfig / LightGBMModelConfig / TrainRequest sections (use those identifiers
to find the surrounding text) and update each table row to have a single space
on both sides of every pipe delimiter (e.g., change "a|b" or "|b" to "a | b" and
"| b|" to "| b |") so all table pipe spacing is consistent with the compact
style rule.
Security improvements: - Add constructor validation for season_length >= 1 in SeasonalNaiveForecaster - Add constructor validation for window_size >= 1 in MovingAverageForecaster - Add path traversal prevention in ForecastingService.predict() - Validate .joblib extension and artifacts directory containment - Log rejection reasons for security auditing Test improvements: - Fix get_settings patching to wrap ForecastingService construction - Add tests for constructor validation - Add tests for path traversal and extension validation Documentation fixes: - Fix config parameter names in ARCHITECTURE.md (season_length, window_size) - Fix README example to use season_length instead of seasonal_period - Fix markdown issues in PRP-5 (code fences, ATX headings) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
w7-learn
approved these changes
Feb 1, 2026
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@app/features/forecasting/models.py`:
- Around line 312-323: The set_params method on SeasonalNaiveForecaster
currently assigns attributes directly and can bypass constructor validation,
allowing invalid values (e.g., zero/negative sizes) to be set; update set_params
to validate incoming params before applying them (reuse the same checks the
constructor uses or call an internal _validate_params helper), reject or raise
ValueError for invalid invariants (like non-positive seasonal_period or window
sizes), and only setattr after validation; apply the same pattern to the
analogous setter at lines ~411-422 so all parameter-updating paths enforce the
same invariants.
In `@docs/ARCHITECTURE.md`:
- Around line 243-247: The table violates MD060 (compact-table spacing); update
the separator and cell padding so every pipe has a single space after and before
cell content. Replace the separator row (currently
|-------|-------------|-------------------|) with a spaced version (e.g., | ---
| --- | --- |) and ensure each data/header row around the headers "Model",
"Description", "Config Parameters" and the rows containing `naive`,
`seasonal_naive`, and `moving_average` has one space between the pipes and the
cell text.
In `@PRPs/PRP-5-forecasting.md`:
- Around line 828-835: The markdown currently lists the filename
app/features/forecasting/__init__.py without being rendered as code which
triggers MD050; update the Task 15 section so the filename and any inline file
references are rendered as code (e.g., wrap __init__.py and the FILE: line in
backticks or include the whole file header in a fenced code block) so the file
name appears as code; ensure the change is applied in the Task 15 block where
FILE: app/features/forecasting/__init__.py and the __all__ export instructions
are declared.
🧹 Nitpick comments (1)
app/features/forecasting/service.py (1)
251-252: Reuse the persistence-layer base_dir guard.
This keeps path validation centralized and resilient if this call site changes.🔁 Suggested tweak
- bundle = load_model_bundle(resolved_path) + bundle = load_model_bundle(resolved_path, base_dir=artifacts_dir)
Comment on lines
+312
to
+323
| def set_params(self, **params: Any) -> SeasonalNaiveForecaster: # noqa: ANN401 | ||
| """Set model parameters. | ||
|
|
||
| Args: | ||
| **params: Parameter names and values to set. | ||
|
|
||
| Returns: | ||
| self (for method chaining). | ||
| """ | ||
| for key, value in params.items(): | ||
| setattr(self, key, value) | ||
| return self |
There was a problem hiding this comment.
Validate invariants in set_params to avoid invalid state.
set_params can currently bypass the constructor guards and create zero/negative sizes, leading to runtime errors later.
🛠️ Suggested fix
def set_params(self, **params: Any) -> SeasonalNaiveForecaster: # noqa: ANN401
"""Set model parameters."""
for key, value in params.items():
+ if key == "season_length" and value < 1:
+ raise ValueError("season_length must be >= 1")
setattr(self, key, value)
return self def set_params(self, **params: Any) -> MovingAverageForecaster: # noqa: ANN401
"""Set model parameters."""
for key, value in params.items():
+ if key == "window_size" and value < 1:
+ raise ValueError("window_size must be >= 1")
setattr(self, key, value)
return selfAlso applies to: 411-422
🤖 Prompt for AI Agents
In `@app/features/forecasting/models.py` around lines 312 - 323, The set_params
method on SeasonalNaiveForecaster currently assigns attributes directly and can
bypass constructor validation, allowing invalid values (e.g., zero/negative
sizes) to be set; update set_params to validate incoming params before applying
them (reuse the same checks the constructor uses or call an internal
_validate_params helper), reject or raise ValueError for invalid invariants
(like non-positive seasonal_period or window sizes), and only setattr after
validation; apply the same pattern to the analogous setter at lines ~411-422 so
all parameter-updating paths enforce the same invariants.
Comment on lines
+243
to
+247
| | Model | Description | Config Parameters | | ||
| |-------|-------------|-------------------| | ||
| | `naive` | Last observed value | None | | ||
| | `seasonal_naive` | Previous season value | `season_length` (e.g., 7 for weekly) | | ||
| | `moving_average` | Mean of last N observations | `window_size` (default: 7) | |
There was a problem hiding this comment.
Fix MD060 table pipe spacing.
markdownlint flags compact-table spacing here.
✅ Suggested fix
-| Model | Description | Config Parameters |
-| `naive` | Last observed value | None |
-| `seasonal_naive` | Previous season value | `season_length` (e.g., 7 for weekly) |
-| `moving_average` | Mean of last N observations | `window_size` (default: 7) |
+| Model | Description | Config Parameters |
+| `naive` | Last observed value | None |
+| `seasonal_naive` | Previous season value | `season_length` (e.g., 7 for weekly) |
+| `moving_average` | Mean of last N observations | `window_size` (default: 7) |🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 244-244: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the right for style "compact"
(MD060, table-column-style)
[warning] 244-244: Table column style
Table pipe is missing space to the left for style "compact"
(MD060, table-column-style)
🤖 Prompt for AI Agents
In `@docs/ARCHITECTURE.md` around lines 243 - 247, The table violates MD060
(compact-table spacing); update the separator and cell padding so every pipe has
a single space after and before cell content. Replace the separator row
(currently |-------|-------------|-------------------|) with a spaced version
(e.g., | --- | --- | --- |) and ensure each data/header row around the headers
"Model", "Description", "Config Parameters" and the rows containing `naive`,
`seasonal_naive`, and `moving_average` has one space between the pipes and the
cell text.
Comment on lines
+828
to
+835
| ### Task 15: Update module __init__.py exports | ||
|
|
||
| ```yaml | ||
| FILE: app/features/forecasting/__init__.py | ||
| ACTION: MODIFY | ||
| IMPLEMENT: | ||
| - Export all public classes | ||
| - __all__ list (sorted alphabetically) |
There was a problem hiding this comment.
Avoid MD050 by rendering __init__.py as code.
✅ Suggested fix
-### Task 15: Update module __init__.py exports
+### Task 15: Update module `__init__.py` exports🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 828-828: Strong style
Expected: asterisk; Actual: underscore
(MD050, strong-style)
[warning] 828-828: Strong style
Expected: asterisk; Actual: underscore
(MD050, strong-style)
🤖 Prompt for AI Agents
In `@PRPs/PRP-5-forecasting.md` around lines 828 - 835, The markdown currently
lists the filename app/features/forecasting/__init__.py without being rendered
as code which triggers MD050; update the Task 15 section so the filename and any
inline file references are rendered as code (e.g., wrap __init__.py and the
FILE: line in backticks or include the whole file header in a fenced code block)
so the file name appears as code; ensure the change is applied in the Task 15
block where FILE: app/features/forecasting/__init__.py and the __all__ export
instructions are declared.
This was referenced Feb 1, 2026
This was referenced May 19, 2026
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
2 participants
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
This PR merges the dev branch into main, including two major features:
PRP-4: Feature Engineering Layer
POST /featuresets/computeandPOST /featuresets/previewendpointsPRP-5: Forecasting Module
naive,seasonal_naive,moving_averageBaseForecasterinterface following scikit-learn conventionsModelBundlepersistence with joblib serializationPOST /forecasting/trainandPOST /forecasting/predictendpointsAdditional Changes
Test plan
uv run pytest -v)uv run mypy app/anduv run pyright app/)uv run ruff check .)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.