janpeter/scrum4me-mcp
1
0
Fork 0
Code Issues 6 Pull requests 1 Projects Releases Packages Wiki Activity Actions 1

feat(pr-review): plan vinden via PR-beschrijving en commit-hashes (ST-052) #183

Merged
janpeter merged 15 commits from feat/pr-review-plan-linking into main 2026-10-04 20:17:28 +02:00
Conversation 3 Commits 15 Files changed 17 +1427 -13
janpeter commented 2026-10-04 19:52:57 +02:00
Owner
Copy link

Waarom

De autonome PR-review (s4m-codex-reviewer) schreef bijna altijd "geen gekoppeld plan gevonden". resolvePrLinkedPlan kende maar twee routes: een implementatiejob met dezelfde pr_url, of een Pbi.pr_url met een PLAN-doc. Interactief gemaakte PR's hebben geen van beide. Over 60 dagen kreeg ~4 % van de reviews een plan mee (787 reviews op 549 PR's).

Wat

Na de twee bestaande routes komen er twee bij. De bestaande routes zelf zijn niet veranderd en geven byte-gelijke output.

  • Route A, de PR-beschrijving (src/lib/pr-refs.ts, resolvePlanViaPrRefs): T-/ST-/PBI--codes worden opgezocht binnen het product van de review-job, en docs/…/plans|specs/*.md-paden worden gelezen op de head-SHA uit de repo van de PR.
  • Route B, de commits (resolvePlanViaCommits): de commits van de PR worden vergeleken met story_logs.commit_hash (een prefix van minstens 7 tekens), binnen het product.
  • Budget: A en B samen zijn begrensd op 100 000 tekens van de geserialiseerde linked_plan. Het plan wordt in een vaste volgorde gevuld: acceptatiecriteria, dan de genoemde taken, dan de planbestanden, dan de overige taken. Wat niet past staat in omitted. Planbestanden worden pas opgehaald als ze aan de beurt zijn.
  • Forgejo (src/git/pr.ts): PrInfo.body, listPullRequestCommitShas en fetchRepoFileAtRef. Ze gooien nooit een fout, zodat de lookup best-effort blijft.
  • wait-for-job: geeft de beschrijving en head-SHA door die het al had opgehaald. Dat kost geen extra Forgejo-call.
  • Prompts: de nieuwe velden zijn beschreven. Plan-conform betekent nu: geen tegenspraak met het plan, en af wat de PR zelf zegt af te ronden. Een story die over meerdere PR's loopt, blokkeert zo niet onterecht.
  • scripts/probe-pr-linked-plan.ts: een alleen-lezen proef op echte data.

Verificatie

  • npm test, inclusief typecheck:tests: 250 testbestanden en 2070 tests geslaagd. npm run typecheck is schoon.
  • RED-controle op de budgettest: zonder budget falen beide budgettests.
  • Proef op de echte database en Forgejo:
    • Van de 25 recentste PR's kreeg er eerst geen enkele een plan mee; nu zijn dat er 9.
    • Scrum4Me#297, Ops-dashboard#280 en Scrum4Me#291 krijgen hun plan via de beschrijving (pr_refs).
    • Het grootste resultaat is 58 171 tekens.

Bewust niet

  • Geen opzoeking over productgrenzen heen (besluit JP). Werk voor mcp, docker en workers dat in Scrum4Me-sprints is gepland, krijgt daardoor geen plan. Dat geldt voor 6 van de 16 PR's zonder plan.
  • Geen schemawijziging en geen nieuw MCP-tool.

Werk

ST-052 (T-157 t/m T-162), PBI-32, sprint S-2026-10-04-1 op product SC2. Plan met review record (dubbel GO na twee rondes): docs/superpowers/plans/2026-10-04-pr-review-plan-linking.md.

Uitrol na merge: de worker-image die de PR_REVIEW-jobs claimt opnieuw bouwen met de nieuwe MCP_GIT_REF. Prompt en code rollen samen uit.

🤖 Generated with Claude Code

## Waarom De autonome PR-review (`s4m-codex-reviewer`) schreef bijna altijd "geen gekoppeld plan gevonden". `resolvePrLinkedPlan` kende maar twee routes: een implementatiejob met dezelfde `pr_url`, of een `Pbi.pr_url` met een PLAN-doc. Interactief gemaakte PR's hebben geen van beide. Over 60 dagen kreeg ~4 % van de reviews een plan mee (787 reviews op 549 PR's). ## Wat Na de twee bestaande routes komen er twee bij. De bestaande routes zelf zijn niet veranderd en geven byte-gelijke output. - **Route A, de PR-beschrijving** (`src/lib/pr-refs.ts`, `resolvePlanViaPrRefs`): `T-`/`ST-`/`PBI-`-codes worden opgezocht binnen het product van de review-job, en `docs/…/plans|specs/*.md`-paden worden gelezen op de head-SHA uit de repo van de PR. - **Route B, de commits** (`resolvePlanViaCommits`): de commits van de PR worden vergeleken met `story_logs.commit_hash` (een prefix van minstens 7 tekens), binnen het product. - **Budget:** A en B samen zijn begrensd op 100 000 tekens van de geserialiseerde `linked_plan`. Het plan wordt in een vaste volgorde gevuld: acceptatiecriteria, dan de genoemde taken, dan de planbestanden, dan de overige taken. Wat niet past staat in `omitted`. Planbestanden worden pas opgehaald als ze aan de beurt zijn. - **Forgejo** (`src/git/pr.ts`): `PrInfo.body`, `listPullRequestCommitShas` en `fetchRepoFileAtRef`. Ze gooien nooit een fout, zodat de lookup best-effort blijft. - **`wait-for-job`:** geeft de beschrijving en head-SHA door die het al had opgehaald. Dat kost geen extra Forgejo-call. - **Prompts:** de nieuwe velden zijn beschreven. Plan-conform betekent nu: geen tegenspraak met het plan, en af wat de PR zelf zegt af te ronden. Een story die over meerdere PR's loopt, blokkeert zo niet onterecht. - **`scripts/probe-pr-linked-plan.ts`:** een alleen-lezen proef op echte data. ## Verificatie - `npm test`, inclusief `typecheck:tests`: 250 testbestanden en 2070 tests geslaagd. `npm run typecheck` is schoon. - RED-controle op de budgettest: zonder budget falen beide budgettests. - Proef op de echte database en Forgejo: - Van de 25 recentste PR's kreeg er eerst geen enkele een plan mee; nu zijn dat er 9. - Scrum4Me#297, Ops-dashboard#280 en Scrum4Me#291 krijgen hun plan via de beschrijving (`pr_refs`). - Het grootste resultaat is 58 171 tekens. ## Bewust niet - **Geen opzoeking over productgrenzen heen** (besluit JP). Werk voor mcp, docker en workers dat in Scrum4Me-sprints is gepland, krijgt daardoor geen plan. Dat geldt voor 6 van de 16 PR's zonder plan. - Geen schemawijziging en geen nieuw MCP-tool. ## Werk ST-052 (T-157 t/m T-162), PBI-32, sprint S-2026-10-04-1 op product SC2. Plan met review record (dubbel GO na twee rondes): `docs/superpowers/plans/2026-10-04-pr-review-plan-linking.md`. **Uitrol na merge:** de worker-image die de PR_REVIEW-jobs claimt opnieuw bouwen met de nieuwe `MCP_GIT_REF`. Prompt en code rollen samen uit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) 
Interactief gemaakte PR's krijgen nu nooit een linked_plan (~4% dekking).
Plan voegt twee routes toe: verwijzingen in de PR-beschrijving en
commit-hashes uit story_logs, gescoped op het product van de job.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Eén totaalbudget voor de nieuwe routes in plaats van veldlimieten,
references optioneel, prompt-conformiteit geherformuleerd, proef met
expliciete PR-URL's.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Interactief gemaakte PR's noemen hun werk al in de beschrijving (T-/ST-/
PBI-codes en docs/…/plans|specs-paden), maar de plan-lookup van de
PR-review las die nooit. extractPrRefs haalt ze er puur uit; de resolver
zoekt ze later op binnen het product van de review-job. Getest op vijf
echte PR-beschrijvingen.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
De plan-lookup van de PR-review heeft drie dingen nodig die er nog niet
waren: de beschrijving (die getPullRequestState al ophaalde maar
weggooide), de commit-SHA's van een PR (route B) en een planbestand op
de head-SHA (route A). Alle drie geven bij een fout { error } terug,
zodat de lookup best-effort blijft.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Interactief gemaakte PR's kregen nooit een linked_plan (~4% dekking): de
lookup kende alleen een implementatiejob met dezelfde pr_url of een
Pbi.pr_url. Na die twee routes (ongewijzigd, byte-gelijke output) volgen
nu route A (codes en planpaden uit de beschrijving, binnen het product
van de job, paden op head-SHA) en route B (PR-commits tegen
story_logs.commit_hash, prefix >= 7).

A en B delen één budget van 100 000 geserialiseerde tekens, gevuld in
vaste volgorde; wat niet past komt in omitted. Een binaire zoektocht kapt
af op de geserialiseerde lengte, omdat escaping die groter maakt dan de
tekst. wait-for-job geeft de al opgehaalde beschrijving en head-SHA door.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Draait dezelfde functies als wait-for-job tegen de echte DB en Forgejo
en print per PR alleen metadata (bron, verwijzingen, omitted, grootte,
en of route B ook iets vond). Zo is de dekking vóór en na meetbaar
zonder planinhoud of tokens te printen.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
feat(pr-review): prompts kennen de nieuwe plan-bronnen en gedeeltelijke dekking
Some checks failed
CI / Final merge attestation and immutable publication (pull_request) Has been skipped
Details
CI / PR candidate (never published) (pull_request) Failing after 8m48s
Details
43b1268194 
De reviewer krijgt nu ook een plan via de PR-beschrijving of commits.
Een story loopt vaak over meerdere PR's; met de oude eis "correct en
volledig" zou een goede deel-PR op COMMENT blijven hangen. Plan-conform
betekent nu: geen tegenspraak met het plan, en af wat de PR zelf zegt af
te ronden. De body noemt de bron en de verwijzingen; de zin bij een
ontbrekend plan blijft ongewijzigd.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
s4m-codex-reviewer reviewed 2026-10-04 20:00:26 +02:00
s4m-codex-reviewer left a comment
Copy link

COMMENT

Plan gekoppeld via pbi. De vier resolverroutes, productscope, Forgejo-helpers en promptuitbreidingen zijn grotendeels conform het gekoppelde plan. Geen blokkerende findings gevonden; onderstaande randgevallen verdienen aanpassing.

  • MINOR — src/lib/pr-linked-plan.ts:385: Witruimte in acceptatiecriteria of taakplannen telt als bruikbare inhoud. Gereproduceerd met uitsluitend \n: route A retourneert pr_refs, waardoor route B met mogelijk wel bruikbare commitplannen niet meer draait. Controleer inhoud met trim() en test deze fallback.
  • MINOR — src/lib/pr-linked-plan.ts:352: Na een budgetweigering kunnen latere korte items alsnog worden opgenomen. Met vijf stories van elk 19.855 AC-tekens, T-1 met 1.000 plantekens en T-2 met short wordt T-1 overgeslagen maar T-2 opgenomen (99.767 JSON-tekens). Dit wijkt af van ontwerpkeuze 6: na het eerste gedeeltelijk/niet passende item gaat alles daarna naar omitted. Implementeer de stopregel of documenteer de gewijzigde semantiek en test dit grensgeval.

Verificatie op commit 43b1268194ef51b77154951872e5744f84394d3c: 57 gerichte tests geslaagd; beide TypeScript-configuraties geslaagd na lokale Prisma-clientgeneratie. Volledige suite: 2.066 geslaagd, 69 overgeslagen, 4 mislukt. Dezelfde vier fouten in Git-fixtures zijn afzonderlijk gereproduceerd op basiscommit a0cdd98c7c93d7cb600e772d1312363cb525a9bb; ze zijn geen aangetoonde regressie van deze PR. De uitrol en architectuurdoc zijn vervolgwerk na merge volgens het plan.

# COMMENT Plan gekoppeld via pbi. De vier resolverroutes, productscope, Forgejo-helpers en promptuitbreidingen zijn grotendeels conform het gekoppelde plan. Geen blokkerende findings gevonden; onderstaande randgevallen verdienen aanpassing. - **MINOR — src/lib/pr-linked-plan.ts:385:** Witruimte in acceptatiecriteria of taakplannen telt als bruikbare inhoud. Gereproduceerd met uitsluitend ` \n`: route A retourneert `pr_refs`, waardoor route B met mogelijk wel bruikbare commitplannen niet meer draait. Controleer inhoud met `trim()` en test deze fallback. - **MINOR — src/lib/pr-linked-plan.ts:352:** Na een budgetweigering kunnen latere korte items alsnog worden opgenomen. Met vijf stories van elk 19.855 AC-tekens, T-1 met 1.000 plantekens en T-2 met `short` wordt T-1 overgeslagen maar T-2 opgenomen (99.767 JSON-tekens). Dit wijkt af van ontwerpkeuze 6: na het eerste gedeeltelijk/niet passende item gaat alles daarna naar `omitted`. Implementeer de stopregel of documenteer de gewijzigde semantiek en test dit grensgeval. Verificatie op commit `43b1268194ef51b77154951872e5744f84394d3c`: 57 gerichte tests geslaagd; beide TypeScript-configuraties geslaagd na lokale Prisma-clientgeneratie. Volledige suite: 2.066 geslaagd, 69 overgeslagen, 4 mislukt. Dezelfde vier fouten in Git-fixtures zijn afzonderlijk gereproduceerd op basiscommit `a0cdd98c7c93d7cb600e772d1312363cb525a9bb`; ze zijn geen aangetoonde regressie van deze PR. De uitrol en architectuurdoc zijn vervolgwerk na merge volgens het plan.
fix(pr-review): stopregel van het budget en witruimte telt niet als plan
All checks were successful
CI / Final merge attestation and immutable publication (pull_request) Has been skipped
Details
CI / PR candidate (never published) (pull_request) Successful in 5m38s
Details
530c3741c8 
Twee bevindingen van s4m-codex-reviewer op #183:
- Na een item dat niet volledig paste kon een later kort item alsnog in
  het plan belanden. Ontwerpkeuze 6 zegt: alles daarna naar omitted. Een
  vlag 'full' dwingt dat nu af, ook voor items zonder tekst en plan-docs.
- Acceptatiecriteria of taakplannen met alleen witruimte telden als
  inhoud, waardoor route A won en route B niet meer draaide.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
s4m-codex-reviewer reviewed 2026-10-04 20:10:11 +02:00
s4m-codex-reviewer left a comment
Copy link

COMMENT

Plan gekoppeld via pbi. De vier resolverroutes, productscope, bestaande outputcontracten en budgetprioriteiten sluiten aan bij het gekoppelde plan. De vervanging van praktijkproef #180 door #291 is expliciet vastgelegd in het uitvoeringsrecord. Uitrol en bewijs van de eerste nieuwe review blijven vervolgstappen na merge.

Findings:

  • WARNING — src/lib/pr-refs.ts:18: De padregex accepteert deelpaden uit ongeldige kandidaten. Gereproduceerd: docs/plans/foo.md.bak levert docs/plans/foo.md; https://example.com/a%20/docs/plans/x.md levert 20/docs/plans/x.md. Als het afgeleide bestand bestaat, wordt het verkeerde plan gekoppeld en kan route A de commitfallback verhinderen. Valideer het volledige padtoken en voeg voor beide gevallen regressietests toe.

Verificatie op head 530c3741c8b6d763c1e790be6e5e7b9196351449: beide TypeScript-checks geslaagd (rechtstreeks via Node), vijf parserfixtures en gerichte controles voor JSON-budget met escaping, omitted en geen documentfetch na budgetuitputting geslaagd. De volledige Vitest-suite kon in deze reviewomgeving niet starten door de noexec-mount van /tmp en native bindings; de in de PR gemelde volledige testrun is dus niet onafhankelijk bevestigd. Productdocumentatie over architectuur en beide typecheckscopes geraadpleegd.

Geen blokkerende finding vastgesteld; wegens de kleine parserbevinding en beperkte volledige testverificatie kies ik COMMENT.

# COMMENT Plan gekoppeld via pbi. De vier resolverroutes, productscope, bestaande outputcontracten en budgetprioriteiten sluiten aan bij het gekoppelde plan. De vervanging van praktijkproef #180 door #291 is expliciet vastgelegd in het uitvoeringsrecord. Uitrol en bewijs van de eerste nieuwe review blijven vervolgstappen na merge. Findings: - **WARNING — src/lib/pr-refs.ts:18**: De padregex accepteert deelpaden uit ongeldige kandidaten. Gereproduceerd: `docs/plans/foo.md.bak` levert `docs/plans/foo.md`; `https://example.com/a%20/docs/plans/x.md` levert `20/docs/plans/x.md`. Als het afgeleide bestand bestaat, wordt het verkeerde plan gekoppeld en kan route A de commitfallback verhinderen. Valideer het volledige padtoken en voeg voor beide gevallen regressietests toe. Verificatie op head `530c3741c8b6d763c1e790be6e5e7b9196351449`: beide TypeScript-checks geslaagd (rechtstreeks via Node), vijf parserfixtures en gerichte controles voor JSON-budget met escaping, `omitted` en geen documentfetch na budgetuitputting geslaagd. De volledige Vitest-suite kon in deze reviewomgeving niet starten door de `noexec`-mount van `/tmp` en native bindings; de in de PR gemelde volledige testrun is dus niet onafhankelijk bevestigd. Productdocumentatie over architectuur en beide typecheckscopes geraadpleegd. Geen blokkerende finding vastgesteld; wegens de kleine parserbevinding en beperkte volledige testverificatie kies ik COMMENT.
fix(pr-review): planpaden als heel token beoordelen
All checks were successful
CI / Final merge attestation and immutable publication (pull_request) Has been skipped
Details
CI / PR candidate (never published) (pull_request) Successful in 5m23s
Details
9adeb452db 
Bevinding van s4m-codex-reviewer op #183: de padregex haalde deelpaden
uit ongeldige tokens (docs/plans/foo.md.bak → docs/plans/foo.md, en uit
een URL met %20 een '20/docs/…'-pad). Bestaat dat afgeleide bestand, dan
koppelt de review het verkeerde plan. De parser splitst nu op witruimte
en markdown-tekens en eist dat het hele token een relatief .md-pad is.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
s4m-codex-reviewer reviewed 2026-10-04 20:17:39 +02:00
s4m-codex-reviewer left a comment
Copy link

COMMENT

Plan gekoppeld via pbi: implementatieplan PR-review plan-linking (ST-052).

Findings

  • MINOR — src/lib/pr-linked-plan.ts:400 — De inhoudscontrole gebeurt na afkappen. Bij een veld met uitsluitend meer dan 20.000 spaties maakt de markering …[afgekapt] het veld volgens hasText inhoudelijk. Route A retourneert dan een leeg plan en verhindert de fallback naar commits. Reproduceerbaar met acceptance_criteria: ' '.repeat(20001): resultaat source: pr_refs, met alleen de afkapmarkering als getrimde inhoud. Controleer inhoud vóór afkappen en voeg dit geval toe aan de whitespace-test.

De diff volgt de geplande routevolgorde, productscope, Forgejo-helpers, payloadvelden en promptaanpassingen. De gewijzigde praktijkacceptatie (#291 in plaats van #180 wegens productscope) is met het besluit vastgelegd. Uitrol en het bewijs van de eerste review na uitrol blijven na-mergewerk.

Tests voor beide routes, bestaande output, fouten, parser en budget zijn aanwezig. Een gerichte uitvoering van de budgetfunctie met escaping gaf 99.997 geserialiseerde tekens en de verwachte omitted-stories. De volledige testsuite en beide typechecks zijn hier niet uitgevoerd; de PR-beschrijving rapporteert 2.070 geslaagde tests, maar CI voor head 9adeb452db9d812d514f88fe7d38d03173222eda staat bij controle nog op pending. Daarom geen APPROVED.

# COMMENT Plan gekoppeld via pbi: implementatieplan PR-review plan-linking (ST-052). ## Findings - **MINOR — src/lib/pr-linked-plan.ts:400** — De inhoudscontrole gebeurt na afkappen. Bij een veld met uitsluitend meer dan 20.000 spaties maakt de markering `…[afgekapt]` het veld volgens `hasText` inhoudelijk. Route A retourneert dan een leeg plan en verhindert de fallback naar commits. Reproduceerbaar met `acceptance_criteria: ' '.repeat(20001)`: resultaat `source: pr_refs`, met alleen de afkapmarkering als getrimde inhoud. Controleer inhoud vóór afkappen en voeg dit geval toe aan de whitespace-test. De diff volgt de geplande routevolgorde, productscope, Forgejo-helpers, payloadvelden en promptaanpassingen. De gewijzigde praktijkacceptatie (#291 in plaats van #180 wegens productscope) is met het besluit vastgelegd. Uitrol en het bewijs van de eerste review na uitrol blijven na-mergewerk. Tests voor beide routes, bestaande output, fouten, parser en budget zijn aanwezig. Een gerichte uitvoering van de budgetfunctie met escaping gaf 99.997 geserialiseerde tekens en de verwachte omitted-stories. De volledige testsuite en beide typechecks zijn hier niet uitgevoerd; de PR-beschrijving rapporteert 2.070 geslaagde tests, maar CI voor head `9adeb452db9d812d514f88fe7d38d03173222eda` staat bij controle nog op pending. Daarom geen APPROVED.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
severity/s2

   
severity/s3

   
severity/s4
No labels
severity/s2
severity/s3
severity/s4
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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
janpeter/scrum4me-mcp!183
No description provided.