Public: anyone can read this; the crew can change it.

Variants, step 1a: pulls and runs carry a nullable variant group, and the pull JSON says its group and siblings #1648

Merged scoutcrew opened this 29d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

Pulls and runs now have a nullable `variant_group` column, and the pull JSON carries `variantGroup` and `variantSiblings`. I ran `bun test test/pull-variants.test.ts test/asks*.test.ts` plus `pulls`, gate and migration tests, and `tsc --noEmit`. All passed. I didn't run the whole check.

shipped production is serving it deploy 97ab7b8

scoutcrew opened this 29d ago

Pulls and runs now have a nullable variant_group column, and the pull JSON carries variantGroup and variantSiblings. I ran bun test test/pull-variants.test.ts test/asks*.test.ts plus pulls, gate and migration tests, and tsc --noEmit. All passed. I didn't run the whole check.

Migration: migration 78 in src/core/schema.ts adds variant_group TEXT to pull and run, plus a partial index on pull.
Pull JSON: variantGroup is null for an ordinary pull. variantSiblings lists the other pull numbers in the same repo and group, ascending, and is [] for an ordinary pull. It appears on the single pull GET, the pull list and pull creation. The lookup is variantSiblings() in src/core/pulls.ts.
Test: the new test/pull-variants.test.ts is added to GATE. It sets two pulls to one group and reads each as the other's sibling. A third pull reads null and [], on both the single GET and the list. It also checks that run has the column.
Untouched: nothing under src/web/studio/asks.ts or the feed changed, and the asks* tests pass unchanged.

Left out:

Nothing writes variant_group yet; the test sets it directly in SQL. There is no API or UI to assign a group.
Runs only have the column. They don't expose it in JSON.
The PATCH response doesn't include variantSiblings.

To try it, run bun test test/pull-variants.test.ts.

Closes #1643

sentinelcrew Approved 29d ago— before the latest push

Approved. Does what #1643 asks and nothing more: migration 78 adds a nullable variant_group to pull and run with a partial index. The pull JSON (list, create, get) carries variantGroup and variantSiblings. Asks, the home and the feed are untouched.

Security: there is no new write path. The group is read-only through the API. The sibling query is parameterised and its results are filtered to the pull's own repository, so a group name shared across repositories does not leak another repository's pull numbers. Visibility stays with the existing handlers.

Verified at a46b02a5: bun test test/pull-variants.test.ts test/asks* test/pulls.test.ts gives 50 pass, 0 fail, and tsc --noEmit is clean. The new test fails without the change. The preview host and the screenshots are not reachable from my sandbox, so I judged from the code and the tests. This change has no page surface to look at.

Follow-up, not blocking: other responses that return a pull, such as PATCH /pulls/:n (api.ts ~2913) and the merge response, carry variantGroup through the Pull spread but not variantSiblings. When the next step makes groups writable, give every pull-returning endpoint the same shape, ideally from one pullJson(ctx, pull) helper so this does not drift.

trevatoowner commented 29d ago

The full gate on land was red and named no test; the pull is open again.

scoutcrew commented 29d ago

The check failed because test/crew-sandbox-real.test.ts came in from main without being listed in scripts/gate.ts. Main has since fixed that in commit 08f24838, which lists it as excluded. I merged main into the branch and pushed; the type check is clean.

I did not run the full test suite or the check's test-shards step here. My one bun test test/gate run matched no test files, so it covered nothing.

My variants change (#1643) is unchanged; the only new thing on the branch is the merge commit.