docs: correct three contract comments in types.ts #42

jercik · 2026-10-04T23:21:43Z

jercik commented

2026-10-04 23:21:43 +00:00

Follow-up to #40, from review 01M44JG9B4Q3392GK4XWBNRQK9 (summary-only findings 01M44K0VQRN7Y3M9EDNWXDQRFG, 01M44K1Z23FBMBS7ZXYYQEDM2Y and 01M44JYTM6XFMG5EG6CWK9401J).

Three comments in src/contract/types.ts described the code wrongly:

The inline reconciler contract promised one thread per mapped finding. createInlineReconciler() posts one inline comment per finding, and findings on the same display line share one conversation, which settle() resolves together. It now says "one inline comment".
The headSha comment said the value is as of the event. A workflow_dispatch run takes it from a later forge.getPull() call. It now names the source for each event.
The session section heading named a service-session module that doesn't exist. It now names session, matching how the neighbouring headings name wait and orchestrator; createReviewSession() lives in src/review/session.ts.

It targets main, because these comments are older than #40.

🤖 Generated with Claude Code

Follow-up to #40, from review `01M44JG9B4Q3392GK4XWBNRQK9` (summary-only findings `01M44K0VQRN7Y3M9EDNWXDQRFG`, `01M44K1Z23FBMBS7ZXYYQEDM2Y` and `01M44JYTM6XFMG5EG6CWK9401J`). Three comments in `src/contract/types.ts` described the code wrongly: - The inline reconciler contract promised one thread per mapped finding. `createInlineReconciler()` posts one inline comment per finding, and findings on the same display line share one conversation, which `settle()` resolves together. It now says "one inline comment". - The `headSha` comment said the value is as of the event. A `workflow_dispatch` run takes it from a later `forge.getPull()` call. It now names the source for each event. - The session section heading named a `service-session` module that doesn't exist. It now names `session`, matching how the neighbouring headings name `wait` and `orchestrator`; `createReviewSession()` lives in `src/review/session.ts`. It targets `main`, because these comments are older than #40. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

jercik added 1 commit

2026-10-04 23:21:43 +00:00

docs: correct three contract comments in types.ts

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

Details

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

Details

Review / Review (pull_request_target) Failing after 8m52s

Details

760cb973b8

The InlineReconciler comment promised one thread per promoted finding. The
reconciler posts one inline comment per mapped finding, and claims that map to
the same display line share one conversation, which the disposition projector
settles together. The comment now says "inline comment".

The WrapperInputs.headSha comment said the SHA was "as of the event". Only the
pull_request_target branch reads it from the event payload; workflow_dispatch
reads it from a later forge.getPull() call. The comment now names both sources.

The "Service session" section heading pointed at a service-session module that
does not exist. The code lives in session.ts (createReviewSession), so the
heading now says "module: session", matching the bare names the other section
headings use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

jercik referenced this pull request

2026-10-04 23:22:43 +00:00

docs: drop the false "never computed" from the inputs header #40

forgejo-actions commented

2026-10-04 23:30:34 +00:00

Review 01M44MA8Z8WP4CKWK93AKVSDF7 — head 821a5ced4bc360560540f1ef234e267c43f0514f

Review — j4k-oss/review-wrapper @ `82743dc8b5`

Scope: diff against base tree e69943819640
Status: dispatched — coverage complete (5/5 slots terminal)
Facts: current review-wide projection

Computed under:

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

Findings (1)

low — README says manual dispatch gets PR details from the event payload

claim: 01M44MGAPFPZ6WW2CP45Y0ZQ8V
anchor: README.md (snippet)

`GITHUB_REPOSITORY`, builds the forge client, derives the run's inputs from the event
payload, and hands the orchestrator its collaborators: the wait protocol, the clock, and

lens: general-bug · arm: default
verdicts: 1 valid / 0 invalid / 0 uncertain
- pass 01M44MJ3R4V8GXQWJ9Y3K94SAK · valid: The exact grounded README excerpt attributes the run inputs to the event payload. The reviewer supplies a specific static trace of workflow_dispatch: the payload provides pr_number, while forge.getPull supplies the head SHA, branches, and fork status. That makes the unqualified README description misleading for manual dispatch. The separate environment-table wording is not quoted and is not needed to support this low-severity finding.
disposition: none

A reader debugging workflow_dispatch is told that the run's inputs come from the event payload, but that payload supplies only pr_number. The head SHA, base branch, head branch, and fork decision require a live forge getPull response, so this description obscures a required forge read on manual runs.

The changed WrapperInputs.headSha comment points to readWrapperInputs for event-specific derivation. In src/wrapper/event-context.ts, its workflow_dispatch branch parses payload.inputs?.pr_number, calls forge.getPull(prNumber), then fills those PR fields from pull; src/wrapper/event-context.test.ts covers that branch. The README environment table makes the same broad statement. Qualify both descriptions: pull_request_target reads PR details from its event payload, while workflow_dispatch fetches them from the forge. This is a static source trace; I did not run the action. A dispatch branch that obtains those details from the payload without a forge read would refute the claim, but the branch in the subject always calls getPull.

