Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 62 additions & 7 deletions .github/workflows/claude-code-review.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
name: Claude Code Review

# Thin caller: the prompt, severity/approval logic, checkout, and pinned action
# refs all live in the shared ShiplightAI/internal-tools/claude-review action.
# This file only owns the trigger, permissions, and repo-specific review focus
# (GitHub requires the trigger + permissions in the caller, and
# claude-code-action's tamper guard requires the token wired here).
# Self-contained: this repo is public, so its review gate is inlined rather than
# called out to the private ShiplightAI/internal-tools/claude-review action. A
# public repo whose CI depends on a private action cannot be read or reproduced
# by outside contributors, never runs for fork PRs (secrets are withheld there),
# and breaks whenever org-side action access changes. Nothing in the shared
# action was secret — the OAuth token is supplied here, from this repo's secrets
# — so inlining costs only the automatic propagation of shared prompt edits.
#
# The two actions below are the ones the shared action ran internally, at the
# same pinned SHAs. Keep them pinned by SHA.
on:
pull_request:
types: [opened, synchronize]
Expand All @@ -18,10 +23,44 @@ jobs:
issues: read
id-token: write # required: claude-code-action mints its App token via OIDC
steps:
- uses: ShiplightAI/internal-tools/claude-review@v1
# Without a token, claude-code-action skips every step and exits 0 — the gate
# reports success having reviewed nothing. A review that silently passes is
# worse than one that visibly fails, so turn the silence into a red check.
- name: Verify review credentials are present
env:
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
run: |
if [ -z "${CLAUDE_CODE_OAUTH_TOKEN}" ]; then
echo "::error::CLAUDE_CODE_OAUTH_TOKEN is empty or unset for this repository."
echo "claude-code-action no-ops without it, so this gate would report success"
echo "without reviewing the diff. Restore the secret at the repository or"
echo "organization level, then re-run."
exit 1
fi

- name: Checkout repository
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
with:
fetch-depth: 1

- name: Run Claude Code Review
uses: anthropics/claude-code-action@51705da45eecce209d4700538bf8377d5b5fc695 # v1
with:
claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
review-focus: |
# The review (including approvals) is submitted by the Claude GitHub App
# token the action mints via OIDC — a distinct identity from the PR author,
# so it can approve without the self-approval restriction.
prompt: |
REPO: ${{ github.repository }}
PR NUMBER: ${{ github.event.pull_request.number }}

Please review this pull request and provide feedback on:
- Code quality and best practices
- Potential bugs or issues
- Security concerns (secret handling, injection, least privilege)
- Performance considerations
- Test coverage

This repo is a quality oracle: it scores how well *other* projects are
verified. Its credibility rests on scores nobody can fake. Weight these
project invariants:
Expand Down Expand Up @@ -61,3 +100,19 @@ jobs:
- Docs must match reality: `README.md` documents the install path and
the package layout, so a stale skill list, command set, or install
ref there is a real defect, not a nit.

Use the repository's CLAUDE.md for guidance on style and conventions.
Be constructive and specific; cite file and line.

Classify every issue you find by severity: CRITICAL, HIGH, MEDIUM, or LOW.

Then submit your review with your Bash tool, choosing the event by severity:
- If there are NO CRITICAL, NO HIGH and no MEDIUM issues, approve:
gh pr review ${{ github.event.pull_request.number }} --approve --body "<summary>"
- If there is any CRITICAL, HIGH or MEDIUM issue, request changes:
gh pr review ${{ github.event.pull_request.number }} --request-changes --body "<summary>"
- If you cannot confidently classify the severity, comment:
gh pr review ${{ github.event.pull_request.number }} --comment --body "<summary>"

