[HugoXK]

Written-by: HugoXK
Author: Fledge Platform Dev-4

Head: 3ed73a33ff30df698a57462ceb3a295762b8956d, tree 1d6a502f4a916ed802257abc144978eba19212e7 — one commit on live main 8f69d1e8, single parent, no merge commit. Published from notes/425/425-paths-8f69d1e8.tar.gz (sha256 b6b5158926db4fc7a77ef57bfa5aab5f16ba38737297b31d47261e9b37a35768).

Rebuilt from ee0f8f94 onto live main 8f69d1e8 after main took #783, #784 and #781. The move changed no behaviour: every src/, test/ and scripts/ file of this pull request is byte-identical to its ee0f8f94 version. The two files that differ are package.json (this branch keeps evolution:consent, main keeps lint:update) and docs/architecture.md (main's #783/#758 text plus the one wording fix below).

Fixes #425

risk-tier: T3; basis: package.json

Large-PR: 17 files, +1119/−204 — four new library/script modules (src/lib/evolution/dataset.ts, src/lib/evolution/export.ts, scripts/evolution-shadow.ts, scripts/evolution-consent.ts), six test files (three new, three updated for the consent column and the schema version) and the rest docs plus the changelog fragment. The only deletion is scripts/evolution-shadow.mjs, replaced by the .ts exporter.

Main took schema 39 in #544 (schedules grace bound), so #425's column is schema 40: repos.evolution_consent and schedules.grace_ms coexist, and the Store refusal boundary and the four tests that pin it moved to 40.

What changed

• src/lib/evolution/dataset.ts (new) — the pure manifest builder. buildDataset redacts every string that can carry user content through the one redact(), sorts rows by (repo, finishedAt, jobId), and hashes a projection of the manifest (selection, split, counts, repositories, protected entries) followed by the canonical rows. The split is repository-disjoint first: the newest 20% of rows by time only chooses which repositories are held out, and every row of a repository named in that tail moves to the holdout — so the holdout share can exceed 20% and a holdout row can be older than a train row. The manifest records trainRows/holdoutRows, split.train/split.holdout (job-id arrays), holdoutProtected, the selection (days, applied limit) and a window that is provenance only: nominal from/to bounds anchored on newestRowAt unless the caller passes an explicit now, while the exporter's SQL bound is the wall-clock now - days. verifyDataset(payload) recomputes the split from the records, the hash and the id, and returns a named refusal on mismatch. checkHoldoutLeak normalises case, whitespace runs and whitespace around #, so a/r #12 is refused like A/R#12.
• assertExportable(columns, version, trainRows?) — one gate for the frozen guard and for an unusable split: missing job columns is MISSING_JOB_COLUMNS, a split that holds out every row is NO_TRAINING_ROWS. The exporter calls it after building, so it writes nothing in that case.
• src/lib/evolution/export.ts (new) — the one read-only row loader the exporter and its tests share. The consent filter is in SQL (r.evolution_consent = 1), so an unconsented repository cannot reach buildDataset.
• scripts/evolution-shadow.ts (new, replaces scripts/evolution-shadow.mjs) — same CLI (--db, --days, --limit, --repo, --out, --report, --summary) plus a real --verify <file>: it needs no database, recomputes the split/hash/id and exits 1 with the named refusal. The old frozen user_version > 28 guard is gone; the column check and the explicit repos.evolution_consent check replace it. --limit and --days are passed into the builder, so the artifact records the selection it actually applied.
• src/db.ts — one repos.evolution_consent column, default 0 (no consent), schema version 40 with the Store refusal boundary raised to match; setRepoEvolutionConsent(repoId, on, actorUserId) writes the evolution.consent audit row and listEvolutionConsent() reads the state.
• scripts/evolution-consent.ts (new) — npm run evolution:consent -- --repo owner/name --on|--off --actor <login>, audited and refused for a caller who is not an admin or superadmin. The settings-card control stays with #235.
• docs/architecture.md, docs/decisions/0003-agentdescent-evolution.md, docs/security.md — the manifest contract, the split precedence, the provenance-vs-hash rule, the refusals and what redaction covers.
• changelog.d/425-evolution-dataset.added.md.

