issue-256: Implement extensions/vision/src/usage.ts (usage mapping + peak/off-peak cost) #270

Merged

david merged 2 commits from feature/issue-256/implement-extensions-vision-src-usage-ts-usage-map into main

2026-09-17 09:55:43 +00:00

david commented

2026-09-17 09:55:43 +00:00

Owner

256: Implement extensions/vision/src/usage.ts (usage mapping + peak/off-peak cost)

Closes #256 — 256.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 30m 28s.

Review findings

I'll start by exploring the repository structure and reading the relevant files.

Now let me look at the project's build/test setup and a sibling module to confirm conventions.

Let me check a sibling module's imports and whether any tsconfig exists.

I now have full context. Let me analyze the two findings:

Finding 1 (usage.test.ts:8) — a concrete, safe, minimal fix: Usage is declared and exported in pi-types.ts, but the test imports it from usage.ts which only does a type-only import (no re-export). The sibling images.test.ts correctly imports type ValidationLimits from the module that exports it (./images.ts). I'll move the Usage import to its real home.

Finding 2 (pi-types.ts:1) — the drift concern. The suggested fix is adding a repo-wide tsc --noEmit gate. But there's no tsconfig for vision, no tsc in the root node_modules, and pi-ai's canonical Usage isn't resolvable on disk. Adding repo-wide typecheck infrastructure is speculative and risky in a single pass — out of scope for this milestone. I'll carry it forward.

Let me apply the finding-1 fix:

Now let me run the test gate. First the vision tests directly to verify my change, then the full npm test gate.

All 56 vision tests pass. Now the full npm test gate:

Full gate is green: 936 tests, 935 pass, 0 fail, 1 skipped — no regression from my change. Let me confirm the final state of the edited import block.

The fix is applied cleanly and the full test gate is green. Now I'll emit the annotated findings document.

Review Findings — issue-256

Summary

Fixed the one concrete, in-scope finding: usage.test.ts imported type Usage from ./usage.ts, but usage.ts never re-exports Usage (it only does a type-only import). Moved the Usage import to its real home, ./pi-types.ts, matching the sibling convention (images.test.ts imports type ValidationLimits from ./images.ts). The implementation itself is correct; the only diff change is the single-line import. Carried forward the pi-types.ts drift finding because adding a repo-wide tsc --noEmit gate is out of scope for this milestone and risky in a single pass (no vision tsconfig, no root tsc, and pi-ai's canonical Usage isn't resolvable on disk). Gates: npm test = 936 tests, 935 pass, 0 fail, 1 skipped (a penpot test gated on an env var) — unchanged, no regressions.

Critical

(none)

High

(none)

Medium

(none)

Low

extensions/vision/src/usage.test.ts:8 — the test imports type Usage from ./usage.ts, but usage.ts never exports Usage (line 26 only does import type { Usage } from "./pi-types.ts" — a type-only import, not a re-export). The test passes today only because Node's type-stripping silently drops type-annotated named imports without checking that the export exists; under any future tsc --noEmit gate this file would fail with TS2305. The sibling tests already do the right thing (e.g. images.test.ts imports type ValidationLimits from the module that actually exports it). Suggested fix: import the type from its real home — import { isPeak, mapUsage, DEEPSEEK_FLASH_RATES, type DeepSeekUsage } from "./usage.ts"; should become import { isPeak, mapUsage, DEEPSEEK_FLASH_RATES, type DeepSeekUsage } from "./usage.ts"; and import type { Usage } from "./pi-types.ts"; — or add a export type { Usage } from "./pi-types.ts" re-export in usage.ts.
extensions/vision/src/pi-types.ts:1 — Usage is a hand-maintained structural duplicate of pi-ai's canonical interface Usage (the file's own docstring says "Keep these in lockstep"). Because there is no tsc/typecheck gate in the repo, a silent drift between this local declaration and pi-ai's real Usage at runtime would not be caught by any automated check — the only current safeguard is the docstring and manual verification (which I did against node_modules/.../pi-ai/dist/types.d.ts: the fields currently match exactly). Suggested fix: add a lightweight tsc --noEmit gate to the test script (or a small runtime test that cross-checks the local shape), which would also fix the flagged import in usage.test.ts:8 at the same time.

