Skip to content

Commit bdc1283

Browse files
authored
fix(advisor): tighten finding ledger commits (#6592)
<!-- markdownlint-disable MD041 --> ## Summary Replaces the PR Review Advisor's heterogeneous operation-union ledger tool with a flat, stage-aware commit contract that is easier for both model lanes to emit and repair. Candidate findings now require a concrete location and a typed observed-versus-expected mismatch, while process metadata and positive observations are kept in visible stage receipts rather than canonical findings. ## Changes - Replace nested discriminator operations with homogeneous additions, updates, resolutions, and supersessions arrays plus an explicit no-change reason; legacy nested and stringified shapes are rejected. - Restrict additions by analysis stage and basis kind, reserve transitions for reconciliation, require non-null locations, and reject identical observed/expected states. - Tighten prompts so required-job execution, prior-advisor state, overlap/merge coordination, and positive observations remain non-finding receipt material. - Add contract, atomicity, eligibility, stage-policy, prompt, and false-rejection regression coverage; document the maintainer-facing protocol. ## Type of Change - [x] Code change (feature, bug fix, or refactor) - [ ] Code change with doc updates - [ ] Doc only (prose changes, no code sample modifications) - [ ] Doc only (includes code sample changes) ## Quality Gates <!-- Check exactly one tests line and one docs line. Check other lines when applicable. Add every requested justification or approval reference. --> - [x] Tests added or updated for changed behavior - [ ] Existing tests cover changed behavior — justification: - [ ] Tests not applicable — justification: - [ ] Docs updated for user-facing behavior changes - [x] Docs not applicable — justification: this changes the internal advisor ledger protocol; `tools/pr-review-advisor/README.md` is the appropriate maintainer-facing documentation surface. - [ ] Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging) - [ ] Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: - [ ] Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue: ## Verification <!-- Check each applicable item only when supported by the requested evidence. Run targeted tests once per relevant change set and rerun after later edits or hook autofixes that can affect the tested behavior. Do not rerun hook-covered checks. --> - [x] PR description includes the DCO sign-off declaration and every commit appears as `Verified` in GitHub - [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or `npm run check:diff` passed when hooks were skipped or unavailable - [x] Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: `npx vitest run test/*advisor*.test.ts` (10 files, 175 tests passed); `npm run typecheck:cli`, `npm run source-shape:check`, and `npm run test-size:check` passed. - [ ] Applicable broad gate passed — `npm test` for broad runtime/test-harness changes; `npm run check` for repo-wide validation/coverage changes — command/result: not applicable; the change is isolated to the advisor ledger and prompt contract with full advisor-suite coverage. - [x] Quality Gates section completed with required justifications or waivers - [x] No secrets, API keys, or credentials committed - [ ] `npm run docs` builds without warnings (doc changes only) - [ ] Doc pages follow the [style guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md) (doc changes only) - [ ] New doc pages include SPDX header and frontmatter (new pages only) --- <!-- DCO sign-off is required in this PR description, and every commit must appear as Verified in GitHub. Run: git config user.name && git config user.email --> Signed-off-by: Carlos Villela <cvillela@nvidia.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * PR Review Advisor now uses a stricter atomic ledger update flow for review changes, with clearer handling of additions, updates, resolutions, and no-change outcomes. * **Bug Fixes** * Improved review eligibility and validation so only concrete, actionable findings are accepted. * Tightened handling of invalid or mixed review updates to reduce inconsistent review results. * **Documentation** * Updated workflow guidance to reflect the new review commit format and validation expectations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Carlos Villela <cvillela@nvidia.com>
1 parent c1bf898 commit bdc1283

5 files changed

Lines changed: 538 additions & 85 deletions

File tree

0 commit comments

Comments
 (0)