Skip to content

fix: ground adapter reviews in traceable evidence - #48

Open
audreina17 wants to merge 1 commit into
harbor-framework:mainfrom
audreina17:fix/adapter-review-evidence-20261023
Open

audreina17 wants to merge 1 commit into
harbor-framework:mainfrom
audreina17:fix/adapter-review-evidence-20261023

Conversation

@audreina17

Copy link
Copy Markdown
Contributor

The Claude review on PR #36 found useful defects but also declared generation logic sound while upstream and generated-task evidence remained unavailable. Require reviews to cover Harbor standards, upstream fidelity and version traceability, and implementation correctness from generation through agent execution and grading.

Require exact source locations, explicit evidence levels, and bounded conclusions. Group findings by root cause and report coverage and remaining gaps in a compact table rather than repeating the full checklist. Missing evidence remains unverified. Existing acceptance requirements, trusted-base guidance, pinned checkout, read-only tools, and freshness guards are preserved; no metadata schema or acceptance threshold is added.

Validation: all 11 existing contract tests pass; YAML parses and diff checks pass. An offline comparison against the recorded PR #36 review covers missing pins, persona/checks handoffs, reward integrity, asymmetric or one-run parity, unsupported positive conclusions, and unavailable history. No prompt-testing framework or additional tests are added. Actual model behavior still needs live validation after this PR and the structural-report fixes are merged; no contributor review was triggered. Devin is outside this change.

Now, as an adapter review bot, go through every check item below. For each item, determine if it passes or fails.

IMPORTANT: If there is a previous bot review comment on this PR, do NOT rely on its conclusions. Review the code from scratch with fresh eyes following the format defined below. However, DO check whether any bugs or issues flagged in the previous review have been fixed — explicitly verify each one and call out whether it is now resolved or still present.
## Evidence and report contract

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.

This block duplicates the guidance this PR also adds to skills/review-adapter/SKILL.md, and the workflow already injects that file as <Trusted Review Guidance> (the "Load trusted review guidance" step). So the model receives roughly the same ~50 lines twice, and the two copies will drift.

They have already diverged slightly: the evidence labels in SKILL.md (L116) include "independently reproduced behavior", while the list here (L293) omits it and says the workflow cannot reproduce execution.

Suggestion: keep the evidence/report contract in SKILL.md as the single source of truth, and reduce this block to a short pointer plus the workflow-specific constraints: read-only/no execution, the blob URL format for citations at the reviewed SHA, and the history-unavailable note.

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