Skip to content

Commit d97db81

Browse files
marcusclaude
andcommitted
docs: add 'why does this exist' comments to non-obvious logic
Annotate budget calculations (multiplier formula, copilot conversion, daytime reservation, reserve), tmux scraping (timeouts, capture ranges, content thresholds, fallback regex), orchestrator (review keyword heuristic, hand-rolled ASCII lowercasing), and task selection (scoring constants, assignment lifetime). Also fixes the factually incorrect existing comment that claimed "2x on day before reset, 3x on last day" when the formula actually gives 1x/2x. Comments only — no behavioral changes. Nightshift-Task: why-annotator Nightshift-Ref: https://github.com/marcus/nightshift Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 68bbf11 commit d97db81

File tree

4 files changed

+50
-17
lines changed

4 files changed

+50
-17
lines changed

internal/budget/budget.go

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,11 @@ func (m *Manager) CalculateAllowance(provider string) (*AllowanceResult, error)
168168
return nil, fmt.Errorf("invalid budget mode: %s", mode)
169169
}
170170

171-
// Apply reserve enforcement
171+
// Reserve holds back a fraction of the base budget so that a single
172+
// nightshift run can't consume the entire remaining allocation.
172173
result = m.applyReserve(result, reservePercent)
174+
// Snapshot allowance before daytime reservation so the CLI can show
175+
// what the budget would be without predicted daytime usage deducted.
173176
result.AllowanceNoDaytime = result.Allowance
174177
if m.trend != nil {
175178
predicted, err := m.trend.PredictDaytimeUsage(provider, m.nowFunc(), weeklyBudget)
@@ -224,10 +227,11 @@ func (m *Manager) calculateWeeklyAllowance(weeklyBudget int64, usedPercent float
224227

225228
remainingWeekly := float64(weeklyBudget) * (1 - usedPercent/100)
226229

227-
// Aggressive end-of-week multiplier
230+
// Aggressive end-of-week: spend remaining budget faster as the reset
231+
// approaches, since unspent tokens are wasted after the weekly reset.
232+
// Formula: 3 - remainingDays → 1x with 2 days left (no boost), 2x on last day.
228233
multiplier := 1.0
229234
if m.cfg.Budget.AggressiveEndOfWeek && remainingDays <= 2 {
230-
// 2x on day before reset, 3x on last day
231235
multiplier = float64(3 - remainingDays)
232236
}
233237

@@ -312,10 +316,10 @@ func (m *Manager) GetUsedPercent(provider string) (float64, error) {
312316
if m.copilot == nil {
313317
return 0, fmt.Errorf("copilot provider not configured")
314318
}
315-
// Copilot uses monthly request limits, not weekly token budgets
316-
// Convert weekly budget to monthly limit for consistency
317-
// Note: This is a simplification; actual monthly limits should be configured separately
318-
monthlyLimit := weeklyBudget * 4 // Approximate: 4 weeks per month
319+
// Copilot's API reports usage against a monthly request cap, not a weekly
320+
// token budget. Multiply by 4 to convert our weekly budget figure into
321+
// an approximate monthly limit so the percentage math stays consistent.
322+
monthlyLimit := weeklyBudget * 4
319323
return m.copilot.GetUsedPercent(mode, monthlyLimit)
320324

321325
default:

internal/orchestrator/orchestrator.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -831,9 +831,10 @@ func ExtractPRURL(text string) string {
831831
}
832832

833833
// inferReviewPassed attempts to detect pass/fail from unstructured text.
834+
// This is a best-effort fallback for when the review agent returns prose
835+
// instead of the requested JSON. We count keyword hits rather than relying
836+
// on a single phrase because agent output format is unpredictable.
834837
func (o *Orchestrator) inferReviewPassed(output string) bool {
835-
// Simple heuristic: look for positive indicators
836-
// This is a fallback when JSON parsing fails
837838
passIndicators := []string{
838839
"passed", "approved", "looks good", "lgtm", "ship it",
839840
"no issues", "complete", "correct", "successful",
@@ -870,7 +871,6 @@ func containsIgnoreCase(s, substr string) bool {
870871
return false
871872
}
872873

873-
// Convert to lowercase for comparison
874874
sLower := toLowerASCII(s)
875875
substrLower := toLowerASCII(substr)
876876

@@ -882,6 +882,10 @@ func containsIgnoreCase(s, substr string) bool {
882882
return false
883883
}
884884

885+
// toLowerASCII lowercases ASCII letters without allocating through
886+
// strings.ToLower's full Unicode machinery. The review-keyword matching
887+
// in inferReviewPassed only uses ASCII terms, so Unicode case-folding is
888+
// unnecessary overhead on potentially large agent output.
885889
func toLowerASCII(s string) string {
886890
b := make([]byte, len(s))
887891
for i := 0; i < len(s); i++ {

internal/tasks/selector.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@ func (s *Selector) SetTaskSources(sources []string) {
5656

5757
// ScoreTask calculates the priority score for a task.
5858
// Formula: base_priority + staleness_bonus + context_bonus + task_source_bonus
59+
//
60+
// Scoring rationale:
61+
// - Staleness (0.1/day, capped at 3.0): gentle upward pressure so neglected
62+
// tasks eventually surface, without overwhelming explicit priority.
63+
// - Context bonus (+2): tasks mentioned in claude.md/agents.md are likely
64+
// relevant to current work, so they should rank above stale-but-irrelevant tasks.
65+
// - Task source bonus (+3): tasks backed by a td issue or GitHub issue represent
66+
// explicit human intent, so they outrank context mentions.
5967
func (s *Selector) ScoreTask(taskType TaskType, project string) float64 {
6068
var score float64
6169

@@ -260,7 +268,10 @@ func (s *Selector) SelectAndAssign(budget int64, project string) *ScoredTask {
260268
return nil
261269
}
262270

263-
// Mark as assigned to prevent duplicate selection
271+
// Mark as assigned to prevent duplicate selection by concurrent runs.
272+
// Assignments are persisted in SQLite and automatically cleared after
273+
// 2 hours (see ClearStaleAssignments) so a crashed run doesn't
274+
// permanently block a task type.
264275
taskID := makeTaskID(string(task.Definition.Type), project)
265276
s.state.MarkAssigned(taskID, project, string(task.Definition.Type))
266277

internal/tmux/scraper.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ func ScrapeClaudeUsage(ctx context.Context) (UsageResult, error) {
3232
return UsageResult{}, ErrTmuxNotFound
3333
}
3434

35+
// 45s overall timeout: must cover CLI startup (~10-20s), the /usage command
36+
// (~5-10s), plus margin for trust prompts and slow CI machines.
3537
ctx, cancel := context.WithTimeout(ctx, 45*time.Second)
3638
defer cancel()
3739

@@ -79,7 +81,12 @@ func ScrapeClaudeUsage(ctx context.Context) (UsageResult, error) {
7981
return UsageResult{}, err
8082
}
8183

82-
// Wait for usage output
84+
// 15s timeout: /usage output appears within a few seconds once the CLI is
85+
// ready; 15s gives generous margin without inflating the overall 45s budget.
86+
// 300ms poll interval: fast enough to detect output promptly without
87+
// hammering tmux capture-pane in a tight loop.
88+
// -S -200: capture last 200 lines of scrollback — usage output can appear
89+
// well above the visible viewport when the TUI has pushed content up.
8390
output, err := session.WaitForPattern(ctx, claudeWeekRegex, 15*time.Second, 300*time.Millisecond, "-S", "-200")
8491
if err != nil {
8592
return UsageResult{}, err
@@ -109,6 +116,7 @@ func ScrapeCodexUsage(ctx context.Context) (UsageResult, error) {
109116
return UsageResult{}, ErrTmuxNotFound
110117
}
111118

119+
// 45s overall timeout — same rationale as Claude: startup + command + margin.
112120
ctx, cancel := context.WithTimeout(ctx, 45*time.Second)
113121
defer cancel()
114122

@@ -167,7 +175,7 @@ func ScrapeCodexUsage(ctx context.Context) (UsageResult, error) {
167175
return UsageResult{}, err
168176
}
169177

170-
// Wait for status output
178+
// Same timing/capture rationale as the Claude scraper above.
171179
output, err := session.WaitForPattern(ctx, codexWeekRegex, 15*time.Second, 300*time.Millisecond, "-S", "-200")
172180
if err != nil {
173181
return UsageResult{}, err
@@ -259,11 +267,16 @@ func waitForSubstantialContent(ctx context.Context, session *Session, timeout ti
259267
return lastOutput, fmt.Errorf("timeout waiting for CLI (%d non-empty lines seen)",
260268
countNonEmptyLines(StripANSI(lastOutput)))
261269
case <-ticker.C:
270+
// -S -50: only need recent 50 lines to detect startup; the full
271+
// 200-line capture is reserved for the actual usage/status output.
262272
output, err := session.CapturePane(ctx, "-S", "-50")
263273
if err != nil {
264274
continue
265275
}
266276
lastOutput = output
277+
// >5 non-empty lines distinguishes a rendered TUI from a bare shell
278+
// prompt (typically 1-2 lines). Threshold is intentionally low to
279+
// avoid false negatives on minimal TUI layouts.
267280
if countNonEmptyLines(StripANSI(output)) > 5 {
268281
return output, nil
269282
}
@@ -334,10 +347,11 @@ func parseCodexResetTimes(output string) (sessionReset, weeklyReset string) {
334347
weeklyReset = m[1]
335348
}
336349

337-
// Fallback: if primary weekly regex didn't match, find the last "(resets HH:MM on D Mon)"
338-
// in the output. The weekly line is always shown last in Codex /status.
339-
// Only use the fallback when we find a match distinct from the session reset
340-
// (avoids misidentifying the 5h line as weekly when it's the only line).
350+
// Fallback: Codex /status format has changed across versions. When the
351+
// structured "Weekly limit" line isn't found, fall back to grabbing the
352+
// last "(resets HH:MM on D Mon)" in the output — the weekly line is
353+
// always printed last. We only accept a match distinct from the session
354+
// reset to avoid misidentifying the 5h line as weekly.
341355
if weeklyReset == "" {
342356
fallbackRe := regexp.MustCompile(`\(resets\s+(\d{1,2}:\d{2}\s+on\s+\d{1,2}\s+\w+)\)`)
343357
matches := fallbackRe.FindAllStringSubmatch(output, -1)

0 commit comments

Comments
 (0)