mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 14:34:42 +01:00
test(shared-libs-plan-callers): judge scope by forbidden verb+object actions, not an exclusion grammar; replay six refusals and four passing censuses
This commit is contained in:
1 parent
41287009bd
commit
2a6b766c5b
5 files changed
+201
-18
No files matched your search
@@ -0,0 +1,23 @@
|
||||
{
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 \u2014 Which parser should the two new Retry-After callers use?\nProject/branch/task: fixture app, branch main, planning src/import-worker.ts and src/sync-route.ts (not yet written).\nELI10: Both new files need to turn a Retry-After header into a number of seconds, exactly the way the scheduler already does. The scheduler gets that from one small existing function, lib/retry-after.ts, whose signature already matches what the plan asks for (string or null, injected clock, caller fallback, 3600 cap). The draft plan instead writes a fresh copy of that logic inside each new file. The question is whether to import the existing function or write two copies.\nStakes if we pick wrong: with copies, a header like '-5' or a past date can quietly produce a different delay in each file, and the plan's fixed compatibility requirement cannot be proven; with reuse but thin tests, a later change to the helper could break all three callers before anyone notices.\nRecommendation: A because the plan fixes behavior to the existing helper, so importing it is the only way to make equivalence true by construction, and the extra contract tests are a few lines.\nCompleteness: A=10/10, B=7/10, C=5/10\nPros / cons:\nA) Reuse helper + contract tests (recommended)\n \u2705 Equivalence with scheduler behavior holds by construction, including engine-dependent Date.parse edges like '-5' \u2192 0\n \u2705 Adds about six shared assertions that document null+fallback, past date \u2192 0, ceiling and injected now, which today's test omits\n \u274c Any regression in lib/retry-after.ts now affects three callers at once instead of one\nB) Reuse helper, existing tests\n \u2705 Same by-construction equivalence as A with the smallest possible diff (two import lines plus caller tests)\n \u2705 No change to the shared test file, so nothing outside the two new callers is touched\n \u274c Edge cases the new callers rely on (custom fallback, past dates, ceiling on dates, non-zero now) stay unproven at the contract level\nC) Local parser per caller\n \u2705 Each caller is self-contained and can be read without following an import\n \u2705 No shared blast radius: a bug in one copy cannot break the scheduler\n \u274c Three copies of the same ~14 lines that must stay identical forever; the plan's fixed-compatibility requirement is only provable by copying verbatim, and drift is silent\nNet: trading a slightly wider shared blast radius (already accepted for the scheduler) for provable compatibility and about 26 fewer implementation lines; B is A without the proof, C is the draft's duplication.",
|
||||
"header": "Parser reuse",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Reuse helper + contract tests",
|
||||
"description": "Import retrySeconds from lib/retry-after.ts in both new callers and pass each caller's injected now and fallback through. Extend test/retry-after.test.ts with ~6 assertions (null + custom fallback, whitespace \u2192 fallback, past HTTP-date \u2192 0, HTTP-date beyond ceiling \u2192 3600, unsafe-integer \u2192 fallback, non-zero injected now). Each caller's integration test asserts now and fallback are forwarded and the 3600 ceiling holds. lib/retry-after.ts semantics and existing callers unchanged. Completeness 10/10. human ~1 h / CC ~5 min. \u2705 Equivalence by construction, including engine-dependent edges. \u2705 Contract edges the callers depend on become tested once. \u274c A helper regression affects scheduler plus both callers; mitigated by the shared tests."
|
||||
},
|
||||
{
|
||||
"label": "Reuse helper, existing tests",
|
||||
"description": "Import retrySeconds from lib/retry-after.ts in both new callers exactly as in A, but leave test/retry-after.test.ts at its current 5 assertions. Each caller's integration test asserts now and fallback are forwarded and the ceiling holds. lib/retry-after.ts semantics and existing callers unchanged. Completeness 7/10. human ~30 min / CC ~3 min. \u2705 Same by-construction equivalence, smallest diff. \u2705 Touches nothing outside the two new callers and their tests. \u274c Null + custom fallback, past dates, date ceiling and non-zero now remain unproven at the shared-contract level."
|
||||
},
|
||||
{
|
||||
"label": "Local parser per caller",
|
||||
"description": "Keep the draft: write a parser inside src/import-worker.ts and inside src/sync-route.ts reproducing lib/retry-after.ts:2-15, each with its own ~8-assertion edge suite plus the integration test. lib/retry-after.ts and existing callers untouched. Completeness 5/10. human ~3 h / CC ~15 min. \u2705 Self-contained files, no import to follow. \u2705 No shared blast radius. \u274c Three copies that must stay identical; engine-dependent Date.parse behavior cannot be proven equal without verbatim copying, and later drift is silent."
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,23 @@
|
||||
{
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 \u2014 Should the two new callers reuse lib/retry-after.ts or carry local parsers?\nProject/branch/task: main \u2014 plan review of PLAN.md (import-worker and sync-route Retry-After parsing under the fixed scheduler contract).\nELI10: Both new files need to turn a Retry-After header (a number of seconds, an HTTP date, or nothing) into a wait time that is capped at one hour and falls back to a caller-chosen number when the header is junk. The scheduler already does exactly this through one shared function in lib/retry-after.ts, which has its own contract test. The plan drafts a fresh copy of that logic inside each new file. Two more copies means three places that must stay identical forever, while one import means the behavior cannot drift.\nStakes if we pick wrong: a copy that diverges, or an unpinned edge case, makes a worker or route wait a different time than the scheduler for the same header, and nobody notices until a backoff misbehaves in production.\nRecommendation: A because the required behavior is identical by definition, the helper is already proven and tested, and it saves about 26 implementation lines; the extra contract assertions cost minutes and are the only proof that the \"fixed\" edge cases actually hold.\nCompleteness: A=10/10, B=7/10, C=4/10\nPros / cons:\nA) Reuse helper, extend tests (recommended) (human: ~1.5 h / CC: ~5 min)\n \u2705 Both callers import retrySeconds(value, now, fallback) from lib/retry-after.ts, the same call shape src/scheduler.ts:2 already uses, so semantics match by construction\n \u2705 Adds the missing shared-contract assertions the callers depend on (empty or whitespace string returns fallback, \"0\" returns 0, past date returns 0, far-future date returns 3600, unsafe integer returns fallback, unparseable date returns the caller fallback) plus one integration test per caller that passes an explicit now and a non-default fallback\n \u274c Grows test/retry-after.test.ts by about 6-8 lines and widens the blast radius: a future helper bug or hardening change reaches scheduler, import-worker and sync-route at once\nB) Reuse helper, existing tests (human: ~1 h / CC: ~3 min)\n \u2705 Same import wiring and the same ~26-line implementation saving as A, with no change to the shared test file\n \u2705 Lowest-touch diff: only the two new source files and their two integration tests\n \u274c The five existing assertions never exercise past dates, the date ceiling, empty strings or unsafe integers, so the \"fixed\" edge-case compatibility stays unproven at the shared layer\nC) Local parser per caller (human: ~2 h / CC: ~5 min)\n \u2705 Each caller is self-contained; a later change to lib/retry-after.ts cannot alter these two files\n \u2705 Matches the plan as currently drafted, so no plan amendment is needed\n \u274c Adds about 28 duplicated lines, creates two more copies that must replicate every quirk of the runtime's Date.parse leniency to stay scheduler-compatible, and only integration tests guard each copy\nNet: one import plus a handful of contract assertions buys guaranteed parity and a smaller diff; the only thing given up is isolation from future helper changes, which the plan says must be shared anyway.",
|
||||
"header": "Parser reuse",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Reuse helper, extend tests (recommended)",
|
||||
"description": "\u2705 Both callers import retrySeconds(value, now, fallback) from lib/retry-after.ts, the same call shape src/scheduler.ts:2 already uses, so semantics match by construction\n\u2705 Adds the missing shared-contract assertions the callers depend on (empty or whitespace string returns fallback, \"0\" returns 0, past date returns 0, far-future date returns 3600, unsafe integer returns fallback, unparseable date returns the caller fallback) plus one integration test per caller that passes an explicit now and a non-default fallback\n\u274c Grows test/retry-after.test.ts by about 6-8 lines and widens the blast radius: a future helper bug or hardening change reaches scheduler, import-worker and sync-route at once"
|
||||
},
|
||||
{
|
||||
"label": "Reuse helper, existing tests",
|
||||
"description": "\u2705 Same import wiring and the same ~26-line implementation saving as A, with no change to the shared test file\n\u2705 Lowest-touch diff: only the two new source files and their two integration tests\n\u274c The five existing assertions never exercise past dates, the date ceiling, empty strings or unsafe integers, so the \"fixed\" edge-case compatibility stays unproven at the shared layer"
|
||||
},
|
||||
{
|
||||
"label": "Local parser per caller",
|
||||
"description": "\u2705 Each caller is self-contained; a later change to lib/retry-after.ts cannot alter these two files\n\u2705 Matches the plan as currently drafted, so no plan amendment is needed\n\u274c Adds about 28 duplicated lines, creates two more copies that must replicate every quirk of the runtime's Date.parse leniency to stay scheduler-compatible, and only integration tests guard each copy"
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
@@ -0,0 +1,106 @@
|
||||
[
|
||||
{
|
||||
"census": "37176837432",
|
||||
"input": {
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 \u2014 Reuse the shared Retry-After parser for the two proposed callers, or copy it locally?\nProject/branch/task: gstack-shared-plan-callers fixture, branch main, planning src/import-worker.ts and src/sync-route.ts.\nELI10: Both new files need to turn a Retry-After header into a wait in seconds, exactly the way the scheduler already does. The scheduler already gets that from one tested function in lib/retry-after.ts. The plan instead writes a fresh copy of that logic inside each new file. Copies look identical today but can drift apart silently, so a future fix to one would not reach the others.\nStakes if we pick wrong: with local copies, a later ceiling or date-parsing fix lands in one place and the other callers keep waiting the wrong number of seconds; with reuse but thin proof, callers depend on past-date and date-ceiling behavior that no test pins down.\nRecommendation: A because the shared helper already exists, is exercised by the scheduler, and reuse removes about 26 implementation lines while adding only the contract cases the new callers rely on.\nCompleteness: A=10/10, B=7/10, C=5/10\nPros / cons:\nA) Reuse helper, full proof (recommended)\n \u2705 Zero new parser logic; both callers import retrySeconds(value, now, fallback) exactly as src/scheduler.ts does today\n \u2705 Adds the missing regression cases (past date \u2192 0, date beyond cap \u2192 3600, unsafe integer \u2192 fallback, whitespace-only \u2192 fallback) to test/retry-after.test.ts, plus one delegation assertion per caller integration test\n \u274c Any future bug in the shared helper now reaches the scheduler and both new callers at once (human: ~1 h / CC: ~5 min)\nB) Reuse helper, existing proof\n \u2705 Same import wiring as A with no changes to the shared test file\n \u2705 Smallest possible diff: roughly two lines per caller plus the integration tests the plan already promises\n \u274c Past-date and date-ceiling paths the callers depend on remain untested, so a regression there would reach production unnoticed (human: ~45 min / CC: ~3 min)\nC) Local parser per caller\n \u2705 Each caller is fully self-contained and can diverge later without touching shared code\n \u2705 No change to lib/retry-after.ts or its test file\n \u274c Adds two more 15-line copies of logic that already exists three times in the repo, each needing its own full contract test to meet the every-behavior-tested bar (human: ~2 h / CC: ~10 min)\nNet: trading a slightly larger shared blast radius for one tested source of truth and about 26 fewer implementation lines.",
|
||||
"header": "Parser reuse",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Reuse helper, full proof",
|
||||
"description": "Both src/import-worker.ts and src/sync-route.ts import retrySeconds from lib/retry-after.ts and call retrySeconds(header, now, fallback); no local parser. Shared contract stays unchanged. Adds four regression cases to test/retry-after.test.ts (past date \u2192 0, date beyond 3600 s cap \u2192 3600, unsafe integer \u2192 fallback, whitespace-only \u2192 fallback). Each caller's integration test asserts its wait equals the helper's output for an injected now, a null header and one date header. Implementation \u2248 +4 lines vs ~30 drafted (saves ~26); tests \u2248 +6. Blast radius: scheduler plus both callers. Existing copies in src/retry-route.ts and src/retry-worker.ts stay untouched. (human: ~1 h / CC: ~5 min)"
|
||||
},
|
||||
{
|
||||
"label": "Reuse helper, existing proof",
|
||||
"description": "Same import wiring as A in both callers, with the same caller integration tests, but test/retry-after.test.ts stays at its current five cases. Shared contract unchanged. Implementation \u2248 +4 lines (saves ~26); tests \u2248 +2. The past-date \u2192 0, date-ceiling, unsafe-integer and whitespace-only paths remain unpinned by any test. Existing copies stay untouched. (human: ~45 min / CC: ~3 min)"
|
||||
},
|
||||
{
|
||||
"label": "Local parser per caller",
|
||||
"description": "Keep the plan as drafted: each caller carries its own 15-line copy of the parser matching lib/retry-after.ts byte-for-byte. No change to lib/retry-after.ts or its test. Each caller integration test must also repeat the full contract cases (null, invalid, trimmed digits, cap, date, past date) so each copy is independently pinned. Implementation \u2248 +30 lines (saves 0); tests \u2248 +16-20. Any future parser fix must be applied to each copy separately. (human: ~2 h / CC: ~10 min)"
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"census": "37179171083",
|
||||
"input": {
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 \u2014 Where should import-worker.ts and sync-route.ts get their Retry-After parser?\nProject/branch/task: repo on main \u2014 plan \"Import and synchronization retry planning\" (PLAN.md), two proposed callers under the fixed scheduler contract.\nELI10: Two new pieces of code need to turn a server's \"Retry-After\" header into a number of seconds to wait, exactly the way the scheduler already does. The repo already has one proven function for this, lib/retry-after.ts, with a passing test, and the scheduler already uses it. The plan draft instead copies the parsing logic into each new file. Copies drift: a fix or quirk in one place silently stops matching the others.\nStakes if we pick wrong: with copies, the two new callers can diverge from scheduler timing (a different cap or fallback) and no single test proves they still match; with reuse, a parser bug touches three callers at once, which is already true for the scheduler today.\nRecommendation: A because lib/retry-after.ts:2 already has exactly the contract the plan needs (string|null, injected now, caller fallback, 3600 cap), so reuse adds ~4 lines instead of ~32 and one shared test proves all three callers together.\nCompleteness: A=10/10, B=7/10, C=3/10\nPros / cons:\nA) Reuse lib + full tests (recommended)\n \u2705 Zero new parser code: each caller is one import plus a call passing its own fallback and now (human: ~1 h / CC: ~5 min)\n \u2705 Adds the missing contract assertions (empty/whitespace \u2192 fallback, unsafe integer \u2192 fallback, past date \u2192 0, date capped at 3600, fractional seconds round up) so every edge the new callers rely on is proven once\n \u274c Total diff grows by roughly 8 test lines, and a future parser defect reaches the scheduler plus both new callers at once\nB) Reuse lib, existing tests\n \u2705 Same ~28-line implementation saving as A with the smallest possible diff (human: ~45 min / CC: ~3 min)\n \u2705 The existing 5-assertion contract test already runs green and the scheduler already depends on it\n \u274c Past-date, empty-string, unsafe-integer and date-ceiling behavior stay untested, so a regression there would reach the new callers unnoticed\nC) Local parser per caller\n \u2705 Each caller is self-contained; a parser change in one file cannot affect the scheduler or the other caller (human: ~1.5 h / CC: ~5 min)\n \u2705 Matches the plan exactly as drafted, so no plan amendment is needed\n \u274c Two more ~14-line copies (retry-route.ts and retry-worker.ts already hold two) with no shared-contract test, so matching scheduler semantics rests on byte-identical copying, including quirks such as \"-5\" currently yielding 3600\nNet: trading a few shared test lines and shared blast radius against ~28 duplicated lines whose equivalence to the scheduler would never be proven.",
|
||||
"header": "Parser source",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Reuse lib + full tests (recommended)",
|
||||
"description": "\u2705 Zero new parser code: each caller is one import plus a call passing its own fallback and now (human: ~1 h / CC: ~5 min)\n\u2705 Adds the missing contract assertions (empty/whitespace \u2192 fallback, unsafe integer \u2192 fallback, past date \u2192 0, date capped at 3600, fractional seconds round up) so every edge the new callers rely on is proven once\n\u274c Total diff grows by roughly 8 test lines, and a future parser defect reaches the scheduler plus both new callers at once"
|
||||
},
|
||||
{
|
||||
"label": "Reuse lib, existing tests",
|
||||
"description": "\u2705 Same ~28-line implementation saving as A with the smallest possible diff (human: ~45 min / CC: ~3 min)\n\u2705 The existing 5-assertion contract test already runs green and the scheduler already depends on it\n\u274c Past-date, empty-string, unsafe-integer and date-ceiling behavior stay untested, so a regression there would reach the new callers unnoticed"
|
||||
},
|
||||
{
|
||||
"label": "Local parser per caller",
|
||||
"description": "\u2705 Each caller is self-contained; a parser change in one file cannot affect the scheduler or the other caller (human: ~1.5 h / CC: ~5 min)\n\u2705 Matches the plan exactly as drafted, so no plan amendment is needed\n\u274c Two more ~14-line copies (retry-route.ts and retry-worker.ts already hold two) with no shared-contract test, so matching scheduler semantics rests on byte-identical copying, including quirks such as \"-5\" currently yielding 3600"
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"census": "37182865432",
|
||||
"input": {
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 \u2014 Where should the two new callers get their Retry-After parser?\nProject/branch/task: repo on main; the plan adds src/import-worker.ts and src/sync-route.ts, both needing Retry-After parsing that matches src/scheduler.ts.\nELI10: The scheduler already reads Retry-After headers through one shared function in lib/retry-after.ts, and a passing test pins how it behaves. The plan has each new file write its own copy of that logic. Two more copies means three places that must agree forever on the same tricky rules (blank header, date in the past, giant number, one-hour cap), and a fix in one copy silently misses the others.\nStakes if we pick wrong: a hand-written copy drifts from the scheduler, so an import or sync retries at a different time than the scheduler would for the same header, and nobody notices until production.\nRecommendation: A because the existing helper already matches the callers' stated contract exactly (string|null, injected now, 3600 ceiling, caller fallback) and the extra proof is a few assertions.\nCompleteness: A=10/10, B=7/10, C=3/10\nPros / cons:\nA) Reuse helper + full proof (recommended) (human: ~1 hour / CC: ~5 min)\n \u2705 Both callers import the function the scheduler already uses, so matching semantics is guaranteed by construction, not by hand.\n \u2705 Adds regression assertions for the four currently untested edges the callers rely on: blank string, unsafe integer, past date, date beyond the one-hour cap.\n \u274c Touches test/retry-after.test.ts, a file the plan did not list, though only with assertions that pin current behavior.\nB) Reuse helper, existing tests (human: ~45 min / CC: ~3 min)\n \u2705 Same guaranteed-matching semantics as A with the smallest diff: two imports plus two caller integration tests.\n \u2705 No change outside the two new files and their tests.\n \u274c Blank header, past dates and the date-beyond-cap path stay unproven, so a later helper edit could change what the new callers do without a failing test.\nC) Keep local parsers (human: ~2 hours / CC: ~10 min)\n \u2705 Each new file is self-contained and can be read without following an import.\n \u2705 No dependency on lib/retry-after.ts for the new code paths.\n \u274c Three hand-maintained copies of the same rules; \"must match scheduler semantics\" becomes a manual promise with no shared test enforcing it, and ~30 duplicated lines.\nNet: trading one extra test-file edit for semantics that are shared by construction and pinned on every edge the new callers depend on, versus duplicating logic the repo already owns.",
|
||||
"header": "Parser reuse",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Reuse helper + full proof (recommended)",
|
||||
"description": "\u2705 Both callers import the function the scheduler already uses, so matching semantics is guaranteed by construction, not by hand.\n\u2705 Adds regression assertions for the four currently untested edges the callers rely on: blank string, unsafe integer, past date, date beyond the one-hour cap.\n\u274c Touches test/retry-after.test.ts, a file the plan did not list, though only with assertions that pin current behavior. (human: ~1 hour / CC: ~5 min)"
|
||||
},
|
||||
{
|
||||
"label": "Reuse helper, existing tests",
|
||||
"description": "\u2705 Same guaranteed-matching semantics as A with the smallest diff: two imports plus two caller integration tests.\n\u2705 No change outside the two new files and their tests.\n\u274c Blank header, past dates and the date-beyond-cap path stay unproven, so a later helper edit could change what the new callers do without a failing test. (human: ~45 min / CC: ~3 min)"
|
||||
},
|
||||
{
|
||||
"label": "Keep local parsers",
|
||||
"description": "\u2705 Each new file is self-contained and can be read without following an import.\n\u2705 No dependency on lib/retry-after.ts for the new code paths.\n\u274c Three hand-maintained copies of the same rules; \"must match scheduler semantics\" becomes a manual promise with no shared test enforcing it, and ~30 duplicated lines. (human: ~2 hours / CC: ~10 min)"
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
},
|
||||
{
|
||||
"census": "37193478719",
|
||||
"input": {
|
||||
"questions": [
|
||||
{
|
||||
"question": "D1 \u2014 Which parser should the two new callers use under the fixed scheduler contract?\nProject/branch/task: repo on main; PLAN.md adds src/import-worker.ts and src/sync-route.ts, both needing Retry-After parsing identical to src/scheduler.ts.\nELI10: The scheduler already uses a small, tested helper (lib/retry-after.ts) that turns a Retry-After header into a wait in seconds, caps it at an hour, and falls back to a caller-supplied default when the header is missing or garbage. The plan drafts a fresh copy of that logic inside each new caller. Copies look harmless today but drift apart on edge cases over time, and nobody tests the copies directly.\nStakes if we pick wrong: two more untested parser copies (three total with the existing duplicates) that can disagree with the scheduler on edge cases such as past dates, blank headers or huge numbers, producing different retry waits per caller.\nRecommendation: A because the helper already exists, is used by the scheduler and has a contract test, so reuse costs two import lines and the only real work is proving the edge cases the new callers rely on.\nCompleteness: A=10/10, B=7/10, C=4/10\nPros / cons:\nA) Reuse helper + full proof (recommended)\n \u2705 Zero new parser code: each caller imports retrySeconds from lib/retry-after.ts, exactly as src/scheduler.ts:1 does today.\n \u2705 Contract test gains the edge cases the callers depend on (blank header, past date \u2192 0, unsafe integer, date beyond 3600 s, fractional-second rounding), all verified by probe against the current helper.\n \u274c A future bug in the shared helper now reaches the scheduler and both new callers at once (three call sites). (human: ~1 h / CC: ~5 min)\nB) Reuse helper, existing tests\n \u2705 Same two-line wiring as A with no parser duplication.\n \u2705 Smallest diff: only the two caller integration tests are added.\n \u274c Edge cases the new callers rely on (blank header, past date, unsafe integer, date cap) stay unproven in test/retry-after.test.ts; a regression there would pass CI. (human: ~45 min / CC: ~4 min)\nC) Local parsers as drafted\n \u2705 Each caller is self-contained; a helper change cannot alter its behavior.\n \u2705 Matches the current plan text, so no plan amendment is needed.\n \u274c Adds ~30 duplicated implementation lines with no direct tests, and the copies can silently diverge from the scheduler's contract the plan says is fixed. (human: ~2 h / CC: ~10 min)\nNet: trading a slightly wider blast radius for one proven parser versus three untested copies of the same 15 lines.",
|
||||
"header": "Parser reuse",
|
||||
"multiSelect": false,
|
||||
"options": [
|
||||
{
|
||||
"label": "Reuse helper + full proof",
|
||||
"description": "\u2705 Both new callers import retrySeconds from lib/retry-after.ts (one line each), the same wiring src/scheduler.ts uses today; no parser code is written.\n\u2705 test/retry-after.test.ts keeps its 5 assertions and adds the edge cases the callers depend on: blank/whitespace header \u2192 fallback, past date \u2192 0, unsafe-integer string \u2192 fallback, date \u2265 3600 s ahead \u2192 3600, fractional-second date \u2192 ceil. Each new caller gets its integration test asserting header/null, injected now and its own fallback flow through and the result is used.\n\u274c A future helper bug reaches the scheduler and both new callers at once. Helper semantics, src/retry-route.ts and src/retry-worker.ts stay unchanged (out of scope). (human: ~1 h / CC: ~5 min)"
|
||||
},
|
||||
{
|
||||
"label": "Reuse helper, existing tests",
|
||||
"description": "\u2705 Same one-line import per caller as A; no parser duplication.\n\u2705 Smallest diff: only the two caller integration tests are added; test/retry-after.test.ts is left at its current 5 assertions.\n\u274c Blank header, past date, unsafe integer and date-cap behavior stay unproven by the contract test, so a helper regression on those paths would pass CI while the callers rely on them. Helper semantics and existing callers stay unchanged. (human: ~45 min / CC: ~4 min)"
|
||||
},
|
||||
{
|
||||
"label": "Local parsers as drafted",
|
||||
"description": "\u2705 Each caller carries its own 15-line parser copy and cannot be changed by a helper edit.\n\u2705 Matches the current plan text; only the two caller integration tests are added.\n\u274c About 30 duplicated implementation lines with no direct tests; the copies must stay byte-identical to lib/retry-after.ts to honor the fixed contract and can silently drift. Helper and existing callers stay unchanged. (human: ~2 h / CC: ~10 min)"
|
||||
}
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
}
|
||||
]
|
||||
@@ -1,25 +1,10 @@
|
||||
import type { SharedQuestionSelector } from './shared-libs-eval-fixture';
|
||||
|
||||
/**
|
||||
* A whole clause that only lists excluded work: a bare list of scope nouns and
|
||||
* an exclusion predicate, e.g. "Existing copies and helper hardening stay
|
||||
* unchanged", "Hardening is outside this decision", "Existing-caller migration
|
||||
* and helper hardening stay out of scope", and the verbless "Scheduler semantics,
|
||||
* existing callers and helper hardening unchanged". A clause with any other verb, such
|
||||
* as "Harden helper parsing so behavior stays unchanged", is not an exclusion.
|
||||
*/
|
||||
const SCOPE_ITEM = String.raw`(?:(?:the|existing|current|its|all|both|helper|parser|lib|shared|callers?|scheduler)[\s-]+)*(?:copies|callers?|hardening|migrations?|semantics|behaviou?r|contract|helper|parser)`;
|
||||
const SCOPE_STATE = String.raw`(?:unchanged|untouched|excluded|out of scope|outside|not part)`;
|
||||
// The predicate may combine exclusion states: "stay unchanged/out of scope", "remain unchanged and excluded".
|
||||
const SCOPE_EXCLUSION = new RegExp(String.raw`^${SCOPE_ITEM}(?:\s*,\s*${SCOPE_ITEM})*(?:,?\s+and\s+${SCOPE_ITEM})?(?:\s+(?:is|are|stays?|remains?))?\s+`
|
||||
+ String.raw`${SCOPE_STATE}(?:\s*(?:\/|,|\bor\b|\band\b)\s*${SCOPE_STATE})*(?:\s+(?:of\s+)?(?:this|the)\s+(?:decision|scope|plan|change))?[.!]?$`, 'i');
|
||||
|
||||
/** Separate explicit exclusions from proposals; do not erase a following "but" clause. */
|
||||
/** Drop explicitly negated clauses; do not erase a following "but" clause. */
|
||||
function affirmativeCommitments(text: string): string {
|
||||
return text.split(/\n|;|(?<=[.!?])\s+|\s+but\s+|\s+however,?\s+/i).map(raw => {
|
||||
let clause = raw.replace(/^[✅❌\s]+/, '').replace(/\s*\((?:no|not|without|never)\b[^()]*\)/gi, '').trim();
|
||||
if (/^(?:do not|don't|never|no\b|without\b)/i.test(clause)) return '';
|
||||
if (SCOPE_EXCLUSION.test(clause)) return '';
|
||||
clause = clause.replace(/\b(?:without|do not|don't|never)\b.*$/i, '');
|
||||
return clause;
|
||||
}).filter(Boolean).join('\n');
|
||||
@@ -101,9 +86,16 @@ export function createSharedPlanReuseSelector(): SharedQuestionSelector {
|
||||
}
|
||||
// Inspect the question as well as the selected option: a harmless label must
|
||||
// not authorize an extra commitment hidden in its brief or description.
|
||||
const proposed = affirmativeCommitments(context + '\n' + commitment);
|
||||
// A Recommendation's "because" clause is rationale about the choice ("the
|
||||
// plan fixes behavior to the existing helper"); the recommended choice and
|
||||
// every other brief and option line carry what the option commits to.
|
||||
const briefCommitments = context.split('\n')
|
||||
.map(line => /^\s*Recommendation\s*:/i.test(line) ? line.replace(/\s+because\b.*$/i, '') : line).join('\n');
|
||||
const proposed = affirmativeCommitments(briefCommitments + '\n' + commitment);
|
||||
const expansions = [
|
||||
/\b(?:harden\w*|tighten\w*|strict(?:er)?|saniti[sz]\w*|coerc\w*)\b/i,
|
||||
// Forbidden actions are verb + object, never a bare noun: "helper hardening
|
||||
// unchanged" or "a future hardening change" names work, it does not propose it.
|
||||
/\b(?:harden(?:s|ed|ing)?|tighten(?:s|ed|ing)?|saniti[sz](?:e|es|ed|ing)|coerc(?:e|es|ed|ing))\s+(?:(?:the|its|our|existing|shared|current|retry-after|numeric|header|malformed(?:-header)?|helper's|parser's)\s+)*(?:helper|parser|parsing|retrySeconds|lib\/retry-after\.ts|inputs?|headers?|values?|validation|scheduler|fallback|contract)\b|\bstricter\s+(?:parsing|validation|checks?|helper|parser)\b|\b(?:with|plus|including|adds?|adding)\s+(?:(?:helper|parser|input|header)\s+)?(?:hardening|tightening|sanitization)\b|\b(?:hardening|tightening|sanitization)\s+(?:included|added|applied|too|as well)\b/i,
|
||||
/\b(?:add(?:s|ing)?|insert(?:s|ing)?|introduc(?:e|es|ing)|implement(?:s|ing)?|appl(?:y|ies|ying)|enabl(?:e|es|ing)|creat(?:e|es|ing))\s+(?:(?:a|an|the|one|new|shared|extra|explicit|validation|numeric|malformed|input|parser)\s+)*(?:guard|validator|validation|normalization)\b/i,
|
||||
/\b(?:chang(?:e|es|ing)|alter(?:s|ing)?|modif(?:y|ies|ying)|patch(?:es|ing)?|fix(?:es|ing)?|updat(?:e|es|ing)|replac(?:e|es|ing))\s+(?:(?:the|existing|shared|current|its|our)\s+)*(?:(?:retry-after|numeric|malformed|header)\s+)*(?:helper|parser|scheduler|behavior|semantics|contract|parsing|fallback|ceiling|cap|retrySeconds|lib\/retry-after\.ts)\b/i,
|
||||
/\b(?:raise|lower|increase|decrease|remove|drop|bypass|disable)\b[^.!?\n]{0,60}\b(?:ceiling|cap|fallback|limit|bound)\b/i,
|
||||
|
||||
@@ -7,6 +7,9 @@ import capturedParity from './fixtures/shared-libs-plan-callers-parity-367761045
|
||||
import capturedOutOfScope from './fixtures/shared-libs-plan-callers-out-of-scope-37151477069.json';
|
||||
import capturedSlashScope from './fixtures/shared-libs-plan-callers-slash-scope-37174266054.json';
|
||||
import capturedVerbless from './fixtures/shared-libs-plan-callers-verbless-scope-37186854666.json';
|
||||
import capturedFutureRisk from './fixtures/shared-libs-plan-callers-future-risk-37195203538.json';
|
||||
import passingCensuses from './fixtures/shared-libs-plan-callers-passing-censuses.json';
|
||||
import capturedFixesBehavior from './fixtures/shared-libs-plan-callers-fixes-behavior-local.json';
|
||||
|
||||
// Exact native R1 from the September 22 timeout. R2 was saved in an Edit, but
|
||||
// never sent as a native AUQ; its public draft fields are reconstructed below.
|
||||
@@ -393,6 +396,39 @@ describe('bounded shared-code planning actor', () => {
|
||||
expect(run.refusals).toEqual([]);
|
||||
});
|
||||
|
||||
// Controls: the questions this actor answered in passing censuses stay answerable.
|
||||
test.each(passingCensuses.map(entry => [entry.census, entry.input] as const))('passing census %s question is still answered', async (_census, input) => {
|
||||
const run = actor();
|
||||
const result = await run.callback('AskUserQuestion', structuredClone(input));
|
||||
expect(Object.keys(result.updatedInput.answers)).toEqual([input.questions[0].question]);
|
||||
expect(run.refusals).toEqual([]);
|
||||
});
|
||||
|
||||
// Local paid trial on the redesign refused this exact question over its
|
||||
// recommendation rationale: "the plan fixes behavior to the existing helper".
|
||||
test('actual native recommendation rationale is explanation, not a commitment', async () => {
|
||||
const run = actor();
|
||||
const result = await run.callback('AskUserQuestion', capturedFixesBehavior);
|
||||
expect(result.updatedInput.answers).toEqual({ [capturedFixesBehavior.questions[0].question]: capturedFixesBehavior.questions[0].options[0].label });
|
||||
expect(run.refusals).toEqual([]);
|
||||
});
|
||||
|
||||
test('an expansion stated as its own brief line is still refused', async () => {
|
||||
const input = structuredClone(capturedFixesBehavior);
|
||||
input.questions[0].question += '\nThis also hardens the parser against malformed headers.';
|
||||
const run = actor();
|
||||
await expect(run.callback('AskUserQuestion', input)).rejects.toThrow('question expands');
|
||||
});
|
||||
|
||||
// Census 37195203538 refused this exact question over a risk bullet: "a future
|
||||
// helper bug or hardening change reaches scheduler ...". Naming work is not proposing it.
|
||||
test('actual native risk wording that names hardening keeps unchanged-helper reuse answerable', async () => {
|
||||
const run = actor();
|
||||
const result = await run.callback('AskUserQuestion', capturedFutureRisk);
|
||||
expect(result.updatedInput.answers).toEqual({ [capturedFutureRisk.questions[0].question]: capturedFutureRisk.questions[0].options[0].label });
|
||||
expect(run.refusals).toEqual([]);
|
||||
});
|
||||
|
||||
// Census 37186854666 refused this exact question: options end "Scheduler semantics,
|
||||
// existing callers and helper hardening unchanged", the same exclusion without a verb.
|
||||
test('actual native verbless exclusion keeps unchanged-helper reuse answerable', async () => {
|
||||
@@ -406,6 +442,9 @@ describe('bounded shared-code planning actor', () => {
|
||||
'Harden helper parsing so behavior stays unchanged.',
|
||||
'Harden helper parsing unchanged.',
|
||||
'Helper hardening unchanged and tighten helper validation.',
|
||||
'Also hardening the parser against malformed headers.',
|
||||
'Use stricter parsing for numeric values.',
|
||||
'Sanitize header values before calling retrySeconds.',
|
||||
'Existing callers stay unchanged/harden the helper.',
|
||||
'Helper hardening stays unchanged or tighten helper validation.',
|
||||
'Existing copies stay unchanged and tighten helper validation.',
|
||||
|
||||
Reference in new issue
Block a user