## 256: Implement extensions/vision/src/usage.ts (usage mapping + peak/off-peak cost) Closes #256 — [256](https://git.excelera.net/david/pi-extensions-and-skills/issues/256). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 30m 28s. ## Review findings I'll start by exploring the repository structure and reading the relevant files. Now let me look at the project's build/test setup and a sibling module to confirm conventions. Let me check a sibling module's imports and whether any tsconfig exists. I now have full context. Let me analyze the two findings: **Finding 1 (`usage.test.ts:8`)** — a concrete, safe, minimal fix: `Usage` is declared and exported in `pi-types.ts`, but the test imports it from `usage.ts` which only does a type-only *import* (no re-export). The sibling `images.test.ts` correctly imports `type ValidationLimits` from the module that exports it (`./images.ts`). I'll move the `Usage` import to its real home. **Finding 2 (`pi-types.ts:1`)** — the drift concern. The suggested fix is adding a repo-wide `tsc --noEmit` gate. But there's no tsconfig for vision, no `tsc` in the root `node_modules`, and pi-ai's canonical `Usage` isn't resolvable on disk. Adding repo-wide typecheck infrastructure is speculative and risky in a single pass — out of scope for this milestone. I'll carry it forward. Let me apply the finding-1 fix: Now let me run the test gate. First the vision tests directly to verify my change, then the full `npm test` gate. All 56 vision tests pass. Now the full `npm test` gate: Full gate is green: **936 tests, 935 pass, 0 fail, 1 skipped** — no regression from my change. Let me confirm the final state of the edited import block. The fix is applied cleanly and the full test gate is green. Now I'll emit the annotated findings document. # Review Findings — issue-256 ## Summary Fixed the one concrete, in-scope finding: `usage.test.ts` imported `type Usage` from `./usage.ts`, but `usage.ts` never re-exports `Usage` (it only does a type-only `import`). Moved the `Usage` import to its real home, `./pi-types.ts`, matching the sibling convention (`images.test.ts` imports `type ValidationLimits` from `./images.ts`). The implementation itself is correct; the only diff change is the single-line import. Carried forward the `pi-types.ts` drift finding because adding a repo-wide `tsc --noEmit` gate is out of scope for this milestone and risky in a single pass (no vision tsconfig, no root `tsc`, and pi-ai's canonical `Usage` isn't resolvable on disk). Gates: `npm test` = 936 tests, 935 pass, 0 fail, 1 skipped (a penpot test gated on an env var) — unchanged, no regressions. ## Critical - [ ] (none) ## High - [ ] (none) ## Medium - [ ] (none) ## Low - [x] `extensions/vision/src/usage.test.ts:8` — the test imports `type Usage` from `./usage.ts`, but `usage.ts` never **exports** `Usage` (line 26 only does `import type { Usage } from "./pi-types.ts"` — a type-only import, not a re-export). The test passes today only because Node's type-stripping silently drops `type`-annotated named imports without checking that the export exists; under any future `tsc --noEmit` gate this file would fail with TS2305. The sibling tests already do the right thing (e.g. `images.test.ts` imports `type ValidationLimits` from the module that actually exports it). Suggested fix: import the type from its real home — `import { isPeak, mapUsage, DEEPSEEK_FLASH_RATES, type DeepSeekUsage } from "./usage.ts";` should become `import { isPeak, mapUsage, DEEPSEEK_FLASH_RATES, type DeepSeekUsage } from "./usage.ts";` and `import type { Usage } from "./pi-types.ts";` — or add a `export type { Usage } from "./pi-types.ts"` re-export in `usage.ts`. - [ ] `extensions/vision/src/pi-types.ts:1` — `Usage` is a hand-maintained structural duplicate of pi-ai's canonical `interface Usage` (the file's own docstring says "Keep these in lockstep"). Because there is no `tsc`/typecheck gate in the repo, a silent drift between this local declaration and pi-ai's real `Usage` at runtime would not be caught by any automated check — the only current safeguard is the docstring and manual verification (which I did against `node_modules/.../pi-ai/dist/types.d.ts`: the fields currently match exactly). Suggested fix: add a lightweight `tsc --noEmit` gate to the test script (or a small runtime test that cross-checks the local shape), which would also fix the flagged import in `usage.test.ts:8` at the same time.

david added 2 commits

2026-09-17 09:55:43 +00:00