Skip to content

fix: update DIFC test assertions to match new notice format#2552

Merged
lpcox merged 1 commit intomainfrom
fix/difc-test-assertions
Mar 26, 2026
Merged

fix: update DIFC test assertions to match new notice format#2552
lpcox merged 1 commit intomainfrom
fix/difc-test-assertions

Conversation

@lpcox
Copy link
Copy Markdown
Collaborator

@lpcox lpcox commented Mar 26, 2026

Fixes 2 test failures in call_backend_tool_difc_test.go caused by notice format changes that landed on main:

  • Phase2 write block: Test expected "DIFC policy violation" but evaluator now emits "DIFC Violation:" (changed in b1cb2a7)
  • Phase5 filter notice: Test searched for "DIFC" or "filtered" (lowercase) but notice now says "[Filtered]" (capital F, changed in 8ecdf2c)

Minimal fix — updates the assertion strings to match the current code.

Tests expected old '[DIFC]' prefix and 'DIFC policy violation' error
string, but commits b1cb2a7 and 8ecdf2c changed these to '[Filtered]'
and 'DIFC Violation:' respectively.

- Phase2 write block: 'DIFC policy violation' → 'DIFC Violation:'
- Phase5 filter notice: add 'Filtered' (capital F) to search terms

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 26, 2026 03:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates DIFC-related test assertions in the unified server test suite to align with recently changed DIFC error/notice message formats, unblocking CI failures after upstream formatting changes.

Changes:

  • Adjust Phase 2 write-block test to expect the current DIFC violation error prefix ("DIFC Violation:").
  • Relax/update Phase 5 filter-mode notice detection to match the new notice capitalization/prefix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 490 to 495
// At least one content item should mention DIFC filtering notice.
var foundNotice bool
for _, c := range result.Content {
if tc, ok := c.(*sdk.TextContent); ok {
if strings.Contains(tc.Text, "DIFC") || strings.Contains(tc.Text, "filtered") {
if strings.Contains(tc.Text, "Filtered") || strings.Contains(tc.Text, "DIFC") || strings.Contains(tc.Text, "filtered") {
foundNotice = true
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filter-notice assertion is quite permissive (it matches any content containing "DIFC" or lowercase "filtered"), which can let the test pass even if the specific notice format regresses. Since buildDIFCFilteredNotice currently prefixes notices with "[Filtered]", consider asserting on that exact prefix (and update the nearby comment that says the notice should mention "DIFC", since the current notice text may not include "DIFC" at all).

See below for a potential fix:

	// At least one content item should contain the DIFC filtering notice prefix.
	var foundNotice bool
	for _, c := range result.Content {
		if tc, ok := c.(*sdk.TextContent); ok {
			if strings.HasPrefix(tc.Text, "[Filtered]") {

Copilot uses AI. Check for mistakes.
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