Always include the full severity-labelled findings in the review body.
claude_args: '--allowed-tools "Bash(gh issue view:*),Bash(gh search:*),Bash(gh issue list:*),Bash(gh pr review:*),Bash(gh pr diff:*),Bash(gh pr view:*),Bash(gh pr list:*)"'
4 changes: 2 additions & 2 deletions .quality/evidence/004-quality-scoring/quality-map.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ expectations:
description: >-
The release snapshot computes its metrics from published formulas with
explicit guardrails, and counts how many checks sit in each state — stale,
manual-only, missing, blocked, accepted, deferred. Where no mapping exists
manual-only, missing, accepted, deferred. Where no mapping exists
for a priority level, the figure is labelled unavailable instead of being
approximated.
source_type: "IMPLEMENTATION"
Expand All @@ -305,7 +305,7 @@ expectations:
- id: "metrics-state-counts"
type: "contract"
path: "tests/contract/analytics.contract.test.ts"
test_case: "counts stale, manual-only, missing, blocked, accepted, and deferred contexts"
test_case: "counts stale, manual-only, missing, accepted, and deferred contexts"
contexts: ["local", "pr-ci"]
- id: "metrics-unavailable-label"
type: "contract"
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ details.

![Quality Explorer showing the overview for a project: a quality score of
100/100 alongside separate coverage, evidence confidence, and structure
confidence scores, the gaps and release blockers counts, and the runtime
confidence scores, the feature and gap counts, and the runtime
observation set fetched from a CI workflow run.](docs/assets/quality-explorer-overview.png)

## What Quality does
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ expectations:
# as tolerated risk for this check. An accepted gap stays visible but stops
# counting as an open gap; accepting the category that drives the check's status
# (missing / manual-only / weak) also lifts its quality/coverage score. Accepting
# a state gap (blocked/stale/deferred/unavailable/failing) is count-only. An agent
# a state gap (stale/deferred/unavailable/failing) is count-only. An agent
# may PROPOSE an acceptance but must never accept a risk on the owner's behalf.
accepted_gaps: [] # Optional: subset of [missing, blocked, stale, deferred, manual-only, weak, failing, unavailable]
accepted_gaps: [] # Optional: subset of [missing, stale, deferred, manual-only, weak, failing, unavailable]

# A proof definition is just the test TYPE (a fact about the artifact) at a
# PATH. Evidence confidence is derived downstream from `type` via a
Expand Down
4 changes: 2 additions & 2 deletions agent-skills/quality/references/map-feature/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -213,11 +213,11 @@ confidence, reported beside structure confidence and never substituting for it.)
### `accepted_gaps` — accepted risk (human-gated)