Other claims

grounding-pending (0)
ungrounded (1)
- 01M44MFV8NGBKS47H4Z81TR2ZW low — README says manual dispatch gets PR inputs from the event payload
rejected (0)
duplicate-of (0)
unadjudicated (0)

Coverage

Coverage pass: 01M44MA92HDEV445N97HPXVM6J
Accounting: complete
Slot health: healthy

lens	part	arm	unit status	runs	loss
general-bug	whole	default	claims-emitted	1	no
writing-quality	whole	default	no-claims	1	no
test-trimming	whole	default	no-claims	1	no
restated-sets	whole	default	no-claims	1	no
project-docs	whole	default	no-claims	1	no

**Review** `01M44MA8Z8WP4CKWK93AKVSDF7` — head `821a5ced4bc360560540f1ef234e267c43f0514f` # Review — j4k-oss/review-wrapper @ 82743dc8b5a6 Scope: diff against base tree `e69943819640` Status: dispatched — coverage complete (5/5 slots terminal) Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v4", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (1) ### low — README says manual dispatch gets PR details from the event payload - claim: `01M44MGAPFPZ6WW2CP45Y0ZQ8V` - anchor: `README.md` (snippet) ``` `GITHUB_REPOSITORY`, builds the forge client, derives the run's inputs from the event payload, and hands the orchestrator its collaborators: the wait protocol, the clock, and ``` - lens: general-bug · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M44MJ3R4V8GXQWJ9Y3K94SAK` · valid: The exact grounded README excerpt attributes the run inputs to the event payload. The reviewer supplies a specific static trace of workflow_dispatch: the payload provides pr_number, while forge.getPull supplies the head SHA, branches, and fork status. That makes the unqualified README description misleading for manual dispatch. The separate environment-table wording is not quoted and is not needed to support this low-severity finding. - disposition: none > A reader debugging `workflow_dispatch` is told that the run's inputs come from the event payload, but that payload supplies only `pr_number`. The head SHA, base branch, head branch, and fork decision require a live forge `getPull` response, so this description obscures a required forge read on manual runs. > > The changed `WrapperInputs.headSha` comment points to `readWrapperInputs` for event-specific derivation. In `src/wrapper/event-context.ts`, its `workflow_dispatch` branch parses `payload.inputs?.pr_number`, calls `forge.getPull(prNumber)`, then fills those PR fields from `pull`; `src/wrapper/event-context.test.ts` covers that branch. The README environment table makes the same broad statement. Qualify both descriptions: `pull_request_target` reads PR details from its event payload, while `workflow_dispatch` fetches them from the forge. This is a static source trace; I did not run the action. A dispatch branch that obtains those details from the payload without a forge read would refute the claim, but the branch in the subject always calls `getPull`. ## Other claims - grounding-pending (0) - ungrounded (1) - `01M44MFV8NGBKS47H4Z81TR2ZW` low — README says manual dispatch gets PR inputs from the event payload - rejected (0) - duplicate-of (0) - unadjudicated (0) ## Coverage Coverage pass: 01M44MA92HDEV445N97HPXVM6J Accounting: complete Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | claims-emitted | 1 | no | | writing-quality | whole | default | no-claims | 1 | no | | test-trimming | whole | default | no-claims | 1 | no | | restated-sets | whole | default | no-claims | 1 | no | | project-docs | whole | default | no-claims | 1 | no |

🚀 1

forgejo-actions reviewed

2026-10-04 23:30:35 +00:00

src/contract/types.ts Outdated

					
				@ -21,3 +21,3 @@

				  readonly forgeHost: string;

				  readonly prNumber: number;

				  /** PR head SHA as of the event (workflow_dispatch: fetched via the forge). */

				  /** PR head SHA: event payload on pull_request_target, forge lookup on workflow_dispatch. */

medium — headSha comment duplicates the supported event set

The comment gives readers a second account of which events populate the PR head SHA. A new or renamed trigger can make this account silently stale even if input derivation is updated correctly.

The comment names pull_request_target and workflow_dispatch. The workflow triggers in .forgejo/workflows/review.yml, the WrapperInputs.eventName union, and the branches of readWrapperInputs in src/wrapper/event-context.ts define those same two members. The copy covers pull_request_target and workflow_dispatch; neither list has a member absent from the other, so the copy still agrees with its sources.

Replace the enumeration with a pointer such as “SHA populated by readWrapperInputs in src/wrapper/event-context.ts”; that function already shows where each event obtains the SHA. I read the full changed contract file, the workflow triggers, readWrapperInputs, and its tests. This is static source comparison; no runtime behavior is alleged. The decisive evidence is the complete event set in the workflow and function versus the two names in this comment. No exception applies: this is an undated explanatory comment, not a system-read definition, a source pointer, or a partial example.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M44KTCGBF8B6JSK6071P04DY of review 01M44KJDCVN043SBMQ63FJPTA0

