ci: smoke-run TypeScript extractor against a fixture#192
Merged
Conversation
This was referenced May 30, 2026
The existing `node --check` step on the extractor entrypoints only confirms the file parses — a `.mjs` that parses cleanly can still emit a catalog that violates `docs/pipeline-contract.md` (wrong `schema_version`, missing required fields, non-deterministic ordering, regressed kind classification). Add a fixture-based smoke test that exercises the actual extraction path and asserts contract conformance in CI. The fixture (`extractors/typescript/tests/fixtures/catalog.ts`) covers the five declaration shapes called out in the issue: a plain interface (`FlowsheetEntry`), a Zod schema (`ListenerSchema`), a Drizzle table (`stations`), a generic type alias (`PageEnvelope<T>`), and a re-export (`FlowsheetEntry as FlowsheetEntryRe`). The runner (`extractors/typescript/tests/test_smoke.sh`) runs the extractor twice and asserts: `schema_version == "1.1"`, the extractor provenance block is well-formed, `entries[]` is an array, every expected declaration name is present, each name maps to its expected kind, every entry carries the contract-required field set, `extends`/`references` are arrays (never null), and the two runs are byte-identical (determinism guard). Wired into the `pipeline` job after the `Syntax-check extractor entrypoints` step; the runner is also added to the `shellcheck` job's file list. Bash style and assertion shape match the existing runners under `pipeline/queries/_tests/` and `extractors/swift/tests/`. Out-of-scope per the issue: end-to-end extract→classify→cluster runs and parallel smoke tests for `function-catalog.mjs` / `file-hashes.mjs`. The runner doesn't generalize trivially across the three extractors (different catalog shapes, different expected fields), so the stretch goal stays as a follow-up. Closes #170
Addresses review feedback on the assertion-design bugs and a few high-value coverage gaps in the smoke test.
Assertion design:
- Run the assertion phase under `set +e` so every assertion fires; under `set -euo pipefail`, the earlier `ACTUAL_SCHEMA=$(jq ...)` / `ACTUAL_NAMES=$(jq | sort | tr)` / `actual=$(jq ...)` captures aborted the script on any malformed JSON, bypassing the PASS/FAIL tally the wrapper was designed around.
- Switch the required-fields check from `has("x")` to `.x != null` for scalar fields. jq's `has` returns true for null values; a regression that silently nulled `references_count`, `line`, `is_test`, or other required scalars would have passed.
- Add a `(.entries | length > 0)` guard in front of the `[ ... ] | all` comprehensions so an extractor regression that emits zero entries surfaces explicitly rather than getting two misleading vacuous PASSes.
- End-anchor the version regex (`^\d+\.\d+\.\d+$`); the unanchored form passed trailing garbage like `"0.5.0junk"` or `"0.5.0 (dev)"`.
- Drop the `2>$WORK_DIR/run*.stderr` redirection. The captured stderr was never displayed and the EXIT trap wiped it, hiding the extractor's `WARNING:` / `ERR file:` lines and any crash diagnostics. Letting stderr stream to the terminal makes failures self-explanatory in CI logs.
- `assert_kind` now checks cardinality before comparing kind, so a duplicate-name or missing-name regression produces a clear FAIL instead of a vague multi-line or empty `actual`.
Coverage:
- Add `secondary.ts` as a second fixture file so the determinism guard actually exercises inter-file ordering rather than collapsing to a single file's source-position order.
- Add a self-reference (`previous_entry: FlowsheetEntry | null`) on FlowsheetEntry so the references walker has at least one populated `references[]`; add an explicit assertion that the self-reference is emitted.
- Add `SyncResult = { ok: true; ... } | { ok: false; ... }` to cover `kind: type-alias-union`, a shape the original four-entry fixture didn't reach.
- Add content assertions on the drizzle table (`db_table_name == "stations"`, non-empty `fields`) and the zod schema (non-empty `fields`) so regressions in `extractDrizzleTableFields` / first-arg pull / `extractZodObjectFields` that leave entries structurally complete but semantically empty get caught.
- Remove the same-file `export { FlowsheetEntry as FlowsheetEntryRe }` line. It produced no catalog entry (no `moduleSpecifier`), so no assertion verified anything beyond parser survival — coverage identical to the existing `node --check` step.
Deferred to follow-ups: wiring the existing node:test suite into CI; verifying `.extractor.version` matches `package.json`; varying `exported: true/false` in the fixture; covering type-alias-intersection / type-alias-infer-model kinds.
Verified locally: 16/16 assertions pass on the updated fixture; the new assertions catch their targeted regressions when fed synthetic bad output (null scalars, empty entries, trailing-garbage version, missing self-ref, null db_table_name).
Second-round review surfaced three assertion-design holes the first hardening pass missed.
Self-reference assertion (line 176): `[.entries[] | select(.name == "FlowsheetEntry") | ... | index("FlowsheetEntry")] | all(. != null)` is vacuously true on empty selector — `[] | all(...)` is true in jq. A regression that drops FlowsheetEntry entirely or renames it would silently PASS the references-walker check. The `(.entries | length > 0) and` guard the first pass added to the two surrounding assertions was not applied here. Fix: gate on `[.entries[] | select(.name == "FlowsheetEntry")] | length > 0` first, then assert via `any(.name == "FlowsheetEntry")` over the selected entries.
Drizzle and zod content assertions (lines 186, 193): `.entries[] | select(.name == "X") | <predicate>` streams one boolean per match into `jq -e`. `jq -e` exits 0 if the LAST output is truthy, not if all are — verified empirically: `jq -n -e 'false, true'` exits 0; `jq -n -e 'true, false'` exits 1. So on a duplicate-name regression (one degraded entry + one valid), emission order can mask the bad one. Mitigated upstream by `assert_kind`'s cardinality check, but the assertions are brittle in isolation. Fix: switch to the `[...] | all` pattern already used by the required-fields and arrays-are-arrays assertions, with the same `length > 0` guard.
Fixture comment (catalog.ts line 6): documented references shape as `["FlowsheetEntry"]` (string array) — the actual extractor emission is `[{name: "FlowsheetEntry", kind: "type-ref"}]` (object array), which the assertion on line 176 already accounts for via `map(.name)`. Doc-only drift; corrected for accuracy.
Verified locally: 16/16 PASS on the real fixture; all three regression classes (FlowsheetEntry missing, FlowsheetEntry without self-ref, duplicate stations with mixed validity) are now caught.
jakebromberg
force-pushed
the
ci-smoke-typescript-extractor
branch
from
May 30, 2026 19:20
d369ef2 to
0a29282
Compare
This was referenced May 30, 2026
jakebromberg
added a commit
that referenced
this pull request
May 30, 2026
Follow-up to #192. Wires the existing `node --test test/*.test.mjs` suite (97 assertions, ~1.5s) into the pipeline job in CI, placed after the deps install and before the smoke step so a red unit test short-circuits the slower fixture-driven smoke run. The two layers are complementary: node:test covers the references walker, intersection resolver, $inferSelect detector, dedupe and sort comparators; the smoke step covers contract-required-fields, schema-version pin, and multi-run determinism. Closes #197.
jakebromberg
added a commit
that referenced
this pull request
May 30, 2026
…n + infer-model kinds Three follow-up enhancements deferred from PR #192's review (closes #198), bundled because each is small and they all land in the same fixture/smoke-test file pair. (1) Replace the end-anchored semver regex on `.extractor.version` with an exact-equality check against `extractors/typescript/package.json`'s `.version` read at script start — a regex check passes forever on a stale hardcoded literal that happens to be parseable, but the extractor reads the live package.json on every run, so an `EXPECTED_VERSION` baseline catches that regression class. (2) Add a non-exported `interface InternalDraftEntry` to the fixture and pair it with an `assert_exported` helper that pins one positive (`FlowsheetEntry == true`) and one negative (`InternalDraftEntry == false`); without the negative, a regression in `exportedMod()` that flipped the default would be invisible since every existing fixture entry is exported. (3) Add `FlowsheetEntryWithStation = FlowsheetEntry & { station_id: number }` and `Station = typeof stations.$inferSelect` to the fixture, with `assert_kind` calls pinning them to `type-alias-intersection` and `type-alias-infer-model` respectively — these were the last two `type-alias-*` kinds untested by smoke and exercise the 5-iteration intersection resolver and the modern Drizzle `$inferSelect` detection path in `type-catalog.mjs`. All 21 smoke assertions pass; `npm test` still green at 97/97.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The existing
node --checkstep on the extractor entrypoints only confirms the file parses — a.mjsthat parses cleanly can still emit a catalog that violatesdocs/pipeline-contract.md(wrongschema_version, missing required fields, non-deterministic ordering, regressed kind classification). This PR adds a fixture-based smoke test that exercises the actual extraction path in CI.extractors/typescript/tests/fixtures/catalog.ts— covers the five declaration shapes called out in the issue (plain interface, Zod schema, Drizzle table, generic type alias, re-export).extractors/typescript/tests/test_smoke.sh— runs the extractor twice and asserts contract conformance.pipelinejob afterSyntax-check extractor entrypoints; also added to theshellcheckjob's file list.Assertions
schema_version == "1.1"(matches the contract)extractorprovenance block well-formed (language=typescript,name=type-catalog, semverversion)entries[]is an arrayFlowsheetEntry,ListenerSchema,PageEnvelope,stations)kind(interface,zod-object,type-alias-object,drizzle-table)name,kind,package,file,line,is_test,extends,references,references_count)extendsandreferencesare arrays on every entry (nevernull)Out of scope
Per the issue: end-to-end extract→classify→cluster runs and parallel smoke tests for
function-catalog.mjs/file-hashes.mjs. The runner doesn't generalize trivially across the three extractors (different catalog shapes, different expected fields), so the stretch goal stays as a follow-up ticket.Test plan
extractors/typescript/tests/test_smoke.shexits 0 locally (11 assertions pass).shellcheck extractors/typescript/tests/test_smoke.shis clean.pipeline/queries/_tests/test_canonical.shstill passes (unchanged).Closes #170