Skip to content

Skip empty rubrics review runs#412

Draft
sepo-agent-app[bot] wants to merge 4 commits into
mainfrom
agent/implement-issue-410/codex-26977545719
Draft

Skip empty rubrics review runs#412
sepo-agent-app[bot] wants to merge 4 commits into
mainfrom
agent/implement-issue-410/codex-26977545719

Conversation

@sepo-agent-app

@sepo-agent-app sepo-agent-app Bot commented Jun 4, 2026

Copy link
Copy Markdown

Summary

  • Add a lightweight rubrics-review preflight before provider/runtime setup.
  • Skip rubric model runs, artifacts, and PR comments when rubric reads are disabled, the rubrics branch is missing, or no active rubrics are selected.
  • Update docs and workflow tests for the skip behavior.

Verification

  • npm run build
  • node --test dist/__tests__/envelope.test.js dist/__tests__/rubrics.test.js
  • git diff --check

Closes #410

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

Comment thread .github/workflows/agent-rubrics-review.yml Outdated
@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

sepo-agent-app Bot commented Jun 4, 2026

Copy link
Copy Markdown
Author

Sepo is dispatching follow-up automation.

Source Next Target Round Status
fix-pr review PR #412 5 / 40 Dispatched

Reason: agent planner selected review: fix-pr succeeded on open PR #412 and pushed current head 1d2f8eb; the next useful automated step is review to verify the prior NEEDS_REWORK findings are resolved.

@sepo-agent-app

sepo-agent-app Bot commented Jun 4, 2026

Copy link
Copy Markdown
Author

Rubrics Review

Total Score Verdict Rubrics Scored
100 PASS 8
Dimension Rubric Result Score Evidence
coding_workflow/generic Keep docs in sync pass 7/7 Docs now state rubrics review skips model work when reads are disabled, agent/rubrics is missing, or no active rubrics are selected: rubrics.md, supported-workflows.md.
coding_workflow/generic Reuse existing code pass 8/8 The workflow reuses existing resolve-policy.js, download-agent-rubrics, and select.js for the preflight instead of adding a separate rubric parser or selector: agent-rubrics-review.yml.
coding_style/generic Prefer simple implementations pass 7/7 The implementation is a direct preflight and if gating change, plus two narrowly scoped setup-action inputs for avoiding duplicate dependency/build work: setup-agent-runtime/action.yml.
coding_workflow/generic Make surgical changes pass 8/8 The PR touches only rubrics-review workflow behavior, the shared runtime setup action needed by that workflow, focused tests, and matching docs.
coding_workflow/generic Validate preview contracts pass 7/7 The skip path validates read policy, branch availability, selected rubric count, and invalid-rubric warning count before deciding whether to run: agent-rubrics-review.yml. Tests assert ordering, gates, skip strings, and comment/artifact gating: envelope.test.ts.
communication/generic Keep status comments compact pass 6/6 Empty rubric runs emit a concise Actions notice/step summary and skip PR comment posting: agent-rubrics-review.yml, agent-rubrics-review.yml.
coding_workflow/generic Prefer agent-derived decisions pass 7/7 The skip decision is derived from current policy resolution and selected rubric count, not from a new repo variable or duplicate user setting: agent-rubrics-review.yml.
coding_workflow/generic Prefer explicit structured inputs pass 7/7 The setup action adds separate install_dependencies and build_runtime boolean inputs, then the rubrics workflow passes them explicitly for the second provider-tool setup call: setup-agent-runtime/action.yml, agent-rubrics-review.yml.

Notes

  • The current head addresses the earlier clean-checkout blocker by building the agent runtime before invoking .agent/dist CLIs, while still gating provider resolution, model execution, artifact upload, and PR comment posting on should_run.
  • The implementation matches issue Skip rubrics review if there are no rubrics / rubrics branch is not present? #410’s requested behavior: no rubrics branch or zero selected active rubrics produces an Actions-visible skip, not a model-generated N/A PR comment.
  • No rubric-specific follow-up needed.

Findings

  • INFO: No rubric-specific violations found.

Final Rubric Verdict

PASS


codex | gpt-5.5/xhigh | Macmini-runner-1

@sepo-agent-app

sepo-agent-app Bot commented Jun 4, 2026

Copy link
Copy Markdown
Author

AI Review Synthesis

Dual-agent review by Claude and Codex.
Requested by @lolipopshock.

Summary of PR/Issue

Review

The current head resolves the prior BLOCKING runtime-order issue. Codex and the rubrics lane report no blocking or warning issues; Claude flags one non-blocking selector-parity maintenance risk.

Issue Severity Description
Selection parity remains implicit WARNING The preflight selector flags currently match the real rubrics-review selection path, but nothing enforces that relationship. Future drift could reintroduce a run where preflight and model selection disagree.

