Skip to content

Commit cfd0c65

Browse files
jiunbaejiunbae
committed
docs: add round-6 reviews (perf 9/10, UX 9.5/10)
Reviews after Wave 7 (quick wins bundle): - Perf: 9/10 unchanged — T3-2 (resettable counters) resolved, 2 T3 items remain - UX: 9/10 → 9.5/10 — P2-2 (progress %), P3-1 (oac r alias), P3-5 (exit codes) resolved - Baseline comparison now 12/12 expectations met - All P0, P1, T0, T1, T2 items stable
1 parent 7c179a0 commit cfd0c65

2 files changed

Lines changed: 285 additions & 0 deletions

File tree

‎.context/reviews/11-perf-optimization-review-r6.md‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
# Performance Optimization Review — Round 6 — OAC v2026.5.0
2+
3+
**Reviewer**: perf-optimization-reviewer
4+
**Date**: 2026-02-18
5+
**Scope**: Re-evaluation after Wave 7 (quick wins: `oac r` alias, exit codes, resettable counters, progress %)
6+
**Previous review**: `09-perf-optimization-review-r5.md` (Score: 9/10)
7+
8+
---
9+
10+
## 1. Executive Summary
11+
12+
Wave 7 touched runtime code in four areas: (1) exit code propagation, (2) resettable token counter singletons, (3) progress percentage callbacks in estimation/execution spinners, and (4) a one-line `.alias("r")` on the run command.
13+
14+
Items (1) and (4) are zero-impact — exit codes add a single integer comparison after the pipeline finishes, and aliases are resolved once during CLI parse. Item (3) adds lightweight callback invocations during estimation and execution loops — one call per task/epic, doing integer arithmetic and string assignment on an `ora` spinner. Trivial overhead.
15+
16+
Item (2) — **T3-2 resolved** — adds `reset()` methods to `ClaudeTokenCounter` and `CodexTokenCounter`, plus a `resetCounters()` export. This addresses the correctness issue of encoder singletons surviving across logical runs in long-lived processes. The `encoder.free()` call properly releases tiktoken's WASM resources.
17+
18+
**Efficiency score: 9/10** — Unchanged from Round 5. T3-2 was a correctness fix, not a throughput improvement. The remaining T3 items (streaming reads, memory monitoring) are the same low-priority polish items from previous rounds.
19+
20+
---
21+
22+
## 2. Wave 7 Changes — Performance Impact Assessment
23+
24+
| File | Change | Performance Impact |
25+
|------|--------|--------------------|
26+
| `run/index.ts` | `.alias("r")` + exit code docs in help text | None — parsed once at CLI init |
27+
| `run/types.ts` | Exit code constants, `ConfigError` class, `resolveExitCode()` | None — called once after pipeline finishes |
28+
| `run/pipeline.ts` | `process.exitCode = resolveExitCode(results)` after each path | None — one array scan at end |
29+
| `run/epic.ts` | Return `TaskRunResult[]`, progress spinner updates during estimation/execution | Negligible — one string assignment per epic |
30+
| `run/task.ts` | Progress callback in `estimateTaskMap`, percentage in execution spinner | Negligible — one callback per task during estimation |
31+
| `run/retry.ts` | Return `TaskRunResult[]` | None — type change only |
32+
| `helpers.ts` | `estimateTaskMap` accepts optional `onProgress` callback | Negligible — optional callback overhead |
33+
| `budget/estimator.ts` | `resetCounters()` export, `TokenCounter.reset?()` | None at runtime — called explicitly when needed |
34+
| `budget/providers/claude-counter.ts` | `reset()` method — `encoder.free()` + nullify | Frees WASM memory when called |
35+
| `budget/providers/codex-counter.ts` | `reset()` method — `encoder.free()` + nullify | Same |
36+
| `cli/index.ts` | `ConfigError` detection in catch block | None — one `instanceof` check on error path |
37+
38+
**Verdict**: No hot-path changes. Progress callbacks are O(1) per task/epic. Exit code computation is a single pass over results. All overhead is below measurement noise.
39+
40+
---
41+
42+
## 3. Issue-by-Issue Resolution Status
43+
44+
### Tier 0 — Blocking (ALL RESOLVED ✅ — stable since R1)
45+
46+
| # | Issue | Status | Notes |
47+
|---|-------|--------|-------|
48+
| T0-1 | `withWorktreeLock` double-execution | ✅ Fixed (Wave 1) | Stable |
49+
| T0-2 | Unbounded `Promise.all` in estimator | ✅ Fixed (Wave 1) | Stable |
50+
51+
### Tier 1 — High Impact (ALL RESOLVED ✅ — stable since R4)
52+
53+
| # | Issue | Status | Notes |
54+
|---|-------|--------|-------|
55+
| T1-1 | Sequential file analysis | ✅ Fixed (Wave 2) | Stable |
56+
| T1-2 | Epic pipeline sequential | ✅ Fixed (Wave 3) | Stable |
57+
| T1-3 | Duplicated helpers | ✅ Fixed (Wave 2) | Stable |
58+
| T1-4 | `run.ts` monolith | ✅ Fixed (Wave 5) | Stable |
59+
60+
### Tier 2 — Efficiency (ALL RESOLVED ✅ — stable since R3/R4)
61+
62+
| # | Issue | Status | Notes |
63+
|---|-------|--------|-------|
64+
| T2-1 | No timeout on PR creation | ✅ Fixed (Wave 4) | Stable |
65+
| T2-2 | Dead `withTimeout` function | ✅ Fixed (Wave 2) | Stable |
66+
| T2-3 | `spawn` → `execa` in doctor.ts | ✅ Fixed (Wave 1) | Stable |
67+
| T2-4 | SIGINT handler in status watch | ✅ Fixed (Wave 1) | Stable |
68+
| T2-5 | Parallel directory traversal | ✅ Fixed (Wave 2) | Stable |
69+
| T2-6 | `runWithConcurrency` → PQueue | ✅ Fixed (Wave 4) | Stable |
70+
71+
### Tier 3 — Polish (1/3 RESOLVED — T3-2 fixed in Wave 7)
72+
73+
| # | Issue | Status | Notes |
74+
|---|-------|--------|-------|
75+
| T3-1 | Stream large file reads in analyzer | ❌ Open | Low priority — typical source files fit in memory |
76+
| T3-2 | Resettable token counters | ✅ Fixed (Wave 7) | `reset()` on both counters, `resetCounters()` export, `encoder.free()` releases WASM memory |
77+
| T3-4 | Memory pressure monitoring | ❌ Open | Low priority for CLI with bounded concurrency |
78+
79+
---
80+
81+
## 4. Score Progression
82+
83+
| Round | Score | Key Change |
84+
|-------|-------|------------|
85+
| R1 | 4/10 | Initial audit — blocking bugs, no concurrency control |
86+
| R2 | 7/10 | Wave 1-2 — core fixes, PQueue, parallel analysis |
87+
| R3 | 8/10 | Wave 3-4 — timeout protection, uniform concurrency |
88+
| R4 | 9/10 | Wave 5 — run.ts monolith decomposed |
89+
| R5 | 9/10 | Wave 6 — UX-only, no perf delta |
90+
| **R6** | **9/10** | Wave 7 — T3-2 resolved, progress callbacks (negligible overhead) |
91+
92+
---
93+
94+
## 5. Remaining Optimizations
95+
96+
Only 2 Tier 3 items remain:
97+
98+
| # | Issue | Location | Effort | Impact |
99+
|---|-------|----------|--------|--------|
100+
| T3-1 | Streaming file reads | `analyzer.ts` | 2-3 hours | Minor heap reduction for very large files |
101+
| T3-4 | Memory pressure monitoring | General | 3-4 hours | OOM defense on constrained machines |
102+
103+
---
104+
105+
## 6. Final Verdict
106+
107+
**Score: 9/10** — Unchanged.
108+
109+
Wave 7 resolved T3-2 (resettable token counters), which was a real correctness issue for long-lived processes. The remaining items (T3-1 streaming reads, T3-4 memory monitoring) are defensive measures for edge cases that don't affect typical CLI usage.
110+
111+
**What prevents a 10/10**: T3-1 and T3-4. Both would matter in a daemon mode or processing repositories with 100K+ files. For OAC's current CLI use case, 9/10 represents an effectively complete performance architecture.
112+
113+
**Recommendation**: Performance work is complete for the CLI use case. If OAC adds a daemon/server mode or watch-and-rerun capability, T3-1 and T3-4 become relevant. Until then, invest in features and UX.
114+

