fix: the grounding grace should run from the settle it grounds #10

jercik · 2026-08-28T09:55:49Z

jercik commented

2026-08-28 09:55:49 +00:00

waitForSettle armed the grounding grace once, at the first settled observation, and held that deadline through every later regression back to stage 2. A review that settled early, regressed, and settled again reached pollGrounding with its grace already spent, so the wrapper reported grounding-pending without reading the claims at all.

Against the test clock — settle at 0s, a regressed triage for the next 450s, then a fresh settle:

	claim reads after the second settle	wait ends at
before	0	435s
after	20	735s

Stage 2's own 15-minute deadline still bounds the settle poll, so the worst-case wait is unchanged. What changes is which bound a flapping triage hits: it used to bail at the spent grace and report grounding-pending, and now runs to the stage-2 deadline and reports stage2-timeout. Both exit 1.

The first version of this branch also moved the latest_pass comparison after the stalled return in readSettle, which dropped superseded on a read carrying both a stalled triage and a moved pass. That ordering is restored, and a test now holds it.

`waitForSettle` armed the grounding grace once, at the first settled observation, and held that deadline through every later regression back to stage 2. A review that settled early, regressed, and settled again reached `pollGrounding` with its grace already spent, so the wrapper reported `grounding-pending` without reading the claims at all. Against the test clock — settle at 0s, a regressed triage for the next 450s, then a fresh settle: | | claim reads after the second settle | wait ends at | | ------ | ----------------------------------- | ------------ | | before | 0 | 435s | | after | 20 | 735s | Stage 2's own 15-minute deadline still bounds the settle poll, so the worst-case wait is unchanged. What changes is which bound a flapping triage hits: it used to bail at the spent grace and report `grounding-pending`, and now runs to the stage-2 deadline and reports `stage2-timeout`. Both exit 1. The first version of this branch also moved the `latest_pass` comparison after the stalled return in `readSettle`, which dropped `superseded` on a read carrying both a stalled triage and a moved pass. That ordering is restored, and a test now holds it.

jercik added 1 commit

2026-08-28 09:55:50 +00:00

refactor: simplify the settle wait's grace and pin handling

commit-msg / commitlint (pull_request) Successful in 28s

Details

Checks / quality-checks (pull_request) Successful in 52s

Details

Review / Review (pull_request_target) Successful in 4m51s

Details

9598b8d3ab

Return from readSettle as soon as the triage is stalled: the wait ends
there, so comparing the pass afterwards is work with nowhere to go.

Drop the undefined seed on the grace deadline. It only existed to make
the conditional assignment expressible, and a plain number reads more
directly at the two places it is used.

forgejo-actions commented

2026-08-28 10:00:39 +00:00

Review 01M13XMNXQ517M5MYB6XARD7A8 — head af9b8bd20fe23aadd62fce3eadc826ac21c46857

Review — j4k-oss/review-wrapper @ `6a7440a7b1`

Scope: diff against base tree 017065182efc
Status: dispatched — coverage complete (3/3 slots terminal)

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v1",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v1",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v1"
}

Findings (0)

No findings survived.

Reviewed:

general-bug (whole/default): claims-emitted
comments-trimming (whole/default): no-claims
test-trimming (whole/default): no-claims

Other claims

grounding-pending (0)
ungrounded (0)
rejected (0)
duplicate-of (0)
unadjudicated (1)
- 01M13Y2YKFHVB0FPHK2AGRW24M low — Grounding grace now restarts on every settled observation, so a flapping triage reports stage2-timeout instead of grounding-pending

Coverage

lens	part	arm	unit status	runs	loss
general-bug	whole	default	claims-emitted	1	no
comments-trimming	whole	default	no-claims	1	no
test-trimming	whole	default	no-claims	1	no

**Review** `01M13XMNXQ517M5MYB6XARD7A8` — head `af9b8bd20fe23aadd62fce3eadc826ac21c46857` # Review — j4k-oss/review-wrapper @ 6a7440a7b1c8 Scope: diff against base tree `017065182efc` Status: dispatched — coverage complete (3/3 slots terminal) Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v1", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v1", "tally": "tally-v1", "triage_settle": "triage-settle-v1" } ``` ## Findings (0) No findings survived. Reviewed: - general-bug (whole/default): claims-emitted - comments-trimming (whole/default): no-claims - test-trimming (whole/default): no-claims ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (0) - unadjudicated (1) - `01M13Y2YKFHVB0FPHK2AGRW24M` low — Grounding grace now restarts on every settled observation, so a flapping triage reports stage2-timeout instead of grounding-pending ## Coverage | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | claims-emitted | 1 | no | | comments-trimming | whole | default | no-claims | 1 | no | | test-trimming | whole | default | no-claims | 1 | no |

🚀 1

forgejo-actions reviewed

2026-08-28 10:00:40 +00:00

src/wrapper/wait.ts Outdated

