claude-code
db2fba45e7
See docs/prs for the PR document. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Reviewed-on: #121 Co-authored-by: claude-code <claude-code@javagil.de>
218 lines
11 KiB
Markdown
218 lines
11 KiB
Markdown
# PR#121: Diagrams of PR Lens in the Result Comment
|
|
|
|
Version: minor
|
|
|
|
> **WARNING:** This document describes only the change applied in this PR.
|
|
> It may already be outdated once the next PR is merged.
|
|
> Historic PR documentation is not maintained along with new PRs; treat it as a snapshot.
|
|
|
|
## Related Links
|
|
|
|
- [Concept and trials by hand](../konzept-pr-lens-diagramme.md), in German.
|
|
- [Decision: Node.js in the base archive](../DECISIONS.md).
|
|
- [PR Lens](https://github.com/coldteadotai/pr-lens), MIT.
|
|
|
|
## Release Notes
|
|
|
|
- With `[diagrams] enabled = true` in the host config, the result comment shows architecture and data-flow diagrams
|
|
of PR Lens, as animated SVGs.
|
|
They never change the verdict; the default is off.
|
|
- The base archive of the release build is now `werkrevisor-base-<hash>.tar.zst`, with Node.js and PR Lens.
|
|
|
|
## Problem
|
|
|
|
The result comment of werkrevisor lists verdict, reasons and findings, but does not show what a PR touches.
|
|
A human reviewer has to read the diff to see which components and data flows change.
|
|
|
|
PR Lens draws architecture and data-flow diagrams of a diff.
|
|
Trials by hand on 2026-10-18 showed useful diagrams for hsadmin-ng PR#319,
|
|
at about 0.21 USD and 35 s per review with `anthropic/claude-sonnet-5.5`,
|
|
and Gitea on git.javagil.de shows attached SVGs inline and animated.
|
|
|
|
## Non-Goals
|
|
|
|
- The diagrams do not change the verdict, the status, the approval or the exit code.
|
|
- No review by PR Lens; werkrevisor stays the only review.
|
|
- No diagrams for `hs-review` on dev.hostsharing.net yet; see "Follow-up PRs".
|
|
- No canvas on prlens.dev; PR Lens talks only to OpenRouter.
|
|
- No repository key: only the host config switches the diagrams on, since they cost a second model call.
|
|
|
|
## Specification
|
|
|
|
### Feature: the result comment shows diagrams of PR Lens
|
|
|
|
#### Scenario#121.01: an enabled repository gets the diagrams in its result comment
|
|
|
|
… so that a human reviewer sees what a PR touches without reading the whole diff.
|
|
|
|
- **Given** a host config with `[diagrams] enabled = true` and a PR whose review completes
|
|
- **When** werkrevisor publishes its result
|
|
- **Then** the result comment has the SVGs as attachments, links them under "Diagrams" with a note
|
|
that they show what the PR touches, not what it does, and the verdict and exit code stay those of the review
|
|
|
|
##### Verified by
|
|
|
|
- [test_prlens.DiagramsInTheResultCommentTest.test_an_enabled_repository_gets_the_diagrams_in_its_result_comment](../../tests/test_prlens.py)
|
|
- [test_prlens.DiagramsInTheResultCommentTest.test_an_error_result_gets_no_diagrams](../../tests/test_prlens.py)
|
|
|
|
#### Scenario#121.02: diagrams are off by default
|
|
|
|
… so that no repository pays for a second model call that its operator did not choose.
|
|
|
|
- **Given** a host config without `[diagrams]`
|
|
- **When** werkrevisor publishes its result
|
|
- **Then** it calls no PR Lens and attaches nothing
|
|
|
|
##### Verified by
|
|
|
|
- [test_prlens.DiagramsInTheResultCommentTest.test_diagrams_are_off_by_default](../../tests/test_prlens.py)
|
|
|
|
#### Scenario#121.03: a failure of PR Lens leaves the review unchanged
|
|
|
|
… so that a missing diagram never ends a review or changes its result.
|
|
|
|
- **Given** an enabled repository and a PR Lens that fails, times out or answers without diagrams
|
|
- **When** werkrevisor publishes its result
|
|
- **Then** the result comment has no diagrams, the verdict and exit code stay those of the review,
|
|
and the log has one redacted line
|
|
|
|
##### Verified by
|
|
|
|
- [test_prlens.DiagramsInTheResultCommentTest.test_a_failure_of_pr_lens_leaves_the_review_unchanged](../../tests/test_prlens.py)
|
|
- [test_prlens.DrawTest.test_a_failed_or_slow_pr_lens_raises_a_diagram_error](../../tests/test_prlens.py)
|
|
|
|
#### Scenario#121.04: a rerun on the same head reuses the attached diagrams
|
|
|
|
… so that a restart of the build costs no second drawing.
|
|
|
|
- **Given** a result comment for the head that holds the SVGs of an earlier run as attachments
|
|
- **When** the review runs again for the same head
|
|
- **Then** the result comment links these SVGs, and PR Lens draws nothing new
|
|
|
|
##### Verified by
|
|
|
|
- [test_prlens.DiagramsInTheResultCommentTest.test_a_rerun_on_the_same_head_reuses_the_attached_diagrams](../../tests/test_prlens.py)
|
|
|
|
#### Scenario#121.05: PR Lens gets no Gitea token
|
|
|
|
… so that werkrevisor does not hand its Gitea token to third-party code.
|
|
|
|
- **Given** a review that holds a Gitea token and an OpenRouter key
|
|
- **When** werkrevisor starts PR Lens
|
|
- **Then** PR Lens gets the OpenRouter key and the diff, but neither the Gitea token nor any other secret
|
|
|
|
##### Verified by
|
|
|
|
- [test_prlens.DrawTest.test_pr_lens_gets_the_model_key_and_no_other_secret](../../tests/test_prlens.py);
|
|
red/green: with `env = dict(environ)` the test fails, with the fix it passes.
|
|
|
|
#### Scenario#121.06: the result comment holds no more diagrams than Gitea takes
|
|
|
|
… so that Gitea's limit of attachments per comment costs at most the diagrams above it.
|
|
|
|
- **Given** a PR Lens answer with more diagrams than `max_diagrams`, or than Gitea's `[attachment] MAX_FILES`
|
|
- **When** werkrevisor attaches them
|
|
- **Then** the result comment links the first `max_diagrams` diagrams that Gitea took,
|
|
and the log names how many were left out and the limit
|
|
|
|
##### Verified by
|
|
|
|
- [test_prlens.DrawTest.test_at_most_max_diagrams_are_attached](../../tests/test_prlens.py)
|
|
- [test_prlens.DiagramsInTheResultCommentTest.test_diagrams_beyond_the_attachment_limit_of_gitea_are_left_out](../../tests/test_prlens.py);
|
|
red/green: before the fix the failed upload left all diagrams unlinked.
|
|
|
|
#### Scenario#121.07: a possible secret keeps the diff from PR Lens
|
|
|
|
… so that a leaked secret does not reach a third party through the diagrams.
|
|
|
|
- **Given** an enabled repository and a PR whose diff holds a possible secret,
|
|
or gets one by a push after the review
|
|
- **When** werkrevisor publishes its result, and later runs again for the same head
|
|
- **Then** no run sends a diff with the secret to PR Lens; PR Lens only ever gets the diff that the secret scan saw
|
|
|
|
##### Verified by
|
|
|
|
- [test_prlens.DiagramsInTheResultCommentTest.test_a_possible_secret_keeps_the_diff_from_pr_lens](../../tests/test_prlens.py);
|
|
red/green: without the check of the `secrets` reason the test fails, with it it passes.
|
|
- [test_prlens.DiagramsInTheResultCommentTest.test_a_rerun_without_attached_diagrams_draws_none](../../tests/test_prlens.py)
|
|
- [test_prlens.DiagramsInTheResultCommentTest.test_pr_lens_gets_the_diff_that_passed_the_secret_scan](../../tests/test_prlens.py);
|
|
red/green: before the fix PR Lens got the diff of the later push.
|
|
|
|
#### Scenario#121.08: a review without the model draws no diagrams
|
|
|
|
… so that `--no-llm` sends nothing to a model and costs nothing.
|
|
|
|
- **Given** an enabled repository
|
|
- **When** werkrevisor reviews a PR with `--no-llm`
|
|
- **Then** it calls no PR Lens
|
|
|
|
##### Verified by
|
|
|
|
- [test_prlens.DiagramsInTheResultCommentTest.test_no_llm_draws_no_diagrams](../../tests/test_prlens.py);
|
|
red/green: without the check of `use_llm` the test fails, with it it passes.
|
|
|
|
## Solution
|
|
|
|
- `Dockerfile`: a stage `base` adds Debian's `nodejs` 20.19 and `/opt/pr-lens`,
|
|
installed with `npm ci --ignore-scripts` from `tools/pr-lens/package-lock.json`,
|
|
and `npm audit --audit-level=high` fails the image build.
|
|
npm stays in the build stage `pr-lens`.
|
|
The 5 packages are MIT or ISC: `@coldtea/pr-lens-cli` 0.13.2, `-renderer` 0.3.2, `-schema` 0.7.0, `zod`, `yaml`.
|
|
- `tools/release_rootfs.py`: the base archive is `werkrevisor-base-<hash>.tar.zst`;
|
|
the hash covers the `Dockerfile` without its last stage and the lockfile.
|
|
`tools/build-rootfs --base` builds and exports the stage `base`.
|
|
- `werkrevisor/pr_lens.mjs`: werkrevisor reads PRs only through the Gitea API and has no checkout,
|
|
so it cannot run `pr-lens analyze`.
|
|
The script takes the diff on stdin and uses the prompt, schema, validation and repair round of the pinned PR Lens,
|
|
then renders the light theme.
|
|
It imports modules that `@coldtea/pr-lens-cli` does not export; a new version needs a check of these imports.
|
|
- `werkrevisor/prlens.py`: starts the script with a timeout and a minimal environment,
|
|
names the SVGs `pr-lens-<view>.svg` and keeps at most `max_diagrams`.
|
|
- `werkrevisor/cli.py`: `_attach_diagrams` runs after the result comment, the approval and the status,
|
|
only for a published result that is not ERROR.
|
|
Only a fresh review with the model and without a possible secret draws;
|
|
a rerun only links SVGs that are attached already, since it does not know whether its first run found a secret.
|
|
PR Lens gets the diff that the review scanned, never one fetched again after a push.
|
|
An upload that Gitea refuses ends the uploads; the comment links the diagrams that Gitea took.
|
|
It reads the attached SVGs through `GET …/comments/{id}/assets`,
|
|
since `GET` of the comment leaves them out on git.javagil.de.
|
|
- `werkrevisor/gitea.py`: `get_issue_comment`, `list_comment_assets` and `create_comment_asset` with a multipart body.
|
|
- `[diagrams]` in the host config: `enabled`, `model`, `max_diff_bytes`, `max_output_tokens`, `timeout_seconds`,
|
|
`max_diagrams`.
|
|
- The inside diagram has a step 11, "Diagrams".
|
|
|
|
End to end on 2026-10-18: the image drew 3 diagrams of hsadmin-ng PR#319 in 33 s,
|
|
and `GiteaClient` attached them to a comment in werkrevisor-bench issue 89;
|
|
a second pass linked them again without a new drawing.
|
|
|
|
Deployment: the base archive changes its name, so the build `release` fails until it lies on mih09.
|
|
Before the merge, build it in the state of this PR with `tools/build-rootfs --base`
|
|
and copy it to `/home/storage/mih09/users/werkator/`.
|
|
|
|
The Codex review of `babb25a` found that the diagrams bypassed the secret veto and `--no-llm`;
|
|
Scenario#121.07 and Scenario#121.08 fix both.
|
|
The Codex review of `e716ea6` found that a push after the review could still send unscanned lines;
|
|
the review of `b232092` found that an upload above Gitea's limit left all diagrams unlinked.
|
|
|
|
## Open Questions
|
|
|
|
- [x] Does the bot token suffice for the upload?
|
|
The bot edits its own result comments already; it has no access to the private test repository,
|
|
so the first enabled repository shows whether it may attach files; a missing right only prints a line.
|
|
- [x] Variant A (a second call through PR Lens) or B (the review model draws in the same call)?
|
|
A: PR-Agent answers in a fixed YAML format, so B would mean a change inside PR-Agent; see the concept.
|
|
- [x] How does PR Lens run on mih09 without a container engine?
|
|
Node.js in the read-only base archive; see [DECISIONS.md](../DECISIONS.md).
|
|
|
|
## Additional Changes
|
|
|
|
- `docs/werkrevisor.werkbaum`: `#ops.release` and the two Podman nodes of PR#114 and PR#115 are `[^]`;
|
|
they are in use since 2026-10-17.
|
|
- `.dockerignore` lets the two files of `tools/pr-lens/` into the build context.
|
|
|
|
## Follow-up PRs
|
|
|
|
- Switch the diagrams on for a first repository, e.g. werkrevisor itself.
|
|
- `hs-review` puts the SVGs next to its draft for dev.hostsharing.net; the human attaches them when posting.
|
|
- The cost of the diagrams in the usage record and in the budget of a review.
|