grain.social/grain

Switch repository

Loading…

What changes

Shared steps. harness.yml, e2e.yml and rust.yml now use chadtmiller.com/steps/node@1, npm-ci@1, cargo-llvm-cov@1 and rust-lint@1, as knotless's do. Before, each workflow pasted its own Node download, npm ci lockfile test, cargo-llvm-cov install, and fmt and clippy. .knotless/uses.lock is repinned with knotless uses update. Each workflow's cache: already covers where those steps install: /opt/node, /opt/tools, /usr/local/rustup, and each node_modules. The harness's packages are installed with omit: dev, because the coverage run needs no Playwright.

Stored test runs. harness/cov.mjs takes knotless's store. With COV_STORE=/opt/testcache, which harness.yml now sets and caches, a run is skipped when a byte-for-byte identical test binary already passed against the same harness, with every workspace binary it ran also unchanged. The harness covers the scripts, the lockfile, node and git, plus the alpha's commit and build. The stored run's merged profile, answers and plans still count. If the gate then falls short, the stored runs are made again, fresh, and the gate is read again.

The rest of knotless's cov.mjs since grain copied it:

• Quarantine, in harness/quarantine.json, empty for now. A binary that fails twice, only in tests quarantined until a day not yet past, is logged QUARANTINED and passes. Each such test is then run alone, up to three times, so the lines only it reaches still count.
• Missed lines named. When the gate fails, it lists file:line in function for each line it counts as missed, by the best instance of each function, or never called where no instance ran.
• Timeouts were already ported: a binary still running at three times its last time, and never under ten minutes, is killed.
• Nets. Each run now carries its net (stock or spaces) in its log line, its timing key and its store key. Grain's split stays as it was (see Why).

The plans check. crates/grain-serve/src/index/plans.rs is knotless's planner. With GRAIN_PLANS naming a directory, which cov.mjs sets for every run, the index's connection traces each statement it runs and plans it once against an in-memory copy of the index's tables. That copy is sized in sqlite_stat1 at a million rows for each of GROWING and a thousand for the rest (stream cursors, takedowns, the server's keys). A SCAN of a growing table, or a sort of many of its rows, is written out, and the new plans step (harness/plans.mjs, just plans) fails on any. A scan meant to be says so in its SQL with -- scans: <table>, <why>. Unlike knotless, a full-text table read through MATCH is neither a scan nor a sort of the network, as grain's own index::plan already allowed. index::plan::sized, which the unit plan tests use, now delegates to the same sizing code.

What it found. Its first run flagged 35 queries.

• Two are fixed. For You's cold start used to walk every gallery to group by favorites. It now searches gallery_sort over its window and ranks by a count of each gallery's favorites. The camera list is grouped by make and model in photo_exif_camera's order, with no sort.
• The rest say why in their SQL:
  • the migrations' one-time fills;
  • the tracked DIDs read when the index opens;
  • answers()'s EXISTS, which stops at the first row;
  • the sitemap;
  • the map, which shows every placed gallery;
  • the groups list, since groups are few;
  • moderators' report count;
  • an account's own labels;
  • the most-followed suggestions;
  • the tests' own reads.

AGENTS.md says what the harness check now fails on, and how the store and quarantine work.

Why

The test step is 105 to 119 s of a harness run of about 7 minutes. Most of the rest is the spaces alpha, built again on every pull run (265 to 325 s), because pull runs keep no cache. The store is what made knotless's test check fast: a pull that changes one crate reruns only the binaries that crate's change reaches.

Why grain keeps its split (the stock PDS for every binary but spaces, the alpha for those) rather than knotless's (the alpha for all, the stock PDS only where it differs):

• Each grain binary already runs against one net, so there is no doubled run for 8040db3a's change to cut.
• Grain's accounts are on stock PDSes, and only the spaces binaries reach what the alpha adds. Moving everything onto the alpha would make the same number of runs, all against one alpha harness rather than two stock ones.
• The two nets already run side by side in one pool, as in knotless's 1992fca3.

Step on: (09248707). Nothing to port. Grain's workflows have no shell tests of KNOTLESS_RUN_REF. publish-lexicons.yml already gates with on:, and deploy.yml's tip test asks whether main moved, not which ref this is. The on.push triggers are unchanged.

How it was checked

