fix(context): remove redundant allocation and document analyzer proof - #1196
Conversation
Use the conditional array directly as the mutable row accumulator instead of copying it immediately through an outer array spread. Preserve recommendation, gate, container, blocker and unblock identity collection. CodeFactor returned a successful PR check with a no-useless-spread notice; the stricter release analyzer gate correctly refused that outstanding annotation. Record the escape and correction under the existing execution-role PM owner, and require exact-head hosted analyzer proof before merging this correction. Validation: 44 existing context packing, egress, relevance and execution-role tests pass, repository ESLint passes, record-integrity passes, and the changelog is generated through pm-changelog. PM: pm-08mt4k.
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR removes an unnecessary array spread in context-usage row accumulation by using the conditional array directly, without changing delivered-identity traversal. It also updates PM records and generated changelog metadata; review should confirm the required 44 tests, lint, build, record-integrity checks, hosted coverage, and exact-head CodeFactor annotation gate. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe PR removes a redundant array spread from context result collection, records regression and analyzer verification, adds exact-head review guidance, updates issue history, refreshes metadata, and reorders two changelog entries. ChangesContext correction and verification
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🟡 Moderate · up to The allocation simplification preserves the resulting rows, but the required static validation has not reached a confirmed terminal successful state for the current revision. Merge should wait for that result to avoid releasing without the required analyzer proof. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
|
@greptileai Please review the correction to the redundant array allocation and its PM evidence. The previous green CodeFactor check had one notice, which the stricter release analyzer gate correctly rejected. No quality threshold or behavior has been weakened; existing context-use and final-egress tests pass. |
|
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="CHANGELOG.md" line_range="7" />
<code_context>
### Fixed
-- Windows published-release verifier test can preempt its own readiness and cleanup contract ([pm-euqm7x](https://github.com/unbraind/pm-cli/blob/main/.agents/pm/issues/pm-euqm7x.toon))
- pm next recommends outcome-milestone gates as actionable work, and summary.containers counts a bucket the envelope never emits ([pm-08mt4k](https://github.com/unbraind/pm-cli/blob/main/.agents/pm/issues/pm-08mt4k.toon))
+- Windows published-release verifier test can preempt its own readiness and cleanup contract ([pm-euqm7x](https://github.com/unbraind/pm-cli/blob/main/.agents/pm/issues/pm-euqm7x.toon))
- Commander usage refusals omit token_accounting even when --token-accounting is parsed ([pm-bfa1ob](https://github.com/unbraind/pm-cli/blob/main/.agents/pm/issues/pm-bfa1ob.toon))
</code_context>
<issue_to_address>
**nitpick:** The changelog change removes the `pm-08mt4k` release-note entry without adding it elsewhere in the shown diff, so the completed context-usage fix is omitted from the generated release notes.
**Suggested fix:** Retain the `pm-08mt4k` entry in the appropriate changelog section and correct only its ordering or formatting.
</issue_to_address>Sourcery assessment
Approved.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
The guide correctly scopes the allocation correction and the separate zero-annotation release proof. All required hosted checks pass, and hosted-analysis-gate confirms exact_commit evidence with CodeFactor outstanding_annotations=0 and DeepScan new_issues=0. |
|
Read the explicit free OSS quota notice. This is not a completed review; the full review will be requested after the stated reset around 07:21 UTC. Other required checks and exact-head analyzer proof have passed. Read the completed full review on f729327: no code or documentation findings. The watcher has now completed successfully, including Gates (static), which was still running when the bot checked. The refreshed exact-head hosted-analysis gate confirms zero CodeFactor annotations, zero new DeepScan issues and strict branch protection. The diff-level private-function docstring heuristic is separate from the repository-defined source/export documentation gate, which passed. |
|
The action-not-completed section reports the free review cooldown. The corrected head now has zero outstanding CodeFactor annotations and no new DeepScan issues. I will request the full review again after included capacity resets. |
|
Read the coverage result on 3e949c1: all modified coverable lines and all tests passed. The mandatory hosted full-coverage gate also completed successfully. |
|
The review approval is noted, but its sole proposed correction is false. The pm-08mt4k changelog entry is an unchanged context line at line 7; only the Windows entry moves below it after canonical package regeneration. Both entries remain exactly once, and the generated check passes. The direct inline response provides that evidence. |
Document the repository review helper, terminal check receipts, complete comment inventory and explicit full-SHA hosted analyzer gate in the agent guide. Explain that a successful CodeFactor check can retain notices and that only the stricter zero-annotation proof establishes release eligibility. Record the verified correction and reusable workflow under pm-08mt4k, keeping reviewer quota limitations separate from approvals and preserving explicit dispositions for false or stale feedback. Validation: build, docs/skills and link/content checks, record integrity, and package-generated changelog. Runtime remains identical to 3e949c1, whose full hosted checks and exact-commit analyzer proof passed. PM: pm-08mt4k.
|
@greptileai The free-review reset has elapsed. Please review final head f729327, which combines the redundant-array correction with the documented review-helper and exact-head analyzer-proof sequence. The runtime is identical to 3e949c1, whose full hosted checks and strict analyzer proof passed with zero CodeFactor annotations and zero new DeepScan issues. The subsequent guide and PM evidence update passed local build, docs/skills and record-integrity checks. The earlier changelog observation was a diff-reading error: pm-08mt4k remains at line 7 and only the Windows entry moves below it. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== PR metadata =='
gh pr view 1196 --repo unbraind/pm-cli --json number,title,baseRefOid,headRefOid,files,commits,statusCheckRollup,reviews,comments
echo '== Changed files =='
gh pr diff 1196 --repo unbraind/pm-cli --name-only
echo '== Diff, excluding large context only =='
gh pr diff 1196 --repo unbraind/pm-cli --patch
echo '== Commit check-runs for stated runtime head =='
gh api "repos/unbraind/pm-cli/commits/3e949c195/check-runs?per_page=100" \
--jq '.check_runs[] | [.name, .status, .conclusion, .details_url] | `@tsv`' || true
echo '== Commit check-runs for final head =='
gh api "repos/unbraind/pm-cli/commits/f7293276a/check-runs?per_page=100" \
--jq '.check_runs[] | [.name, .status, .conclusion, .details_url] | `@tsv`' || trueLength of output: 99266 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Locate candidate scripts and workflows =='
fd -a -t f '^(context-usage\.ts|pr-review-loop\.mjs|hosted-analysis-gate\.mjs)$|\.ya?ml$' . \
| sed -n '1,160p'
echo '== Source structure =='
ast-grep outline src/sdk/context-usage.ts --items all --view expanded
echo '== Review helper structure =='
ast-grep outline scripts/reviews/pr-review-loop.mjs --items all --view expanded
echo '== Hosted analyzer gate structure =='
ast-grep outline scripts/release/hosted-analysis-gate.mjs --items all --view expanded
echo '== collectNextResultItemIds and call sites =='
rg -n -A45 -B10 '\bcollectNextResultItemIds\b|\basObjectRecord\b' src/sdk/context-usage.ts
rg -n -A4 -B4 '\bcollectNextResultItemIds\s*\(' --glob '*.ts' .
echo '== Relevant test definitions and assertions =='
rg -n -A10 -B6 'context-usage|final egress|recommended|gate_needed|held_by_others|containers' \
tests --glob '*.{ts,tsx}' | sed -n '1,320p'
echo '== Final-head guide section and referenced helper command syntax =='
sed -n '185,240p' docs/AGENT_GUIDE.md
rg -n -A10 -B6 'watch|inventory|acknowledge-inline|acknowledge' scripts/reviews/pr-review-loop.mjs
rg -n -A12 -B6 'process\.argv|hosted-analysis|annotations|DeepScan|branch protection' \
scripts/release/hosted-analysis-gate.mjs
echo '== Final-head static gate and analyzer workflow definitions =='
rg -n -A35 -B10 'Gates \(static\)|hosted-analysis-gate|Release analyzer readiness' \
.github --glob '*.{yml,yaml}' || true
echo '== Current final-head checks =='
gh api "repos/unbraind/pm-cli/commits/f7293276add7a10f0fec41e9275c4be5c047bee7/check-runs?per_page=100" \
--jq '.check_runs[] | select(.name == "Gates (static)" or .name == "Release analyzer readiness (main)") | [.name,.status,.conclusion,.completed_at,.details_url] | `@tsv`'Length of output: 78985 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="unbraind/pm-cli"
sha="f7293276add7a10f0fec41e9275c4be5c047bee7"
echo '== Final-head required and non-success check states =='
gh api "repos/${repo}/commits/${sha}/check-runs?per_page=100" \
--jq '.check_runs[] | select(.conclusion != "success" and .conclusion != "neutral" and .conclusion != "skipped") | [.name,.status,(.conclusion // ""),.output.title,.output.annotations_count,.details_url] | `@tsv`'
echo '== Final-head CodeFactor check evidence =='
gh api "repos/${repo}/commits/${sha}/check-runs?per_page=100" \
--jq '.check_runs[] | select(.name == "CodeFactor") | {name,status,conclusion,output:{title:.output.title,summary:.output.summary,annotations_count:.output.annotations_count}}'
echo '== Final-head DeepScan commit status =='
gh api "repos/${repo}/commits/${sha}/status" \
--jq '.statuses[] | select(.context == "DeepScan") | {context,state,description,target_url}'
echo '== Fresh PR review inventory =='
gh api graphql -F owner="unbraind" -F name="pm-cli" -F pr=1196 -f query='
query($owner: String!, $name: String!, $pr: Int!) {
repository(owner:$owner,name:$name) {
pullRequest(number:$pr) {
headRefOid
reviewThreads(first:100) {
nodes {
isResolved
comments(first:20) { nodes { author { login } body path line originalLine } }
}
}
reviews(first:100) { nodes { author { login } state commit { oid } body } }
}
}
}' --jq '.data.repository.pullRequest'
echo '== Changed-file whitespace errors =='
git diff --check 22a7a1a9b829c540036340f5f7ca8f354b3cc008 f7293276add7a10f0fec41e9275c4be5c047bee7
echo '== Final PM record closeout and history tail =='
sed -n '1,170p' .agents/pm/issues/pm-08mt4k.toon
tail -n 8 .agents/pm/history/pm-08mt4k.jsonlLength of output: 42178
✅ Action performedFull review finished. |
|
The full review found no code/documentation findings and correctly waited for the remaining static check. The watcher has since reached terminal success for all required checks on f729327. The independent hosted-analysis gate also passes on that exact head with zero outstanding CodeFactor annotations, zero new DeepScan issues and strict branch protection. The changelog entry remains present as verified; no source changes followed this review. |
The release analyzer gate rejected the context-usage row accumulator because CodeFactor reported an unnecessary outer array spread, even though its PR check conclusion was successful. Use the conditional array directly and preserve all delivered-identity traversal behavior.
Tracked under pm-08mt4k, with the original release refusal, correction, linked verification, and structured closure recorded through PM. The agent guide now documents the review helper, complete feedback inventory, and mandatory exact-head zero-annotation analyzer proof before merge. This completes release readiness for #1195. The changelog is generated by the latest pm-changelog package.
Validation: 44 existing context packing, final-egress, relevance and execution-role tests; full repository ESLint; build, docs/skills and record-integrity assurance. The exact-head hosted-analysis gate must explicitly confirm zero outstanding CodeFactor annotations before merge; a successful check conclusion alone is insufficient. Hosted coverage and all required CI gates remain mandatory.