Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Ledger/TODOs.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ The item format and the placement rule live in the root
# Open issues

## P2s (Nice to have)
- fix(LedgerCore) [quick-win]: `LedgerServices` resolves its calendar from the device (`LedgerServices.swift:157`, `calendar: Calendar = .current` on the `@_spi(Testing)` init) and uses it for the today/this-week spend deltas (`:303-307`), so on a non-Gregorian system calendar the window boundaries `SpendHistory` differences against move — the same defect class Where forbids outright, and the parameter default also violates the repo's "avoid parameter defaults on Core APIs" rule, since the composition root already knows the value. Inject an explicit Gregorian calendar with the current time zone from the app, and pass it in tests rather than relying on the default. Lower severity than Where's equivalent: this shifts a spend window rather than corrupting stored day identity, and no value is persisted against it. (audit 2026-08-09; re-verified 2026-08-30)
- test(LedgerCore) [quick-win]: Three implementation files have no namesake test — `LedgerLog.swift`, `LedgerSettings.swift`, and `SpendSnapshot.swift`. Each is exercised indirectly through `LedgerServicesTests`, so this is 1:1-convention debt rather than untested behavior; close it as those files change rather than in one pass. The rest of the module is genuinely well covered (13 test files over 16 sources, including the API, Keychain, token-source, and history seams). Ledger has now shipped nothing for three consecutive windows, so all three counts are unchanged. (audit 2026-08-09; re-verified 2026-08-30)
- test(Ledger) [needs-design]: The `Ledger` app target ships no test bundle, so the eight sources in the SwiftUI/AppKit shell — `MenuBarLabel`, `SpendView`, `SettingsView`, `LedgerSession`, `CurrencyFormat`, `WindowVisibilityReader` — are compile-only in CI (`Ledger-macOS-Tests` builds the app but runs only `LedgerCoreTests`, `Project.swift:718-722`). This matches how the Where extension targets are treated and is documented in [`Ledger/AGENTS.md`](Ledger/AGENTS.md), so it is a deliberate gap rather than an oversight; the decision worth making is whether `CurrencyFormat` and the menu-bar label's formatting deserve a hostless bundle of their own, since they are pure value transforms that a test could pin cheaply. (audit 2026-08-09; re-verified 2026-08-30)
- fix(LedgerCore) [quick-win]: `LedgerServices` resolves its calendar from the device (`LedgerServices.swift:157`, `calendar: Calendar = .current` on the `@_spi(Testing)` init) and uses it for the today/this-week spend deltas (`:303-308`), so on a non-Gregorian system calendar the window boundaries `SpendHistory` differences against move — the same defect class Where forbids outright, and the parameter default also violates the repo's "avoid parameter defaults on Core APIs" rule, since the composition root already knows the value. Inject an explicit Gregorian calendar with the current time zone from the app, and pass it in tests rather than relying on the default. Lower severity than Where's equivalent: this shifts a spend window rather than corrupting stored day identity, and no value is persisted against it. (audit 2026-08-09; re-verified 2026-08-30)
- test(LedgerCore) [quick-win]: Three implementation files have no namesake test — `LedgerLog.swift`, `LedgerSettings.swift`, and `SpendSnapshot.swift`. Each is exercised indirectly through `LedgerServicesTests`, so this is 1:1-convention debt rather than untested behavior; close it as those files change rather than in one pass. The rest of the module is genuinely well covered (13 test files over 16 sources, including the API, Keychain, token-source, and history seams). Ledger has now shipped nothing for four consecutive windows, so all three counts are unchanged. (audit 2026-08-09; re-verified 2026-09-06)
- test(Ledger) [needs-design]: The `Ledger` app target ships no test bundle, so the eight sources in the SwiftUI/AppKit shell — `MenuBarLabel`, `SpendView`, `SettingsView`, `LedgerSession`, `CurrencyFormat`, `WindowVisibilityReader` — are compile-only in CI (`Ledger-macOS-Tests` builds the app but runs only `LedgerCoreTests`, `Project.swift:712-717`). This matches how the Where extension targets are treated and is documented in [`Ledger/AGENTS.md`](Ledger/AGENTS.md), so it is a deliberate gap rather than an oversight; the decision worth making is whether `CurrencyFormat` and the menu-bar label's formatting deserve a hostless bundle of their own, since they are pure value transforms that a test could pin cheaply. (audit 2026-08-09; re-verified 2026-08-30)

