fix(T-1008): Comparison context skips uppercase TASKS.md

[ArjenSchwarz]

Fixes Transit ticket T-1008: Comparison context skips uppercase TASKS.md

[claude]

---

Code Review: fix(T-1008) — Comparison context skips uppercase TASKS.md

Overview

This PR fixes a consistency gap where readSpecContext only checked for lowercase tasks.md, while the rest of the codebase (run.go, status/gatherer.go) already handled both tasks.md and TASKS.md. The fix brings the comparison context in line with those existing code paths. The approach is correct and clean.

Code Quality

The fix is correct. The struct field rename from name string to names []string is clean, the inner loop with early-break-on-success is idiomatic, and the content == nil sentinel is reliable (Go zero-initialises []byte to nil, and a successful ReadFile of an empty file returns []byte{} not nil).

Consistent with the rest of the codebase. run.go:505-508 and status/gatherer.go:269-270 already probe both cases; this fix closes the gap in readSpecContext.

Issues

Pre-existing duplication (not introduced here, but worth noting). readSpecContext exists in two places with near-identical logic:

• cmd/orbit/compare.go:325 (package-level function, called from orbit compare)
• internal/orbit/comparison.go:312 (method on Orbit, called from orbit run --variants)

The PR correctly patches both copies. Long-term, extracting this into a shared helper would eliminate the drift risk that caused this bug in the first place.

Missing test coverage for the internal/orbit copy. TestReadSpecContext_UppercaseTasksFile exercises the cmd/orbit version. There is no equivalent test for (o *Orbit).readSpecContext() in internal/orbit/comparison.go. Since both copies are structurally identical this is low risk, but a regression there would not be caught.

requirements.md and design.md do not get the same fallback treatment. This is probably intentional — the spec and run.go only document dual-case support for the tasks file. Worth a brief comment confirming that, so a future contributor does not wonder why the pattern is not uniform.

Suggestions

1. Consider extracting a shared helper to avoid the two-file drift problem long-term. Both callers could delegate to a shared function. Out of scope for a targeted bug fix, but a good follow-up.

2. Add a test for the internal/orbit path or add a comment explaining why it is intentionally untested.

3. Clarify the asymmetry in the specFiles slice with a short comment explaining why tasks gets a fallback but requirements/design do not.

Test Coverage

The new test (TestReadSpecContext_UppercaseTasksFile) is well-structured: uses t.TempDir(), writes only the uppercase file, and asserts the content appears in the output. It follows project conventions correctly. The only gap is the missing companion test for internal/orbit/comparison.go.

Summary

The fix is correct, focused, and consistent with the existing codebase. The main actionable item is adding a test (or explanation) for the internal/orbit copy. Everything else is low-priority follow-up. Looks good to merge.

[claude]

Code Review

Overview

This PR fixes a case-sensitivity bug (T-1008) where readSpecContext only looked for tasks.md (lowercase) and missed TASKS.md on case-sensitive filesystems (Linux). The fix generalises the spec-file lookup to support a list of name candidates with first-match semantics.

---

Code Quality & Style

• Minimal and focused — the change is scoped exactly to the reported bug. The struct field rename from name string to names []string is clean and self-documenting.
• Consistent application — the same fix is correctly applied to both copies of readSpecContext (cmd/orbit/compare.go and internal/orbit/comparison.go), avoiding a split-brain situation.
• The content == nil sentinel for "no file found" is correct: os.ReadFile returns a non-nil []byte{} for empty files, so the nil check reliably distinguishes "nothing read" from "empty file read".

---

Suggestions / Observations

1. Code duplication (pre-existing, out of scope) — readSpecContext is implemented identically in two packages. That is fine for this PR, but it means future changes (like this one) must be applied twice. Worth a follow-up to extract a shared helper.

2. Incomplete case coverage for other spec files — requirements.md and design.md only list their lowercase variant. If the same uppercase-filename issue can arise for those files, it should be addressed consistently. If the repo convention is that only TASKS.md is ever uppercase (matching the auto-detection logic in main.go which already checks both), then leaving the others as-is is fine — but worth a comment explaining the deliberate asymmetry.

3. Priority when both files exist — the first-match wins (lowercase before uppercase). That is a reasonable default but is not tested. A small test asserting that tasks.md takes precedence over TASKS.md when both exist would make the contract explicit.

---

Test Coverage

• Good: TestReadSpecContext_UppercaseTasksFile directly exercises the bug path and will catch regressions.
• Gap: The test only covers cmd/orbit/compare.go's standalone readSpecContext. The method on *Orbit in internal/orbit/comparison.go has no corresponding new test. Given the duplication, adding a parallel test (or at least noting the gap) would be safer.
• Suggestion: Add a test case for the "both files exist" scenario to pin the priority behaviour.

---

Potential Issues

None blocking. The logic is straightforward and the fix is correct for its stated scope.

---

Summary

Solid, focused bug fix. Main actionable items are:

• Consider adding tests for the priority-ordering case (both files present)
• Consider adding a test for the internal/orbit copy of the function
• Optional: apply the same multi-name pattern to requirements.md / design.md for consistency, or add a comment explaining why only Tasks needs it