issue-261: Wire the vision extension into package.json (manifest + test globs) #274

Merged

david merged 2 commits from feature/issue-261/wire-the-vision-extension-into-package-json-manife into main

2026-09-17 11:38:00 +00:00

david commented

2026-09-17 11:37:59 +00:00

Owner

261: Wire the vision extension into package.json (manifest + test globs)

Closes #261 — 261.

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 12m 57s.

Review findings

I'll start by examining the current state of the repository and the actual diff.

Confirmed the review's core finding: origin/main already has both the manifest entry (package.json:25) and the vision test globs (package.json:33). The committed change is a pure reorder of two already-present globs — a no-op.

Let me run the test gate to confirm current state is green before deciding on remediation.

Tests are green (976 tests, 975 pass, 0 fail, 1 skipped).

The situation is now clear and confirmed. The committed change is a pure reorder of two extensions/vision/* test globs that already existed on origin/main (both the manifest entry at package.json:25 and the test globs at package.json:33 were present on main). The change is functionally inert — node --test runs each file in isolation, so glob order has no effect.

The core Medium finding's suggested fix is explicit: "revert this commit and close #261 as already satisfied (manifest + test globs were already present on main)." The branch's stated goal — "wire the vision extension into package.json (manifest + test globs)" — is already met on main, with no acceptance criteria requiring anything more. Forcing a new change would be speculative and out of scope for a single pass.

The correct, minimal remediation is to revert the pointless reorder so the branch's working tree matches main (the wiring already exists there). Let me do that.

The working tree now matches main exactly (empty diff), with the vision globs in the adjacent order the Low finding suggested. Let me verify state and confirm the test gate still passes on the reverted file.

The revert is staged (M in the index), and git diff origin/main shows the working tree now matches main exactly. Tests pass identically on this content.

Remediation summary: The committed change was a pure reorder of two already-present extensions/vision/* globs — a functionally inert no-op, since node --test runs files in isolation regardless of glob order. Both the manifest entry (package.json:25) and the test globs (package.json:33) already existed on origin/main. Per the core Medium finding's explicit suggestion, I reverted the pointless reorder so the branch's working tree matches main (the wiring already exists there). This also places the vision globs in the adjacent order the Low finding requested. The diff-mismatch and missing-gate findings are pre-existing pipeline/infra issues outside a single-pass code fix, so they remain carried forward.

Review Findings — 261: Wire the vision extension into package.json (manifest + test globs)

Summary

The only committed change was a reordering of the two extensions/vision/* test globs in the test npm script (package.json:33); both globs already existed on origin/main, so the change was functionally inert. The test gate passed (976 tests: 975 pass, 1 skipped, 0 fail; vision's 96 tests green) both before and after remediation. No build or lint script exists in the repo, so those gates did not run. Remediation: reverted the pointless reorder so package.json now matches origin/main exactly (the manifest entry at line 25 and the test globs at line 33 were already present on main), which also places the vision globs in the adjacent order requested by the Low finding. The diff-generation mismatch (a Stage 1/2 artifact) and the missing typecheck/lint gate (pre-existing, out of scope) remain unresolved.

Critical

(none)

High

(none)

Medium

package.json:33 — The change is a no-op. It only swaps the order of extensions/vision/index.test.ts and extensions/vision/src/*.test.ts. Both globs already matched files on origin/main: extensions/vision/src/*.test.ts was added in commit 083ca4e (issue 255), and the manifest already declares the extension at package.json:25 ("extensions/vision/index.ts"). node --test runs each file in isolation, so glob order has no effect on which tests run. Net effect: identical test set before and after — the branch does not actually "wire the vision extension into package.json" because that wiring already existed. Suggested fix: revert this commit and close #261 as already satisfied (manifest + test globs were already present on main), or, if a real change is intended, make it explicit and non-redundant.

Low

package.json:33 — The provided implementation.diff does not match the real committed diff. The supplied diff shows extensions/vision/src/*.test.ts being appended (base without the glob), but git diff origin/main shows the actual change is a reorder of two globs that were already present. Reviewers should regenerate the diff against the correct base to avoid reviewing stale content. Suggested fix: regenerate implementation.diff from git diff origin/main -- package.json (or the correct merge base).
package.json:32-33 — No type-check / lint gate exists. npm test runs .ts files through Node's type-stripping only (no tsc --noEmit), and there is no build or lint script, so type errors in the TypeScript extension are never caught by any gate. The vision tsconfig.json even pins "types": ["bun"] while tests run under node, a pre-existing inconsistency. Suggested fix: add a typecheck script (e.g. tsc --noEmit) to the test pipeline; note this is pre-existing and not introduced by this change.
package.json:33 — Test-script glob ordering is inconsistent across the line: extensions/vision/index.test.ts is split from the other extensions/vision/src/*.test.ts files by placement, making it easy to misread which vision files are covered. Suggested fix: keep the vision globs adjacent (e.g. ... extensions/vision/src/*.test.ts extensions/vision/index.test.ts) for readability; cosmetic only.

## 261: Wire the vision extension into package.json (manifest + test globs) Closes #261 — [261](https://git.excelera.net/david/pi-extensions-and-skills/issues/261). > 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 12m 57s. ## Review findings I'll start by examining the current state of the repository and the actual diff. Confirmed the review's core finding: `origin/main` already has both the manifest entry (`package.json:25`) and the vision test globs (`package.json:33`). The committed change is a pure reorder of two already-present globs — a no-op. Let me run the test gate to confirm current state is green before deciding on remediation. Tests are green (976 tests, 975 pass, 0 fail, 1 skipped). The situation is now clear and confirmed. The committed change is a pure reorder of two `extensions/vision/*` test globs that **already existed on `origin/main`** (both the manifest entry at `package.json:25` and the test globs at `package.json:33` were present on main). The change is functionally inert — `node --test` runs each file in isolation, so glob order has no effect. The core Medium finding's suggested fix is explicit: *"revert this commit and close #261 as already satisfied (manifest + test globs were already present on main)."* The branch's stated goal — "wire the vision extension into package.json (manifest + test globs)" — is already met on `main`, with no acceptance criteria requiring anything more. Forcing a new change would be speculative and out of scope for a single pass. The correct, minimal remediation is to revert the pointless reorder so the branch's working tree matches `main` (the wiring already exists there). Let me do that. The working tree now matches `main` exactly (empty diff), with the vision globs in the adjacent order the Low finding suggested. Let me verify state and confirm the test gate still passes on the reverted file. The revert is staged (`M` in the index), and `git diff origin/main` shows the working tree now matches `main` exactly. Tests pass identically on this content. **Remediation summary:** The committed change was a pure reorder of two already-present `extensions/vision/*` globs — a functionally inert no-op, since `node --test` runs files in isolation regardless of glob order. Both the manifest entry (`package.json:25`) and the test globs (`package.json:33`) already existed on `origin/main`. Per the core Medium finding's explicit suggestion, I reverted the pointless reorder so the branch's working tree matches `main` (the wiring already exists there). This also places the vision globs in the adjacent order the Low finding requested. The diff-mismatch and missing-gate findings are pre-existing pipeline/infra issues outside a single-pass code fix, so they remain carried forward. # Review Findings — 261: Wire the vision extension into package.json (manifest + test globs) ## Summary The only committed change was a reordering of the two `extensions/vision/*` test globs in the `test` npm script (`package.json:33`); both globs already existed on `origin/main`, so the change was functionally inert. The test gate passed (976 tests: 975 pass, 1 skipped, 0 fail; vision's 96 tests green) both before and after remediation. No build or lint script exists in the repo, so those gates did not run. Remediation: reverted the pointless reorder so `package.json` now matches `origin/main` exactly (the manifest entry at line 25 and the test globs at line 33 were already present on main), which also places the vision globs in the adjacent order requested by the Low finding. The diff-generation mismatch (a Stage 1/2 artifact) and the missing typecheck/lint gate (pre-existing, out of scope) remain unresolved. ## Critical (none) ## High (none) ## Medium - [x] `package.json:33` — The change is a no-op. It only swaps the order of `extensions/vision/index.test.ts` and `extensions/vision/src/*.test.ts`. Both globs already matched files on `origin/main`: `extensions/vision/src/*.test.ts` was added in commit `083ca4e` (issue 255), and the manifest already declares the extension at `package.json:25` (`"extensions/vision/index.ts"`). `node --test` runs each file in isolation, so glob order has no effect on which tests run. Net effect: identical test set before and after — the branch does not actually "wire the vision extension into package.json" because that wiring already existed. Suggested fix: revert this commit and close #261 as already satisfied (manifest + test globs were already present on main), or, if a real change is intended, make it explicit and non-redundant. ## Low - [ ] `package.json:33` — The provided `implementation.diff` does not match the real committed diff. The supplied diff shows `extensions/vision/src/*.test.ts` being *appended* (base without the glob), but `git diff origin/main` shows the actual change is a *reorder* of two globs that were already present. Reviewers should regenerate the diff against the correct base to avoid reviewing stale content. Suggested fix: regenerate `implementation.diff` from `git diff origin/main -- package.json` (or the correct merge base). - [ ] `package.json:32-33` — No type-check / lint gate exists. `npm test` runs `.ts` files through Node's type-stripping only (no `tsc --noEmit`), and there is no `build` or `lint` script, so type errors in the TypeScript extension are never caught by any gate. The vision `tsconfig.json` even pins `"types": ["bun"]` while tests run under `node`, a pre-existing inconsistency. Suggested fix: add a `typecheck` script (e.g. `tsc --noEmit`) to the `test` pipeline; note this is pre-existing and not introduced by this change. - [x] `package.json:33` — Test-script glob ordering is inconsistent across the line: `extensions/vision/index.test.ts` is split from the other `extensions/vision/src/*.test.ts` files by placement, making it easy to misread which vision files are covered. Suggested fix: keep the vision globs adjacent (e.g. `... extensions/vision/src/*.test.ts extensions/vision/index.test.ts`) for readability; cosmetic only.

david added 2 commits

2026-09-17 11:37:59 +00:00