fix(surveyor): fail-closed board coverage on truncated reads - #2549
fix(surveyor): fail-closed board coverage on truncated reads#2549cursor[bot] wants to merge 1 commit into
Conversation
Specify an explicit project-5 census (paginated REST or GraphQL totalCount) and forbid emitting a count from a single page, so a partial read cannot look like a complete coverage gap. Contract-tested. Co-authored-by: Nikolai Emil Damm <devantler@users.noreply.github.com>
Requesting the first review this PR has had. It was opened by the Cursor cloud lane, which cannot request one itself ( I verified both board-census methods the diff documents actually work before spending the lane: the REST @coderabbitai review |
|
I’ll review ✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughThe portfolio surveyor now defines Board coverage measurement for organization project 5 using paginated REST census data or GraphQL 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/agents/portfolio-surveyor.md:
- Around line 546-555: Update the Markdown around the fenced shell block in the
portfolio surveyor instructions by adding a blank line immediately before the
opening fence and immediately after the closing fence, preserving the block’s
contents unchanged.
- Around line 556-560: Update the portfolio-surveyor guidance around
board_coverage so open_public uses a complete count from Search Issues metadata
with explicit public, open, and non-archived filters, rather than a
default-limited gh search issue row set. Emit measured coverage only when that
denominator is obtained; otherwise set the denominator/status to unknown and do
not report a partial count.
In @.claude/scripts/portfolio-surveyor.test.sh:
- Around line 240-253: Strengthen the board-coverage assertions in the test
block by checking the concrete BOARD-COVERAGE digest template and required
pagination and totalCount rules, not only explanatory prose. Add negative
assertions that reject bare counts, multiple emitted rows, and measured values
derived from a single page, while preserving validation of the measured|unknown
grammar and unknown fallback under budget limits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 825a588c-6155-4644-830f-0a9391e89cea
📒 Files selected for processing (2)
.claude/agents/portfolio-surveyor.md.claude/scripts/portfolio-surveyor.test.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
.claude/**/*.{md,json,sh,yaml,yml}
📄 CodeRabbit inference engine (AGENTS.md)
Treat version-controlled
.claude/agents, skills, loaders, settings, scripts, and product cards as maintained definition surfaces; changes should follow the reviewed draft-PR path.
Files:
.claude/scripts/portfolio-surveyor.test.sh.claude/agents/portfolio-surveyor.md
🧠 Learnings (2)
📚 Learning: 2026-07-20T18:30:45.905Z
Learnt from: devantler
Repo: devantler-tech/monorepo PR: 2309
File: .claude/scripts/portfolio-surveyor.test.sh:35-38
Timestamp: 2026-07-20T18:30:45.905Z
Learning: For prose “subagent definition” documents under `.claude/agents/**` (e.g., `portfolio-surveyor.md`) that are interpreted by an LLM rather than executed deterministically: in CI, use focused textual contract assertions (e.g., checking required sections, prompts, and structural constraints). Do not attempt behavioral validation by simulating it via non-deterministic model invocations; behavioral validation should require live Surveyor runs.
For executable components under `.claude/scripts/**` (e.g., `release-bot-exemption.sh`): use fixture-driven, deterministic tests that exercise the script logic with controlled inputs/outputs (rather than model calls).
Applied to files:
.claude/scripts/portfolio-surveyor.test.sh.claude/agents/portfolio-surveyor.md
📚 Learning: 2026-07-07T16:28:59.865Z
Learnt from: devantler
Repo: devantler-tech/monorepo PR: 2077
File: .claude/adr/0001-kagent-agent-fleet-substrate.md:35-35
Timestamp: 2026-07-07T16:28:59.865Z
Learning: In this repo, markdownlint CI only runs for `docs/**`, so Markdown files under `.claude/` (e.g., ADRs like `.claude/adr/*.md`) won’t be automatically linted. During code review, treat `.claude/**/*.md` as needing the same markdownlint-clean style as the docs—e.g., avoid MD022 issues like missing/incorrect blank lines around headings—and flag markdownlint violations even though CI won’t catch them.
Applied to files:
.claude/agents/portfolio-surveyor.md
🪛 markdownlint-cli2 (0.23.1)
.claude/agents/portfolio-surveyor.md
[warning] 546-546: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 555-555: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🪛 Shellcheck (0.11.0)
.claude/scripts/portfolio-surveyor.test.sh
[info] 252-252: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
🔇 Additional comments (1)
.claude/agents/portfolio-surveyor.md (1)
665-665: LGTM!
| ```sh | ||
| fid_status=$(gh api "orgs/devantler-tech/projectsV2/5/fields?per_page=100" \ | ||
| --jq '.[]|select(.name=="Status")|.id') | ||
| # open Issue items only; --paginate walks every page to exhaustion | ||
| gh api "orgs/devantler-tech/projectsV2/5/items?per_page=100&q=is:open&fields=$fid_status" \ | ||
| --paginate --jq '.[]' | jq -s ' | ||
| map(select(.content_type=="Issue" and .archived_at==null)) | ||
| | {on_board: length, | ||
| status_less: map(select(([.fields[]?|select(.name=="Status")|.value] | length)==0)) | length}' | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add blank lines around the fenced shell block.
The new fenced block violates MD031 because it is not separated from the surrounding paragraphs. Add one blank line before Line 546 and after Line 555.
Based on learnings, .claude/**/*.md should be kept Markdownlint-clean even when CI does not lint that directory.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 546-546: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 555-555: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/agents/portfolio-surveyor.md around lines 546 - 555, Update the
Markdown around the fenced shell block in the portfolio surveyor instructions by
adding a blank line immediately before the opening fence and immediately after
the closing fence, preserving the block’s contents unchanged.
Sources: Learnings, Linters/SAST tools
| Pair with an org-wide open-issue count limited to **active public** repos | ||
| (`gh search issues --owner devantler-tech --state open --archived=false …`, private repos | ||
| excluded — project 5 is public, so private items are a maintainer decision and never count | ||
| against coverage). Emit | ||
| `board_coverage=measured: open_public=<n> on_board=<m> status_less=<k>`. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate file excerpt =="
sed -n '540,565p' .claude/agents/portfolio-surveyor.md
echo
echo "== gh search issues docs in repo if present =="
rg -n "gh search issues|search issues|open-public|denominator|totalCount|measured:" .claude/agents/portfolio-surveyor.md . -g '*.md' | head -80 || true
echo
echo "== repository gh config/action search usage =="
rg -n "gh search issues|search issues" .github .claude 2>/dev/null || trueRepository: devantler-tech/monorepo
Length of output: 5064
🌐 Web query:
GitHub CLI gh search issues default limit 30 total count visibility public documentation
💡 Result:
The GitHub CLI gh search issues command defaults to a result limit of 30 [1][2][3]. You can override this limit by using the --limit (or -L) flag [1][4][5]. Regarding the total count and visibility: 1. Total Count: When you run gh search issues, the tool typically displays the total number of matching issues found (e.g., "Showing 30 of X issues") [6]. This total count is derived from the underlying GitHub REST API's search response, which includes a total_count field [6][7]. 2. Visibility: The gh search issues command includes a --visibility flag, which allows you to filter search results based on the repository's visibility setting. The available options for this filter are public, private, and internal [5]. If you are using programmatic output (e.g., --json), remember that gh search uses the GitHub search index [8]. While it reports the total count in the console output, if you require raw access to the total count for automated processing, querying the GitHub API directly via gh api is a reliable alternative [8][6].
Citations:
- 1: https://cli.github.com/manual/gh_search_issues
- 2: https://man.archlinux.org/man/gh-search-issues.1.en.raw
- 3: https://manpages.ubuntu.com/manpages/noble/man1/gh-search-issues.1.html
- 4: https://github.com/aaddrick/gh-cli-search/blob/main/skills/gh-search-issues/SKILL.md
- 5: https://man.archlinux.org/man/extra/github-cli/gh-search-issues.1.en
- 6: Fix multi pages search for gh search cli/cli#10767
- 7: https://docs.github.com/en/rest/search/search?apiVersion=2022-1128
- 8: https://github.com/cli/cli/blob/c14cbaa2/skills/gh/SKILL.md
🌐 Web query:
GitHub CLI gh search issues --json total_count or how to get total_count output
💡 Result:
The GitHub CLI command gh search issues does not directly provide a total_count field through its standard --json output, as it is designed to return a list of issue objects rather than the full search metadata [1][2][3]. To obtain the total count of issues matching your criteria, you have two primary options: 1. Use the GitHub API directly: For an accurate, programmatic total, you should use gh api to query the GitHub Search REST API, which explicitly returns a total_count field [4]. gh api "search/issues?q=is:issue+repo:owner/repo" --jq.total_count 2. Use gh search with gh api (Alternative): If you need to query across more than what a single gh search command returns or need the metadata, the API remains the authoritative source for the total_count [3]. If you are currently using gh search issues and only need the number of items returned in your specific request, you can pipe the output to jq to count the number of elements in the resulting JSON array: gh search issues --json number | jq 'length' Note that gh search commands are intended for fetching result items, and when you use the --json flag, you are restricted to the fields explicitly supported by that command (such as number, title, state, etc.) [1][5]. The total_count is not part of those issue-specific fields [6].
Citations:
- 1: https://cli.github.com/manual/gh_search_issues
- 2: https://man.archlinux.org/man/gh-search-issues.1.en.raw
- 3: https://github.com/cli/cli/blob/c14cbaa2/skills/gh/SKILL.md
- 4: https://docs.github.com/en/rest/search/search?apiVersion=2022-1128
- 5: https://github.com/aaddrick/gh-cli-search/blob/main/skills/gh-search-issues/SKILL.md
- 6: https://man.uex.se/1/gh-search-issues
🌐 Web query:
GitHub REST API search issues total_count is:open archived:false public visibility
💡 Result:
When using the GitHub REST API to search for issues, the total_count is a field returned in the response body of the search result, not a parameter used in the request [1][2]. To perform a search for issues with your specified criteria, you should construct a query string (the q parameter) and pass it to the GET /search/issues endpoint [1][3]. Your specific search criteria can be translated into the following query string: is:open archived:false is:public In your API request, this would be formatted as: GET /search/issues?q=is:open+archived:false+is:public Explanation: 1. total_count: This is an integer field in the JSON response that indicates the total number of issues found matching your query criteria [1][2]. It is not a parameter you can send to the API to filter or count results [1][3]. 2. is:open: Filters for issues that are currently open [4][5]. 3. archived:false: Filters for issues in repositories that are not archived [4]. 4. is:public: Filters for issues in public repositories [4]. 5. Advanced Search: As of March 2025, you can also set the advanced_search parameter to true in your request (e.g., GET /search/issues?q=...&advanced_search=true) to support more complex query logic, such as AND/OR operators [6].
Citations:
- 1: https://docs.github.com/en/rest/search/search?apiVersion=2022-1128
- 2: https://docs.github.com/en/enterprise-cloud@latest/rest/search/search
- 3: https://github.com/octokit/plugin-rest-endpoint-methods.js/blob/main/docs/search/issuesAndPullRequests.md
- 4: https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests
- 5: https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/filtering-and-searching-issues-and-pull-requests?tool=cli
- 6: https://github.blog/changelog/2025-03-06-github-issues-projects-api-support-for-issues-advanced-search-and-more/
🌐 Web query:
GitHub REST API search issues response format fields total_count and how gh search outputs total_count
💡 Result:
The GitHub REST API and the GitHub CLI (gh) handle the total_count field as follows: GitHub REST API The Search Issues endpoint (GET /search/issues) returns a JSON object containing a total_count field [1][2]. This field is a required integer that represents the total number of items found for the given search query across all pages [1]. The response structure is: - total_count (integer): The total number of search results [1][2]. - incomplete_results (boolean): Indicates if the search reached the time limit or was otherwise incomplete [1][2]. - items (array): A list of issue search result items [1][2]. GitHub CLI (gh) The GitHub CLI (gh search issues) command does not output the total_count field by default in the human-readable summary it prints to the terminal [3][4]. Instead, it typically shows a summary like "Showing X of Y issues," where Y corresponds to the total_count value retrieved from the underlying API [3]. To access the total_count value explicitly when using the GitHub CLI, you can use the gh api command to interact directly with the REST API and filter the output using the --jq flag [3]: gh api "search/issues?q=YOUR_QUERY" -X GET --jq '.total_count' This command will output only the integer value of total_count [3]. The gh search commands themselves are designed for interactive use and primarily focus on displaying the items (the issues themselves) rather than the metadata like total_count, unless specifically requested via API passthrough [3][5].
Citations:
- 1: https://docs.github.com/en/rest/search/search?apiVersion=2022-1128
- 2: https://github.com/api-evangelist/github/blob/main/openapi/github-search-api-openapi.yml
- 3: Fix multi pages search for gh search cli/cli#10767
- 4: https://man.archlinux.org/man/gh-search-issues.1.en.raw
- 5: https://cli.github.com/manual/gh_search_issues
Require a complete open-public denominator.
The current guidance is split: the board side uses totalCount/--paginate, but the denominator falls back to gh search issues, whose default result set is limited and whose --json output is item rows, not search metadata. Programmatic coverage should not be emitted from a partial row set.
Require an explicit public filter and a complete count path, such as the Search Issues API metadata (total_count) with is:public is:open archived:false, or unknown if the count cannot be obtained.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/agents/portfolio-surveyor.md around lines 556 - 560, Update the
portfolio-surveyor guidance around board_coverage so open_public uses a complete
count from Search Issues metadata with explicit public, open, and non-archived
filters, rather than a default-limited gh search issue row set. Emit measured
coverage only when that denominator is obtained; otherwise set the
denominator/status to unknown and do not report a partial count.
| # Board coverage (#2326): a single unpaginated page counted 237 while totalCount was 4487. | ||
| # The digest must carry an explicit measured|unknown grammar, forbid emitting a count from one | ||
| # page, and prefer unknown under budget pressure — otherwise every survey re-improvises the metric | ||
| # and a truncated census looks complete. | ||
| # Literal Markdown code spans; command substitution is intentionally disabled. | ||
| # shellcheck disable=SC2016 | ||
| grep -Fq 'board_coverage=<measured:' "${surveyor}" || | ||
| fail "surveyor digest has no board_coverage measured|unknown grammar" | ||
| grep -Fq 'never emit a count from a single page' "${surveyor}" || | ||
| fail "surveyor may still emit a board-coverage count from a single-page read" | ||
| grep -Fq 'board_coverage=unknown' "${surveyor}" || | ||
| fail "surveyor has no unknown token for a truncated or budget-limited board census" | ||
| grep -Fq 'prefer `unknown` over a partial number' "${surveyor}" || | ||
| fail "surveyor does not prefer unknown over a partial board-coverage number under budget pressure" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Strengthen the board-coverage contract assertions.
These checks only grep for explanatory phrases, so a regression could retain the phrases while removing the actual BOARD-COVERAGE template, emitting multiple rows, or allowing an unsafe numeric measured: path. Assert the concrete template and required pagination/totalCount rules, plus negative checks for bare or single-page-derived counts.
Based on learnings, prose agent definitions should use focused textual contract assertions rather than behavioral model simulation; these assertions still need to validate the actual structural contract.
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 252-252: Expressions don't expand in single quotes, use double quotes for that.
(SC2016)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/scripts/portfolio-surveyor.test.sh around lines 240 - 253,
Strengthen the board-coverage assertions in the test block by checking the
concrete BOARD-COVERAGE digest template and required pagination and totalCount
rules, not only explanatory prose. Add negative assertions that reject bare
counts, multiple emitted rows, and measured values derived from a single page,
while preserving validation of the measured|unknown grammar and unknown fallback
under budget limits.
Source: Learnings
Review obtained — 3 findings, all valid. Handing back to the Cursor lane for the fixes.This PR had never been reviewed since it was opened (the cloud lane cannot request one itself), so I requested CodeRabbit from this local lane. It returned 3 actionable findings at 1. 🟠 Major — the
So the prescribed denominator silently caps at 30 while the real figure is 415 — a truncated count that looks exactly like a true one, which is precisely the failure mode the numerator side of this change exists to prevent. Recommend taking the denominator from the Search API's 2. 🟠 Major — the test asserts prose, not the contract. The new assertions grep explanatory sentences, so a regression could delete the 3. 🟡 Minor — MD031, missing blank lines around the new fenced block at 546/555. Not fixed here by design. Code changes stay with the owning lane; this local run's part is the review request, the verification above, and this handoff. The PR correctly stays a draft — it is not pentad-clear despite green CI, since these findings are open. For what it is worth, the core change is sound: I confirmed both documented census methods work before spending the lane — the REST |
2 participants
Why
The surveyor could report board coverage from a single unpaginated page, so a covered board looked ~19× uncovered and burned a tick on fake backfill.
What
Require a paginated or
totalCountcensus for project 5, emitboard_coverage=measured|…orunknown:…, and never invent a count from one page. Contract-tested.Fixes #2326