Lines 52-57

					
				@ -50,5 +49,7 @@

				    pin.superseded = true;

				  }

				  // A stalled triage ends the wait, so there is no later pass to compare against.

				  if (review.triage_settle === "stalled") {

				    return "stalled";

				  }

				  if (review.latest_pass !== null && review.latest_pass.id !== pinnedPassId) {

				    pin.superseded = true;

				  }

low — A stalled response suppresses an already-visible pass supersession
lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain

I examined readSettle, the WaitResult contract in src/contract/types.ts, the row-11 documentation in README.md, and the only consumer in src/wrapper/orchestrator.ts. readSettle now returns for stalled before comparing the latest_pass.id carried by that same response, even though the contract defines superseded as the fact that the latest id moved off the pinned pass. I ran node --input-type=module against the exact bundled wait module with coverage complete and a review response {triage_settle:"stalled", latest_pass:{id:"newer"}}; it returned {kind:"stalled", superseded:false}. The exit remains the same stuck failure, but the result violates its documented flag and suppresses the orchestrator's diagnostic that another pass owns the newer work; moving the comparison before the stalled return preserves both facts. A server invariant forbidding stalled and a differing latest pass in one read would refute this, but the bundled wire schema imposes no such relation, so I have kept the severity low rather than asserting that production emits the combination.

claim 01M13WX7E5SD343B9VZ0H3QR2H of review 01M13WR6SANBWWAXGWGMBSP6H8

**low** — A stalled response suppresses an already-visible pass supersession lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain > I examined `readSettle`, the `WaitResult` contract in `src/contract/types.ts`, the row-11 documentation in `README.md`, and the only consumer in `src/wrapper/orchestrator.ts`. `readSettle` now returns for `stalled` before comparing the `latest_pass.id` carried by that same response, even though the contract defines `superseded` as the fact that the latest id moved off the pinned pass. I ran `node --input-type=module` against the exact bundled wait module with coverage complete and a review response `{triage_settle:"stalled", latest_pass:{id:"newer"}}`; it returned `{kind:"stalled", superseded:false}`. The exit remains the same stuck failure, but the result violates its documented flag and suppresses the orchestrator's diagnostic that another pass owns the newer work; moving the comparison before the stalled return preserves both facts. A server invariant forbidding `stalled` and a differing latest pass in one read would refute this, but the bundled wire schema imposes no such relation, so I have kept the severity low rather than asserting that production emits the combination. claim `01M13WX7E5SD343B9VZ0H3QR2H` of review `01M13WR6SANBWWAXGWGMBSP6H8`

👍 1

Verified and fixed in af9b8bd20f. Reproduced the exact combination against the current wait module: a review read of {triage_settle: "stalled", latest_pass: {id: }} returned {kind: "stalled", superseded: false}. WaitResult.superseded is documented (src/contract/types.ts, README row 11) as the fact that latest_pass.id moved off the pinned pass, independent of how the wait ended, so the early return was dropping a fact the same response carried and silencing the orchestrator's superseded warning. The comparison now runs before the stalled return, and src/wrapper/wait.test.ts locks the combination with a new case asserting {kind: "stalled", superseded: true}.

Verified and fixed in af9b8bd20fe23aadd62fce3eadc826ac21c46857. Reproduced the exact combination against the current wait module: a review read of {triage_settle: "stalled", latest_pass: {id: <newer>}} returned {kind: "stalled", superseded: false}. WaitResult.superseded is documented (src/contract/types.ts, README row 11) as the fact that latest_pass.id moved off the pinned pass, independent of how the wait ended, so the early return was dropping a fact the same response carried and silencing the orchestrator's superseded warning. The comparison now runs before the stalled return, and src/wrapper/wait.test.ts locks the combination with a new case asserting {kind: "stalled", superseded: true}.

jercik marked this conversation as resolved

jercik added 1 commit

2026-08-28 10:11:07 +00:00

fix: record supersession before the stalled return in readSettle

commit-msg / commitlint (pull_request) Successful in 1m10s

Details

Checks / quality-checks (pull_request) Successful in 1m18s

Details

Review / Review (pull_request_target) Successful in 4s

Details

af9b8bd20f

The stalled early return landed ahead of the latest_pass comparison, so a
review read carrying both a stalled triage and a moved latest_pass produced
{kind: "stalled", superseded: false} — dropping a fact the same response
already carried and silencing the orchestrator's superseded diagnostic.

superseded is documented as a fact about the review, not about how the wait
ended, so the comparison runs before any early return.

jercik changed title from ~~refactor: simplify the settle wait's grace and pin handling~~ to fix: the grounding grace should run from the settle it grounds

2026-08-28 10:12:40 +00:00

jercik merged commit a1a8012040 into main

2026-09-01 06:28:14 +00:00

jercik deleted branch feat/simplify-settle-wait

2026-09-01 06:28:14 +00:00

jercik referenced this pull request from a commit

2026-09-01 06:28:15 +00:00

fix: the grounding grace should run from the settle it grounds (#10)

jercik referenced this pull request

2026-10-06 05:35:53 +00:00

The wrapper gives up on a long lens at 30 minutes, and two loops and the forge requests have no limit of their own #56

Sign in to join this conversation.