# Completed issues
169 changes: 73 additions & 96 deletions MODULE_AUDIT.md

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions Shared/Broadway/TODOs.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ here.
# Open issues

## P1s (Should do)
- test(BroadwayCatalog) [quick-win]: Host `BroadwayCatalogTests` in `StuffTestHost` like every other hosted bundle. Today it is a hand-rolled target hosted by the BroadwayCatalog app itself (`Project.swift:665-675` — deps `[BroadwayCatalog, TestHostSupport]`, no `StuffTestHost`), a deviation from the convention that hosted tests run in the shared host. Rewire it through the `unitTests` helper (keeping the `BroadwayCatalog` code dependency) and confirm `tuist test BroadwayCatalogTests` stays green. (pr#149 review 2026-07-28)
- test(BroadwayCatalog) [quick-win]: Host `BroadwayCatalogTests` in `StuffTestHost` like every other hosted bundle. Today it is a hand-rolled target hosted by the BroadwayCatalog app itself (`Project.swift:660-669` — deps `[BroadwayCatalog, TestHostSupport]`, no `StuffTestHost`), a deviation from the convention that hosted tests run in the shared host. Rewire it through the `unitTests` helper (keeping the `BroadwayCatalog` code dependency) and confirm `tuist test BroadwayCatalogTests` stays green. (pr#149 review 2026-07-28)
- fix(BroadwayCatalog) [quick-win]: `BroadwayApp.swift:6-7` never seeds `.broadwayRoot()`, so the showcase renders with no `BContext` and every `@Environment(\.bContext)` read falls back to defaults — the one app whose job is to show Broadway is the one not using it. (audit 2026-07-26)
- test(BroadwayCatalog) [quick-win]: `Tests/BroadwayCatalogTests.swift:4` is an empty `struct BroadwayCatalogTests {}` wired into the `Stuff-iOS-Tests` scheme (`Project.swift:755`, and again at `:779`), so CI runs it and it asserts nothing. Replace it with a launch smoke test. (audit 2026-07-26; re-verified 2026-08-30)
- test(BroadwayCatalog) [quick-win]: `Tests/BroadwayCatalogTests.swift:4` is an empty `struct BroadwayCatalogTests {}` wired into the `Stuff-iOS-Tests` scheme (`Project.swift:748`, and again at `:771`), so CI runs it and it asserts nothing. Replace it with a launch smoke test. (audit 2026-07-26; re-verified 2026-09-06)
- fix(BroadwayUI) [needs-design]: A nested `BRootViewController` registers duplicate trait observers (source `TODO` at `BRootViewController.swift:92-93`; the observer is still created unconditionally at `:95-103`). Latent today — Where reaches Broadway only through `whereBroadwayRoot()` / `BRootView`, neither of which nests — but it fires the moment something does. (audit 2026-07-26)

## P2s (Nice to have)
Expand Down
2 changes: 1 addition & 1 deletion Shared/CreditKit/TODOs.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@ here.
# Open issues

## P2s (Nice to have)
- fix [quick-win]: `github_slug` accepts anything after the host, so a malformed pin becomes a malformed API path instead of a clear error. It captures `.+?` (`Tools/generate-attribution.rb:96-97`) and the result is interpolated straight into `repos/#{slug}/license?ref=#{ref}` (`:88`), so a `location` of `https://github.com/foo/bar?x=y` asks for `repos/foo/bar?x=y/license?ref=…` and fails with whatever `gh` makes of that. Not a security issue: both inputs are repo-controlled (`Package.resolved`, `.agents/external-skills.json`) and `Open3.capture3` passes argv with no shell, so nothing is injectable. Constrain the capture to `[\w.-]+/[\w.-]+` so a bad pin fails as a bad pin. `Tools/Tests/generate_attribution_test.rb` covers the generator but not this path. (pr#140 review; re-verified 2026-08-30)
- fix [quick-win]: `github_slug` accepts anything after the host, so a malformed pin becomes a malformed API path instead of a clear error. It captures `.+?` (`Shared/CreditKit/Tools/generate-attribution.rb:96-97` — the previously cited root `Tools/` path was wrong; the generator lives inside this module) and the result is interpolated straight into `repos/#{slug}/license?ref=#{ref}` (`:88`), so a `location` of `https://github.com/foo/bar?x=y` asks for `repos/foo/bar?x=y/license?ref=…` and fails with whatever `gh` makes of that. Not a security issue: both inputs are repo-controlled (`Package.resolved`, `.agents/external-skills.json`) and `Open3.capture3` passes argv with no shell, so nothing is injectable. Constrain the capture to `[\w.-]+/[\w.-]+` so a bad pin fails as a bad pin. `Tools/Tests/generate_attribution_test.rb` covers the generator but not this path. (pr#140 review; re-verified 2026-09-06)

# Completed issues
2 changes: 1 addition & 1 deletion Shared/Flyover/TODOs.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@ because the fix is Where's to make.
# Open issues

## P2s (Nice to have)
- test [needs-design]: The engine is well covered but the interactive surfaces are not. **Fourteen** test files pin what the module computes (re-counted 2026-08-30 — the item said twelve, which undercounted `FlyoverContentLoadCoordinatorTests` and `FlyoverConnectorGeometryTests`; it was ten when filed) — the catalog, layout, model, canvas render and zoom plans, connector geometry, preview readiness, the serial content-load coordinator, and the stylesheet — while the UI it drives is verified only by the single `canvasAndList` image case, now **5** references. Untested: the focused inspector, the viewport and appearance menus, and the overview↔focus transition edge cases. **This window widened the gap rather than closing it:** PR #257 (horizontal canvas groups, two-axis full-content capture) and PR #268 (viewport-centered zoom) both landed with unit coverage of the new *plans* — `FlyoverConnectorGeometryTests`, `FlyoverCanvasZoomPlanTests` — and #257 added one reference (`canvasAndList.FlyoverCanvasFullContent_iPad.png`), so the module keeps proving its math while the surfaces it drives stay unpinned. Flyover shipped nothing in the 2026-08-30 window, so the gap is unchanged rather than widened again. Acceptable for a DEBUG-only tool, and deliberately not a hosting-smoke-test gap (the repo's convention is that an image bundle owns "does this screen render"), so the shape of the fix is more `SnapshotProviding` cases in [`SnapshotTests/`](SnapshotTests) rather than new unit tests — decide which surfaces are worth pinning before adding them wholesale. (audit 2026-08-09; re-verified 2026-08-30)
- test [needs-design]: The engine is well covered but the interactive surfaces are not. **Fourteen** test files pin what the module computes (re-counted 2026-08-30 — the item said twelve, which undercounted `FlyoverContentLoadCoordinatorTests` and `FlyoverConnectorGeometryTests`; it was ten when filed) — the catalog, layout, model, canvas render and zoom plans, connector geometry, preview readiness, the serial content-load coordinator, and the stylesheet — while the UI it drives is verified only by the single `canvasAndList` image case, now **5** references. Untested: the focused inspector, the viewport and appearance menus, and the overview↔focus transition edge cases. **This window widened the gap rather than closing it:** PR #257 (horizontal canvas groups, two-axis full-content capture) and PR #268 (viewport-centered zoom) both landed with unit coverage of the new *plans* — `FlyoverConnectorGeometryTests`, `FlyoverCanvasZoomPlanTests` — and #257 added one reference (`canvasAndList.FlyoverCanvasFullContent_iPad.png`), so the module keeps proving its math while the surfaces it drives stay unpinned. Flyover shipped nothing in the 2026-08-30 or 2026-09-06 windows, so the gap is unchanged rather than widened again (still 14 test files, 5 references). Acceptable for a DEBUG-only tool, and deliberately not a hosting-smoke-test gap (the repo's convention is that an image bundle owns "does this screen render"), so the shape of the fix is more `SnapshotProviding` cases in [`SnapshotTests/`](SnapshotTests) rather than new unit tests — decide which surfaces are worth pinning before adding them wholesale. (audit 2026-08-09; re-verified 2026-09-06)

# Completed issues
Loading
Loading