A per-check list of gap **categories** a human has reviewed and accepted as tolerated
risk: a subset of `missing, blocked, stale, deferred, manual-only, weak, failing,
risk: a subset of `missing, stale, deferred, manual-only, weak, failing,
unavailable`. An accepted gap stays visible but stops counting as an **open** gap;
accepting the category that drives the check's status (`missing` / `manual-only` /
`weak`) also lifts its quality/coverage score, while accepting a state category
(`blocked`/`stale`/`deferred`/`unavailable`/`failing`) is count-only. It never
(`stale`/`deferred`/`unavailable`/`failing`) is count-only. It never
changes evidence confidence. Like the gates, it is **human-gated**: the agent may
*propose* "accept this as tolerated risk" but must never write `accepted_gaps` for
the owner. Remove the category to un-accept.
Expand Down
5 changes: 0 additions & 5 deletions apps/explorer/src/app/quality-explorer/help/scoring/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,6 @@ const gapTerms: readonly GlossaryItem[] = [
definition:
"A gap where some evidence exists, but the proof is not automated enough, not gated, or not complete enough."
},
{
term: "Release blocker",
definition:
"A gap serious enough that the current quality model treats it as blocking release readiness."
},
{
term: "Copy fix prompt",
definition:
Expand Down
2 changes: 1 addition & 1 deletion docs/concepts/glossary.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ aggregate score.

Open gaps use these categories:

`missing`, `blocked`, `stale`, `deferred`, `manual-only`, `weak`, `failing`, and
`missing`, `stale`, `deferred`, `manual-only`, `weak`, `failing`, and
`unavailable`.

The distinction matters:
Expand Down
4 changes: 2 additions & 2 deletions docs/how-to/accept-a-known-gap.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ rationale that the file cannot store.
| Accepted category | Effect |
| --- | --- |
| `missing`, `manual-only`, or `weak` | The gap stops counting as open, and its structural coverage/static-quality penalty is removed. |
| `blocked`, `stale`, `deferred`, `failing`, or `unavailable` | The gap stops counting as open; the scores do not change. |
| `stale`, `deferred`, `failing`, or `unavailable` | The gap stops counting as open; the scores do not change. |

In every case:

Expand Down Expand Up @@ -81,5 +81,5 @@ whether proof investment is being postponed indefinitely, or whether accepted
decisions need an expiry process outside Quality.

**The score did not change.** That is expected for state categories such as
`blocked` or `failing`. Acceptance changes open-gap reporting, not the observed
`stale` or `failing`. Acceptance changes open-gap reporting, not the observed
software result.
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@shiplightai/quality-core",
"version": "0.2.0",
"version": "0.3.0",
"type": "module",
"description": "Deterministic analysis engine for evidence-backed software quality maps.",
"license": "MIT",
Expand Down
11 changes: 4 additions & 7 deletions packages/core/src/analytics/compute-release-snapshot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,16 +156,14 @@ function structuredMetrics(input: {
availability: highPriorityAvailability,
guardrail: "Only explicit gate-context evidence is counted as gated."
}),
...(["stale", "manual-only", "missing", "blocked"] as const).map((category) =>
...(["stale", "manual-only", "missing"] as const).map((category) =>
metric({
metricId:
category === "stale"
? "stale-evidence"
: category === "manual-only"
? "manual-only-exposure"
: category === "missing"
? "missing-evidence"
: "blocked-gaps",
: "missing-evidence",
numerator: gapRecords.filter((record) => record.category === category).length,
denominator: gapRecords.length,
records: gapRecords.filter((record) => record.category === category).map((record) =>
Expand Down Expand Up @@ -204,7 +202,7 @@ export function buildAnalyticsView(input: BuildAnalyticsInput): AnalyticsView {
summary: missingSummary(input.targetId),
filters: input.filters ?? {},
metrics: [],
riskSummary: { blockers: [], acceptedRisks: [], deferredRisks: [] },
riskSummary: { acceptedRisks: [], deferredRisks: [] },
baselineComparison: buildBaselineComparison({ currentSnapshotId: input.targetId, records: [] }),
guardrails: ["Release analytics require a selected feature."],
missingSelection: {
Expand All @@ -223,7 +221,7 @@ export function buildAnalyticsView(input: BuildAnalyticsInput): AnalyticsView {
summary: missingSummary(input.targetId),
filters: input.filters ?? {},
metrics: [],
riskSummary: { blockers: [], acceptedRisks: [], deferredRisks: [] },
riskSummary: { acceptedRisks: [], deferredRisks: [] },
baselineComparison: buildBaselineComparison({ currentSnapshotId: input.targetId, records: [] }),
guardrails: ["Selected feature is unavailable."],
missingSelection: {
Expand Down Expand Up @@ -282,7 +280,6 @@ export function buildAnalyticsView(input: BuildAnalyticsInput): AnalyticsView {
metrics,
filteredMetric,
riskSummary: {
blockers: metrics.find((item) => item.metricId === "blocked-gaps")?.drilldownRecords ?? [],
acceptedRisks: metrics.find((item) => item.metricId === "accepted-risks")?.drilldownRecords ?? [],
deferredRisks: metrics.find((item) => item.metricId === "deferred-risks")?.drilldownRecords ?? []
},
Expand Down
10 changes: 0 additions & 10 deletions packages/core/src/analytics/metric-definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,16 +51,6 @@ export const metricDefinitions = [
excludedCriteria: "Partial evidence is not counted as missing.",
limitations: ["Missing source fields are labeled unavailable."]
},
{
metricId: "blocked-gaps",
title: "Blocked Gaps",
formulaName: "blocked-gap-count",
numeratorDefinition: "Blocked gap records",
denominatorDefinition: "All source-backed gap records",
includedCriteria: "Gap category must be blocked.",
excludedCriteria: "Deferred-only gaps without blocked context are excluded.",
limitations: ["Blocker reasons are preserved from source text."]
},
{
metricId: "accepted-risks",
title: "Accepted Impact",
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/analytics/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ export interface MetricResult {
}

export interface ReleaseRiskSummary {
readonly blockers: readonly MetricDrilldownRecord[];
readonly acceptedRisks: readonly MetricDrilldownRecord[];
readonly deferredRisks: readonly MetricDrilldownRecord[];
}
Expand Down
9 changes: 0 additions & 9 deletions packages/core/src/gap-triage/classify-gaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ import type {

const categoryLabels: Record<GapCategory, string> = {
missing: "Missing evidence",
blocked: "Blocked evidence",
stale: "Stale evidence",
deferred: "Deferred evidence",
"manual-only": "Manual-only evidence",
Expand All @@ -39,7 +38,6 @@ const categoryLabels: Record<GapCategory, string> = {

export const gapCategoryOrder: readonly GapCategory[] = [
"missing",
"blocked",
"stale",
"deferred",
"manual-only",
Expand Down Expand Up @@ -138,10 +136,6 @@ function categoriesFor(input: {
categories.add("missing");
}

if (includesStandaloneAny(evidenceText, ["blocked"]) || input.assessmentStatus.toLowerCase() === "blocked") {
categories.add("blocked");
}

if (includesStandaloneAny(evidenceText, ["deferred"])) {
categories.add("deferred");
}
Expand Down Expand Up @@ -264,9 +258,6 @@ function fallbackCategories(row: FallbackCoverageRow | undefined, hasEvidence: b
if (!hasEvidence) {
categories.add("missing");
}
if (includesStandaloneTerm(text, "blocked")) {
categories.add("blocked");
}
if (includesStandaloneTerm(text, "deferred")) {
categories.add("deferred");
}
Expand Down
19 changes: 13 additions & 6 deletions packages/core/src/owner-view/risk-summary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ export function riskBadgeFor(input: {
}): OwnerRiskBadge {
const status = input.status?.toUpperCase() ?? "";

if (status.includes("BLOCK")) {
return "Blocked";
}

if (status.includes("FAIL")) {
// Markdown-fallback rows carry the Result cell verbatim from the scanned project,
// so any stated-but-not-passing result arrives here as a producer claim. "BLOCK"
// is still such a claim even though the blocked gap category is gone — it maps to
// the surviving Gap vocabulary rather than being dropped.
if (status.includes("FAIL") || status.includes("BLOCK")) {
return "Gap";
}

Expand All @@ -24,7 +24,14 @@ export function riskBadgeFor(input: {
return "Missing";
}

if (status.includes("COVER") || status.includes("PASS") || input.evidenceConfidence?.toUpperCase() === "HIGH") {
if (status.includes("COVER") || status.includes("PASS")) {
return "Covered";
}

// Confidence may only settle a row that states no result of its own. The scanned
// project controls this text, so a high-confidence claim must never promote an
// unrecognized status to Covered.
if (status.length === 0 && input.evidenceConfidence?.toUpperCase() === "HIGH") {
return "Covered";
}

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/owner-view/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { IndexSourceClassification, IndexSourceReference } from "../project-index/types";

export type OwnerViewState = "ready" | "missingTarget" | "directOpen";
export type OwnerRiskBadge = "Covered" | "Partial" | "Gap" | "Blocked" | "Missing" | "Unknown";
export type OwnerRiskBadge = "Covered" | "Partial" | "Gap" | "Missing" | "Unknown";

export interface OwnerTargetSummary {
readonly targetId: string;
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/quality-structure/assessment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export function deriveExpectationAssessment(
// Only the three EVIDENCE-STRENGTH categories drive `status` (and therefore the
// score): no evidence → "missing", manual-only → "manual-only", otherwise "weak"
// (IMPLICIT/PARTIAL). `status` is never a state category, so accepting one of the
// state/text categories (blocked/stale/deferred/unavailable, and fallback-only
// state/text categories (stale/deferred/unavailable, and fallback-only
// "failing") is intentionally count-only — those never lower the score, so there
// is nothing to lift here. This penalty-category mapping mirrors the status ladder
// above and the categories gap-triage `categoriesFor` emits for the same status.
Expand Down
Loading
Loading