OSAC-959: Add internal-proposals/ directory & prd/design docs for agentic-sdlc measurement, harden docs routing - #160
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tchughesiv The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughAdds internal proposal routing and review guidance, introduces the OSAC-959 agentic SDLC measurement PRD/design, and adds tested naming validation enforced by GitHub Actions. ChangesInternal proposals
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant GitHubActions
participant Git
participant NamingChecker
PullRequest->>GitHubActions: trigger for internal-proposals changes
GitHubActions->>Git: resolve base SHA and changed paths
GitHubActions->>NamingChecker: run tests and validate paths
NamingChecker->>Git: inspect base and live refs
NamingChecker-->>GitHubActions: return validation status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
tchughesiv
changed the title
|
@tchughesiv: This pull request references OSAC-959 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the feature to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
tchughesiv
changed the title
tchughesiv
changed the title
tchughesiv
changed the title
39882e0 to
cf30c95
Compare
…ring docs
Gives internal engineering process/tooling work (no tenant-facing surface,
no CRD/API footprint) a durable, reviewed home of its own inside
osac-workspace, distinct from enhancement-proposals (scoped to OSAC
product/tenant-facing features). Not scoped to any one initiative --
OSAC-959 (agentic-SDLC measurement) is simply the first occupant and the
ticket whose ambiguity surfaced the need for it.
- internal-proposals/README.md: location decision, naming convention,
review process, and a rubric-calibration caveat for prd-review/
design-review, validated by self-running both skills against a real
OSAC feature PRD/design and against the OSAC-959 PRD/design here --
design-review scored the internal design 8/8 (N/As documented, not
penalized); prd-review failed the internal PRD 6/10, driven mostly by
genuinely fixable writing gaps rather than the WHAT=0 auto-fail clause
(that clause is about content-only deliverables, not internal-vs-
tenant-facing scope, and did not fire)
- internal-proposals/OSAC-959-agentic-sdlc-measurement/{prd,design}.md:
migrated from a personal enhancement-proposals fork branch that was
never opened as a PR there; prd.md additionally revised post-review to
drop internal codenames/personal-fork paths, split out a separable
cost-telemetry capability, and sharpen scope bullets into checkable
outcomes
- AGENTS.md: 'Internal-Only Proposals' section pointing to
internal-proposals/README.md, a mechanical Jira-label routing signal
(osac-agentic-sdlc) for /prd:ingest and /design:ingest, and a
publish-time override skipping the enhancement-proposals-scoped
docs-repo/config.json flow in favor of a same-repo commit
- tools/check_internal_proposals_naming.py (+ 27 unit tests) and
.github/workflows/internal-proposals-naming.yml: naming validation CI
enforcing OSAC-<key>-<slug>/{prd,design}.md, adapted from
enhancement-proposals' check_ep_naming.py, with grandfathering for
pre-existing paths
- skills/prd-review/SKILL.md, skills/design-review/SKILL.md: add
internal-proposals/<slug>/{prd,design}.md as an example input path,
pointing to the rubric caveat above -- no rubric logic changed
Assisted-by: Claude Code <noreply@anthropic.com>
Signed-off-by: Tommy Hughes <tohughes@redhat.com>
cf30c95 to
8c8a986
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/internal-proposals-naming.yml:
- Around line 20-21: Update the workflow’s top-level permissions configuration
to add a restrictive fallback, while preserving the existing job-level contents:
read scope. Ensure future jobs inherit minimal permissions unless they
explicitly declare their own narrower requirements.
In `@AGENTS.md`:
- Around line 180-183: Update the Git workflow instructions in AGENTS.md at
lines 180-183 and internal-proposals/README.md at lines 55-57 to require the
Assisted-by: Claude Code <noreply@anthropic.com> trailer on AI-assisted commits
and prohibit AI Co-Authored-By trailers; keep both duplicated workflow
descriptions consistent.
In `@internal-proposals/OSAC-959-agentic-sdlc-measurement/design.md`:
- Around line 169-171: Update the Security Considerations section describing
qualitative_finding_quality to remove the claim that no new exposure is
introduced. Specify classification and redaction requirements for
reference-review.md, document provider retention and access guarantees, and
explicitly exclude content not approved for external LLM processing before
characterizing the resulting exposure profile.
- Around line 129-140: Document the pinned qualitative_finding_quality judge’s
output scale and how its LLM responses are parsed and mapped to annotations
before applying the min_mean: 3.5 threshold. Clarify the alignment used for
annotation comparison and Cohen’s κ, while keeping the existing deterministic
judge definitions and thresholds unchanged.
In `@internal-proposals/OSAC-959-agentic-sdlc-measurement/prd.md`:
- Around line 49-50: Update the phased validation completion criteria in the
referenced PRD section to require a golden dataset and reproducible judge/human
agreement report only for Phases 1–2. Define phase-appropriate acceptance
criteria for Phases 3–4 based on their operational metrics, Org Pulse trends,
and reporting deliverables, avoiding a universal calibration requirement.
In `@internal-proposals/README.md`:
- Around line 45-48: Update the Markdown code block around the
internal-proposals path examples in README.md to include the text language
identifier, preserving the example contents and formatting so Markdownlint rule
MD040 passes.
In `@tools/check_internal_proposals_naming.py`:
- Around line 87-94: Return the combined grandfathering condition directly from
is_grandfathered, preserving the existing checks that require a non-None
base_sha or live_base_ref before calling path_exists_at_ref.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e83ee9d6-f555-4f46-947b-fb29b7b4fe3f
📒 Files selected for processing (9)
.github/workflows/internal-proposals-naming.ymlAGENTS.mdinternal-proposals/OSAC-959-agentic-sdlc-measurement/design.mdinternal-proposals/OSAC-959-agentic-sdlc-measurement/prd.mdinternal-proposals/README.mdskills/design-review/SKILL.mdskills/prd-review/SKILL.mdtools/check_internal_proposals_naming.pytools/test_check_internal_proposals_naming.py
| permissions: | ||
| contents: read |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
Optional: add a top-level permissions: fallback.
The job already scopes contents: read, which is correctly minimal. Static analysis still flags the workflow for lacking a top-level permissions: block — adding one as defense-in-depth guards against a future job being added here without its own explicit scope (which would otherwise inherit the broader default token permissions).
🔒 Proposed defense-in-depth addition
concurrency:
group: internal-proposals-naming-${{ github.event.pull_request.number }}
cancel-in-progress: true
+
+permissions:
+ contents: read📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| permissions: | |
| contents: read | |
| permissions: | |
| contents: read | |
| permissions: | |
| contents: read |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/internal-proposals-naming.yml around lines 20 - 21, Update
the workflow’s top-level permissions configuration to add a restrictive
fallback, while preserving the existing job-level contents: read scope. Ensure
future jobs inherit minimal permissions unless they explicitly declare their own
narrower requirements.
Source: Linters/SAST tools
| 2. Follow `osac-workspace`'s own [Git Workflow](#git-workflow) instead of the | ||
| skill's docs-repo git steps: `git commit -s`, push to this repo's `fork` | ||
| remote, open the PR from `fork/<branch>` against `osac-workspace`'s | ||
| `origin/main` — not the `enhancement-proposals` repo's fork/origin (the |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Keep duplicated Git workflow instructions compliant with AI-attribution policy.
Both workflow descriptions require signed commits but omit the required AI-assisted commit metadata.
AGENTS.md#L180-L183: document theAssisted-by: Claude Code <noreply@anthropic.com>trailer and prohibit AICo-Authored-Bytrailers.internal-proposals/README.md#L55-L57: mirror the same requirement so the README does not contradict the root workflow.
As per coding guidelines, AI-assisted commits must include a Red Hat Assisted-by or Generated-by trailer.
📍 Affects 2 files
AGENTS.md#L180-L183(this comment)internal-proposals/README.md#L55-L57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` around lines 180 - 183, Update the Git workflow instructions in
AGENTS.md at lines 180-183 and internal-proposals/README.md at lines 55-57 to
require the Assisted-by: Claude Code <noreply@anthropic.com> trailer on
AI-assisted commits and prohibit AI Co-Authored-By trailers; keep both
duplicated workflow descriptions consistent.
Source: Coding guidelines
| **Scoring model.** `prd-review`: 0–2 per dimension, `/10` total, PASS ≥7 with no zero on any dimension. `design-review`: 0–2 per dimension, `/8` total, PASS ≥5 with no zero on any dimension. Primary scoring is harness-native judges declared in `eval-prd-review.yaml`/`eval-design-review.yaml`: | ||
|
|
||
| ```yaml | ||
| judges: | ||
| - name: rubric_scoring # deterministic check judge; regex-parses the skill's own rubric table | ||
| - name: critical_findings_recall # deterministic check judge; fuzzy-matches annotated critical findings | ||
| - name: qualitative_finding_quality # LLM prompt judge; only judge affected by model-family choice | ||
| thresholds: | ||
| rubric_scoring: { min_pass_rate: 1.0 } | ||
| critical_findings_recall: { min_pass_rate: 1.0 } | ||
| qualitative_finding_quality: { min_mean: 3.5 } | ||
| ``` |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg 'internal-proposals/OSAC-959-agentic-sdlc-measurement/design.md|CLAUDE.md|AGENTS.md' || true
echo "== relevant design excerpts =="
file="internal-proposals/OSAC-959-agentic-sdlc-measurement/design.md"
if [ -f "$file" ]; then
wc -l "$file"
sed -n '1,230p' "$file" | cat -n
fi
echo "== search related files =="
git ls-files | rg 'eval-(prd|design)-review|kubernetes|eval|judge|scoring|rubric|criteria|metrics|measurement' || trueRepository: osac-project/osac-workspace
Length of output: 38710
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== eval config files =="
for f in evals/review/eval-prd-review.yaml evals/review/eval-design-review.yaml evals/review/harness.lock; do
if [ -f "$f" ]; then
echo "--- $f ($(wc -l < "$f") lines) ---"
cat -n "$f"
fi
done
echo "== docs around eval runner/judges =="
for f in evals/review/README.md evals/review/docs/case-schema.md evals/review/docs/measurement-taxonomy.md internal-proposals/OSAC-959-agentic-sdlc-measurement/prd.md; do
if [ -f "$f" ]; then
echo "--- $f ($(wc -l < "$f") lines) ---"
rg -n "qualitative_finding_quality|Rubert|score|mean|threshold|min_mean|judge|annotations|Cohen|κ" "$f" -C 3 || true
fi
done
echo "== all repo references to qualitative anchor =="
rg -n "qualitative_finding_quality|min_mean: 3\.5|Rubert|Rubert|Cohen|minimum mean|min_mean|score scale|annotations.yaml" . -S || trueRepository: osac-project/osac-workspace
Length of output: 28381
🌐 Web query:
opendatahub-io agent-eval-harness v1.22.0 qualitative finding quality min_mean 3.5 score.py Rubert score scale
💡 Result:
In the opendatahub-io/agent-eval-harness framework (current as of v1.22.0), the min_mean parameter is a configuration setting used in the eval.yaml file to define regression thresholds for numeric LLM-based judges [1][2]. Specifically, min_mean: 3.5 sets a requirement that the average score for a designated quality metric must be at least 3.5 to pass the regression gate [1][3]. The harness utilizes a scoring system where LLM judges provide numeric scores [4][2]. While the framework's official documentation and templates do not strictly define a global scale for all judges, the frequent use of min_mean: 3.5 in examples implies a common 1–5 scale (often used in Likert-style qualitative evaluations) [1][3][4]. Users configure these thresholds under the thresholds section of their eval.yaml to detect regressions across evaluation runs [1][2]. Key technical points regarding this configuration: - Purpose: min_mean is specifically designed for numeric judges, such as LLM-as-a-judge, to establish an average performance floor [2]. - Regression Gating: If the calculated mean score for a specified judge falls below the min_mean value, the evaluation run can be flagged as a failure, preventing regressions [1][2]. - Other Thresholds: The framework also supports min_pass_rate (for boolean/deterministic checks) and min_win_rate (for pairwise comparisons) as alternative or complementary gating mechanisms [1][2]. The score.py file mentioned is a core component of the harness responsible for executing these scoring/judging logic and aggregating results, which are then compared against the thresholds defined in the configuration [5][1].
Citations:
- 1: https://github.com/opendatahub-io/agent-eval-harness/blob/main/README.md
- 2: Investigate opendatahub-io/agent-eval-harness for skill evals fullsend-ai/fullsend#499
- 3: https://github.com/EmilienM/agent-eval-harness
- 4: https://github.com/opendatahub-io/agent-eval-harness/blob/main/skills/eval-analyze/references/eval-yaml-template.md
- 5: https://github.com/opendatahub-io/agent-eval-harness/blob/main/skills/eval-run/SKILL.md
🌐 Web query:
"opendatahub-io/agent-eval-harness" "v1.22.0" "score.py" "Rubert" "qualitative"
💡 Result:
In the opendatahub-io/agent-eval-harness framework, score.py is the primary scoring CLI tool used to execute evaluation judges and generate results [1][2]. It processes evaluation runs by loading test case outputs and applying judges configured in an eval.yaml file [1][3]. Regarding your specific query: 1. Rubert: There is no native or explicit reference to a Rubert model or algorithm within the core score.py scoring logic of the opendatahub-io/agent-eval-harness repository [1][4][3]. The framework is designed to be model-agnostic, allowing users to configure various LLM judges via prompt files or inline definitions [4][5]. 2. Qualitative Assessment: Qualitative scoring is a core function of the framework [4][5]. This is typically handled through LLM-as-a-judge configurations where a model is prompted to provide both a quantitative score (e.g., 1–5) and a qualitative rationale [1][3]. The score.py tool manages this by: - Forcing structured output (e.g., via tool calls) to ensure the model returns both a numerical value and a text-based justification [1]. - Supporting user-defined rubrics that instruct the LLM on how to evaluate qualitative aspects such as accuracy, completeness, and adherence to guidelines [4][5]. - Facilitating human review of these outputs, where judge scores and rationales are presented to allow for the collection of qualitative feedback and pattern analysis [4][5]. If you are seeing references to Rubert, it may be a custom modification, a specific judge prompt implementation within your local eval configuration, or an external dependency used in a downstream integration [6]. The framework's modular architecture allows developers to define custom judge modules or prompt files that could integrate specific models or qualitative frameworks [4][3]. [1][4][3]
Citations:
- 1: https://github.com/opendatahub-io/agent-eval-harness/blob/main/skills/eval-run/scripts/score.py
- 2: https://github.com/opendatahub-io/agent-eval-harness/blob/main/skills/eval-run/SKILL.md
- 3: https://github.com/opendatahub-io/agent-eval-harness/blob/main/skills/eval-run/references/data-pipeline.md
- 4: https://github.com/opendatahub-io/agent-eval-harness
- 5: https://github.com/opendatahub-io/agent-eval-harness/blob/main/README.md
- 6: Add vendored agent-eval-harness scoring engine (eval harness, 1/3) redhat-community-ai-tools/code-to-docs#21
🌐 Web query:
"agent-eval-h Harness" "qualitative_finding_quality" "Rubert" "min_mean"
💡 Result:
The terms you mentioned relate to configuration settings within the opendatahub-io/agent-eval-harness framework, which is designed to evaluate AI agents and skills [1][2]. In this framework, qualitative_finding_quality typically refers to a specific metric or evaluator category used to assess the quality of outputs or qualitative findings generated by an agent [1][2]. The configuration allows for regression detection using defined thresholds [1][2]. Regarding the specific configuration values: - min_mean: This is a threshold setting representing the minimum average score required for a judge or metric to pass regression testing [1][3]. For example, a setting of min_mean: 3.5 implies that the average quality score must be at least 3.5 to avoid triggering a failure in the CI pipeline [1][2]. - Rubert: In the context of NLP, RuBERT is a well-known Russian-language BERT model developed by DeepPavlov [4]. While RuBERT is not a native component of the agent-eval-harness core (which typically relies on LLMs as judges), it is possible that in your specific environment, a fine-tuned version of RuBERT (often used for classification or scoring tasks, such as sentiment or poem evaluation) [5][6] is being utilized as a custom judge or model provider for specific evaluation tasks. Summary of usage in agent-eval-harness: The harness supports various judge types, including LLMs and custom checkers [3]. If you see Rubert referenced alongside min_mean in a config, it indicates that an evaluation is likely being performed by a model (possibly a fine-tuned RuBERT) where its output scores are being aggregated and checked against the min_mean threshold [1][3].
Citations:
- 1: https://github.com/opendatahub-io/agent-eval-harness
- 2: https://github.com/opendatahub-io/agent-eval-harness/blob/main/README.md
- 3: Investigate opendatahub-io/agent-eval-harness for skill evals fullsend-ai/fullsend#499
- 4: https://github.com/RussianNLP/RuSentEval
- 5: https://huggingface.co/numblilbug/rubert-cased-poem-evalutation
- 6: https://huggingface.co/ilyali034/rubert-sentence-similarity
Define the qualitative judge’s output scale and annotation mapping before gating on min_mean: 3.5.
The rubric scores are documented as 0–2, but qualitative_finding_quality uses a 3.5 mean gate without stating whether the judge scores 0–5 or another range, or how the LLM output is parsed and aligned with annotations/Cohen’s κ. Record that scale/mapping for the pinned judge.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal-proposals/OSAC-959-agentic-sdlc-measurement/design.md` around lines
129 - 140, Document the pinned qualitative_finding_quality judge’s output scale
and how its LLM responses are parsed and mapped to annotations before applying
the min_mean: 3.5 threshold. Clarify the alignment used for annotation
comparison and Cohen’s κ, while keeping the existing deterministic judge
definitions and thresholds unchanged.
| ### Security Considerations | ||
|
|
||
| The harness's LLM judge (`qualitative_finding_quality`) sends the EP Review Bot's already-posted PR comment plus the case's human-authored reference review to an external LLM API (`claude-sonnet-4-6` via the configured provider) for scoring. This is a strictly smaller data-exposure footprint than an earlier iteration of this design: there is no second skill-under-test execution sending full PRD/design document content to a second LLM call — only the bot's own already-public PR comment and the golden reference review are sent. This is the same data-exposure profile the EP Review Bot already has in production today for its half of the exchange — no new exposure is introduced. No credentials, secrets, or tenant data pass through this path. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate target =="
fd -a 'design\.md$' . | sed 's#^\./##' | rg "internal-proposals/.*/design\.md|OSAC-959-agentic-sdlc-measurement/design\.md" || true
echo "== target section =="
file="internal-proposals/OSAC-959-agentic-sdlc-measurement/design.md"
if [ -f "$file" ]; then
nl -ba "$file" | sed -n '140,190p'
else
echo "target not found"
fi
echo "== references to reference-review/qualitative_finding_quality/claude-sonnet =="
rg -n "reference-review|qualitative_finding_quality|claude-sonnet|external LLM|LLM judge|Experiencer" .Repository: osac-project/osac-workspace
Length of output: 329
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="internal-proposals/OSAC-959-agentic-sdlc-measurement/design.md"
if [ -f "$file" ]; then
awk '{printf "%5d\t%s\n", NR, $0}' "$file" | sed -n '140,200p'
else
echo "target not found"
fi
echo "== references to reference-review/qualitative_finding_quality/claude-sonnet =="
rg -n "reference-review|qualitative_finding_quality|claude-sonnet|external LLM|LLM judge|Experiencer" .Repository: osac-project/osac-workspace
Length of output: 23664
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== referenced files =="
for f in evals/review/eval-prd-review.yaml evals/review/eval-design-review.yaml evals/review/docs/case-schema.md internal-proposals/OSAC-959-agentic-sdlc-measurement/design.md; do
if [ -f "$f" ]; then
echo "--- $f ---"
awk '{printf "%5d\t%s\n", NR, $0}' "$f" | sed -n '1,120p'
else
echo "missing $f"
fi
done
echo "== find source references to reference-review content =="
rg -n "reference_review|reference-review|load_case_record|quality_finding|finding_quality|prompt|judge" evals internal-proposals/OSAC-959-agg -S || trueRepository: osac-project/osac-workspace
Length of output: 28114
Do not claim that this introduces “no new exposure.”
The qualitative_finding_quality path adds reference-review.md content to the LLM judge input. Define classification/redaction requirements, provider retention and access guarantees, and an explicit exclusion rule for content not approved for external LLM processing before saying the exposure profile is the same.
Suggested wording change
-This is the same data-exposure profile the EP Review Bot already has in production today for its half of the exchange — no new exposure is introduced.
+Reference reviews are additional data sent to the configured external LLM provider. Before scoring, classify and sanitize them, verify provider retention/access controls, and exclude any sensitive content that is not approved for external processing.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ### Security Considerations | |
| The harness's LLM judge (`qualitative_finding_quality`) sends the EP Review Bot's already-posted PR comment plus the case's human-authored reference review to an external LLM API (`claude-sonnet-4-6` via the configured provider) for scoring. This is a strictly smaller data-exposure footprint than an earlier iteration of this design: there is no second skill-under-test execution sending full PRD/design document content to a second LLM call — only the bot's own already-public PR comment and the golden reference review are sent. This is the same data-exposure profile the EP Review Bot already has in production today for its half of the exchange — no new exposure is introduced. No credentials, secrets, or tenant data pass through this path. | |
| ### Security Considerations | |
| The harness's LLM judge (`qualitative_finding_quality`) sends the EP Review Bot's already-posted PR comment plus the case's human-authored reference review to an external LLM API (`claude-sonnet-4-6` via the configured provider) for scoring. This is a strictly smaller data-exposure footprint than an earlier iteration of this design: there is no second skill-under-test execution sending full PRD/design document content to a second LLM call — only the bot's own already-public PR comment and the golden reference review are sent. Reference reviews are additional data sent to the configured external LLM provider. Before scoring, classify and sanitize them, verify provider retention/access controls, and exclude any sensitive content that is not approved for external processing. No credentials, secrets, or tenant data pass through this path. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal-proposals/OSAC-959-agentic-sdlc-measurement/design.md` around lines
169 - 171, Update the Security Considerations section describing
qualitative_finding_quality to remove the claim that no new exposure is
introduced. Specify classification and redaction requirements for
reference-review.md, document provider retention and access guarantees, and
explicitly exclude content not approved for external LLM processing before
characterizing the resulting exposure profile.
| - No numeric target thresholds exist yet for MTTR improvement, RCA accuracy, or velocity change — the framework establishes baselines first; specific targets are not yet defined. [Clarify: Remaining Gaps] | ||
| - End-to-end validation against real use cases proceeds in phases rather than all at once: an initial planning-review golden set, followed later by real bug-fix outcome validation once bug-fix evaluation is integrated — full validation is not available from day one. Each phase is complete when its golden dataset and a baseline judge/human agreement report both exist and are reproducible, giving reviewers a concrete, inspectable checkpoint rather than an open-ended validation effort. [Jira: OSAC-959] |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make phase completion criteria phase-specific.
This assumption requires every phase to produce a golden dataset and judge/human agreement report, but internal-proposals/OSAC-959-agentic-sdlc-measurement/design.md defines those artifacts only for Phases 1-2. Phases 3-4 instead deliver operational metrics, Org Pulse trends, and reports, so the current wording creates an impossible or ambiguous Definition of Done.
Restrict the calibration requirement to Phases 1-2 and define separate acceptance criteria for later phases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal-proposals/OSAC-959-agentic-sdlc-measurement/prd.md` around lines 49
- 50, Update the phased validation completion criteria in the referenced PRD
section to require a golden dataset and reproducible judge/human agreement
report only for Phases 1–2. Define phase-appropriate acceptance criteria for
Phases 3–4 based on their operational metrics, Org Pulse trends, and reporting
deliverables, avoiding a universal calibration requirement.
| ``` | ||
| internal-proposals/<jira-key>-<feature-slug>/prd.md | ||
| internal-proposals/<jira-key>-<feature-slug>/design.md | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the example block.
Markdownlint reports MD040 at Line 45. Use a language such as text so the README passes the documented lint checks.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal-proposals/README.md` around lines 45 - 48, Update the Markdown code
block around the internal-proposals path examples in README.md to include the
text language identifier, preserving the example contents and formatting so
Markdownlint rule MD040 passes.
Source: Linters/SAST tools
| def is_grandfathered( | ||
| path: str, base_sha: str | None, live_base_ref: str | None | ||
| ) -> bool: | ||
| if base_sha is not None and path_exists_at_ref(base_sha, path): | ||
| return True | ||
| if live_base_ref is not None and path_exists_at_ref(live_base_ref, path): | ||
| return True | ||
| return False |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Optional: return the boolean condition directly.
is_grandfathered can be simplified to a single return expression instead of two early return Trues + a fallback.
♻️ Proposed simplification
def is_grandfathered(
path: str, base_sha: str | None, live_base_ref: str | None
) -> bool:
- if base_sha is not None and path_exists_at_ref(base_sha, path):
- return True
- if live_base_ref is not None and path_exists_at_ref(live_base_ref, path):
- return True
- return False
+ return (
+ (base_sha is not None and path_exists_at_ref(base_sha, path))
+ or (live_base_ref is not None and path_exists_at_ref(live_base_ref, path))
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def is_grandfathered( | |
| path: str, base_sha: str | None, live_base_ref: str | None | |
| ) -> bool: | |
| if base_sha is not None and path_exists_at_ref(base_sha, path): | |
| return True | |
| if live_base_ref is not None and path_exists_at_ref(live_base_ref, path): | |
| return True | |
| return False | |
| def is_grandfathered( | |
| path: str, base_sha: str | None, live_base_ref: str | None | |
| ) -> bool: | |
| return ( | |
| (base_sha is not None and path_exists_at_ref(base_sha, path)) | |
| or (live_base_ref is not None and path_exists_at_ref(live_base_ref, path)) | |
| ) |
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 92-94: Return the condition directly
Inline condition
(SIM103)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/check_internal_proposals_naming.py` around lines 87 - 94, Return the
combined grandfathering condition directly from is_grandfathered, preserving the
existing checks that require a non-None base_sha or live_base_ref before calling
path_exists_at_ref.
Source: Linters/SAST tools
|
Closing without merging. This started from a real need — Eran Cohen's Slack request to flag internal-tooling PRs (like OSAC-959, agentic-SDLC measurement) as "not an OSAC feature" before they hit the EP Review Bot's tenant-facing rubric. This PR built out a durable answer: an Validation (running Going a lighter route instead: for ad-hoc internal engineering docs like OSAC-959, skip building workspace-native process around them and just get them reviewed directly (e.g., Google Docs) — no bot, no rubric, no routing logic needed, because the doc never enters the Assisted-by: Claude Code noreply@anthropic.com |
2 participants
Background
OSAC-959 ("Create metrics
and methods to evaluate the success of the agentic-sdlc process over
time") is internal engineering tooling to measure how well AI-agent-driven
development is working for the OSAC team, not an OSAC product/tenant-facing
capability.
This came up in Slack because OSAC-959's PRD/design docs had nowhere to
go:
enhancement-proposalsis scoped to OSAC product features withtenant-facing surface and CRD/API footprint (see that repo's README, "Is
my proposed change an enhancement?"), and its EP Review Bot scores every
PRD/design against a rubric built for exactly that shape of work —
CRD-heavy, tied to one of OSAC's four canonical personas. OSAC-959 has
none of that by design, so submitting it there would mean either scoring
it against criteria that don't apply (most of Architecture is N/A, no
personas affected, no comparable exemplar in the reference library) or
carving out a bot-side exception for it.
Rather than stretch
enhancement-proposals' scope or special-case itsbot, this PR gives internal engineering process/tooling work a durable,
reviewed home of its own — inside
osac-workspace, where the rest ofthis kind of work (
evals/,skills/,.design/context/) alreadylives.
internal-proposals/isn't scoped to agentic-SDLC workspecifically — it's a general home for any internal engineering
proposal with no tenant-facing surface and no CRD/API footprint,
regardless of subject matter. OSAC-959 is simply the first occupant and
the ticket whose ambiguity surfaced the need for it; the
osac-agentic-sdlcJira label check in
AGENTS.mdis one convenience heuristic for oneinitiative's tickets, not the directory's defining scope.
What's in this PR
internal-proposals/— new workspace-native directory for PRD/designdocs that are internal engineering process/tooling, not OSAC product
features. Includes a
README.mdwith the location decision, namingconvention, review process, and a rubric-calibration caveat for
self-reviewing with
prd-review/design-review. Migrates theOSAC-959 PRD and design
docs here from a personal
enhancement-proposalsfork branch that wasnever opened as a PR there.
AGENTS.md— adds an "Internal-Only Proposals" section pointing tointernal-proposals/README.md, plus two hardening additions:/prd:ingest//design:ingest: checkthe Jira issue's
Labels(already captured in01-requirements.md)for
osac-agentic-sdlcas a fast first-pass default — confirmed withthe user, not silently applied, since the label means "part of the
agentic-SDLC initiative," not "definitely non-tenant-facing."
/prd:publish//design:publish's "ResolveDocs Repo" step assumes a separate checked-out docs repo and caches
the answer in
.artifacts/prd/config.json(scoped toenhancement-proposals).internal-proposals/is part of this repo,so that step is skipped in favor of a same-repo commit using this
repo's own remotes.
Naming validation CI —
tools/check_internal_proposals_naming.py(adapted from
enhancement-proposals'check_ep_naming.py), its unittests (27, all passing), and a path-scoped GitHub Actions workflow
(
.github/workflows/internal-proposals-naming.yml) enforcingOSAC-NNNN-slug/{prd,design}.mdnaming on PRs touchinginternal-proposals/, with grandfathering for pre-existing paths.Validation
python3 -m unittest test_check_internal_proposals_naming -v— 27/27 passactionlint .github/workflows/internal-proposals-naming.yml— cleanpython3 -m py_compileon both new scripts — cleannon-compliant directory name (
internal-proposals/bad-name-no-jira-key/),confirmed
check-namingfailed with the expected message, then revertedit — CI correctly returned to green.
Assisted-by: Claude Code noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes
Tests