fix: trim semantic review input for broad changes

This commit is contained in:
hanshaoshuai
2026-06-17 17:28:32 +08:00
committed by HanShaoshuai-k
parent 76f5419a0d
commit 1f2164c7c2
5 changed files with 355 additions and 47 deletions

View File

@@ -252,6 +252,64 @@ func TestClientRejectsOversizedRequestWithDefaultLimitBeforeHTTP(t *testing.T) {
}
}
func TestClientPostsBroadChangedSurfaceWithinRequestLimit(t *testing.T) {
var calls int
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
calls++
_, _ = w.Write([]byte(`{"choices":[{"message":{"content":"{\"verdict\":\"pass\",\"findings\":[]}"}}]}`))
}))
defer srv.Close()
c := Client{
BaseURL: srv.URL,
APIKey: "test-key",
Model: "semantic-review-v1",
Timeout: time.Second,
MaxRequestBytes: 64 * 1024,
AllowedModels: map[string]bool{"semantic-review-v1": true},
}
if _, err := c.Review(context.Background(), broadChangedFacts(434, 44)); err != nil {
t.Fatalf("Review() broad changed surface error = %v", err)
}
if calls != 1 {
t.Fatalf("server calls = %d, want 1", calls)
}
}
func TestClientPostsBroadOutputCandidatesWithinRequestLimit(t *testing.T) {
var calls int
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
calls++
body, err := io.ReadAll(r.Body)
if err != nil {
t.Fatalf("read request body: %v", err)
}
if len(body) > 64*1024 {
t.Fatalf("request bytes = %d, want <= 65536", len(body))
}
if strings.Contains(string(body), "verbose_output_field_") {
t.Fatalf("request leaked verbose output fields: %s", string(body))
}
_, _ = w.Write([]byte(`{"choices":[{"message":{"content":"{\"verdict\":\"pass\",\"findings\":[]}"}}]}`))
}))
defer srv.Close()
c := Client{
BaseURL: srv.URL,
APIKey: "test-key",
Model: "semantic-review-v1",
Timeout: time.Second,
MaxRequestBytes: 64 * 1024,
AllowedModels: map[string]bool{"semantic-review-v1": true},
}
if _, err := c.Review(context.Background(), broadOutputCandidateFacts(40)); err != nil {
t.Fatalf("Review() broad output candidates error = %v", err)
}
if calls != 1 {
t.Fatalf("server calls = %d, want 1", calls)
}
}
func TestClientFallsBackToJSONObjectWhenJSONSchemaIsRejected(t *testing.T) {
var formats []string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

View File

@@ -22,6 +22,7 @@ func BuildPrompt(f facts.Facts) []Message {
{Role: "system", Content: strings.Join([]string{
"You review a projected lark-cli quality-gate semantic input view.",
"Use only the provided JSON view.",
"The changed_summary may summarize broad changed surfaces; review only listed facts, not omitted summarized items.",
"Use fact_ref values exactly when writing finding evidence.",
"Only facts.commands, facts.skills, facts.errors, and facts.outputs fact_ref values may be blocker evidence.",
"Evidence entries must be exact fact_ref strings such as \"facts.commands[0]\" with no explanations, labels, or suffix text.",

View File

@@ -43,7 +43,7 @@ func TestBuildPromptContainsSemanticReviewContract(t *testing.T) {
}
}
func TestBuildInputViewSelectsChangedFactsWithStableRefs(t *testing.T) {
func TestBuildInputViewSelectsChangedReviewCandidatesWithStableRefs(t *testing.T) {
view := BuildInputView(facts.Facts{
SchemaVersion: 1,
Commands: []facts.CommandFact{
@@ -78,17 +78,17 @@ func TestBuildInputViewSelectsChangedFactsWithStableRefs(t *testing.T) {
if got := singleRef(t, view.Skills); got != "facts.skills[1]" {
t.Fatalf("skill ref = %q, want facts.skills[1]", got)
}
if got := singleRef(t, view.SkillQuality); got != "facts.skill_quality[1]" {
t.Fatalf("skill quality ref = %q, want facts.skill_quality[1]", got)
if len(view.SkillQuality) != 0 {
t.Fatalf("skill quality len = %d, want 0 without diagnostics", len(view.SkillQuality))
}
if got := singleRef(t, view.Errors); got != "facts.errors[1]" {
t.Fatalf("error ref = %q, want facts.errors[1]", got)
}
if got := singleRef(t, view.Outputs); got != "facts.outputs[1]" {
t.Fatalf("output ref = %q, want facts.outputs[1]", got)
if len(view.Outputs) != 0 {
t.Fatalf("outputs len = %d, want 0 without reject diagnostics", len(view.Outputs))
}
if got := singleRef(t, view.Examples); got != "facts.examples[1]" {
t.Fatalf("example ref = %q, want facts.examples[1]", got)
if len(view.Examples) != 0 {
t.Fatalf("examples len = %d, want 0 without diagnostics", len(view.Examples))
}
if view.ChangedSummary.Commands != 1 ||
view.ChangedSummary.Skills != 1 ||

View File

@@ -61,8 +61,6 @@ type SkillQualityInput struct {
facts.SkillQualityFact
}
func (i SkillQualityInput) ref() string { return i.FactRef }
type ErrorInput struct {
FactRef string `json:"fact_ref"`
facts.ErrorFact
@@ -71,8 +69,14 @@ type ErrorInput struct {
func (i ErrorInput) ref() string { return i.FactRef }
type OutputInput struct {
FactRef string `json:"fact_ref"`
facts.OutputFact
FactRef string `json:"fact_ref"`
Command string `json:"command"`
Domain string `json:"domain,omitempty"`
Changed bool `json:"changed,omitempty"`
Source string `json:"source,omitempty"`
IsList bool `json:"is_list"`
HasDefaultLimit bool `json:"has_default_limit"`
HasDecisionField bool `json:"has_decision_field"`
}
func (i OutputInput) ref() string { return i.FactRef }
@@ -82,11 +86,9 @@ type ExampleInput struct {
facts.CommandExample
}
func (i ExampleInput) ref() string { return i.FactRef }
func BuildInputView(f facts.Facts) InputView {
selected := newInputSelection(f)
selected.addChangedFacts()
selected.addChangedReviewCandidates()
var viewDiagnostics []facts.DiagnosticFact
for _, diag := range f.Diagnostics {
@@ -115,37 +117,44 @@ func BuildInputView(f facts.Facts) InputView {
}
}
func (s *inputSelection) addChangedFacts() {
func (s *inputSelection) addChangedReviewCandidates() {
for i, cmd := range s.f.Commands {
if cmd.Changed {
if cmd.Changed && commandReviewCandidate(cmd) {
s.commands[i] = true
}
}
for i, skill := range s.f.Skills {
if skill.Changed {
if skill.Changed && skillReviewCandidate(skill) {
s.skills[i] = true
}
}
for i, skill := range s.f.SkillQuality {
if skill.Changed {
s.skillQuality[i] = true
}
}
for i, errFact := range s.f.Errors {
if errFact.Changed {
if errFact.Changed && errorReviewCandidate(errFact) {
s.errors[i] = true
}
}
for i, output := range s.f.Outputs {
if output.Changed {
if output.Changed && outputReviewCandidate(output) {
s.outputs[i] = true
}
}
for i, example := range s.f.Examples {
if example.Changed {
s.examples[i] = true
}
}
}
func commandReviewCandidate(cmd facts.CommandFact) bool {
return cmd.NameConflictsExisting || cmd.FlagAliasConflict
}
func skillReviewCandidate(skill facts.SkillFact) bool {
return skill.ReferencesInvalidCommand
}
func errorReviewCandidate(errFact facts.ErrorFact) bool {
return errFact.Boundary && errFact.RequiredHint && errFact.HintActionCount == 0
}
func outputReviewCandidate(_ facts.OutputFact) bool {
// default_output is observe-first in the current rollout; reject diagnostics add exact output context.
return false
}
type inputSelection struct {
@@ -172,6 +181,24 @@ func newInputSelection(f facts.Facts) *inputSelection {
func (s *inputSelection) diagnosticContext(diag facts.DiagnosticFact) *inputSelection {
out := newInputSelection(s.f)
switch {
case diag.Rule == "command_naming" || diag.Rule == "flag_naming":
s.addDiagnosticCommands(out, diag)
case strings.HasPrefix(diag.Rule, "default_output"):
s.addDiagnosticOutputs(out, diag)
case strings.HasPrefix(diag.Rule, "skill_"):
s.addDiagnosticSkills(out, diag)
s.addDiagnosticSkillQuality(out, diag)
s.addDiagnosticExamples(out, diag)
case strings.HasPrefix(diag.Rule, "example_dry_run"):
s.addDiagnosticExamples(out, diag)
case diag.Rule == "no_bare_helper_error":
s.addDiagnosticErrors(out, diag)
}
return out
}
func (s *inputSelection) addDiagnosticCommands(out *inputSelection, diag facts.DiagnosticFact) {
for i, cmd := range s.f.Commands {
if diagnosticCommandMatches(diag, cmd.Path, cmd.CanonicalPath) ||
diagnosticMentions(diag, cmd.Path) ||
@@ -179,6 +206,9 @@ func (s *inputSelection) diagnosticContext(diag facts.DiagnosticFact) *inputSele
out.commands[i] = true
}
}
}
func (s *inputSelection) addDiagnosticSkills(out *inputSelection, diag facts.DiagnosticFact) {
for i, skill := range s.f.Skills {
if diagnosticLocationMatches(diag.File, diag.Line, skill.SourceFile, skill.Line) ||
diagnosticCommandMatches(diag, skill.CommandPath) ||
@@ -186,11 +216,17 @@ func (s *inputSelection) diagnosticContext(diag facts.DiagnosticFact) *inputSele
out.skills[i] = true
}
}
}
func (s *inputSelection) addDiagnosticSkillQuality(out *inputSelection, diag facts.DiagnosticFact) {
for i, skill := range s.f.SkillQuality {
if samePath(diag.File, skill.SourceFile) {
out.skillQuality[i] = true
}
}
}
func (s *inputSelection) addDiagnosticErrors(out *inputSelection, diag facts.DiagnosticFact) {
for i, errFact := range s.f.Errors {
if diagnosticLocationMatches(diag.File, diag.Line, errFact.File, errFact.Line) ||
diagnosticCommandMatches(diag, errFact.CommandPath, errFact.Command) ||
@@ -199,12 +235,18 @@ func (s *inputSelection) diagnosticContext(diag facts.DiagnosticFact) *inputSele
out.errors[i] = true
}
}
}
func (s *inputSelection) addDiagnosticOutputs(out *inputSelection, diag facts.DiagnosticFact) {
for i, output := range s.f.Outputs {
if diagnosticCommandMatches(diag, output.Command) ||
diagnosticMentions(diag, output.Command) {
out.outputs[i] = true
}
}
}
func (s *inputSelection) addDiagnosticExamples(out *inputSelection, diag facts.DiagnosticFact) {
for i, example := range s.f.Examples {
if diagnosticLocationMatches(diag.File, diag.Line, example.SourceFile, example.Line) ||
diagnosticCommandMatches(diag, example.CommandPath) ||
@@ -212,11 +254,10 @@ func (s *inputSelection) diagnosticContext(diag facts.DiagnosticFact) *inputSele
out.examples[i] = true
}
}
return out
}
func includeDiagnosticInView(diag facts.DiagnosticFact, selected, context *inputSelection) bool {
if diag.Action != report.ActionWarning {
if diag.Action == report.ActionReject {
return true
}
return selected.intersects(context)
@@ -284,7 +325,17 @@ func (s *inputSelection) outputInputs() []OutputInput {
out := make([]OutputInput, 0, countSelected(s.outputs))
for i, ok := range s.outputs {
if ok {
out = append(out, OutputInput{FactRef: factRef("outputs", i), OutputFact: s.f.Outputs[i]})
output := s.f.Outputs[i]
out = append(out, OutputInput{
FactRef: factRef("outputs", i),
Command: output.Command,
Domain: output.Domain,
Changed: output.Changed,
Source: output.Source,
IsList: output.IsList,
HasDefaultLimit: output.HasDefaultLimit,
HasDecisionField: output.HasDecisionField,
})
}
}
return out

View File

@@ -5,6 +5,7 @@ package semantic
import (
"encoding/json"
"strconv"
"strings"
"testing"
@@ -12,15 +13,17 @@ import (
"github.com/larksuite/cli/internal/qualitygate/report"
)
func TestInputViewKeepsChangedFactsWithOriginalRefs(t *testing.T) {
func TestInputViewKeepsChangedReviewCandidatesWithOriginalRefs(t *testing.T) {
f := facts.Facts{
SchemaVersion: 1,
Commands: []facts.CommandFact{
{Path: "old noisy command", Source: "shortcut"},
{Path: "docs +clean", Changed: true, Source: "shortcut"},
{Path: "docs +fetch", Changed: true, Source: "shortcut", NameConflictsExisting: true},
},
Skills: []facts.SkillFact{
{SourceFile: "skills/lark-old/SKILL.md", Line: 3, Raw: "old noisy skill"},
{SourceFile: "skills/lark-doc/SKILL.md", Line: 8, Raw: "changed clean skill", Changed: true},
{SourceFile: "skills/lark-doc/SKILL.md", Line: 9, Raw: "changed skill", Changed: true, ReferencesInvalidCommand: true},
},
SkillQuality: []facts.SkillQualityFact{
@@ -29,10 +32,12 @@ func TestInputViewKeepsChangedFactsWithOriginalRefs(t *testing.T) {
},
Errors: []facts.ErrorFact{
{File: "old.go", Line: 10, Boundary: true, RequiredHint: true},
{File: "cmd/docs.go", Line: 19, Changed: true, Boundary: true, RequiredHint: true, HintActionCount: 1},
{File: "cmd/docs.go", Line: 20, Changed: true, Boundary: true, RequiredHint: true},
},
Outputs: []facts.OutputFact{
{Command: "old list", IsList: true},
{Command: "docs clean-list", Changed: true, IsList: true, HasDefaultLimit: true, HasDecisionField: true},
{Command: "docs list", Changed: true, IsList: true},
},
Examples: []facts.CommandExample{
@@ -42,31 +47,155 @@ func TestInputViewKeepsChangedFactsWithOriginalRefs(t *testing.T) {
}
view := BuildInputView(f)
if got := singleRef(t, view.Commands); got != "facts.commands[1]" {
t.Fatalf("command ref = %q, want facts.commands[1]", got)
if got := singleRef(t, view.Commands); got != "facts.commands[2]" {
t.Fatalf("command ref = %q, want facts.commands[2]", got)
}
if got := singleRef(t, view.Skills); got != "facts.skills[1]" {
t.Fatalf("skill ref = %q, want facts.skills[1]", got)
if got := singleRef(t, view.Skills); got != "facts.skills[2]" {
t.Fatalf("skill ref = %q, want facts.skills[2]", got)
}
if got := singleRef(t, view.SkillQuality); got != "facts.skill_quality[1]" {
t.Fatalf("skill quality ref = %q, want facts.skill_quality[1]", got)
if len(view.SkillQuality) != 0 {
t.Fatalf("skill quality len = %d, want 0 without diagnostics", len(view.SkillQuality))
}
if got := singleRef(t, view.Errors); got != "facts.errors[1]" {
t.Fatalf("error ref = %q, want facts.errors[1]", got)
if got := singleRef(t, view.Errors); got != "facts.errors[2]" {
t.Fatalf("error ref = %q, want facts.errors[2]", got)
}
if got := singleRef(t, view.Outputs); got != "facts.outputs[1]" {
t.Fatalf("output ref = %q, want facts.outputs[1]", got)
if len(view.Outputs) != 0 {
t.Fatalf("outputs len = %d, want 0 without reject diagnostics", len(view.Outputs))
}
if got := singleRef(t, view.Examples); got != "facts.examples[1]" {
t.Fatalf("example ref = %q, want facts.examples[1]", got)
if len(view.Examples) != 0 {
t.Fatalf("examples len = %d, want 0 without diagnostics", len(view.Examples))
}
data, err := json.Marshal(view)
if err != nil {
t.Fatalf("marshal view: %v", err)
}
if strings.Contains(string(data), "old noisy") {
t.Fatalf("view leaked unchanged noisy facts: %s", data)
for _, forbidden := range []string{"old noisy", "docs +clean", "changed clean skill", "docs clean-list"} {
if strings.Contains(string(data), forbidden) {
t.Fatalf("view leaked non-candidate fact %q: %s", forbidden, data)
}
}
}
func TestInputViewSummarizesBroadChangedCommandSurface(t *testing.T) {
f := broadChangedFacts(434, 44)
view := BuildInputView(f)
if view.ChangedSummary.Commands != 434 || view.ChangedSummary.Outputs != 44 {
t.Fatalf("changed summary = %#v", view.ChangedSummary)
}
if len(view.Commands) != 0 || len(view.Outputs) != 0 {
t.Fatalf("broad clean surface leaked details: commands=%d outputs=%d", len(view.Commands), len(view.Outputs))
}
messages := BuildPrompt(f)
if len(messages) != 2 {
t.Fatalf("messages len = %d, want 2", len(messages))
}
if got := len(messages[1].Content); got > 8192 {
t.Fatalf("prompt user content bytes = %d, want <= 8192", got)
}
if strings.Contains(messages[1].Content, "service command 433") {
t.Fatalf("prompt leaked broad command details: %s", messages[1].Content)
}
}
func TestInputViewKeepsSemanticCandidateInsideBroadChangedSurface(t *testing.T) {
f := broadChangedFacts(200, 20)
f.Commands[137].NameConflictsExisting = true
f.Outputs[11].HasDefaultLimit = false
view := BuildInputView(f)
if got := singleRef(t, view.Commands); got != "facts.commands[137]" {
t.Fatalf("command ref = %q, want facts.commands[137]", got)
}
if len(view.Outputs) != 0 {
t.Fatalf("outputs len = %d, want 0 without reject diagnostics", len(view.Outputs))
}
}
func TestInputViewOmitsVerboseOutputFields(t *testing.T) {
f := facts.Facts{
SchemaVersion: 1,
Outputs: []facts.OutputFact{{
Command: "base +record-list",
Domain: "base",
Changed: true,
Source: "shortcut",
Fields: []string{"items", "has_more", strings.Repeat("verbose_output_field_", 200)},
IsList: true,
HasDefaultLimit: true,
HasDecisionField: false,
}},
Diagnostics: []facts.DiagnosticFact{{
Rule: "default_output_contract",
Action: report.ActionReject,
File: "command-manifest",
CommandPath: "base +record-list",
}},
}
view := BuildInputView(f)
if got := singleRef(t, view.Outputs); got != "facts.outputs[0]" {
t.Fatalf("output ref = %q, want facts.outputs[0]", got)
}
data, err := json.Marshal(view)
if err != nil {
t.Fatalf("marshal view: %v", err)
}
text := string(data)
if strings.Contains(text, "verbose_output_field_") || strings.Contains(text, "fields") {
t.Fatalf("semantic view leaked verbose output fields: %s", text)
}
if !strings.Contains(text, `"has_decision_field":false`) {
t.Fatalf("semantic view should make missing decision field explicit: %s", text)
}
}
func TestBuildPromptKeepsManyOutputCandidatesWithinRequestLimit(t *testing.T) {
f := broadOutputCandidateFacts(40)
for i := 0; i < 80; i++ {
f.Commands = append(f.Commands, facts.CommandFact{
Path: "shortcut list " + strconv.Itoa(i),
Domain: "shortcut",
Changed: true,
Source: "shortcut",
Flags: []string{strings.Repeat("verbose_flag_", 100)},
})
f.Skills = append(f.Skills, facts.SkillFact{
SourceFile: "skills/lark-shortcut/SKILL.md",
Line: i + 1,
Raw: strings.Repeat("verbose skill guidance ", 100),
CommandPath: "shortcut list " + strconv.Itoa(i),
Changed: true,
})
}
f.Diagnostics = append(f.Diagnostics, facts.DiagnosticFact{
Rule: "default_output",
Action: report.ActionWarning,
File: "command-manifest",
Message: "shortcut list 1 looks like a list command without an explicit default limit flag",
CommandPath: "shortcut list 1",
})
messages := BuildPrompt(f)
var view InputView
if err := json.Unmarshal([]byte(messages[1].Content), &view); err != nil {
t.Fatalf("prompt user content is not input view JSON: %v", err)
}
if len(view.Commands) != 0 || len(view.Skills) != 0 {
t.Fatalf("default-output view leaked unrelated context: commands=%d skills=%d", len(view.Commands), len(view.Skills))
}
if len(view.Outputs) != 0 {
t.Fatalf("default-output warnings should not enter semantic view without reject diagnostics: outputs=%d", len(view.Outputs))
}
if got := len(messages[1].Content); got > 16*1024 {
t.Fatalf("prompt user content bytes = %d, want <= 16384", got)
}
if strings.Contains(messages[1].Content, "verbose_output_field_") ||
strings.Contains(messages[1].Content, "verbose_flag_") ||
strings.Contains(messages[1].Content, "verbose skill guidance") {
t.Fatalf("prompt leaked verbose output fields: %s", messages[1].Content)
}
}
@@ -163,6 +292,32 @@ func TestInputViewDropsUnchangedWarningDiagnostics(t *testing.T) {
}
}
func TestInputViewDropsUnselectedLabelDiagnostics(t *testing.T) {
f := facts.Facts{
SchemaVersion: 1,
Commands: []facts.CommandFact{{
Path: "drive +task_result",
Changed: true,
Source: "shortcut",
}},
Diagnostics: []facts.DiagnosticFact{{
Rule: "command_naming",
Action: report.ActionLabel,
File: "command-manifest",
Message: "drive +task_result has non-kebab-case command segments",
CommandPath: "drive +task_result",
}},
}
view := BuildInputView(f)
if len(view.Commands) != 0 {
t.Fatalf("commands len = %d, want 0 for label-only diagnostic", len(view.Commands))
}
if len(view.Diagnostics) != 0 {
t.Fatalf("diagnostics len = %d, want 0 for label-only diagnostic", len(view.Diagnostics))
}
}
func TestBuildPromptUsesInputViewInsteadOfFullFacts(t *testing.T) {
f := facts.Facts{
SchemaVersion: 1,
@@ -198,6 +353,49 @@ func TestBuildPromptDescribesErrorHintRubric(t *testing.T) {
}
}
func broadChangedFacts(commands, outputs int) facts.Facts {
f := facts.Facts{SchemaVersion: 1}
for i := 0; i < commands; i++ {
f.Commands = append(f.Commands, facts.CommandFact{
Path: "service command " + strconv.Itoa(i),
Domain: "service",
Changed: true,
Source: "service",
Flags: []string{"tenant_key", "page_token", "page_size", "user_id_type"},
})
}
for i := 0; i < outputs; i++ {
f.Outputs = append(f.Outputs, facts.OutputFact{
Command: "service command " + strconv.Itoa(i),
Domain: "service",
Changed: true,
Source: "service",
Fields: []string{"items", "has_more", "page_token"},
IsList: true,
HasDefaultLimit: true,
HasDecisionField: true,
})
}
return f
}
func broadOutputCandidateFacts(outputs int) facts.Facts {
f := facts.Facts{SchemaVersion: 1}
for i := 0; i < outputs; i++ {
f.Outputs = append(f.Outputs, facts.OutputFact{
Command: "shortcut list " + strconv.Itoa(i),
Domain: "shortcut",
Changed: true,
Source: "shortcut",
Fields: []string{"items", "has_more", strings.Repeat("verbose_output_field_", 200)},
IsList: true,
HasDefaultLimit: i%2 == 0,
HasDecisionField: false,
})
}
return f
}
type refItem interface {
ref() string
}