Skip to content

docs: add /review-pr and /agent-brief documentation - #74

Open
jflowers wants to merge 4 commits into
mainfrom
opsx/command-docs
Open

docs: add /review-pr and /agent-brief documentation#74
jflowers wants to merge 4 commits into
mainfrom
opsx/command-docs

Conversation

@jflowers

@jflowers jflowers commented May 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • /review-pr documentation — Post-PR review command with CI causality analysis (PR-caused vs pre-existing failure classification), fix branch offering, comparison table with /review-council, and in-line PR comments with 15-comment cap.
  • /agent-brief documentation — AGENTS.md lifecycle management: create mode (project signal analysis), audit mode (5-tier scoring rubric), bridge file management (CLAUDE.md, .cursorrules), and doctor integration.
  • Quick reference table — New table at the top of common-workflows.md listing all 8 documented slash commands at a glance.
  • Spec artifact fixes — Constitution alignment corrected (website vs org constitution), quick reference table requirement updated (create vs update), content sources documented, /agent-brief placement decision resolved.

Closes #59, closes #60

jflowers added 2 commits May 2, 2026 15:09
- Document /review-pr with CI causality analysis, fix branch offering,
  comparison table vs /review-council, and in-line PR comments
- Document /agent-brief with create/audit modes, 5-tier scoring rubric,
  bridge file management, and doctor integration
- Create quick reference table listing all 8 documented slash commands
- Fix spec artifacts per review council (constitution alignment, quick
  reference table scope, content sources, placement decision)

@yvonnedevlinrh yvonnedevlinrh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: REQUEST CHANGES

Build passes cleanly (npm run build — exit 0). Pure Markdown additions, no custom HTML/CSS/JS. Constitution principles all satisfied structurally, but issue coverage gaps need resolution before closing #60.

Findings

