Skip to content

feat: implement Lane 1 Review/SDLC core (W1-W7) - #58

Closed
briansrls wants to merge 2 commits into
mainfrom
claude/review-sdlc-core-LFc5V
Closed

briansrls wants to merge 2 commits into
mainfrom
claude/review-sdlc-core-LFc5V

Conversation

@briansrls

Copy link
Copy Markdown
Contributor

W1: gunbc-review CLI binary — entry point that builds the review DAG, resolves credentials from env/policy, and executes in Real mode. Input: git diff via --repo-path/-r and --base-ref/-b. Output: structured findings JSON to stdout.

W2: Credential smoke test — dry-run mode validates the full credential chain (GCP WIF → Secret Manager → LLM API key) with mock specs.

W3: Multi-provider support — --provider flag supports openai and anthropic. Credential policy override via GUNBC_CREDENTIAL_POLICY_JSON.

W4: Abstract 4-dimension review model — domain-agnostic dimensions: Coherence (bugs, contradictions), Quality (standards), Requirements (goal accomplishment), Aspirational (must-fix/defer/accept). Each dimension has typed input ports (artifact, criteria, depth, context). Dimensions with no criteria are skipped (opt-in). Aspirational runs last, seeing merged findings from the other three.

W5: Coding review profile — ReviewProfile maps dimensions to domain-specific criteria. coding_review_profile loads criteria from AGENT.md + clippy.toml (quality), issue body (requirements), invariant docs (coherence), refactoring heuristics (aspirational). Follows I6 (no escape hatches) — file contents are passed as parameters.

W6: CI status as review context — gunbc-pipeline --pr N queries CI via gh run list and injects failure context into the requirements dimension. --depth XS|S|M|L|XL flag controls review thoroughness.

W7: gunbc-pipeline command — orchestrates the full daily flow: fetch CI status → build coding profile → run review → output summary with actionable items categorized as must-fix / defer / accept.

https://claude.ai/code/session_01WSV1TUHvgpw9JnG6gB9xV7

W1: `gunbc-review` CLI binary — entry point that builds the review DAG,
resolves credentials from env/policy, and executes in Real mode. Input:
git diff via --repo-path/-r and --base-ref/-b. Output: structured
findings JSON to stdout.

W2: Credential smoke test — dry-run mode validates the full credential
chain (GCP WIF → Secret Manager → LLM API key) with mock specs.

W3: Multi-provider support — --provider flag supports openai and
anthropic. Credential policy override via GUNBC_CREDENTIAL_POLICY_JSON.

W4: Abstract 4-dimension review model — domain-agnostic dimensions:
Coherence (bugs, contradictions), Quality (standards), Requirements
(goal accomplishment), Aspirational (must-fix/defer/accept). Each
dimension has typed input ports (artifact, criteria, depth, context).
Dimensions with no criteria are skipped (opt-in). Aspirational runs
last, seeing merged findings from the other three.

W5: Coding review profile — ReviewProfile maps dimensions to
domain-specific criteria. coding_review_profile loads criteria from
AGENT.md + clippy.toml (quality), issue body (requirements), invariant
docs (coherence), refactoring heuristics (aspirational). Follows I6
(no escape hatches) — file contents are passed as parameters.

W6: CI status as review context — `gunbc-pipeline --pr N` queries CI
via `gh run list` and injects failure context into the requirements
dimension. --depth XS|S|M|L|XL flag controls review thoroughness.

W7: `gunbc-pipeline` command — orchestrates the full daily flow:
fetch CI status → build coding profile → run review → output summary
with actionable items categorized as must-fix / defer / accept.

https://claude.ai/code/session_01WSV1TUHvgpw9JnG6gB9xV7

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 69606fbef6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread gunbc-dag/src/bin/pipeline.rs Outdated
Comment on lines +237 to +239
.criteria_for(gunbc_lib_review::dimension::ReviewDimension::Quality)
.cloned()
.unwrap_or_else(gunbc_lib_review::graph_mock::default_criteria);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Use requirements criteria when requirements context is present

