|
| 1 | +# PR #8421 Reviewer's Guide - Analysis Backlog |
| 2 | + |
| 3 | +> **Goal**: Systematically analyze PR #8421 and complete the reviewer's guide |
| 4 | +> **PR Size**: 146 files, +36,222 / -4,654 lines |
| 5 | +> **Constraint**: ~1,200 LOC analysis per iteration |
| 6 | +> **Created**: 2026-03-12 |
| 7 | +
|
| 8 | +--- |
| 9 | + |
| 10 | +## Analysis Strategy |
| 11 | + |
| 12 | +Given the PR size (~40k LOC) and our working context (~1,200 LOC), we need **~30-35 focused analysis passes** to thoroughly review and document. |
| 13 | + |
| 14 | +### Prioritization |
| 15 | + |
| 16 | +| Priority | Category | Files | Est. Iterations | |
| 17 | +|----------|----------|-------|-----------------| |
| 18 | +| P0 | Library code (core storage) | 10 | 5-6 | |
| 19 | +| P0 | Library code (API layer) | 10 | 4-5 | |
| 20 | +| P1 | Test code (UUID/sync) | 5 | 3-4 | |
| 21 | +| P1 | Test code (shape/batch) | 5 | 3-4 | |
| 22 | +| P2 | Test fixtures | 10 | 2-3 | |
| 23 | +| P2 | Scripts/tooling | 5 | 1-2 | |
| 24 | +| P3 | Documentation (audits) | 7 | 3-4 | |
| 25 | +| P3 | Documentation (proposals) | 15 | 4-5 | |
| 26 | +| P3 | Documentation (schemas) | 12 | 2-3 | |
| 27 | +| P4 | CI/Config | 5 | 1 | |
| 28 | + |
| 29 | +--- |
| 30 | + |
| 31 | +## Work Items |
| 32 | + |
| 33 | +### Phase 1: Library Code Analysis (P0) |
| 34 | + |
| 35 | +#### 1.1 Core Storage Layer |
| 36 | + |
| 37 | +| ID | File | Lines | Status | Notes | |
| 38 | +|----|------|-------|--------|-------| |
| 39 | +| LIB-001 | `lib/server/treatments.js` | +216/-33 | ❌ | UUID handling, identifier promotion | |
| 40 | +| LIB-002 | `lib/server/entries.js` | +100/-29 | ❌ | UUID handling, sysTime+type dedup | |
| 41 | +| LIB-003 | `lib/server/devicestatus.js` | +68/-39 | ❌ | | |
| 42 | +| LIB-004 | `lib/server/activity.js` | +29/-16 | ❌ | | |
| 43 | +| LIB-005 | `lib/server/food.js` | +15/-11 | ❌ | | |
| 44 | +| LIB-006 | `lib/server/profile.js` | +10/-5 | ❌ | | |
| 45 | +| LIB-007 | `lib/server/query.js` | +26/-4 | ❌ | | |
| 46 | +| LIB-008 | `lib/server/bootevent.js` | +6/-16 | ❌ | | |
| 47 | +| LIB-009 | `lib/server/env.js` | +17/-0 | ❌ | | |
| 48 | +| LIB-010 | `lib/data/ddata.js` | +23/-1 | ❌ | | |
| 49 | + |
| 50 | +**Analysis template for each file:** |
| 51 | +```markdown |
| 52 | +### LIB-XXX: filename.js |
| 53 | + |
| 54 | +**Purpose**: [What this file does] |
| 55 | + |
| 56 | +**Changes Summary**: |
| 57 | +- [Bullet points of key changes] |
| 58 | + |
| 59 | +**Key Review Points**: |
| 60 | +1. [Specific things reviewers should check] |
| 61 | + |
| 62 | +**Breaking Changes**: None / [Description] |
| 63 | + |
| 64 | +**Test Coverage**: [Reference to test file] |
| 65 | + |
| 66 | +**Security Considerations**: None / [Description] |
| 67 | +``` |
| 68 | + |
| 69 | +#### 1.2 API Layer |
| 70 | + |
| 71 | +| ID | File | Lines | Status | Notes | |
| 72 | +|----|------|-------|--------|-------| |
| 73 | +| LIB-011 | `lib/api/entries/index.js` | +18/-2 | ❌ | | |
| 74 | +| LIB-012 | `lib/api3/storage/mongoCollection/find.js` | +16/-2 | ❌ | | |
| 75 | +| LIB-013 | `lib/api3/storage/mongoCollection/utils.js` | +3/-3 | ❌ | | |
| 76 | +| LIB-014 | `lib/api3/storage/mongoCollection/modify.js` | +2/-2 | ❌ | | |
| 77 | +| LIB-015 | `lib/api3/generic/patch/operation.js` | +4/-5 | ❌ | | |
| 78 | +| LIB-016 | `lib/api3/generic/update/replace.js` | +3/-0 | ❌ | | |
| 79 | +| LIB-017 | `lib/api3/generic/search/input.js` | +2/-2 | ❌ | | |
| 80 | +| LIB-018 | `lib/api3/generic/collection.js` | +2/-2 | ❌ | | |
| 81 | + |
| 82 | +#### 1.3 Other Library |
| 83 | + |
| 84 | +| ID | File | Lines | Status | Notes | |
| 85 | +|----|------|-------|--------|-------| |
| 86 | +| LIB-019 | `lib/authorization/storage.js` | +15/-6 | ❌ | | |
| 87 | +| LIB-020 | `lib/authorization/delaylist.js` | +1/-1 | ❌ | | |
| 88 | +| LIB-021 | `lib/plugins/openaps.js` | +6/-13 | ❌ | | |
| 89 | +| LIB-022 | `lib/sandbox.js` | +3/-2 | ❌ | | |
| 90 | +| LIB-023 | `lib/language.js` | +6/-7 | ❌ | | |
| 91 | +| LIB-024 | `lib/client/renderer.js` | +2/-0 | ❌ | | |
| 92 | +| LIB-025 | `lib/client/index.js` | +1/-1 | ❌ | | |
| 93 | +| LIB-026 | `lib/report_plugins/daytoday.js` | +2/-1 | ❌ | | |
| 94 | + |
| 95 | +--- |
| 96 | + |
| 97 | +### Phase 2: Test Code Analysis (P1) |
| 98 | + |
| 99 | +#### 2.1 UUID/Sync Tests |
| 100 | + |
| 101 | +| ID | File | Lines | Status | Gap/Req | |
| 102 | +|----|------|-------|--------|---------| |
| 103 | +| TEST-001 | `tests/api.entries.uuid.test.js` | ~577 | ❌ | GAP-SYNC-045 | |
| 104 | +| TEST-002 | `tests/gap-treat-012.test.js` | ~428 | ❌ | GAP-TREAT-012 | |
| 105 | +| TEST-003 | `tests/identity-matrix.test.js` | ~476 | ❌ | REQ-SYNC-072 | |
| 106 | +| TEST-004 | `tests/objectid-cache.test.js` | ~468 | ❌ | | |
| 107 | +| TEST-005 | `tests/api.deduplication.test.js` | ~200 | ❌ | | |
| 108 | + |
| 109 | +#### 2.2 Shape/Batch Tests |
| 110 | + |
| 111 | +| ID | File | Lines | Status | Notes | |
| 112 | +|----|------|-------|--------|-------| |
| 113 | +| TEST-006 | `tests/sgv-devicestatus.test.js` | ~646 | ❌ | | |
| 114 | +| TEST-007 | `tests/websocket.shape-handling.test.js` | ~643 | ❌ | | |
| 115 | +| TEST-008 | `tests/storage.shape-handling.test.js` | ~410 | ❌ | | |
| 116 | +| TEST-009 | `tests/api.aaps-client.test.js` | ~300 | ❌ | | |
| 117 | +| TEST-010 | `tests/flakiness-control.test.js` | ~306 | ❌ | | |
| 118 | + |
| 119 | +#### 2.3 Test Fixtures |
| 120 | + |
| 121 | +| ID | File | Lines | Status | |
| 122 | +|----|------|-------|--------| |
| 123 | +| FIX-001 | `tests/fixtures/loop-override.js` | ~219 | ❌ | |
| 124 | +| FIX-002 | `tests/fixtures/partial-failures.js` | ~190 | ❌ | |
| 125 | +| FIX-003 | `tests/fixtures/trio-pipeline.js` | ~198 | ❌ | |
| 126 | +| FIX-004 | `tests/lib/test-helpers.js` | ~277 | ❌ | |
| 127 | + |
| 128 | +--- |
| 129 | + |
| 130 | +### Phase 3: Documentation Audit (P2-P3) |
| 131 | + |
| 132 | +#### 3.1 Verify Doc Accuracy |
| 133 | + |
| 134 | +| ID | Document | Status | Notes | |
| 135 | +|----|----------|--------|-------| |
| 136 | +| DOC-001 | `docs/meta/architecture-overview.md` | ❌ | Verify matches code | |
| 137 | +| DOC-002 | `docs/meta/modernization-roadmap.md` | ❌ | Check completion status | |
| 138 | +| DOC-003 | `docs/audits/*.md` (7 files) | ❌ | Spot-check accuracy | |
| 139 | +| DOC-004 | `docs/proposals/*.md` (15 files) | ❌ | Identify implemented vs proposed | |
| 140 | +| DOC-005 | `docs/requirements/*.md` (3 files) | ❌ | Verify test coverage | |
| 141 | +| DOC-006 | `docs/test-specs/*.md` (4 files) | ❌ | Match to actual tests | |
| 142 | + |
| 143 | +--- |
| 144 | + |
| 145 | +### Phase 4: CI/Config Analysis (P4) |
| 146 | + |
| 147 | +| ID | File | Status | Notes | |
| 148 | +|----|------|--------|-------| |
| 149 | +| CI-001 | `.github/workflows/main.yml` | ❌ | Node version change | |
| 150 | +| CI-002 | `package-lock.json` | ❌ | Dependency audit | |
| 151 | +| CI-003 | `Makefile` | ❌ | New targets | |
| 152 | + |
| 153 | +--- |
| 154 | + |
| 155 | +## Iteration Plan |
| 156 | + |
| 157 | +### Iteration Template |
| 158 | + |
| 159 | +```markdown |
| 160 | +## Iteration N: [Focus Area] |
| 161 | + |
| 162 | +**Files analyzed**: |
| 163 | +- file1.js (+X/-Y) |
| 164 | +- file2.js (+X/-Y) |
| 165 | + |
| 166 | +**Findings**: |
| 167 | +1. ... |
| 168 | + |
| 169 | +**Guide updates**: |
| 170 | +- Section X.Y updated |
| 171 | +- Added review point for Z |
| 172 | + |
| 173 | +**Next iteration**: [Focus] |
| 174 | +``` |
| 175 | + |
| 176 | +### Suggested Iteration Sequence |
| 177 | + |
| 178 | +| Iter | Focus | Work Items | Est. Time | |
| 179 | +|------|-------|------------|-----------| |
| 180 | +| 1 | treatments.js deep dive | LIB-001 | 30 min | |
| 181 | +| 2 | entries.js deep dive | LIB-002 | 30 min | |
| 182 | +| 3 | devicestatus.js, activity.js | LIB-003, LIB-004 | 25 min | |
| 183 | +| 4 | food.js, profile.js, query.js | LIB-005-007 | 20 min | |
| 184 | +| 5 | bootevent, env, ddata | LIB-008-010 | 15 min | |
| 185 | +| 6 | API layer (entries) | LIB-011-014 | 20 min | |
| 186 | +| 7 | API layer (v3 generic) | LIB-015-018 | 20 min | |
| 187 | +| 8 | Other lib files | LIB-019-026 | 25 min | |
| 188 | +| 9 | UUID test files | TEST-001-003 | 30 min | |
| 189 | +| 10 | Sync test files | TEST-004-005 | 25 min | |
| 190 | +| 11 | Shape test files | TEST-006-008 | 30 min | |
| 191 | +| 12 | Other test files | TEST-009-010 | 20 min | |
| 192 | +| 13 | Test fixtures | FIX-001-004 | 20 min | |
| 193 | +| 14 | Doc accuracy check | DOC-001-002 | 25 min | |
| 194 | +| 15 | Doc audit check | DOC-003 | 30 min | |
| 195 | +| 16 | Doc proposals check | DOC-004 | 30 min | |
| 196 | +| 17 | Doc requirements/specs | DOC-005-006 | 25 min | |
| 197 | +| 18 | CI/Config | CI-001-003 | 15 min | |
| 198 | +| 19 | Final guide polish | - | 20 min | |
| 199 | +| 20 | Cross-reference check | - | 20 min | |
| 200 | + |
| 201 | +--- |
| 202 | + |
| 203 | +## Completion Criteria |
| 204 | + |
| 205 | +### For Each Work Item |
| 206 | + |
| 207 | +- [ ] File analyzed and understood |
| 208 | +- [ ] Key changes documented |
| 209 | +- [ ] Review points identified |
| 210 | +- [ ] Breaking changes noted |
| 211 | +- [ ] Test coverage verified |
| 212 | +- [ ] Guide section updated |
| 213 | + |
| 214 | +### For Overall Guide |
| 215 | + |
| 216 | +- [ ] All library files documented |
| 217 | +- [ ] All test files documented |
| 218 | +- [ ] Documentation accuracy verified |
| 219 | +- [ ] Review sessions defined |
| 220 | +- [ ] Checklist complete |
| 221 | +- [ ] Questions for reviewers added |
| 222 | + |
| 223 | +--- |
| 224 | + |
| 225 | +## Workflow Command |
| 226 | + |
| 227 | +```bash |
| 228 | +# Run analysis iterations |
| 229 | +time sdqctl iterate ./workflows/pr-8421-review-analysis.conv -n 20 |
| 230 | + |
| 231 | +# Or step by step |
| 232 | +time sdqctl iterate ./workflows/pr-8421-review-analysis.conv -n 5 # Library core |
| 233 | +time sdqctl iterate ./workflows/pr-8421-review-analysis.conv -n 5 # Library API + tests |
| 234 | +time sdqctl iterate ./workflows/pr-8421-review-analysis.conv -n 5 # More tests + fixtures |
| 235 | +time sdqctl iterate ./workflows/pr-8421-review-analysis.conv -n 5 # Docs + polish |
| 236 | +``` |
| 237 | + |
| 238 | +--- |
| 239 | + |
| 240 | +## References |
| 241 | + |
| 242 | +- [PR #8421](https://github.com/nightscout/cgm-remote-monitor/pull/8421) |
| 243 | +- [Reviewer's Guide (stub)](./PR-8421-reviewers-guide.md) |
| 244 | +- [Worktree](file:///home/bewest/src/worktrees/nightscout/cgm-pr-8447) |
0 commit comments