Add design docs for achievable status fix and course description popups change artifacts
This commit is contained in:
99
docs/superpowers/plans/2026-03-27-achievable-status-fix.md
Normal file
99
docs/superpowers/plans/2026-03-27-achievable-status-fix.md
Normal file
@@ -0,0 +1,99 @@
|
||||
# Achievable Status Fix Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Fix `determineStatuses` so specs that are infeasible alongside achieved specs show as "unreachable" instead of falsely "achievable."
|
||||
|
||||
**Architecture:** Add a feasibility check in `determineStatuses()` after the upper bound check passes. Uses the existing `checkWithS2` helper to LP-solve whether the spec can be achieved alongside the already-achieved set.
|
||||
|
||||
**Tech Stack:** TypeScript, Vitest, javascript-lp-solver
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Write failing test for the bug scenario
|
||||
|
||||
**Files:**
|
||||
- Modify: `app/src/solver/__tests__/optimizer.test.ts`
|
||||
|
||||
- [ ] **Step 1: Add test case for LCM being falsely marked achievable**
|
||||
|
||||
Add to the `determineStatuses` describe block:
|
||||
|
||||
```typescript
|
||||
it('marks spec as unreachable when infeasible alongside achieved specs due to credit sharing', () => {
|
||||
// Bug scenario: CRF+STR achieved, LCM has 10 credit upper bound but
|
||||
// shared courses (spr3, fall3) are consumed by CRF/STR
|
||||
const selectedCourses = [
|
||||
'spr1-collaboration', // LCM, MGT
|
||||
'spr2-financial-services', // BNK, CRF, FIN, FIM
|
||||
'spr3-mergers-acquisitions', // CRF, FIN, LCM, STR(S1)
|
||||
'spr4-foundations-entrepreneurship', // ENT, MGT, STR(S1)
|
||||
'spr5-corporate-finance', // CRF, FIN
|
||||
'sum1-global-immersion', // GLB
|
||||
'sum2-business-drivers', // STR(S1)
|
||||
'sum3-valuation', // BNK, CRF, FIN, FIM
|
||||
'fall1-managing-change', // LCM, MGT, STR(S2)
|
||||
'fall2-decision-models', // MGT, MTO
|
||||
'fall3-corporate-governance', // LCM, MGT, SBI, STR(S1)
|
||||
'fall4-game-theory', // MGT, STR(S1)
|
||||
];
|
||||
const achieved = ['CRF', 'STR', 'MGT'];
|
||||
const statuses = determineStatuses(selectedCourses, [], achieved);
|
||||
|
||||
// LCM upper bound is 10 (>= 9) but infeasible alongside CRF+STR+MGT
|
||||
expect(statuses['LCM']).toBe('unreachable');
|
||||
});
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run the test to verify it fails**
|
||||
|
||||
Run: `cd /home/bill/dev/emba-course-solver/app && npx vitest run src/solver/__tests__/optimizer.test.ts -t "marks spec as unreachable when infeasible alongside achieved specs"`
|
||||
|
||||
Expected: FAIL — `statuses['LCM']` is `'achievable'` but expected `'unreachable'`
|
||||
|
||||
### Task 2: Implement the feasibility check in determineStatuses
|
||||
|
||||
**Files:**
|
||||
- Modify: `app/src/solver/optimizer.ts:146-185`
|
||||
|
||||
- [ ] **Step 3: Add feasibility check after the upper bound gate**
|
||||
|
||||
In `app/src/solver/optimizer.ts`, in the `determineStatuses` function, replace:
|
||||
|
||||
```typescript
|
||||
statuses[spec.id] = 'achievable';
|
||||
```
|
||||
|
||||
(line 181) with:
|
||||
|
||||
```typescript
|
||||
// Verify spec is actually feasible alongside already-achieved specs
|
||||
const testSet = [...achieved, spec.id];
|
||||
const feasResult = checkWithS2(selectedCourseIds, testSet);
|
||||
statuses[spec.id] = feasResult.feasible ? 'achievable' : 'unreachable';
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run the failing test to verify it now passes**
|
||||
|
||||
Run: `cd /home/bill/dev/emba-course-solver/app && npx vitest run src/solver/__tests__/optimizer.test.ts -t "marks spec as unreachable when infeasible alongside achieved specs"`
|
||||
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Run the full test suite to check for regressions**
|
||||
|
||||
Run: `cd /home/bill/dev/emba-course-solver/app && npx vitest run`
|
||||
|
||||
Expected: All tests pass
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
cd /home/bill/dev/emba-course-solver
|
||||
git add app/src/solver/optimizer.ts app/src/solver/__tests__/optimizer.test.ts
|
||||
git commit -m "fix: mark specs as unreachable when infeasible alongside achieved specs
|
||||
|
||||
determineStatuses() was marking specs as 'achievable' based solely on
|
||||
per-specialization upper bounds, ignoring credit sharing with achieved
|
||||
specs. Now performs an LP feasibility check to verify the spec can
|
||||
actually be achieved alongside the current achieved set."
|
||||
```
|
||||
@@ -0,0 +1,60 @@
|
||||
# Fix: "Achievable" status ignores credit sharing with achieved specs
|
||||
|
||||
**Date:** 2026-03-27
|
||||
**Type:** Bugfix
|
||||
|
||||
## Problem
|
||||
|
||||
`determineStatuses()` marks specializations as "achievable" based solely on per-specialization upper bounds (`computeUpperBounds`), which ignore credit sharing between specializations. A spec can show "achievable" (upper bound >= 9) even when it's infeasible alongside the already-achieved higher-priority specs because shared courses have committed their credits elsewhere.
|
||||
|
||||
**Reproduction scenario:**
|
||||
- 11 courses selected, Fall 1 open
|
||||
- Priority: CRF > STR > LCM > MGT
|
||||
- LCM upper bound = 10 (from spr1-collaboration, spr3-M&A, fall1-managing-change, fall3-corporate-governance)
|
||||
- CRF + STR achieved, consuming credits from spr3 and fall3
|
||||
- LCM shows "Achievable" but `checkFeasibility([all courses], ['CRF', 'STR', 'LCM'])` is infeasible for all S2 choices
|
||||
- Selecting Managing Change for Fall 1 achieves MGT (priority 4) instead of LCM (priority 3)
|
||||
|
||||
## Root Cause
|
||||
|
||||
`determineStatuses()` in `optimizer.ts:146-185` checks only:
|
||||
1. Whether the spec is in the achieved set
|
||||
2. Whether the required course gate passes
|
||||
3. Whether `computeUpperBounds` >= 9 (per-spec, ignoring sharing)
|
||||
|
||||
It never checks whether the spec is actually feasible alongside the achieved set.
|
||||
|
||||
## Fix
|
||||
|
||||
In `determineStatuses()`, after a non-achieved spec passes the upper bound check, add a feasibility check: call `checkWithS2(selectedCourseIds, [...achieved, specId])`. If infeasible, mark as `unreachable` instead of `achievable`.
|
||||
|
||||
### Changes
|
||||
|
||||
**`optimizer.ts` — `determineStatuses` function:**
|
||||
- After the upper bound check passes (currently falls through to `statuses[spec.id] = 'achievable'`), add:
|
||||
```ts
|
||||
const testSet = [...achieved, spec.id];
|
||||
const feasResult = checkWithS2(selectedCourseIds, testSet);
|
||||
if (!feasResult.feasible) {
|
||||
statuses[spec.id] = 'unreachable';
|
||||
continue;
|
||||
}
|
||||
```
|
||||
- No new parameters needed — `selectedCourseIds` is already passed to the function.
|
||||
|
||||
### What doesn't change
|
||||
|
||||
- No new status types — reuses existing `unreachable`
|
||||
- No UI changes — `unreachable` already renders correctly with grey styling
|
||||
- `computeUpperBounds` unchanged — still used for credit bar display
|
||||
- `AllocationResult` type unchanged
|
||||
- `checkWithS2` helper already exists in the same file
|
||||
|
||||
### Test updates
|
||||
|
||||
- Add a test for the bug scenario: given the specific course selection and CRF > STR > LCM > MGT ranking, verify LCM status is `unreachable` (not `achievable`) when CRF and STR are achieved
|
||||
- Existing test `marks achievable when required course is in open set` should be unaffected (uses all-open sets with no achieved specs, so feasibility check passes trivially)
|
||||
|
||||
### Performance
|
||||
|
||||
14 specializations total, at most ~10 non-achieved specs to check. Each check is a small LP solve. Negligible overhead.
|
||||
Reference in New Issue
Block a user