Phase 1 builds combined_context from CI/PR/issue data and coding_review_profile_with_requirements stores that context on the requirements dimension, but this selection always pulls ReviewDimension::Quality into config.criteria. In runs with --pr or --issue, the gathered requirements/CI context is therefore dropped before prompt generation, so the review cannot actually validate requirement fulfillment. Select the requirements criteria when context is available (or wire all active dimensions) so this pipeline stage has effect.

Useful? React with 👍 / 👎.

Comment thread lib/review/src/profile.rs Outdated
Comment on lines +130 to +131
pub fn coding_review_profile_from_repo(_repo_path: &Path, depth: FermiDepth) -> ReviewProfile {
coding_review_profile_with_context(depth, &ProjectContext::default())

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Populate repo-based quality context from repo_path

coding_review_profile_from_repo ignores repo_path and always passes ProjectContext::default(), which means AGENT.md/clippy.toml content is never loaded into the quality criteria. Any caller expecting repository-specific standards gets generic checks only, reducing review accuracy across all repos. This helper should read the scoped files under repo_path and fill ProjectContext before building the profile.

Useful? React with 👍 / 👎.

…ojectContext

Two review fixes:

1. Pipeline criteria selection: when --pr or --issue provides requirements
   context, select the requirements dimension criteria instead of always
   using quality. This ensures CI/PR/issue context reaches prompt
   generation for requirement validation.

2. ProjectContext from repo_path: binary entry point now reads AGENT.md
   and clippy.toml from the repo path and passes them as ProjectContext.
   Removed the misleading coding_review_profile_from_repo that silently
   ignored repo_path. All callers now use coding_review_profile_with_context
   or coding_review_profile_with_requirements with explicit ProjectContext.
   File reads happen at the binary I/O boundary (I6 compliant), not in
   the library.

https://claude.ai/code/session_01WSV1TUHvgpw9JnG6gB9xV7
@briansrls briansrls closed this Feb 20, 2026
briansrls added a commit that referenced this pull request May 13, 2026
Modeled CI timing for EnforcedApplication on PB-1 bootstrap; SG-0/P5 receipts
for census + enforced_lens_application + build.rs staged ordering; INVARIANTS
integration-test table row + ci-merge PR body append for #2827.

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 13, 2026
Modeled CI timing for EnforcedApplication on PB-1 bootstrap; SG-0/P5 receipts
for census + enforced_lens_application + build.rs staged ordering; INVARIANTS
integration-test table row + ci-merge PR body append for #2827.

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 13, 2026
Modeled CI timing for EnforcedApplication on PB-1 bootstrap; SG-0/P5 receipts
for census + enforced_lens_application + build.rs staged ordering; INVARIANTS
integration-test table row + ci-merge PR body append for #2827.
briansrls added a commit that referenced this pull request May 13, 2026
Export check_enforced_lens_applications for integration tests; re-invoke it on
the PB-1 snapshot after presence checks. Add a budget-violation .dag fixture
(compiled like gate #92 LAS) that must emit a timing lens-enforcement
ParseError — executable receipt that the host path is live, not declaration-only.

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 13, 2026
SG-0 net-shrink compares EXPECTED_HAND_AUTHORED_* counts vs origin/main;
after merging main the gate #58 row is a net +1. The machine prepend
declared +0 and tripped CI.
briansrls added a commit that referenced this pull request May 13, 2026
Add gate_58_test_parse_timing_budget_violation_max_ns_pair next to the
violation message template so integration tests pin ParseError kind,
empty fixes, witness file span, and parsed (declared, usage) ns pair
(TESTING.md: avoid pinning English diagnostic copy).

Align the budget-violation fixture test with the same predicates.

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 13, 2026
INVARIANTS + sg0 append still named the removed V3_STD_STAGING_ORDER_EDGES
table; co-receipt text now matches build.rs (import v3.std.* → Kahn ranks
among co-ranked STAGED_FILES).

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 13, 2026
…larify gate #58 receipt vs Workflow thesis

- timing_lens: fold Practice-4 checkpoint into 🟢/🟡/🔴 classification above the sum (INVARIANTS P1).
- t_ci_workflow: replace weak "not Workflow directly" phrasing with explicit T-LAS receipt vs
  T-Lens-Self-Application dissolution boundary (modeled_gunbc_ci_workflow stays adjacent substrate).
- Regenerate PB-1 bootstrap snapshots after .dag authority edits.

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 13, 2026
… carrier

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 13, 2026
… carrier

Composer-2 11080: module header matched substrate arity (TimingEnforcementProjected).

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 13, 2026
* feat(v3): gate #58 apply_lens_self_application_demonstrated

Modeled CI timing for EnforcedApplication on PB-1 bootstrap; SG-0/P5 receipts
for census + enforced_lens_application + build.rs staged ordering; INVARIANTS
integration-test table row + ci-merge PR body append for #2827.

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* test(v3): prove gate #58 timing EnforcedApplication runs

Export check_enforced_lens_applications for integration tests; re-invoke it on
the PB-1 snapshot after presence checks. Add a budget-violation .dag fixture
(compiled like gate #92 LAS) that must emit a timing lens-enforcement
ParseError — executable receipt that the host path is live, not declaration-only.

Co-authored-by: Cursor <cursoragent@cursor.com>

* WIP: apply_lens_self_application_demonstrated

* ci(sg0): fix PR #2827 hand-path delta to match census

SG-0 net-shrink compares EXPECTED_HAND_AUTHORED_* counts vs origin/main;
after merging main the gate #58 row is a net +1. The machine prepend
declared +0 and tripped CI.

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* chore(v3): rustfmt enforced_lens_application imports

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* refactor(v3): generalize std staging load-order edges in build.rs

Replace the timing_lens ↔ t_ci_demo pairwise comparator with
V3_STD_STAGING_ORDER_EDGES so new default-rank cross-references append
a (before, after) pair instead of bespoke match arms (Codex/Claude review
on PR #2827). Sync INVARIANTS + sg0 PR-body append wording with the new
mechanism; dissolution remains import-graph topo in the producer.

Co-authored-by: Cursor <cursoragent@cursor.com>

* WIP: apply_lens_self_application_demonstrated

* test(v3): assert gate #58 timing receipt structurally, not prose

Add gate_58_test_parse_timing_budget_violation_max_ns_pair next to the
violation message template so integration tests pin ParseError kind,
empty fixes, witness file span, and parsed (declared, usage) ns pair
(TESTING.md: avoid pinning English diagnostic copy).

Align the budget-violation fixture test with the same predicates.

Co-authored-by: Cursor <cursoragent@cursor.com>

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* fix(ci): make PR #2827 P5 append one receipt per INVARIANTS (b)

Codex review 10958: sg0-pr-body-append.2827.txt claimed exactly one P5(b)
receipt then listed three numbered receipts. Rewrite as a single explicit
deferral (SG-0 row + INVARIANTS table path) with co-listed compiler paths
as the same-surface co-receipt, matching the INVARIANTS table wording.

Co-authored-by: Cursor <cursoragent@cursor.com>

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* docs: sync gate #58 P5 receipts with import-derived std staging

INVARIANTS + sg0 append still named the removed V3_STD_STAGING_ORDER_EDGES
table; co-receipt text now matches build.rs (import v3.std.* → Kahn ranks
among co-ranked STAGED_FILES).

Co-authored-by: Cursor <cursoragent@cursor.com>

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* docs(std): Practice-4 traffic light for TimingEnforcementProjected; clarify gate #58 receipt vs Workflow thesis

- timing_lens: fold Practice-4 checkpoint into 🟢/🟡/🔴 classification above the sum (INVARIANTS P1).
- t_ci_workflow: replace weak "not Workflow directly" phrasing with explicit T-LAS receipt vs
  T-Lens-Self-Application dissolution boundary (modeled_gunbc_ci_workflow stays adjacent substrate).
- Regenerate PB-1 bootstrap snapshots after .dag authority edits.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(INVARIANTS): gate #58 SG-0 row names 3-param EnforcedApplication carrier

Co-authored-by: Cursor <cursoragent@cursor.com>

* WIP: apply_lens_self_application_demonstrated

* WIP: apply_lens_self_application_demonstrated

* docs(v3-compiler): gate #58 rustdoc names 3-param EnforcedApplication carrier

Composer-2 11080: module header matched substrate arity (TimingEnforcementProjected).

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 13, 2026
Remove duplicate PASSING token from r3-structure acceptance bullet;
closure status + receipts stay authoritative in §1.8 row 58 only.
Addresses codex REQUEST_CHANGES on PR #2941.

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 13, 2026
* docs(r3): promote gate #58 apply_lens_self_application_demonstrated to PASSING

Sync §1.8 ledger and T-Lens-Self-Application acceptance bullet with the
existing bootstrap witnesses and integration tests already on main.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore: nudge dashboard review sweep

No code or doc content change; empty commit to retrigger provider-review
classification on PR #2941 (per Verification Mgr).

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(r3): single-home gate #58 status in program plan (P2 / Practice 5)

Remove duplicate PASSING token from r3-structure acceptance bullet;
closure status + receipts stay authoritative in §1.8 row 58 only.
Addresses codex REQUEST_CHANGES on PR #2941.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 14, 2026
…ments

Add SG-0 hand-authored integration test receipt row for
t_gate_106_show_correct_code_diagnostic_coverage_test.rs per INVARIANTS §P5.
Split sg0 census comments so gate #58 dissolution block sits only above its path.

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 14, 2026
…ments

Add SG-0 hand-authored integration test receipt row for
t_gate_106_show_correct_code_diagnostic_coverage_test.rs per INVARIANTS §P5.
Split sg0 census comments so gate #58 dissolution block sits only above its path.

Co-authored-by: Cursor <cursoragent@cursor.com>
briansrls added a commit that referenced this pull request May 14, 2026
…Completeness + Substrate canvas) (#3134)

* WIP: R3 gate #106: show_correct_code_diagnostic_coverage (T-Tests-As-Data-Com

* fix(#3134): INVARIANTS P5 row for gate #106 SG-0 census + clarify comments

Add SG-0 hand-authored integration test receipt row for
t_gate_106_show_correct_code_diagnostic_coverage_test.rs per INVARIANTS §P5.
Split sg0 census comments so gate #58 dissolution block sits only above its path.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(ci): SG-0 PR-body append for #3134 census net +1

PR #3134 adds one EXPECTED_HAND_AUTHORED_TEST path; CI prepends
scripts/ci-merge/sg0-pr-body-append.<pr>.txt for sg0 discipline checks.
Pair gate #106 harness add with Verification Gap 9 dispatch anchor in
r3-verification-manager brief.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(INVARIANTS): ASCII-order SG-0 row after gate #62 negative-bridge land

Restores strict path ordering in the SG-0 integration-test receipt table:
r3_gate_62 negative-bridge audit row follows r3_gate_60 (both r3_gate_*).

Co-authored-by: Cursor <cursoragent@cursor.com>

* test(gate-106): lock Correction variant payload field sets

Address openai-pro APPROVE_WITH_COMMENTS: assert exact sorted conj
labels for LiveCorrection and DeferredCorrection payloads (ratchet
matches diagnostics.dag alongside existing per-field ty checks).

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
@briansrls
briansrls deleted the claude/review-sdlc-core-LFc5V branch June 1, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants