Skip to content

fix: trim semantic review input for broad PRs - #1505

Merged
HanShaoshuai-k merged 1 commit into
mainfrom
fix/semantic-review-input-projection
Jun 17, 2026
Merged

fix: trim semantic review input for broad PRs#1505
HanShaoshuai-k merged 1 commit into
mainfrom
fix/semantic-review-input-projection

Conversation

@HanShaoshuai-k

@HanShaoshuai-k HanShaoshuai-k commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fix semantic review degradation on broad but small PRs by sending only reviewable semantic candidates to the review prompt, instead of passing verbose changed facts such as full output schemas. This keeps the request size bounded while preserving deterministic gate coverage and semantic review behavior.

Changes

  • Limit the semantic input view to changed candidates that the semantic reviewer can actually judge, such as command naming, flag naming, skill command references, and boundary error hints.
  • Stop sending observe-only default_output facts and verbose output field lists into semantic review.
  • Narrow diagnostic context so rule-specific diagnostics only include the relevant fact surface.
  • Add regression coverage for broad command/output surfaces and request-size limits.

Test Plan

  • go test ./internal/qualitygate/semantic ./internal/qualitygate/cmd/semantic-review -count=1
  • go test ./internal/qualitygate/... -count=1
  • git diff --check
  • Verified the broad shortcut repro stays below the semantic request size limit locally.
  • Verified sandbox fork PRs:
    • #11: broad global clean, CI and semantic review passed, no PR comment.
    • #12: broad service clean, CI and semantic review passed, no PR comment.
    • #13: broad shortcut clean repro, CI and semantic review passed, no request-too-large degradation.
    • #14: broad change plus deterministic skill reference defect, deterministic gate and results failed with PR quality summary as expected.

Related Issues

  • None

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved handling of broad change surfaces to avoid including excessive or irrelevant diagnostic details.
    • Strengthened enforcement of request/prompt size limits when building and sending semantic review payloads.
    • Sanitized generated semantic view content to omit verbose output fields and prevent accidental leakage.
  • Tests

    • Expanded coverage for broad changed-surface summarization, stable reference behavior, and ensuring only intended candidates are retained.
    • Added assertions for payload byte limits and stricter diagnostic/field omission rules.

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 629111b8-6624-4f4f-bbd2-1973564651f9

📥 Commits

Reviewing files that changed from the base of the PR and between e96c3a0 and c8ee0a7.

📒 Files selected for processing (5)
  • internal/qualitygate/semantic/client_test.go
  • internal/qualitygate/semantic/prompt.go
  • internal/qualitygate/semantic/prompt_contract_test.go
  • internal/qualitygate/semantic/view.go
  • internal/qualitygate/semantic/view_test.go
🚧 Files skipped from review as they are similar to previous changes (5)
  • internal/qualitygate/semantic/prompt.go
  • internal/qualitygate/semantic/prompt_contract_test.go
  • internal/qualitygate/semantic/client_test.go
  • internal/qualitygate/semantic/view.go
  • internal/qualitygate/semantic/view_test.go

📝 Walkthrough

Walkthrough

The semantic quality gate's input view construction is refactored to select only "review candidate" changed facts using predicate-based filtering instead of including all changed facts. OutputInput serialization switches from embedding facts.OutputFact to explicit fields. Diagnostic inclusion is narrowed: ActionReject always passes; other actions require fact intersection. A system-prompt line clarifies changed_summary scope. Tests are extensively updated and added to verify candidate selection, size limits, and no verbose field leakage.

Changes

Semantic view review-candidate refactor

Layer / File(s) Summary
OutputInput struct and builder
internal/qualitygate/semantic/view.go
OutputInput drops the embedded facts.OutputFact and declares FactRef, Command, Domain, Changed, Source, IsList, HasDefaultLimit, HasDecisionField explicitly; outputInputs() populates each field directly; SkillQualityInput.ref() method is removed.
Review candidate selection logic
internal/qualitygate/semantic/view.go
BuildInputView switches to addChangedReviewCandidates(), which applies per-fact predicates: commands selected on naming/alias conflicts, skills on invalid references, errors on boundary+hint+zero-count, and outputs explicitly rejected.
Diagnostic context refactoring and inclusion gating
internal/qualitygate/semantic/view.go
diagnosticContext refactored to dispatch by diag.Rule using a switch statement, routing to typed helpers for commands, outputs, skills, skill quality, examples, and errors; includeDiagnosticInView always includes ActionReject but otherwise requires fact intersection with diagnostic-derived context.
System prompt changed_summary instruction
internal/qualitygate/semantic/prompt.go
Adds one system-prompt line stating changed_summary may cover a broad surface and reviewers must rely only on explicitly listed facts.
View tests for review candidates, leakage, and size
internal/qualitygate/semantic/view_test.go
Replaces changed-facts test with review-candidate variant; adds broad-surface summarization tests, semantic candidate retention, verbose field omission, output-candidate size bounds, label diagnostic dropping, and helper generators broadChangedFacts/broadOutputCandidateFacts.
Prompt contract and client request-size tests
internal/qualitygate/semantic/prompt_contract_test.go, internal/qualitygate/semantic/client_test.go
Prompt contract test renamed to ...SelectsChangedReviewCandidatesWithStableRefs with SkillQuality/Outputs/Examples assertions updated to expect empty slices; two new client tests assert broad changed surface and broad output candidates stay within MaxRequestBytes with no verbose_output_field_ in HTTP body.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Hoppy refactor day!
I trimmed the verbose fields away,
Only candidates may stay—
The broad surface gets a summary,
Size limits keep requests from going funny.
Clean facts in, clean facts out,
That's what review gates are about!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely and clearly summarizes the main change: trimming semantic review input for broad PRs, which directly aligns with the primary objective of fixing degradation by filtering unnecessary verbose data.
Description check ✅ Passed The PR description follows the template structure with all required sections: a clear Summary explaining the motivation, a detailed Changes list covering the core modifications, a comprehensive Test Plan with verified steps and sandbox testing, and Related Issues. The description is complete and well-organized.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/semantic-review-input-projection

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jun 17, 2026
@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.23404% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.72%. Comparing base (c5b5aec) to head (c8ee0a7).

Files with missing lines Patch % Lines
internal/qualitygate/semantic/view.go 86.95% 5 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1505   +/-   ##
=======================================
  Coverage   73.72%   73.72%           
=======================================
  Files         780      780           
  Lines       74140    74170   +30     
=======================================
+ Hits        54659    54683   +24     
- Misses      15420    15425    +5     
- Partials     4061     4062    +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@c8ee0a71de058ae3c3c93f391ed9dc8cc7dbb13a

🧩 Skill update

npx skills add larksuite/cli#fix/semantic-review-input-projection -y -g

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown

PR Quality Summary

CI did not complete successfully. Use the failed check links below to decide whether this PR needs a code change or a rerun.

Failed checks

@HanShaoshuai-k
HanShaoshuai-k force-pushed the fix/semantic-review-input-projection branch from 585271e to e96c3a0 Compare June 17, 2026 11:35
@HanShaoshuai-k
HanShaoshuai-k force-pushed the fix/semantic-review-input-projection branch from e96c3a0 to c8ee0a7 Compare June 17, 2026 11:55
@HanShaoshuai-k
HanShaoshuai-k merged commit 1f2164c into main Jun 17, 2026
22 checks passed
@HanShaoshuai-k
HanShaoshuai-k deleted the fix/semantic-review-input-projection branch June 17, 2026 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants