Bill
cb2024f857
Once every elective set is pinned, the Top Plans panel now renders the user's completed selection as a single "Your Plan" card showing the achievement count and pinned courses. Previously the panel went blank because the all-pinned branch in useAppState cleared topPlans, even though the spec strip was already showing the achievement. Synthesizes a single PlanOutcome from pinnedAssignments + optimizationResult.achieved + the priority scorer; TopPlans detects the all-pinned state via ELECTIVE_SETS.every(...) and bypasses the length>0 filter so a 0-spec completed plan still renders honestly.
112 lines
6.3 KiB
Markdown
112 lines
6.3 KiB
Markdown
## Context
|
|
|
|
`useAppState` runs a decision-tree search via a debounced worker effect to populate `topPlans`. When `openSetIds.length === 0` (every set pinned), the effect short-circuits at `appState.ts:185-192`:
|
|
|
|
```ts
|
|
if (openSetIds.length === 0) {
|
|
setTreeResults([]);
|
|
setTopPlans([]);
|
|
setTopPlansPartial(false);
|
|
setSearchProgress(null);
|
|
setTreeLoading(false);
|
|
return;
|
|
}
|
|
```
|
|
|
|
Meanwhile, `optimizationResult` (a separate `useMemo` of `optimize(...)`) IS computed for the same pinned set, and the spec strip shows the correct achievement count. The two views drift out of sync exactly when the user has just finished selecting their courses — the moment when "what did I end up with?" feedback matters most.
|
|
|
|
`TopPlans.tsx:41` further filters out plans with no achievement (`visible = plans.filter(p => p.achievedSpecs.length > 0)`), and renders "No plans yet achieve a specialization…" (lines 100-104) when nothing is visible. So even if we fed a synthesized plan with zero achievements through, today's filter would still hide it.
|
|
|
|
## Goals / Non-Goals
|
|
|
|
**Goals:**
|
|
- When all sets are pinned, Top Plans shows the user's completed plan with its actual achievement count
|
|
- Header reads "Your Plan" so the singular plan doesn't feel like a search result
|
|
- A 0-spec completed plan still renders (the courses are visible; the count is honest)
|
|
- No regression in the open-sets-> 0 cases (search-driven top-K, partial cache hits, streaming improvements)
|
|
|
|
**Non-Goals:**
|
|
- New `PlanRow` styling for the completed-plan case
|
|
- Removing or hiding the adopt-button (every set is already pinned, so the button is a no-op but harmless)
|
|
- Restructuring the search progress strip
|
|
- Caching the synthesized plan (it's free to compute on every render)
|
|
|
|
## Decisions
|
|
|
|
### Synthesize a single `PlanOutcome` in the appState early return
|
|
|
|
Replace the `setTopPlans([])` line with:
|
|
|
|
```ts
|
|
const scorer = makePriorityScorer(state.ranking);
|
|
const completed: PlanOutcome = {
|
|
courseAssignments: pinnedAssignments,
|
|
achievedSpecs: optimizationResult.achieved,
|
|
priorityScore: scorer(optimizationResult.achieved),
|
|
};
|
|
setTopPlans([completed]);
|
|
```
|
|
|
|
`pinnedAssignments` is already computed earlier in the hook. `optimizationResult.achieved` is already available. `makePriorityScorer` is already imported (used elsewhere in the solver layer, can be imported here cheaply).
|
|
|
|
**Alternative considered:** Build a parallel "completed plan" panel component and switch on `openSetIds.length === 0`. Rejected — the existing `PlanRow` already renders exactly the data we need. Adding a parallel layout is more code and more drift surface for one edge case.
|
|
|
|
### Drop the `achievedSpecs.length > 0` filter when there's a single completed plan
|
|
|
|
`TopPlans.tsx` currently does:
|
|
|
|
```ts
|
|
const visible = plans.filter((p) => p.achievedSpecs.length > 0);
|
|
```
|
|
|
|
For the all-pinned case, the synthesized plan must render even when the achievement is empty. The cleanest signal is to detect "is this a completed plan" by inspecting `pinnedCourses` (every elective set has a pinned course), and skip the filter in that case:
|
|
|
|
```ts
|
|
const allPinned = ELECTIVE_SETS.every((s) => pinnedCourses[s.id]);
|
|
const visible = allPinned ? plans : plans.filter((p) => p.achievedSpecs.length > 0);
|
|
```
|
|
|
|
`pinnedCourses` is already a prop on `TopPlans`. No new wiring.
|
|
|
|
**Alternative considered:** Drop the filter unconditionally. Rejected — when the worker is mid-search and only emitted low-achievement candidates, the existing "Searching for high-priority plans…" placeholder is the right UX. The filter exists for a reason; we just need to gate it on whether selection is complete.
|
|
|
|
### Header copy: "Your Plan" when complete, "Top Plans" otherwise
|
|
|
|
```ts
|
|
const heading = allPinned ? 'Your Plan' : 'Top Plans';
|
|
```
|
|
|
|
The "ranked by specs achieved" subtitle stays on the multi-plan path. The single-plan path drops it (one plan can't be ranked).
|
|
|
|
### Empty placeholder copy update
|
|
|
|
When `!loading && visible.length === 0` and `allPinned` is true, the existing "No plans yet achieve a specialization…" message would be misleading (selection IS complete). Since the synthesis above guarantees `visible.length === 1` in the all-pinned branch, this placeholder will never fire in that path. No copy change needed there.
|
|
|
|
### Adopt-button behavior
|
|
|
|
The adopt button calls `onAdopt(plan.courseAssignments)`, which dispatches `pinCourse` for every set. When everything is already pinned, this is a no-op (idempotent). Leaving it visible is harmless. Hiding it would add a special case for no benefit.
|
|
|
|
## Risks / Trade-offs
|
|
|
|
- **Synthesized plan vs. eventual full search** → No risk. The all-pinned branch returns before the search effect runs, so there's no race where the worker overwrites the synthesized plan.
|
|
- **`allPinned` check uses `ELECTIVE_SETS.every(...)`** → Imports `ELECTIVE_SETS` into `TopPlans.tsx`. Already imported there (used for `setNameById`). No new dependency.
|
|
- **Synthesizing on every render** → `useMemo` not strictly required (the synthesis is cheap), but the existing pattern in `useAppState` uses `setTopPlans` once per effect run, which is fine.
|
|
- **Unpin-after-complete UX** → When the user unpins a single course after completing the plan, `openSetIds.length` becomes 1, the effect re-runs as a normal partial-cache search, and the synthesized plan path is no longer used. The cached subset (if any) renders immediately, exactly as today. Verified by inspection — no special handling needed.
|
|
- **External-credits achievement** → `optimizationResult.achieved` is already external-credit-aware (per the prior change). No additional plumbing.
|
|
|
|
## Migration Plan
|
|
|
|
Single-PR change. No data migration. No persisted state changes.
|
|
|
|
1. Update `useAppState` to synthesize the completed plan in the all-pinned branch.
|
|
2. Update `TopPlans` to detect the all-pinned case and switch the header copy and visibility filter.
|
|
3. Browser verify: pin all 12 sets, watch Top Plans render the completed plan with the same achievement count as the spec strip. Unpin one set: search resumes normally.
|
|
4. Bump version to `1.5.1`; CHANGELOG entry.
|
|
|
|
Rollback: revert. v1.5.0 behavior restored.
|
|
|
|
## Open Questions
|
|
|
|
- Should the adopt button on the synthesized plan be replaced with a "Reset" or hidden? Defer to UI polish — keeping it inert is the smallest change.
|
|
- Should the schedule panel get a "plan complete" affordance too? Out of scope for this change; could be a follow-up.
|