Progress

  • Prior same-agent inline thread for the runtime-order blocker was re-fetched, verified as fixed, and resolved.
  • No new inline comments were posted because the selector-parity concern is already covered in PR discussion and is advisory.
  • No inline replies or comment minimizations were performed.
  • Reviewer verification reports npm run build, targeted node tests, git diff --check, and the current GitHub check status is passing.

Issue Details

Selection parity remains implicit

Cause: The preflight in agent-rubrics-review.yml manually mirrors the effective rubrics-review selector behavior in run-agent-task.

Candidate solutions: Add a cross-reference comment at both call sites, or add a test asserting the preflight selector flags stay aligned with the real rubrics-review selector.

Comments: This is not a current correctness defect because the flags match today.

Recommended Next Step

HUMAN_DECISION: the only remaining concern is optional maintenance hardening, not required automated branch-change work.

Final Verdict

MINOR_ISSUES

Action Items

  • No required branch-change work remains.

codex | gpt-5.5/xhigh | zucchini-m4-4

required: false
default: ""
install_dependencies:
description: Whether to install .agent runtime dependencies.

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.

@sepo-agent I'm confused : why this is added in the pr?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It was added as support for the follow-up fix, not as a new user-facing setting.

The rubrics preflight now needs .agent/dist before it can run resolve-policy.js / select.js, so the workflow calls setup-agent-runtime early here:

- name: Setup agent runtime for rubrics preflight
uses: ./.github/actions/setup-agent-runtime
- name: Resolve rubrics review mode
id: rubrics_mode
env:
AGENT_RUBRICS_POLICY: ${{ vars.AGENT_RUBRICS_POLICY || '' }}
ROUTE: rubrics-review
run: node .agent/dist/cli/rubrics/resolve-policy.js

Later, after the provider is resolved, the workflow still needs to install only the selected provider CLI, but it should not run npm ci and npm run build a second time. These inputs let that second call skip dependency install/build and only handle provider tools:

- name: Install rubrics review provider tools
if: ${{ steps.preflight.outputs.should_run == 'true' }}
uses: ./.github/actions/setup-agent-runtime
with:
install_dependencies: "false"
build_runtime: "false"
install_codex: ${{ steps.provider.outputs.install_codex }}

Defaults stay true, so existing callers keep the old behavior:

install_dependencies:
description: Whether to install .agent runtime dependencies.
required: false
default: "true"
build_runtime:
description: Whether to build the .agent TypeScript runtime.
required: false
default: "true"


codex | gpt-5.5/xhigh | zucchini-m4-6

@lolipopshock

Copy link
Copy Markdown
Contributor

@sepo-agent /answer

- name: Download rubrics for review preflight
        if: ${{ steps.rubrics_mode.outputs.read_enabled == 'true' }}
        id: preflight_rubrics
        uses: ./.github/actions/download-agent-rubrics
        with:
          github_token: ${{ steps.auth.outputs.token }}
          ref: ${{ inputs.rubrics_ref || vars.AGENT_RUBRICS_REF || 'agent/rubrics' }}
          path: ${{ runner.temp }}/agent-rubrics-review-preflight
          continue_on_missing: "true"
          bootstrap_if_missing: "false"

      - name: Count active rubrics for review
        if: ${{ steps.preflight_rubrics.outputs.rubrics_available == 'true' && steps.preflight_rubrics.outputs.rubrics_dir != '' }}
        id: preflight_select
        env:
          REQUEST_TEXT: ${{ inputs.request_text }}
          ROUTE: rubrics-review
          RUBRICS_DIR: ${{ steps.preflight_rubrics.outputs.rubrics_dir }}
          RUBRICS_CONTEXT_FILE: ${{ runner.temp }}/rubrics-review-preflight.md
        run: |
          set -euo pipefail
          node .agent/dist/cli/rubrics/select.js \
            --dir "${RUBRICS_DIR}" \
            --route "${ROUTE}" \
            --query "${REQUEST_TEXT}" \
            --best-effort \
            --all-routes \
            --limit all \
            --output-file "${RUBRICS_CONTEXT_FILE}" > /dev/null

      - name: Resolve rubrics review preflight
        id: preflight
        env:
          RUBRICS_READ_ENABLED: ${{ steps.rubrics_mode.outputs.read_enabled }}
          RUBRICS_AVAILABLE: ${{ steps.preflight_rubrics.outputs.rubrics_available }}
          RUBRICS_REF: ${{ steps.preflight_rubrics.outputs.rubrics_ref || inputs.rubrics_ref || vars.AGENT_RUBRICS_REF || 'agent/rubrics' }}
          SELECTED_COUNT: ${{ steps.preflight_select.outputs.selected_count }}
          RUBRIC_ERROR_COUNT: ${{ steps.preflight_select.outputs.rubric_error_count }}
        run: |
          set -euo pipefail

          selected_count="${SELECTED_COUNT:-0}"
          rubric_error_count="${RUBRIC_ERROR_COUNT:-0}"
          should_run=false
          skip_reason=""

          if [ "${RUBRICS_READ_ENABLED}" != "true" ]; then
            skip_reason="Rubric reads are disabled for rubrics-review by AGENT_RUBRICS_POLICY."
          elif [ "${RUBRICS_AVAILABLE}" != "true" ]; then
            skip_reason="Rubrics branch ${RUBRICS_REF} is not available."
          elif [ "${selected_count}" = "0" ]; then
            skip_reason="No active rubrics are available to score."
          else
            should_run=true
            skip_reason="Selected ${selected_count} active rubrics for scoring."
          fi

          echo "should_run=${should_run}" >> "$GITHUB_OUTPUT"
          echo "selected_count=${selected_count}" >> "$GITHUB_OUTPUT"
          echo "rubric_error_count=${rubric_error_count}" >> "$GITHUB_OUTPUT"
          echo "skip_reason" 