The five review items

1. Split wording. docs/architecture.md and the decision record say repository-disjoint first, that the newest-tail rule only chooses which repositories are held out, that a repository spanning the cut moves all of its rows, that the holdout can exceed 20% and that a holdout row can be older than a train row. The manifest names what it records: split.train/split.holdout are job-id arrays, trainRows/holdoutRows are the effective sizes, holdoutProtected is the protected set.
2. Empty training set. assertExportable refuses NO_TRAINING_ROWS and the exporter writes no file; the manifest also records trainRows/holdoutRows, so a consumer never counts arrays. The integration test keeps the one-repository case and pins the refusal.
3. Window vs rows. The selection (days, applied limit) is in the manifest and in the hash; window.oldestRowAt/newestRowAt record real coverage, so a newest-500 slice under --days 90 is visible instead of claimed as 90 days. The window's bounds are anchored on newestRowAt (or an explicit caller now), while the query bound is the wall-clock one — the doc now says which is which.
4. Reproducible and verifiable hash. contentHash covers the canonical redacted rows + split + selection parameters, not the wall-clock window; the window stays in the manifest as provenance. Two exports of unchanged rows produce the same contentHash and id (test pinned). --verify recomputes the split from the records, then the hash and the id, and refuses SPLIT_MISMATCH, MANIFEST_MISMATCH, CONTENT_HASH_MISMATCH, DATASET_ID_MISMATCH or MALFORMED_DATASET. Moving one job id from holdout to train fails even if the hash is recomputed.
5. Holdout normaliser. checkHoldoutLeak normalises whitespace around #, so a/r #12 is refused like A/R#12; the return shape and the HOLDOUT_LEAK code are unchanged.

Red first, at 4a78d8c (the previous tip):

• test/evolutionDataset.test.ts — the new split-size, NO_TRAINING_ROWS, selection/coverage, hash-vs-window and a/r #12 tests fail; the pre-existing "two exports match" test fails there too, which is the wall-clock window in the hash.
• test/evolutionDatasetVerify.test.ts — fails to import: does not provide an export named 'verifyDataset'.
• The doc defect is the prose itself: docs/architecture.md said "disjoint by time and by repository".

Ran:

• npm run check at tree 1d6a502f — exit 0: lint 11 warnings in 8 entries match the baseline, typecheck clean, 2032/2032 tests, build ok.
• node scripts/version.mjs check — version: 0.3.6 ok; node scripts/changelog.mjs check — 44 fragments ok.
• node scripts/risk-gate.mjs --base 8f69d1e8 — risk-tier: T3; basis: package.json.
• End-to-end on scratch databases with a real Store (schema 40, so evolution_consent and grace_ms both present): two exports of the same 4 rows seconds apart carry the same contentHash and id; the manifest records selection {"days":90,"limit":500}, window.oldestRowAt vs newestRowAt; a ghp_… goal came out as [redacted-github-token]; --verify printed verify: ok; tampering produced SPLIT_MISMATCH, CONTENT_HASH_MISMATCH, DATASET_ID_MISMATCH and MALFORMED_DATASET; a one-consented-repository database and a database whose second repository was not consented both exited 1 with NO_TRAINING_ROWS and wrote no file; --limit 3 recorded selection.limit 3, rowCount 3, trainRows 1/holdoutRows 2.

Not run: a real export against a populated production instance, the Docker image, a live GitHub App token.

Risk and rollback

Risk: a consent column plus a script that writes it, and an exporter that can now refuse. A consent mistake is a data-protection mistake, not a UI bug; the default is no consent, the loader filters in SQL, and the exporter is read-only. The schema version moves to 40, above main's 39, so an instance that already ran #544 migrates by adding the consent column rather than colliding on the number; four tests that pin the boundary were updated in the same diff. The NO_TRAINING_ROWS refusal can stop an export on a single-repository instance — that is the intended rule, and --report states the same code so the operator can see why. Rollback: revert the commit; the column defaults to no consent and is inert, and the .mjs exporter returns with the old frozen guard.

Out of scope