[MEDIUM] Cross-framework bridge check not documented (Issue #60)
The Bridge File Management section documents CLAUDE.md/.cursorrules file checks but doesn't mention the cross-framework constitution governance verification described in #60 ("verifies constitution governs both Speckit and OpenSpec when both frameworks exist"). If the upstream /agent-brief command includes this capability, a sentence should be added. If it doesn't, the issue description may overstate the feature — in which case, note that in the PR so the gap is acknowledged.

[LOW] Fix branch user confirmation not explicit (Issue #59)
Issue #59 lists "user confirmation" as a fix-branch safeguard. The doc says "you review it and push when ready" — implicit but not a documented confirmation prompt.

[LOW] Doctor integration check count
Issue #60 says "12 deterministic structural checks"; docs say "7-area health check." These describe different granularity levels — not wrong, but the quantified improvement is lost.

[LOW] Tasks 4.3 and 4.5 unchecked in tasks.md
Visual rendering verification and dark mode verification remain unchecked. Build passes, so this is a process gap rather than a functional risk.

Issue Coverage

Issue Coverage
#59 6/7 criteria covered (1 PARTIAL)
#60 5/8 criteria covered (1 NOT COVERED, 2 PARTIAL)

This PR says "Closes #60" but 3 of 8 acceptance criteria have gaps. Please either address the missing coverage or clarify which criteria don't apply to the shipped command.

Verdict

REQUEST CHANGES — The cross-framework bridge check gap (NOT COVERED) and two PARTIAL criteria against #60 need resolution before this PR can close that issue. The fix is likely small — either a sentence addition if the feature exists upstream, or a note explaining why the criterion doesn't apply.

This review was generated by /uf.review-pr (AI-assisted).

@jflowers jflowers self-assigned this Aug 20, 2026
jflowers added a commit that referenced this pull request Aug 20, 2026
- Rename all /review-pr references to /uf.review-pr throughout blog post
- Rename all /review-council references to /uf.review-council
- Drop "local" qualifier from /uf.review-council description (now supports GitHub review posting)
- Update lifecycle diagram labels to match current command names
- Fix See Also link: remove comparison table claim (depends on unmerged PR #74)

Addresses PR #82 review feedback from @yvonnedevlinrh and @jflowers.

Signed-off-by: jflowers <jflowers@users.noreply.github.com>
Assisted-by: claude-opus
- Remove stale Bridge File Management subsection (upstream PR #463
  removed CLAUDE.md/.cursorrules scaffolding)
- Make fix branch user confirmation explicit in /review-pr docs
- Update doctor integration to specify 11 structural checks
- Check off tasks 4.3 and 4.5 after verification

Addresses PR #74 review feedback from @yvonnedevlinrh.

Signed-off-by: jflowers <jflowers@users.noreply.github.com>
Assisted-by: claude-opus-4-6
@jflowers

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review! All four items addressed in 05e7fa8:

  1. Cross-framework bridge check / Bridge File Management — Removed the entire Bridge File Management subsection. Bridge file scaffolding (CLAUDE.md/.cursorrules) was removed upstream in chore: remove CLAUDE.md and .cursorrules bridge file scaffolding unbound-force#463 (merged Aug 13), so this section documented dead functionality.

  2. Fix branch user confirmation — Updated the pre-existing failures paragraph to explicitly mention "user confirmation" before branch creation, matching the safeguard language from docs: Add /review-pr command documentation — post-PR GitHub review with CI causality analysis #59.

  3. Doctor integration check count — Updated to "11 deterministic structural checks" (the accurate upstream count after bridge file checks were removed in #463, down from 12).

  4. Tasks 4.3 and 4.5 — Verified rendering and dark mode via npm run build, checked off both tasks.

@yvonnedevlinrh yvonnedevlinrh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review Summary

Verdict: REQUEST CHANGES (2 MEDIUM findings)

Build passes (npm run build — 101 pages, exit 0). No CI checks configured on this branch. Core documentation additions (/review-pr, /agent-brief, quick reference table) are well-structured, accurate, and constitution-compliant. Two gaps remain from the prior review cycle.

Findings

[MEDIUM] Bridge File Management — spec requirement undelivered

The delta spec (specs/commands.md, requirement agent-brief-docs) states:

bridge file management MUST document which files are created (CLAUDE.md, .cursorrules), when they are created vs validated, and their purpose

Task 2.5 also requires this. The fix commit (05e7fa8) removed the Bridge File Management subsection citing "upstream PR #463 removed CLAUDE.md/.cursorrules scaffolding" — but no replacement was added and the spec was not amended. Issue #60 still lists bridge files as a key capability.

Action: Either restore the subsection with accurate upstream content, or amend specs/commands.md + note in #60 that the capability was removed upstream.

[MEDIUM] Doctor check count mismatch — 11 vs 12

Issue #60 states "12 deterministic structural checks." The published docs say "11 structural checks." Constitution Principle I (Content Accuracy) requires claims to be verifiable. One of these numbers is wrong.

Action: Verify against the upstream /agent-brief command source and reconcile the docs and issue description.

Resolved from prior review

  • Fix branch user confirmation implicit — Fixed in 05e7fa8. Wording now explicitly states user confirmation is required.
  • Tasks 4.3 and 4.5 unchecked — Checked off in 05e7fa8.

Constitution Compliance

Principle Status
I. Content Accuracy PASS (with caveats above)
II. Minimal Footprint PASS — pure Markdown, no custom HTML/SCSS/JS
III. Visitor Clarity PASS — quick reference table, comparison table, clear structure

This review was generated by /uf.review-pr (AI-assisted).

Upstream PR unbound-force/unbound-force#463 removed bridge file scaffolding
(CLAUDE.md, .cursorrules) from uf init and uf doctor. The spec requirement
to document bridge file management is now obsolete — the feature no longer
exists. Amend the spec with an explanatory note citing the upstream change.

Also updated Issue #60 body to correct the doctor check count from 12 to
11, matching the current upstream reality (verified against source code in
internal/doctor/checks.go).

Addresses PR #74 review feedback from @yvonnedevlinrh.

Signed-off-by: jflowers <jflowers@users.noreply.github.com>
Assisted-by: claude-opus-4-6
@jflowers

Copy link
Copy Markdown
Contributor Author

Both findings addressed:

Bridge File Management (Item 1):
Spec amended in 8cc974b. Upstream PR unbound-force/unbound-force#463 (merged 2026-08-13) removed bridge file scaffolding entirely — CLAUDE.md and .cursorrules are no longer created, validated, or managed by uf. The spec requirement has been amended with an explanatory note citing the upstream change. Documenting a removed feature would violate Constitution Principle I (Content Accuracy).

Doctor Check Count (Item 2):
Verified against source code (internal/doctor/checks.go, function checkAgentContext()): 11 checks is correct (8 guaranteed + 3 conditional). Issue #60 body updated from "12" to "11" with a note explaining the history (PR #122: 12 → PR #135: 13 → PR #463: 11). The documentation was already accurate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

3 participants