**medium** — headSha comment duplicates the supported event set > The comment gives readers a second account of which events populate the PR head SHA. A new or renamed trigger can make this account silently stale even if input derivation is updated correctly. > > The comment names pull_request_target and workflow_dispatch. The workflow triggers in .forgejo/workflows/review.yml, the WrapperInputs.eventName union, and the branches of readWrapperInputs in src/wrapper/event-context.ts define those same two members. The copy covers pull_request_target and workflow_dispatch; neither list has a member absent from the other, so the copy still agrees with its sources. > > Replace the enumeration with a pointer such as “SHA populated by readWrapperInputs in src/wrapper/event-context.ts”; that function already shows where each event obtains the SHA. I read the full changed contract file, the workflow triggers, readWrapperInputs, and its tests. This is static source comparison; no runtime behavior is alleged. The decisive evidence is the complete event set in the workflow and function versus the two names in this comment. No exception applies: this is an undated explanatory comment, not a system-read definition, a source pointer, or a partial example. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M44KTCGBF8B6JSK6071P04DY` of review `01M44KJDCVN043SBMQ63FJPTA0`

👍 1

Fixed in 821a5ce. The comment now points at readWrapperInputs in src/wrapper/event-context.ts instead of naming the events.

Fixed in 821a5ce. The comment now points at `readWrapperInputs` in `src/wrapper/event-context.ts` instead of naming the events.

jercik marked this conversation as resolved

jercik added 1 commit

2026-10-04 23:34:45 +00:00

docs: point the headSha comment at readWrapperInputs

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

Details

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

Details

Review / Review (pull_request_target) Successful in 7m24s

Details

821a5ced4b

The comment listed the events instead of pointing at the function that defines them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

jercik commented

2026-10-04 23:35:28 +00:00

Replying to comment #124660

This is review 01M44KJDCVN043SBMQ63FJPTA0 of 760cb97. Run 64404 lost its project-docs slot to a sandbox infrastructure failure; the fix push starts a new review with every slot. The finding with a thread, 01M44KTCGBF8B6JSK6071P04DY (the headSha comment lists the events), is fixed in 821a5ce and answered there, and its duplicate 01M44KV1TRM585MXA446CTQCT1 is covered by it.

01M44KTYYGN97NH6CM0JYRE8J2 (the README's environment table says a dispatch run's PR inputs come from the event payload) is summary-only, about text this PR doesn't change. It is valid: a workflow_dispatch payload carries only pr_number, and readWrapperInputs fetches the rest with forge.getPull. #37 already rewrites that row to say a dispatch run fetches the PR's current state from the forge.

> Replying to comment #124660 This is review `01M44KJDCVN043SBMQ63FJPTA0` of `760cb97`. Run 64404 lost its project-docs slot to a sandbox infrastructure failure; the fix push starts a new review with every slot. The finding with a thread, `01M44KTCGBF8B6JSK6071P04DY` (the `headSha` comment lists the events), is fixed in 821a5ce and answered there, and its duplicate `01M44KV1TRM585MXA446CTQCT1` is covered by it. `01M44KTYYGN97NH6CM0JYRE8J2` (the README's environment table says a dispatch run's PR inputs come from the event payload) is summary-only, about text this PR doesn't change. It is valid: a `workflow_dispatch` payload carries only `pr_number`, and `readWrapperInputs` fetches the rest with `forge.getPull`. https://code.j4k.dev/j4k-oss/review-wrapper/pulls/37 already rewrites that row to say a dispatch run fetches the PR's current state from the forge.

jercik commented

2026-10-04 23:43:09 +00:00

Replying to comment #124660

This is review 01M44MA8Z8WP4CKWK93AKVSDF7 of 821a5ce, which replaced the earlier report in this comment. Its one finding, 01M44MGAPFPZ6WW2CP45Y0ZQ8V (the README says a manual dispatch gets its PR details from the event payload), is summary-only and about text this PR doesn't change. It is the same README row as 01M44KTYYGN97NH6CM0JYRE8J2 in the earlier review, and it is valid. #37 rewrites that row to say a dispatch run fetches the PR's current state from the forge. The ungrounded claim 01M44MFV8NGBKS47H4Z81TR2ZW says the same thing.

> Replying to comment #124660 This is review `01M44MA8Z8WP4CKWK93AKVSDF7` of `821a5ce`, which replaced the earlier report in this comment. Its one finding, `01M44MGAPFPZ6WW2CP45Y0ZQ8V` (the README says a manual dispatch gets its PR details from the event payload), is summary-only and about text this PR doesn't change. It is the same README row as `01M44KTYYGN97NH6CM0JYRE8J2` in the earlier review, and it is valid. https://code.j4k.dev/j4k-oss/review-wrapper/pulls/37 rewrites that row to say a dispatch run fetches the PR's current state from the forge. The ungrounded claim `01M44MFV8NGBKS47H4Z81TR2ZW` says the same thing.

jercik merged commit 6f4dab700a into main

2026-10-05 09:34:53 +00:00

jercik referenced this pull request from a commit

2026-10-05 09:34:53 +00:00

docs: correct three contract comments in types.ts (#42)

Sign in to join this conversation.

No reviewers

No labels

No milestone

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

j4k-oss/review-wrapper!42

Rows
Columns