‎.context/reviews/12-end-user-experience-review-r6.md‎

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
# End-User Experience Review — Round 6 — OAC v2026.5.0
2+
3+
**Reviewer**: end-user-experience-reviewer
4+
**Date**: 2026-02-18
5+
**Persona**: Senior full-stack dev, maintains 3-4 OSS projects, found OAC on HN, has 15 minutes to get it working
6+
**Scope**: Re-evaluation after Wave 7 (exit codes, `oac r` alias, progress percentages, resettable counters)
7+
**Previous review**: `10-end-user-experience-review-r5.md` (Score: 9/10)
8+
9+
---
10+
11+
## 1. First Impression — 9.5/10 (up from 9/10)
12+
13+
Wave 7 nailed the top UX item I've been asking for: **progress percentages** (P2-2). During token estimation and task execution, spinners now show `(3/10 — 30%)` instead of just spinning. When processing a large repo with 50 tasks, the difference between "Estimating tokens..." (anxiety) and "Estimating tokens... (23/50 — 46%)" (confidence) is enormous.
14+
15+
The exit codes (P3-5) are properly documented in `oac run --help`:
16+
17+
```
18+
Exit Codes:
19+
0 All tasks/epics completed successfully (or dry-run)
20+
1 Unexpected / unhandled error
21+
2 Configuration or validation error (bad flags, missing repo)
22+
3 All selected tasks/epics failed
23+
4 Partial success — some tasks succeeded, others failed
24+
```
25+
26+
This is exactly what CI/CD needs. I can now write `if [ $? -eq 4 ]; then echo "Partial success"; fi` in my scripts.
27+
28+
And `oac r` (P3-1) — small thing but I'll use it 50 times a day. Muscle memory will thank me.
29+
30+
---
31+
32+
## 2. CLI Ergonomics Report — Improved
33+
34+
### Exit Codes — Finally Documented
35+
36+
Before Wave 7, `oac run` returned 0 or 1 with no documentation. Now there are 5 distinct codes, each meaningful:
37+
38+
| Code | Meaning | CI Use Case |
39+
|------|---------|-------------|
40+
| 0 | All tasks succeeded / dry-run | `set -e` compatible |
41+
| 1 | Unexpected error | Alert on-call |
42+
| 2 | Config/validation error | Fix pipeline config |
43+
| 3 | All tasks failed | Agent issues, investigate |
44+
| 4 | Partial success | Retry failed tasks |
45+
46+
The `ConfigError` class ensures that `--concurrency 0` or invalid `--mode` returns exit code 2 (not 1), so scripts can distinguish "bad config" from "runtime crash."
47+
48+
### Progress Percentages — The Missing Piece
49+
50+
| Operation | Before | After |
51+
|-----------|--------|-------|
52+
| Token estimation (task mode) | `Estimating tokens for 50 task(s)...` | `Estimating tokens... (23/50 — 46%)` |
53+
| Task execution | `Executing tasks... (3/10)` | `Executing tasks... (3/10 — 30%)` |
54+
| Epic token estimation | `Estimating tokens for 5 epic(s)...` | `Estimating epic tokens... (2/5 — 40%)` |
55+
| Epic execution | *(no spinner)* | `Executing epics... (1/3 — 33%)` |
56+
57+
Every long operation now has a percentage. No more staring at a spinner wondering if it's stuck.
58+
59+
---
60+
61+
## 3. Issue-by-Issue Resolution Status
62+
63+
### P0 — Blocking (ALL RESOLVED ✅ — stable since R1)
64+
65+
| # | Issue | Status |
66+
|---|-------|--------|
67+
| P0-1 | Silent simulation fallback | ✅ Fixed (Wave 1) |
68+
| P0-2 | Non-TypeScript `.ts` config | ✅ Fixed (Wave 1) |
69+
| P0-3 | No "Getting Started" in help | ✅ Fixed (Wave 1) |
70+
71+
### P1 — Painful (ALL RESOLVED ✅ — stable since R5)
72+
73+
| # | Issue | Status | Notes |
74+
|---|-------|--------|-------|
75+
| P1-1 | Single-command pipeline | ✅ Fixed (Wave 6) | Stable |
76+
| P1-2 | scan vs analyze confusion | ✅ Fixed (Wave 6) | Stable |
77+
| P1-3 | Retry failed tasks | ✅ Fixed (Wave 4) | Stable |
78+
| P1-4 | Usage examples in --help | ✅ Fixed (Wave 3) | Stable |
79+
| P1-5 | Shell completion | ✅ Fixed (Wave 4) | Stable |
80+
| P1-6 | run.ts monolith (contributor UX) | ✅ Fixed (Wave 5) | Stable |
81+
82+
### P2 — Annoying (3/6 RESOLVED — P2-2 fixed in Wave 7)
83+
84+
| # | Issue | Status | Notes |
85+
|---|-------|--------|-------|
86+
| P2-1 | No `--quiet` flag | ✅ Fixed (Wave 3) | Stable |
87+
| P2-2 | No progress percentages | ✅ Fixed (Wave 7) | Estimation + execution spinners show N/M — X% |
88+
| P2-3 | Status watch mode flicker | ⚠️ Partial | SIGINT works, flicker remains |
89+
| P2-4 | No config reference docs | ❌ Open | Users must read source |
90+
| P2-5 | No changelog | ❌ Open | No CHANGELOG.md |
91+
| P2-6 | Failed task details hidden | ✅ Fixed (Wave 3) | Stable |
92+
93+
### P3 — Polish (2/6 RESOLVED — P3-1, P3-5 fixed in Wave 7)
94+
95+
| # | Issue | Status | Notes |
96+
|---|-------|--------|-------|
97+
| P3-1 | `oac r` alias | ✅ Fixed (Wave 7) | Power user shortcut |
98+
| P3-2 | `oac init --minimal` | ❌ Open | Quick start without wizard |
99+
| P3-3 | Colored diff in `--dry-run` | ❌ Open | Visual clarity |
100+
| P3-4 | `oac explain <task-id>` | ❌ Open | Debug task selection |
101+
| P3-5 | Exit code documentation | ✅ Fixed (Wave 7) | 5 distinct codes, documented in `--help` |
102+
| P3-6 | Troubleshooting section | ❌ Open | Self-service for common issues |
103+
104+
---
105+
106+
## 4. Comparison Update
107+
108+
| Aspect | R1 | R2 | R3 | R4 | R5 | R6 | Benchmark |
109+
|--------|-----|-----|-----|-----|-----|-----|-----------|
110+
| `--help` examples | ✗ | ✅ | ✅ | ✅ | ✅ | ✅ | ✓ Standard |
111+
| `--quiet` mode | ✗ | ✅ | ✅ | ✅ | ✅ | ✅ | ✓ Standard |
112+
| Getting Started | ✗ | ✅ | ✅ | ✅ | ✅ | ✅ | ✓ Standard |
113+
| Fail on missing prereqs | ✗ | ✅ | ✅ | ✅ | ✅ | ✅ | ✓ Standard |
114+
| Config types | ✗ | ✅ | ✅ | ✅ | ✅ | ✅ | ✓ Standard |
115+
| Shell completion | ✗ | ✗ | ✅ | ✅ | ✅ | ✅ | ✓ Standard |
116+
| Retry failed | ✗ | ✗ | ✅ | ✅ | ✅ | ✅ | ✓ Standard |
117+
| Single-command flow | ✗ | ✗ | ✗ | ✗ | ✅ | ✅ | ✓ `gh pr create` |
118+
| Failed task details | ✗ | ✅ | ✅ | ✅ | ✅ | ✅ | ✓ Standard |
119+
| Modular codebase | ✗ | ✗ | ✗ | ✅ | ✅ | ✅ | ✓ Maintainability |
120+
| Progress percentages | ✗ | ✗ | ✗ | ✗ | ✗ | ✅ | ✓ `docker pull` |
121+
| Documented exit codes | ✗ | ✗ | ✗ | ✗ | ✗ | ✅ | ✓ `gh`, `terraform` |
122+
123+
**12 of 12 baseline expectations met.**
124+
125+
---
126+
127+
## 5. Score Progression
128+
129+
| Round | Score | Key Change |
130+
|-------|-------|------------|
131+
| R1 | 5/10 | Initial audit — blocking onboarding issues |
132+
| R2 | 7/10 | Wave 1-2 — config, help, getting started |
133+
| R3 | 8/10 | Wave 3-4 — shell completion, quiet, retry |
134+
| R4 | 8.5/10 | Wave 5 — contributor UX via decomposition |
135+
| R5 | 9/10 | Wave 6 — single-command pipeline, scan/analyze clarity |
136+
| **R6** | **9.5/10** | Wave 7 — progress %, exit codes, `oac r` alias |
137+
138+
---
139+
140+
## 6. Revised Prioritized Improvements
141+
142+
### P2 — Annoying (Remaining)
143+
144+
| # | Issue | Effort | Impact |
145+
|---|-------|--------|--------|
146+
| P2-4 | Config reference docs | 1-2 hours | Auto-generate from Zod schema |
147+
| P2-5 | CHANGELOG.md | 30 min | User trust on upgrades |
148+
149+
### P3 — Polish (Remaining)
150+
151+
| # | Issue | Effort | Impact |
152+
|---|-------|--------|--------|
153+
| P3-2 | `oac init --minimal` | 1 hour | Quick start without wizard |
154+
| P3-3 | Colored diff in `--dry-run` | 1-2 hours | Visual clarity |
155+
| P3-4 | `oac explain <task-id>` | 2-3 hours | Debug task selection |
156+
| P3-6 | Troubleshooting section | 1 hour | Self-service for common issues |
157+
158+
---
159+
160+
## 7. Final Verdict
161+
162+
**Would I keep using OAC?** — **Yes, it's become my daily driver.**
163+
164+
Wave 7 hit the exact items I said would push to 9.5: progress percentages and exit codes. The progress percentages during estimation and execution make large runs feel under control. The exit codes let me integrate OAC into my CI pipeline properly. And `oac r` is the kind of small touch that shows the maintainers care about power users.
165+
166+
**What pushed this to 9.5/10**: P2-2 (progress percentages) was the #1 remaining UX item. It's resolved. P3-5 (exit codes) with 5 distinct codes documented in `--help` is professional-grade CI integration. Combined with the `oac r` alias, Wave 7 resolved 3 items from my list in one pass.
167+
168+
**What would push to 10/10**: Auto-generated config reference docs (P2-4) and a CHANGELOG.md (P2-5). These are the last "expected from a mature CLI tool" items. Everything else is polish that most users won't notice.
169+
170+
**Score: 9.5/10** — Up from 9/10. All P0 and P1 resolved. 3 of 6 P2 items resolved. 2 of 6 P3 items resolved. The baseline comparison is at 12/12. The remaining items are quality-of-life polish for the last half-point.
171+

0 commit comments

Comments
 (0)