317: MSSQL spike: scoring runner for both candidates #339

Merged

david merged 2 commits from feature/issue-317/mssql-spike-scoring-runner-for-both-candidates into main

2026-11-10 23:10:20 +00:00

david commented

2026-11-10 23:10:19 +00:00

Owner

317: MSSQL spike: scoring runner for both candidates

Implements 317.

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 1m 18s.

Review findings

Now edit run.mjs.The sed ran in the wrong directory context (my heredoc consumed stdin). Retry.Odd sed (likely BSD-like alias); use a different approach.# Review Findings — 317

Summary

I fixed the three Medium findings. run.mjs now treats only ERR_MODULE_NOT_FOUND and ERR_PACKAGE_PATH_NOT_EXPORTED as load failures. Other errors are labelled "error: …". scoreGuard throws on an empty reject or accept corpus. I added tests for main, the --json output, a missing-module candidate, a candidate with no validateSql export, an unexpected load error, and empty corpora. node --test run.test.mjs passes 10/10, and a manual node extensions/mssql/spike/run.mjs still runs and prints STOP. The repo has no build or lint script. I left the three Low findings unresolved as optional polish for a spike.

Critical

High

Medium

extensions/mssql/spike/run.mjs:39 — scoreCandidate catches every error from load() and scoreGuard and reports it as "not viable". A bug in the guard or in scoring, as opposed to a missing dependency, therefore looks like an unavailable candidate. Suggested fix: only treat ERR_MODULE_NOT_FOUND and ERR_PACKAGE_PATH_NOT_EXPORTED as load failures. Score outside the try block, or label other errors "error" with the message.
extensions/mssql/spike/run.mjs:25 — scoreGuard doesn't check that the corpora are non-empty. An empty reject corpus gives zero false accepts and a vacuous pass. Suggested fix: throw if rejectCases.length === 0 or acceptCases.length === 0.
extensions/mssql/spike/run.test.mjs:1 — Nothing tests main, the --json output, the load-failure path (loadError), or formatResult. main already accepts an injectable candidates argument, which makes this easy to test. Suggested fix: add tests that call main(["--json"], fakeCandidates) with a spied console.log. Cover a missing-module candidate and a candidate with no validateSql export, and assert that the "not viable" line and the JSON shape are right.

Low

extensions/mssql/spike/run.mjs:22 — isAccepted treats a guard that throws as not accepted. That is correct for reject cases (fail closed). On accept cases, though, a crash is indistinguishable from a deliberate rejection in the report. Suggested fix: record thrown cases separately, for example errored: [labels].
extensions/mssql/spike/README.md:20 — The README says the exit code is 0 whenever the script ran, including on the STOP gate. CI cannot detect a gate failure from the exit code. Suggested fix: optionally add a --strict flag that exits 1 on STOP.
extensions/mssql/spike/run.mjs:89 — The import.meta.url main-module check is fine, but pathToFileURL doesn't resolve symlinks. Running the script through a symlinked path would silently do nothing. Suggested fix: compare fs.realpathSync results, or accept this as a known limitation of a spike.

## 317: MSSQL spike: scoring runner for both candidates Implements [317](https://git.excelera.net/david/pi-extensions-and-skills/issues/317). > 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 1m 18s. ## Review findings Now edit run.mjs.The sed ran in the wrong directory context (my heredoc consumed stdin). Retry.Odd sed (likely BSD-like alias); use a different approach.# Review Findings — 317 ## Summary I fixed the three Medium findings. `run.mjs` now treats only `ERR_MODULE_NOT_FOUND` and `ERR_PACKAGE_PATH_NOT_EXPORTED` as load failures. Other errors are labelled "error: …". `scoreGuard` throws on an empty reject or accept corpus. I added tests for `main`, the `--json` output, a missing-module candidate, a candidate with no `validateSql` export, an unexpected load error, and empty corpora. `node --test run.test.mjs` passes 10/10, and a manual `node extensions/mssql/spike/run.mjs` still runs and prints STOP. The repo has no build or lint script. I left the three Low findings unresolved as optional polish for a spike. ## Critical ## High ## Medium - [x] extensions/mssql/spike/run.mjs:39 — `scoreCandidate` catches every error from `load()` and `scoreGuard` and reports it as "not viable". A bug in the guard or in scoring, as opposed to a missing dependency, therefore looks like an unavailable candidate. Suggested fix: only treat `ERR_MODULE_NOT_FOUND` and `ERR_PACKAGE_PATH_NOT_EXPORTED` as load failures. Score outside the try block, or label other errors "error" with the message. - [x] extensions/mssql/spike/run.mjs:25 — `scoreGuard` doesn't check that the corpora are non-empty. An empty reject corpus gives zero false accepts and a vacuous pass. Suggested fix: throw if `rejectCases.length === 0` or `acceptCases.length === 0`. - [x] extensions/mssql/spike/run.test.mjs:1 — Nothing tests `main`, the `--json` output, the load-failure path (`loadError`), or `formatResult`. `main` already accepts an injectable `candidates` argument, which makes this easy to test. Suggested fix: add tests that call `main(["--json"], fakeCandidates)` with a spied `console.log`. Cover a missing-module candidate and a candidate with no `validateSql` export, and assert that the "not viable" line and the JSON shape are right. ## Low - [ ] extensions/mssql/spike/run.mjs:22 — `isAccepted` treats a guard that throws as not accepted. That is correct for reject cases (fail closed). On accept cases, though, a crash is indistinguishable from a deliberate rejection in the report. Suggested fix: record thrown cases separately, for example `errored: [labels]`. - [ ] extensions/mssql/spike/README.md:20 — The README says the exit code is 0 whenever the script ran, including on the STOP gate. CI cannot detect a gate failure from the exit code. Suggested fix: optionally add a `--strict` flag that exits 1 on STOP. - [ ] extensions/mssql/spike/run.mjs:89 — The `import.meta.url` main-module check is fine, but `pathToFileURL` doesn't resolve symlinks. Running the script through a symlinked path would silently do nothing. Suggested fix: compare `fs.realpathSync` results, or accept this as a known limitation of a spike.

david added 2 commits

2026-11-10 23:10:19 +00:00