docs(prp): refresh prp37 after model zoo contracts#304
Merged
Conversation
Reviewer's GuideDocs-only PR that (1) adds a detailed PRP-37 contract probe report verifying PRP-35/36 backend contracts on dev and (2) updates the PRP-37 spec to match the actual backend field names and capabilities (metrics dict shape, model health fields, planner assumptions, and probe filename). Flow diagram for PRP-37 contract probe and docs refreshflowchart TD
A[Task 1: run PRP-37 contract probe] --> B[Read PRP-35/36 backend schemas]
B --> C[Compare backend fields to PRP-37 spec]
C --> D{Drifts found}
D -- Yes --> E[Record drifts in prp-37-contract-probe-report.md]
E --> F[Patch PRP-37 doc field names and scope]
F --> G[Use updated spec for future implementation branch]
D -- No --> H[Keep PRP-37 spec unchanged]
H --> G
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The contract probe report still mentions the old checklist target path (
PRPs/ai_docs/contract-probe-report.md) in its header block even though the PRP now points atPRPs/ai_docs/prp-37-contract-probe-report.md; consider updating that note so the filename is consistent everywhere. - The "Pre-execution housekeeping" section in the probe report includes transient local-environment details (DB revision, unstashed changes) that will age quickly; consider trimming or moving those to a more ephemeral channel (e.g., HANDOFF or an issue comment) to keep this report focused on stable contract facts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The contract probe report still mentions the old checklist target path (`PRPs/ai_docs/contract-probe-report.md`) in its header block even though the PRP now points at `PRPs/ai_docs/prp-37-contract-probe-report.md`; consider updating that note so the filename is consistent everywhere.
- The "Pre-execution housekeeping" section in the probe report includes transient local-environment details (DB revision, unstashed changes) that will age quickly; consider trimming or moving those to a more ephemeral channel (e.g., HANDOFF or an issue comment) to keep this report focused on stable contract facts.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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
Refreshes
PRPs/PRP-37-forecast-intelligence-C-interactive-ui.mdagainst the final PRP-35 + PRP-36 backend contracts now live ondev(PR #299 + PR #303), and lands the PRP-37 Task 1 Contract Probe report. Docs-only — no UI implementation code, no backend code. PRP-37 has NOT been executed yet.What this PR contains
PRPs/ai_docs/prp-37-contract-probe-report.md(new) — Task 1 Contract Probe output. Verifies every PRP-35 / PRP-36 field PRP-37 wires UI to is present ondevat0e2ad9e, withfile:linecites. Verdict: GO with patches — 0 task DEFERRED, every[gate:PRP-35]and[gate:PRP-36]gate satisfied.PRPs/PRP-37-forecast-intelligence-C-interactive-ui.md(patched) — 4 drift items the probe surfaced.Drifts patched in PRP-37
Drift #1 — backend-field naming (mechanical)
bucketed_aggregate_metrics→bucketed_aggregated_metrics(backend ships the past-participle form onModelBacktestResult,app/features/backtesting/schemas.py:206). 9 occurrences renamed.aggregate_metrics.rmse→aggregated_metrics["rmse"]—aggregated_metricsis adict[str, float]and RMSE is a key inside it; there is noAggregateMetricsPydantic class (app/features/backtesting/metrics.py:349). 4 occurrences renamed.AggregateMetricsinterface dropped; surface RMSE asaggregated_metrics["rmse"]per backend's flat-dict shape.Drift #2 —
ModelHealthEntry.n_comparable_runsdoes not existModelHealthEntry.run_count: int(app/features/ops/schemas.py:313). 3 occurrences updated (Goal section, User-visible-behaviour/opsbullet, Task 21INSERTstep). UI pill relabelled "N runs evaluated" rather than "comparable runs"; a future PRP may add a filtered count.Drift #3 — invented
is_known_futureflag dropped from scopeis_known_futurefield exists on any ofPriceAssumption/PromotionAssumption/HolidayAssumption/InventoryAssumption/LifecycleAssumption(app/features/scenarios/schemas.py:37-119). Every planner assumption is hypothetical by definition. The "known future input vs hypothetical" per-row pill is dropped from Task 18 + Success Criteria + the desired-tree comment + the planner.tsx file note (6 prose locations). Themethodbadge (model_exogenousvsheuristic) — already backed byScenarioComparison.method— remains as the supported UX signal.Drift #4 — contract-probe report filename standardized
PRPs/ai_docs/prp-37-contract-probe-report.md(matches the prefixed convention already used byPRPs/ai_docs/prp-35-final-contract-snapshot.md).Why this lands separately from PRP-37 execution
The probe report is the gate the PRP itself defines: Task 1 must verify every cited field before Task 2+ can scaffold UI on top of it. Landing the probe + the docs patch first means the implementation branch starts with a self-consistent PRP and an in-repo, RAG-indexable probe report.
Issue reference
epic(forecast): Forecast Intelligence roadmap (PRP-35/36/37). No PRP-37-specific issue exists yet — this PR is part of the same forecast-intelligence epic.Commit-message scope
The commit lands as
docs(forecast)rather thandocs(prp)— theprpscope is not in.claude/rules/commit-format.md's allow-list and the PreToolUse hook rejects it; the closest semantically-accurate scope in the allow-list isforecast(the affected epic).Verification
git diff --statshowsPRPs/PRP-37-...md(+39/-40) and the newPRPs/ai_docs/prp-37-contract-probe-report.md(+185).git diff --checkclean (no whitespace errors).git diff app/ alembic/ frontend/empty).stash@{0}: On dev: local qwen3 rag demo changes before prp-35.Next step after merge
Branch
feat/forecast-interactive-uioffdevto execute PRP-37 Task 2+ against the cleaned contract.Summary by Sourcery
Refresh PRP-37 forecast-interactive-UI design docs to match the finalized PRP-35/36 backend contracts and add the PRP-37 Task 1 contract probe report.
Documentation: