Repository navigation
chore: regenerate the artifacts at the top of the localization stack - #1582
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Test statistics and coverage snapshots docs/development/testing/testing.md |
Updated aggregate, package, and domain test counts. Added i18n rows to the command and core-package coverage snapshots, marked n/a. |
Priority:
Estimated code review effort: 1 (Trivial) | ~4 minutes
Change: Other
Possibly related PRs
- jmrplens/gitlab-mcp-server#1577: Adds localization code and tests that are reflected in this testing-statistics refresh.
Merge Risk: 
599da
-
No actionable merge-blocking risk was identified in the documentation-only changes.
Pre-merge checks: 
-
Check name Status Explanation Resolution Description check The description is thorough and explains the context: this is the top layer of the localization stack for #1547, it adds no behavior, and it regenerates artifacts. It records what was run, what passed…Rewrite the description to follow the repository template. Fill in the '## Description' section with a short summary. Under 'Related Issue', use 'Refs #1547' or 'Closes #' with the correct number. Check the applicable box under 'Type of Cha…-
Check name Status Explanation Title check The title "chore: regenerate the artifacts at the top of the localization stack" describes the main change: regenerating committed artifacts (here, the testing docs statistics) for the top layer of th… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "chore: regenerate the artifacts at the top of the localization stack" describes the main change: regenerating committed artifacts (here, the testing docs statistics) for the top layer of the localization stack. It follows Conventional Commits style and is specific enough for a teammate scanning history. Its one weakness is that "the artifacts" and "the localization stack" are slightly vague, but the change is still clearly summarized.
-
Full details: Description check
-
Explanation
The description is thorough and explains the context: this is the top layer of the localization stack for
#1547, it adds no behavior, and it regenerates artifacts. It records what was run, what passed, and how each layer was checked. It references an issue (Refs#1547) but does not use the template's 'Closes #' line or fill in the Type of Change, Changes Made, How to Test, or Checklist sections. It is long and detailed but does not follow the repository template structure.Resolution
Rewrite the description to follow the repository template. Fill in the '## Description' section with a short summary. Under 'Related Issue', use 'Refs
#1547' or 'Closes #' with the correct number. Check the applicable box under 'Type of Change' (likely 'CI / build / tooling' or 'Documentation update'). List key changes under 'Changes Made' (for example, the regenerated testing reference counts and the i18n coverage rows). Add numbered steps under 'How to Test'. Keep 'Breaking Changes / Migration Notes' as 'N/A'. Mark the Checklist items that were verified, such as the generator and DCO checks. Add a Screenshots / Logs section if relevant, or leave it out.
-
Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
github-actions
Bot
added
the
documentation
The five layers below rebuild issue 1547 on main as it stands today: the message catalog and GITLAB_MCP_LANGUAGE, the two gates, the move of the person-facing texts into the catalog, the Spanish catalog and the documentation. Every layer below this one runs its suite with GITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred, and this one regenerates the committed artifacts once, over the whole stack. make update-all with the testing reference written by go run ./cmd/gen_testing_docs/ -skip-coverage: every other generator found its output current, and the testing reference moves its test counts to the packages and tests the stack adds. Refs #1547 Signed-off-by: José M. Requena Plens <mail@jmrp.io>
|
jmrplens
added a commit
that referenced
this pull request
Oct 29, 2026
…'s diff, publish the pagination of mr_review.changes_get, and say when a prompt read only the first page of changed files (#1584) An inline discussion or draft note on a file past the first page of a merge request's diff was refused locally as "not in the merge request diff", because the position pre-check read one page of 100 files, and `mr_review.changes_get` returned one page without saying how to continue. Nikita Turchaninov (slowaner) found and fixed both in #1517, and that work is his: the pre-check walks the diff a page at a time and stops at the file, bounded by the new register row RQB-013; it compares the merge request's `diff_refs` with the position before and after the walk; and it leaves the write to GitLab whenever its evidence is incomplete. `changes_get` publishes its pagination, and its Files line reads `N`, `N of total` or `N shown, more available`. This pull request carries #1517 to land it for 3.4.0, with him as co-author, and I'll close #1517 when it merges. His six commits carry no `Signed-off-by:` trailer, which the Developer Certificate of Origin needs on every commit since #1562, and adding one means rewriting his branch, so I squashed them into one commit of mine on today's `main`, `fix(tools): handle paginated merge request diffs`, with a `Co-authored-by:` trailer naming him, and opened it from a branch of this repository. The merge conflicted only in generated files (the README token block, the testing reference and the token footprint), and I regenerated them from the merged tree rather than taking a side. The branch is rebased on the localization stack (#1577 to #1582), where only the testing reference conflicted, regenerated the same way. #1530 and #1531 stay separate, as agreed in the review of #1517. ## What I added on top - `internal/tools/mrchanges/action_specs_test.go`, the one file the pull request adds, opens with the copyright and license header #1563 requires, written by `make fix-file-headers`. - The doc comment of `toolutil.MergeRequestDiffForPositionCheck` says again why skipping the check is safe: the write goes to the same instance with the same credential, so a 401, 403 or 404 on the listing or on the snapshot read is the refusal the write is about to get, and a transient failure, a snapshot that moved or an exhausted page budget leaves the write judged by GitLab alone. Both `validatePosition` comments that send the reader there are true again, and name the snapshot case. - The same gap in the prompts. The five prompts that read a merge request's changed files (`summarize_mr_changes`, `review_mr`, `suggest_mr_reviewers`, `mr_risk_assessment` and `mr_description_quality`) read one page of 100, as their descriptions have said since issue 1169, but counted it as the whole diff: a review or a risk assessment of a merge request changing 250 files read "Files changed: 100", with the line totals of those 100 and nothing saying the other 150 were never read. Their count now reads as `changes_get`'s does, and when the merge request changes more files a line says that every count and list covers the page alone and that `mr_review.changes_get` reads the rest from page 2 with `per_page` 100. A merge request within one page gets exactly the message it got before. `TestMRDiffPrompts_MoreFilesThanOnePage_SayWhatTheyRead` fails on the parent in every partial case, and the prompt section of the site says the same in English and Spanish. The line stays English under `GITLAB_MCP_LANGUAGE=es`, like the rest of a `prompts/get` body, since the model reads it and ADR-0027 keeps what the model reads out of the message catalog. - `suggest_mr_reviewers` reads the merge request and its changed files as `review_mr` does, but its exclusion entry named only `project.members`, so an operator who excluded `mr_review.changes_get` or `merge_request.get` was still served the changed paths through it, and with the line above it would have named the excluded action. It now names both, and `TestRegister_ExcludingAnActionRemovesThePromptServingTheSameData` fails without that. ## The other diff routes I went through every handler that reads a diff for the same gap. Whether GitLab pages a route is what the live record says it takes: only the merge request diffs, the diff versions and the commit diff declare `page` and `per_page`. | Route | Read by | Paged | Affected | | --- | --- | --- | --- | | `GET /projects/:id/merge_requests/:merge_request_iid/diffs` | `mr_review.changes_get` and the position pre-check of `mr_review.discussion_create`, `mr_review.draft_note_create` and `mr_review.draft_note_update` | Yes | Yes, fixed by slowaner's commit | | The same route | the five prompts above | Yes | Yes, fixed here: still one page, now said in the message | | `GET /projects/:id/merge_requests/:merge_request_iid/versions` | `mr_review.diff_versions_list` | Yes | No: it takes `page` and `per_page` and publishes its pagination | | `GET /projects/:id/merge_requests/:merge_request_iid/versions/:version_id` | `mr_review.diff_version_get` | No | No | | `GET /projects/:id/merge_requests/:merge_request_iid/raw_diffs` | `mr_review.raw_diffs` | No | No | | `GET /projects/:id/repository/commits/:sha/diff` | `repository.commit_diff` | Yes | No: it takes `page` and `per_page` and publishes its pagination | | `GET /projects/:id/repository/compare` | `repository.compare` and the `generate_release_notes`, `compare_branches` and `audit_commit_hygiene` prompts | No | No | | `GET /projects/:id/merge_requests/:merge_request_iid/changes` | nothing | No | No | ## How I tested it On the head, `go test ./... -count=1` passes (303 packages), recording the request inventory on the whole unit suite, and `gen_request_inventory -check` finds the committed inventory current. `golangci-lint` passes on every package the branch touches. `audit-1to1-paths`, `audit-1to1-grants`, `check-action-grants`, `check-action-grants-derivation`, `check-tenancy`, `check-surface-quality`, `check-tool-reference`, `check-meta-descriptions`, `check-action-ids`, `check-sdk-context`, `check-md-escaping`, `check-i18n`, `check-test-file-names`, `check-test-subtests`, `check-test-goroutines`, `check-godocs`, `check-llms`, `check-testing-docs`, `check-footprint`, `check-site-stats`, `check-md-tables`, `check-doc-tool-names`, `check-e2e-static`, `check-action-catalog-manifest`, `audit-catalog-first`, `check-file-headers`, `check-dco` and `check-em-dash` pass, and the three `test/e2e/gitlab` packages compile and vet under the `e2e` tag. Every generator of `make update-all` runs clean on the head, the testing reference with `-skip-coverage`, which its own commit refreshed. The site passes `i18n:check`, `facts:check`, `chips:check` and `llms:check`, and `node scripts/check-doc-links.mjs` passes. I did not run the Docker suites against a GitLab. <details> <summary>Design notes from slowaner's description</summary> The position pre-check searches for the requested old or new path one page at a time and stops at the first match. It retains no earlier pages. Discussion create and draft-note create and update share this lookup and preserve the requested body, position and SHAs. Before trusting the listing, the helper reads the merge request's `diff_refs` and compares all three SHAs with the position. It reads them again after finding the file or finishing the listing. Both snapshot reads are kept: the second detects a push during the search, so evidence from a newer diff cannot justify a local refusal. A successful lookup costs k+2 reads for a match on page k, plus the write. This remains best effort, since GitLab validates changes after the final read. A changed, missing or unreadable snapshot, a listing error, cancellation, non-advancing pagination or an exhausted page budget makes the pre-check advisory, and GitLab judges the original write. Only a complete, stable result can produce the existing file or line error. A matching file with no available patch skips line validation. The walk is bounded by RQB-013 at 100 actual list calls of 100 files. A match or completed listing on call 100 is valid; call 101 is never sent. Reaching the bound adds no caller quota and does not reject the write. GitLab documents a maximum `diff_max_files` of 3,000, normally at most 30 pages at 100 files per page; the 100-call bound also ends a malformed walk that keeps advertising another page. GitLab's own diff-size limits still apply. | Item | Answer | | --- | --- | | VAL-001 | Allow: bounds the diff pages one advisory position check may read. | | VAL-002 | `KeyRequest`; mint cost is nothing. Each invocation has its own finite walk; this is not an allowance accumulated by a caller. | | VAL-003 | Class P, as computed for a request bound by the register. No class D finding. | | VAL-004 | `RequestBound`, not a share or fairness guarantee. | | VAL-005 | No claim to protect a process-wide resource or one population from another, so no process partner applies. | | VAL-006 | At 100 list calls, a match or terminal page is accepted. If another page is needed, discard incomplete evidence and let GitLab judge the unchanged write. No table is keyed on caller input. | | VAL-007 | No new refusal, channel, code, HTTP status, `Retry-After` or retry class, on HTTP or STDIO. Existing stable-snapshot position errors remain unchanged. | | VAL-008 | Counts actual diff-list calls within this lookup only. It does not charge an admission, tenant or server quota, and charges no unjudged write. | | VAL-009 | Fixed source constant `mergeRequestDiffPositionMaxPages = 100`; 100 files per call. No new setting, flag or MCP parameter, so operator-supplied zero and malformed values do not apply. | | VAL-010 | Debug diagnostics state why the advisory check was skipped. No new refusal metric or caller/address identity label; existing project/MR context identifies the operation. | | VAL-011 | The same request bound and advisory fallback apply on STDIO. | | VAL-012 | No population-protection or fairness claim, so fairness measurement is not applicable. Boundary, early-stop and fallback behavior are covered by regression tests. | `changes_get` publishes pagination metadata in structured output and Markdown. Guidance describes offset continuation, `next_page` as `page`, with `per_page: 100`. The keyset request option #1517 first added, and its synthetic endpoint tests, were removed during its review, and the existing input fields stay as they are for #1530. </details> Co-authored-by: Nikita Turchaninov <2010628+slowaner@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #1547
This is the top of the localization stack for #1547. The five layers below it add the message catalog and
GITLAB_MCP_LANGUAGEwith English alone, teachaudit_md_escapingthe catalog and add the person-facing text gate, move every text the server shows a person directly into the catalog with English unchanged byte for byte, add the Spanish catalog, and document the setting with ADR-0027. They run their suites underGITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred, and this one regenerates the committed artifacts once, over the whole stack. It adds no behavior of its own.The stack sits on
mainatd6f108bcd, onto which I rebased it on 28 October 2026, layer by layer and in order, since it had been built on8f339f4e8andmaintook 24 pull requests since. Every commit is signed and carries aSigned-off-bytrailer. Every source file the layers add, the planted modules ofcmd/audit_i18nandcmd/audit_md_escapingincluded, carries the copyright and license header of #1563, written bymake fix-file-headersinto the commit that adds it, and the lines the person-facing text gate's findings tree pins moved down three with it. The gate is now step 33 ofmake analyze, after the header gate, so the second layer renumbers every step to/33and moves the header gate's Makefile test to[32/33]. The language field and theAllowPlaintextHTTPfield of #1561 sit side by side in the HTTP overlay and in the server's flag struct, and the package counts inCLAUDE.mdare 312 and 339 after the first layer and 313 and 349 after the second. The regeneration was made on the rebased tree.The regeneration
I ran every generator of
make update-all, with the testing reference written bygo run ./cmd/gen_testing_docs/ -skip-coverage. Every other generator found its output current: the layers move no tool, action, schema, description, resource or prompt, which is what the stack promises for anyone who leaves the setting unset, and the documentation layer already carries what the site pages andserver.jsonsay about the setting. The testing reference moves its package, file and test counts to the ones the stack adds.How I checked it
On this branch, with
GITLAB_MCP_TEST_SNAPSHOT_PARITYunset:go test ./... -count=1passes over all 313 packages.make analyzepasses its 33 steps: 31 on a copy of the tree without.gitor Node, and markdownlint and the copyright and license header gate in the checkout.generatedjob passes,check-testing-docs,check-llms,check-tool-reference,check-site-stats,check-footprint,check-action-grants,check-i18n,check-md-escaping,check-gateway-chars,check-surface-quality,check-server-jsonandcheck-server-json-packagesamong them, and so docheck-dco,check-em-dashandcheck-plan-untracked.make test-e2e-http(811 tests, the 8 that need Docker skipped) andmake test-e2e-stdio(313 tests) pass. Both drive the real binary with the setting unset,en,EN-GB,es,es-419and through the flag, and ask theinteractive.issue_createquestions in English and in Spanish, with the property keys, the option values and the listings unchanged in both.test/e2e/gitlaband its harness compile under thee2etag,go vet -tags e2epasses on them, and the harness's own tests pass.i18n:check,facts:check,chips:check,llms:check,buildandlintpass, andscripts/check-doc-links.mjsfinds every local link and anchor of 352 files.Each layer below passes on its own as well, checked out alone after the rebase:
go build ./...,go vet ./...and with thee2etag, the compile oftest/e2e/gitlab,go test ./... -count=1underGITLAB_MCP_TEST_SNAPSHOT_PARITY=deferred,make golangci-lintover the whole module, the gates each layer's description names (check-i18nfrom the second layer up, where it exists),check-file-headers,check-dco,check-em-dash, markdownlint on the Markdown it changes,check-doc-links, and both transport modules: 790 HTTP and 288 stdio tests on the first two layers, 794 and 292 on the third, 811 and 313 on the fourth and fifth.