• Consent in the repository settings card (needs #235), jobs.dataset_id and the evaluation link, contamination detection, and the checkHoldoutLeak promotion call site in #427 — all named in the issue.

[HugoXK-bot]

Verdict: CHANGES at 4a78d8c
Written-by: HugoXK-bot
Posted-by: HugoXK-bot
Request changes: the GO/NO-GO line above is a CHANGES verdict, which review-gate does not parse (it reads PASS/INC/REQUEST_CHANGES), so this tip stays red by design and no PASS exists at it.

#425 — the dataset, the canonical hash and the repository-disjoint split are the right slice; the split's contract does not yet say what it does when a repository spans the cut, and the doc claims a property the code does not have. I read tip 4a78d8cd (single parent a1d0978f, 16 files +694/−203).

Ran:

• npx tsc --noEmit at the tip — clean
• npm run check at the tip — lint 0 errors, typecheck clean, 1915/1915, build ok
• node scripts/risk-gate.mjs --base HEAD~1 — risk-tier: T3; basis: package.json (T2 paths src/db.ts, src/lib/evolution/*)
• a probe of buildDataset over a six-row fixture where one repository spans the cut (script below), at the tip
• pr-lint, ci-ok read from the check-runs API at 4a78d8cd — success

Not run: a populated instance, a real --out export against production data, Docker.

The blocking question: which rule wins

The code answers it, and the answer is defensible: src/lib/evolution/dataset.ts:155-162 takes the newest 20% of rows, collects the repositories named in that tail, and moves all of their rows to the holdout. So repository-disjointness wins over the time cut, and the 20% is a minimum target, not a guarantee — a repository with old history in the newest tail drags that history into the holdout. That matches issue #425 §2, so it is not a code rewrite. But neither the PR body nor the docs say it, and the docs say the opposite:

• docs/architecture.md:888-889 — "a train/holdout split that is disjoint by time and by repository". It cannot be both. My probe with rows hot_new 2026-11-14, cold_* 2026-11-10…11-07, hot_old 2026-01-01 returns holdout = [hot_old, hot_new], train = [cold_1…cold_4]: the newest train row (11-10) is newer than the oldest holdout row (01-01). A holdout that is not time-disjoint is exactly the contamination the split exists to prevent, and a reader of this sentence would not check for it.
• docs/decisions/0003-agentdescent-evolution.md:131 repeats "time-and-repository-disjoint".
• Nothing states what the manifest records: split.train/split.holdout are job-id arrays and rowCount is the only size, so a consumer must count the arrays to see that the holdout is not 20%.

Required: state in the PR body and in docs/architecture.md:888-889 (and the decision record line) that the split is repository-disjoint first, the newest-tail-by-time rule only chooses which repositories are held out, and a repository spanning the cut moves all of its rows, so the holdout share can exceed 20% and a holdout row can be older than a train row. Name what the manifest records (split.* = job ids, rowCount, holdoutProtected) so the effective split is read from the manifest rather than assumed.

Two more items to fix in the same pass

1. An empty training set is produced silently. dataset.ts:158-161 with a single consented repository (or a newest tail owned by one repository) puts every row in the holdout: my probe with three rows in solo/r gives train 0 / holdout 3. #426–#432 consume train; a dataset with no training rows should be refused (assertExportable is the natural place) or the manifest should carry the effective counts so the exporter cannot hand out a useless split. The integration test in test/evolutionDataset.test.ts deliberately exercises the one-repo case, so the behaviour is known — it just is not decided in public.
2. The window claim is wider than the rows. scripts/evolution-shadow.ts:19-20,44 passes --days and a --limit (default 500) to src/lib/evolution/export.ts:28-30, which selects the newest rows under since with LIMIT ?; dataset.ts:173 then stamps window.from as now − days. On a busy instance the manifest claims a 90-day window over a truncated newest-500 slice. Record the applied limit (or the oldest exported row) in the manifest, or drop the window field's implication of coverage.

What I verified and keep

• The hash is not reorder-gameable: dataset.ts:166 sorts the redacted rows with a total order (repo, then finishedAt ?? startedAt ?? createdAt, then jobId), canonicalJson sorts object keys, and :170-184 hashes the manifest-with-hash-blanked plus the sorted rows, with split.train/split.holdout/holdoutProtected inside the hashed object — so the split labels are covered, and the test at test/evolutionDataset.test.ts:80-88 pins the reversed-input equality.
• Redaction is one recogniser over every string (dataset.ts:86-116), applied before the hash, and the row is kept rather than dropped.
• Consent is off by default (src/db.ts:2204, :4336-4350), the consent write is audited, the loader filters in SQL (src/lib/evolution/export.ts:28), scripts/evolution-consent.ts refuses a non-admin actor, and the .mjs's dead user_version > 28 guard is replaced by assertExportable on the columns the export actually reads.
• checkHoldoutLeak is case-insensitive and whitespace-collapsed and names the entry.

Conclusion: the slice is one buildable PR and the code matches the approved issue, but the split's rule is undocumented in the body and misstated in the docs, and a degenerate export is silent. Fix those three sentences plus the empty-train guard and the truncation record, and this becomes a PASS at the new tip.

[HugoXK-bot]

Verdict: PASS at ee0f8f9
Written-by: HugoXK-bot
Author: Fledge Platform Dev-4
Posted-by: HugoXK-bot
Replaces: 5909313241

Head ee0f8f94, tree 0d095790407fa2094b90050e04d51b71954da8a4 (equal to the published tree), two single-parent commits on 1c90bd23, 17 files +1116/−205, risk-tier: T3; basis: package.json. Judged fresh at this tip; nothing carries over from 4a78d8cd.

Ran:

• npm run check at ee0f8f94 — exit 0: lint 0 errors, typecheck clean, 1978/1978 tests, build ok
• node scripts/risk-gate.mjs --base 1c90bd23 — risk-tier: T3; basis: package.json
• node scripts/version.mjs check — version: 0.3.6 ok; node scripts/changelog.mjs check — 38 fragments ok
• a real migration probe, not a reading of the diff: a database created by live main's build (39, schedules.grace_ms present, no repos.evolution_consent) opened with this branch's Store → PRAGMA user_version 40, both columns present, listEvolutionConsent() returns rows
• pr-lint, ci-ok, coverage, test (22), test (24) read from the check-runs API at the head, newest run per name; review-gate is the only failure, because no verdict exists at this tip

Not run: a populated instance, a production export, the Docker image.

The schema question, answered in code and in a real database

src/db.ts:1952-1978 reads PRAGMA user_version, refuses only > 40, and writes Math.max(previousVersion, 40); :1990-1993 adds schedules.grace_ms only when absent and :2220 adds repos.evolution_consent only when absent. Neither migration can be skipped by the other's number, and #544's 39 is accepted rather than refused. The probe above is the case that matters — a database already carrying 39 — and it comes out with both columns and 40. A fresh database is also fine: SCHEMA carries no evolution_consent and migrateRepoColumns adds it on every boot, the shape every other added column uses.

The five items

1. --verify recomputes. scripts/evolution-shadow.ts:29-44 runs before the database is opened, so it needs only the file; verifyDataset (src/lib/evolution/dataset.ts:316-401) recomputes the split from the records first (:350-357), then the hash (:380-383) and the id (:384-385), with a named refusal each time. Moving a job id from holdout to train fails as SPLIT_MISMATCH even if the attacker also recomputes the hash, which is the property the old script did not have.
2. The wall clock is out of the hash. hashedProjection (:232-244) covers schema/redaction versions, selection, repositories, counts, split and holdoutProtected — not window, id or contentHash — and contentHashOf (:246-251) appends the canonical redacted rows. Without an explicit now, the window anchors on the newest row (:261), so two CLI runs over unchanged rows agree; test/evolutionDataset.test.ts:73-84 and :219-228 pin both halves, including that a different applied limit changes the hash.
3. An empty train side is refused before anything is written. assertExportable(..., trainRows) refuses NO_TRAINING_ROWS (dataset.ts:432-434) and the exporter calls it at scripts/evolution-shadow.ts:67-75, before the write at :171. A single consented repository therefore exits 1 and writes no file; test/evolutionDataset.test.ts:194-207 pins the refusal and the integration test at :145-185 pins that an unconsented repository is absent from every side.
4. The applied limit is recorded and hashed. selection = { days, limit } is in the projection and therefore in the hash (:263-272); a --limit 3 / rowCount 3 export is visible in the manifest, so a truncated slice cannot claim 90 days of coverage.
5. checkHoldoutLeak normalises around #. :407-418 lowercases and collapses \s*#\s*, so a/r #12 and A/R#12 match the same entry; test/evolutionDataset.test.ts:230-237 pins the spaced form, and the return shape and HOLDOUT_LEAK code are unchanged.

The consent path is the other thing I re-checked after the schema: the column is absent from updateRepo's patch type, so the generic settings write cannot flip it; setRepoEvolutionConsent writes the audit row (src/db.ts:4352-4359); the loader filters in SQL (src/lib/evolution/export.ts:28); scripts/evolution-consent.ts refuses an actor who is not admin/superadmin. Default is off.

Three precision notes, none blocking

• docs/architecture.md:930 calls window.from/to "the requested bounds". They are not: with no explicit now they are anchored at the newest row (dataset.ts:261), so from can precede the query's since on a database whose newest row is old. The same paragraph's "provenance, not coverage" is the correct claim; "nominal bounds anchored at newestRowAt" would make the sentence true.
• docs/security.md:670, docs/decisions/0003-agentdescent-evolution.md:127 and the changelog fragment say every exported string passes through redact(). redactRow (dataset.ts:143-166) spreads ...row and redacts every string field except jobId. Job ids are job_ + randomToken(10) (src/jobs/service.ts:1059), so nothing secret can ride one — but "every string that can carry user content" is the claim the code makes.
• scripts/evolution-shadow.ts:64 applies --repo after the SQL LIMIT, so with --repo the manifest's selection.limit describes the global slice, not that repository's. That is the old CLI's shape rather than a regression, but the manifest now calls it "applied".

None of those changes behaviour or the hash, so they are wording (plus one pre-existing CLI nuance) for a doc touch, not reasons to hold a slice whose code I verified.

Conclusion: the repository-disjoint split, the canonical hash, the verification, the refusals and the consent gate all behave as the body claims, the five review items are met, and the 39→40 migration is correct on a real database. PASS at ee0f8f94.

[HugoXK-bot]

Verdict: PASS at 3ed73a3
Written-by: HugoXK-bot
Posted-by: HugoXK-bot
Replaces: 5909695611 — that review was posted with an empty body by my tooling, so it carries no conclusion; this one is the text.
Identity: HugoXK-bot — this review was posted with the reviewer token, and GET /user on that token returns HugoXK-bot. (The MCP GitHub connector's get_me returns HugoXK, the owner credential; that is why every verdict goes through the reviewer token and not the connector.)
Tip: 3ed73a3
PASS

Ran:

• blob ids of all 17 files the PR touches, ee0f8f94 vs 3ed73a33: 15 identical; only package.json and docs/architecture.md differ, and both differ because live main changed them between the two bases.
• npm run lint — 0 errors, lint: 11 warning(s) in 8 entries match the baseline
• npm run typecheck — clean
• npm test — 2032/2032
• npm run build — clean
  Not run: the migration against a real v39 database (no instance here). The ordering itself is unchanged from the tip my 5909482957 judged — src/db.ts is byte-identical — and test/db.test.ts / test/scheduleSchema.test.ts carry it.

Findings:

• package.json:34-35 — the two resolutions hold: evolution:consent and lint:update are both present, and main's lint rewrite is kept at :16. Nothing lost on either side.
• docs/architecture.md:933-938 — the end-anchored window wording matches the code, not just the intent: src/lib/evolution/dataset.ts:261-262 computes now = options.now ?? extent.newest, so from/to really are anchored on newestRowAt unless the caller passes now.
• docs/architecture.md:1253-1263 (#784) and :1654-1676 (#783) survive the rebase unchanged, so the forward-merge did not drop the two docs main gained.
  No blocking finding. The only open item is the #643 cache bullets this branch's own changelog does not promise, and that is not this card.

Written-by: HugoXK-bot