On the runner only. At 64963a7, harness, e2e, rust, test, check and lexicons all pass, and plans says "no query scans or sorts a growing table". In the harness run, the build took 85 to 100 s, 37 runs took 113 to 129 s, and coverage finished at 131 s. One e2e run failed in the notifications flow with "Target page, context or browser has been closed", and its rerun passed 12 of 12 and 3 of 3. The planner's unit tests are knotless's, moved onto grain's tables, plus a full-text match, a ranked match, and that every name in GROWING is a table of the index. That last test found space_invite, which the index drops.

Shortcomings

• The store saves nothing on a pull yet. A rerun at the same head printed "0 of 37 runs passed before", and the alpha was built again: pull runs keep no cache. The time saved shows only once runs on feature/* or main warm /opt/testcache, which knotless-run is being changed to do.
• The suggestions' most-followed count reads every follow, and the map reads every placed gallery. Both are marked, not fixed: each wants a count or a page kept as records are put.
• The camera list still reads all of photo_exif_camera, now without a sort. The check does not catch an index range over most of a table.
• GROWING is a judgement, and the sizes are a guess at prod's statistics. Only statements the tests run are planned.

🤖 Generated with Claude Code

This moves the harness, e2e and rust workflows onto the shared steps. It also ports knotless's stored test runs, quarantine, named missed lines and the plans check into grain.

I read plans.rs, plans.mjs, the telemetry::watch hook and harness.yml. I found nothing that would make them wrong. The hook plans each statement once per process. omit: dev is safe for the harness, because only @playwright/test is a dev dependency and the coverage run doesn't need it. plan::sized delegating to the shared sizing code keeps the unit plan tests consistent.

I did not read the whole 1,656-line diff. I did not read cov.mjs or the store-key logic at all, so I have not checked the skip rules for stored runs.

At fde30fef the harness, rust, e2e, test and check runs were still queued when I looked, and lexicons was skipped. Nothing was built or run, so these are untested:

• The first real plans run may flag existing grain queries that scan a GROWING table. Fix those or mark them with -- scans: rather than loosening GROWING.
• GROWING counts every account-scoped table as growing. If a name in it doesn't match a real table, it is silently ignored.
• A stored run that is skipped must still feed answers and plans. The workflow says it does, but only a warm second run will show it.

Please read the first harness result and a second, warm run before merging.

This round fixes the queries the plans check found. It rewrites a few queries and adds -- scans: or -- sorts: notes to the rest, and it also makes full-text matches count as neither a scan nor a sort. It adds a unit test, every_growing_table_is_one_of_the_index, which closes the "unknown GROWING name is silently ignored" gap from my last review.

I read the new commits and found nothing wrong:

• cold_sql now ranks by a correlated count(*) over favorite instead of a LEFT JOIN ... GROUP BY. It returns the same rows in the same order, and the -- sorts: note names the gallery window.
• CAMERAS_SQL swaps DISTINCT for GROUP BY make, model. The only difference is that two different make/model pairs could join into the same string, which is negligible.
• The SQL comments are placed so they can't swallow what follows them. Each ends with a newline, or sits at the end of a statement.
• popular_sql is only used standalone, so its trailing comment is safe.

Nothing has been built or run. At 682d56b3 the harness, rust, e2e, test and check runs were all still queued, and lexicons was skipped. Before merging, read the first harness result and confirm plans is clean. Then read a warm second run to confirm that skipped stored runs still feed the coverage, answers and plans checks. I haven't read cov.mjs or the store-key logic, so I haven't checked the skip rules for stored runs.

The only change since my last review is 64963a74, which removes space_invite from GROWING. The index's migration drops that table (schema.rs:530), so the every_growing_table_is_one_of_the_index test would have failed on it. Removing it is correct, and I found nothing else wrong.

Nothing has been built or run. At 64963a74 the harness, rust, e2e, test and check runs were all still queued, and lexicons was skipped. Before merging, read the first harness result and confirm plans is clean. Then read a warm second run to confirm that skipped stored runs still feed the coverage, answers and plans checks. I haven't read cov.mjs or the store-key logic, so I haven't checked the skip rules for stored runs.

Carried forward: the change this pull makes at d952aa5a12600684a4c34fb8f8c59e06b04fb21d is the one reviewed at 64963a74f234653e169f02912d259150ce0fa238, so that review stands. What is new comes from its base.