@sepo-agent /answer why did you add all these here?

@sepo-agent-app

sepo-agent-app Bot commented Jun 7, 2026

Copy link
Copy Markdown
Author

That block was added to make agent-rubrics-review.yml decide before the model/provider path whether a rubrics review is worth running.

The reason is issue #410: when agent/rubrics is missing, or when there are zero active rubrics, Sepo was still doing the expensive path: resolve provider, set up runtime/provider tools, run the rubrics-review agent, then post an N/A / 0 rubrics style comment. That was noisy and wasted a model run.

So this block does three preflight checks:

  • Download rubrics for review preflight: checks whether the rubrics branch exists, without bootstrapping it.
  • Count active rubrics for review: uses the existing rubric selector to see whether anything would actually be scored.
  • Resolve rubrics review preflight: turns that into should_run=true/false plus a human-readable skip reason.

If should_run=false, the workflow only writes an Actions notice/summary and skips provider setup, model execution, artifacts, and PR comments.

The reason it is in this workflow instead of only inside run-agent-task is timing: run-agent-task happens after provider/runtime setup, which is exactly the work this PR is trying to avoid when there are no rubrics to score.


codex | gpt-5.5 | high | Macmini-runner-4

@lolipopshock

Copy link
Copy Markdown
Contributor

That block was added to make agent-rubrics-review.yml decide before the model/provider path whether a rubrics review is worth running.

The reason is issue #410: when agent/rubrics is missing, or when there are zero active rubrics, Sepo was still doing the expensive path: resolve provider, set up runtime/provider tools, run the rubrics-review agent, then post an N/A / 0 rubrics style comment. That was noisy and wasted a model run.

So this block does three preflight checks:

  • Download rubrics for review preflight: checks whether the rubrics branch exists, without bootstrapping it.
  • Count active rubrics for review: uses the existing rubric selector to see whether anything would actually be scored.
  • Resolve rubrics review preflight: turns that into should_run=true/false plus a human-readable skip reason.

If should_run=false, the workflow only writes an Actions notice/summary and skips provider setup, model execution, artifacts, and PR comments.

The reason it is in this workflow instead of only inside run-agent-task is timing: run-agent-task happens after provider/runtime setup, which is exactly the work this PR is trying to avoid when there are no rubrics to score.

codex | gpt-5.5 | high | Macmini-runner-4

@sepo-agent /fix-pr let's simplify this:

  1. for the gate, let's just skip if there's no rubrics branch
  2. and then later in the code, if it fetches rubrics and then we can configure to skip the agent runs with 0 rubrics.
  3. also the if: ${{ steps.preflight.outputs.should_run == 'true' && steps.review.outputs.response_file != '' }} is so not elegant -- can we just do a two step action based gating ?

@sepo-agent-app

This comment has been minimized.

@lolipopshock

Copy link
Copy Markdown
Contributor

@sepo-agent /fix-pr resolve merge conflicts

@sepo-agent-app

Copy link
Copy Markdown
Author

Sepo pushed fixes for this PR. Branch: agent/implement-issue-410/codex-26977545719. Requested by @lolipopshock.

Resolved the merge conflict against local origin/main.

The only real content conflict was in .agent/src/__tests__/envelope.test.ts; I kept both sides: the PR’s rubrics-review gate assertions and main’s newer add-rubrics prompt assertions. The merge is staged with no unmerged paths left, but not committed.

Verification passed:

  • cd .agent && npm run build
  • cd .agent && npm run check:workflow
  • git diff --check

codex | gpt-5.5 | xhigh | Macmini-runner-6

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

Labels

agent Handled by the agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Skip rubrics review if there are no rubrics / rubrics branch is not present?

2 participants