chore: expand pnpm check to mirror full CI merge gate#2074
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
🔴❗ Critical (0) ❗🔴
None
🟠⚠️ Major (0) 🟠⚠️
None
🟡 Minor (0) 🟡
None
💭 Consider (0) 💭
None
✅ APPROVE
Summary: Clean docs & script wiring PR that improves DX by giving contributors a single pnpm check command that mirrors the CI merge gate. The documentation corrections accurately reflect the actual pre-push hook behavior (pnpm format, not check:prepush). All changes are accurate, well-structured, and improve contributor experience. Ship it! 🚀
Discarded (5)
| Location | Issue | Reason Discarded |
|---|---|---|
overview.mdx:270 |
Snippet references raw GitHub URL from main branch | Pre-existing pattern not introduced by this PR — out of scope |
| PR description | Claims AGENTS.md documents check:husky/check:prepush but these terms don't appear |
PR description metadata accuracy, not a code issue |
.github/workflows/ci.yml |
CI runs checks redundantly after this change | Out of scope — requires separate CI workflow PR (correctly noted by reviewer as follow-up) |
| Multiple files | Terminology split: 'format verification' vs 'format:check' | Validated as acceptable given different audiences (AGENTS.md for AI, overview.mdx for humans) |
| Multiple files | Documentation aligns with pre-push hook | Positive observation, not an issue |
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-docs |
1 | 0 | 0 | 0 | 0 | 0 | 1 |
pr-review-devops |
3 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-consistency |
4 | 0 | 0 | 0 | 0 | 0 | 4 |
| Total | 8 | 0 | 0 | 0 | 0 | 0 | 5 |
Note: DevOps findings were INFO-level positive observations validating correctness, not issues requiring action.
CI Note: Pre-existing flaky browser test on mainThe CI failure is in This is not caused by this PR (which only touches Evidence: The last 3 CI runs on
The flaky screenshot test appears to have been introduced between |
nick-inkeep
force-pushed
the
feat/local-ci-parity
branch
from
e2f94d2 to
9d5e9c9
Compare
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
🔴❗ Critical (0) ❗🔴
None
🟠⚠️ Major (0) 🟠⚠️
None
🟡 Minor (0) 🟡
None
💭 Consider (1) 💭
💭 1) AGENTS.md:13 Add runtime expectation guidance for pnpm check
Issue: The expanded pnpm check command chains 4+ sequential check suites (turbo check, format:check, env-descriptions, knip) which can take several minutes. Contributors unfamiliar with the codebase might run it during active development expecting quick feedback.
Why: First-contact legibility — setting expectations helps contributors choose the right tool for their workflow (rapid iteration vs. pre-PR verification).
Fix: Consider adding a brief note distinguishing pnpm check (comprehensive gate for pre-PR verification) from faster alternatives (pnpm lint:fix, pnpm typecheck, pnpm test) for rapid iteration. This is optional — the current documentation already lists all components, and experienced contributors will naturally infer the runtime implications.
Refs: AGENTS.md:13
✅ APPROVE
Summary: Clean docs & script wiring PR that improves contributor DX by consolidating CI gate steps into a single pnpm check command. All documentation changes accurately reflect actual behavior (verified against .husky/pre-push and CI workflow). The terminology fix in the second commit ensures consistency. Ship it! 🚀
Discarded (1)
| Location | Issue | Reason Discarded |
|---|---|---|
.github/workflows/ci.yml |
CI now runs format:check, env-descriptions, and knip redundantly (once via pnpm check, once as separate steps) |
Pre-existing observation already noted in prior review. Intentional redundancy for safety. Out of PR scope — would require a separate CI workflow PR. |
Reviewers (4)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-docs |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-devops |
1 | 0 | 0 | 0 | 0 | 0 | 1 |
pr-review-consistency |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-product |
1 | 0 | 1 | 0 | 0 | 0 | 0 |
| Total | 2 | 0 | 1 | 0 | 0 | 0 | 1 |
nick-inkeep
force-pushed
the
feat/local-ci-parity
branch
from
9d5e9c9 to
e4d04ae
Compare
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
🔴❗ Critical (0) ❗🔴
None
🟠⚠️ Major (0) 🟠⚠️
None
🟡 Minor (0) 🟡
None
💭 Consider (0) 💭
None
✅ APPROVE
Summary: Clean docs & script wiring PR that improves contributor DX by consolidating CI gate steps into a single pnpm check command. All documentation changes verified against actual implementation:
- ✅ Pre-push hook correctly documented as
pnpm format(matches.husky/pre-push) - ✅
pnpm checkaccurately described as mirroring full CI gate - ✅ Terminology consistent across
AGENTS.mdandoverview.mdx
No issues found across 3 reviewers. Ship it! 🚀
Discarded (0)
No findings to discard.
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-docs |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-consistency |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
pnpm check previously only ran the turbo subset (lint + typecheck + test). CI also ran format:check, check:env-descriptions, and knip as separate steps. Now pnpm check chains all four, so "green locally = green in CI." Also fixes contributing docs: pre-push hook description was wrong (claimed check:prepush, actually runs pnpm format) and removes the stale caveat about CI running extra checks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Addresses review feedback — line 269 said "format" but the actual script runs format:check (verification, not auto-fix). Now matches line 13 which correctly uses "format:check". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
nick-inkeep
force-pushed
the
feat/local-ci-parity
branch
from
e4d04ae to
161ae4e
Compare
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
🔴❗ Critical (0) ❗🔴
None
🟠⚠️ Major (0) 🟠⚠️
None
🟡 Minor (0) 🟡
None
💭 Consider (0) 💭
None
✅ APPROVE
Summary: Clean docs & script wiring PR that improves contributor DX by consolidating CI gate steps into a single pnpm check command. All documentation changes verified against actual implementation:
- ✅ Pre-push hook correctly documented as
pnpm format(verified against.husky/pre-push) - ✅
pnpm checkaccurately described as mirroring full CI gate (verified againstci.yml) - ✅ Terminology consistent across
AGENTS.mdandoverview.mdx(both useformat:check)
No issues found across 3 reviewers. Ship it! 🚀
Discarded (0)
No findings to discard.
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-docs |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-consistency |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Restructure the quick reference to clearly separate build/dev from verification. Add a pre-push gate (pnpm format → pnpm check) and an iteration table for individual commands. Simplify the completion checklist to reference pnpm check as the single CI gate. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(2) Total Issues | Risk: Medium
🔴❗ Critical (0) ❗🔴
None
🟠⚠️ Major (1) 🟠⚠️
Inline Comments:
- 🟠 Major:
AGENTS.md:19Documentation claimspnpm checkincludesbuildbut it does not - 🟠 Major:
AGENTS.md:277Samebuildissue (duplicate location)
🟡 Minor (1) 🟡
Inline Comments:
- 🟡 Minor:
AGENTS.md:260Terminology inconsistency — usesformatinstead offormat:check
💭 Consider (0) 💭
None
🚫 REQUEST CHANGES
Summary: The verification cheatsheet restructure is a clear improvement for contributor DX — the two-tier approach (pre-push gate vs. development iteration) addresses the prior "Consider" feedback well. However, the documentation claims pnpm check includes build, but neither turbo check (per turbo.json:163-165) nor the root check script (per package.json:36) includes a build step. This factual inaccuracy would mislead AI agents into believing they've verified the build when they haven't.
To resolve: Remove build from the CI gate description at lines 19, 260, and 277. Also fix the format → format:check terminology on line 260 for internal consistency.
Discarded (0)
No findings to discard.
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-docs |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-devops |
1 | 0 | 0 | 0 | 2 | 0 | 0 |
pr-review-consistency |
1 | 0 | 0 | 0 | 1 | 0 | 0 |
| Total | 2 | 0 | 0 | 0 | 3 | 0 | 0 |
Note: pr-review-docs correctly identified AGENTS.md as internal AI tooling docs (out of scope for customer-facing docs review).
… fix format terminology turbo check depends on [lint, typecheck, test] — not build. Also fix `format` → `format:check` in the completion checklist for consistency with the actual script. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
🔴❗ Critical (0) ❗🔴
None
🟠⚠️ Major (0) 🟠⚠️
None
🟡 Minor (0) 🟡
None
💭 Consider (0) 💭
None
✅ APPROVE
Summary: Delta re-review confirms all prior findings (2 MAJOR, 1 MINOR) were correctly addressed in commit 00790400c:
- ✅ Removed incorrect
buildfrompnpm checkdescription at lines 19, 260, 277 - ✅ Fixed
format→format:checkterminology for consistency - ✅ Documentation now accurately reflects
turbo.json(checkdepends on["lint", "typecheck", "test"]) - ✅
.husky/pre-pushrunspnpm format— matches updated docs
Clean docs & script wiring PR that improves contributor DX by giving a single pnpm check command that mirrors the full CI merge gate. Ship it! 🚀
Discarded (0)
No findings to discard.
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-docs |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-consistency |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: All reviewers verified the prior findings were correctly addressed. No new issues introduced in the delta commit.
Drop iteration table to inline list, remove duplicated pre-push block from workflow section (now cross-references Verification), strip parenthetical from checklist item. ~25 lines → ~12 lines, same signal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
🔴❗ Critical (0) ❗🔴
None
🟠⚠️ Major (0) 🟠⚠️
None
🟡 Minor (0) 🟡
None
💭 Consider (0) 💭
None
✅ APPROVE
Summary: Delta re-review of commit 01b0707e7 confirms the compression of AGENTS.md verification/workflow sections is clean:
- ✅ Internal consistency maintained —
pnpm checkis defined once (line 19) and referenced via anchor links and checklist items - ✅ AI artifact quality preserved — compression improves context efficiency (~40 lines → ~15 lines) without losing essential guidance
- ✅ Accuracy verified —
pnpm checkdescription (lint + typecheck + test + format:check + env-descriptions + knip) matches actual implementation inpackage.jsonandturbo.json - ✅ Cross-references work —
[Verification](#verification)anchor correctly resolves to the heading
The prior review already approved after fixing the build terminology and format vs format:check issues in commit 00790400c. This delta is purely compression with no semantic changes. Ship it! 🚀
Discarded (0)
No findings to discard.
Reviewers (2)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-devops |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-consistency |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Note: Both reviewers verified the compression maintains quality and consistency. No issues introduced in the delta commit.
|
🔎💬 Inkeep AI search and chat service is syncing content for source 'Inkeep Agent Framework Docs' |
1 participant
Summary
Expand
pnpm checkto mirror the full CI merge gate and restructure AGENTS.md so AI coding agents know exactly which commands to run and when.Motivation
Previously,
pnpm checkonly ranturbo check(lint + typecheck + test + build per-package), leaving format verification, env-description validation, and knip as CI-only steps. Contributors and AI agents had to know about 5+ separate commands to replicate CI locally. This meant CI failures that could have been caught pre-push.Changes
package.json—checkscript now chains:turbo check→pnpm format:check→pnpm check:env-descriptions→pnpm knipAGENTS.md— replaced flat command bullet list with a two-tier Verification section:pnpm format→pnpm check(one copy-paste block)pnpm checkinstead of 4 separate itemsagents-docs/.../overview.mdx— corrected pre-push hook description (pnpm format, notpnpm check:prepush), consolidated CI description to referencepnpm checkas the single gateTest plan
pnpm checkruns all gate steps locally and matches CI outputGenerated with Claude Code