Skip to content

Commit

Permalink
Filter out blank responses (#270)
Browse files Browse the repository at this point in the history
* I'm observing cases where we have a non empty suggested cell which
ends up getting deleted.
* It looks like this happens for one of two reasons
* The model responses with an empty code block which doesn't get
filtered out
  * We return a response with no blocks in it

* We should filter these out.
* This means we bias towards generating irrelvant suggestions rather
than no suggestions.
* Right now I think this is helpful because otherwise people might get
confused and wonder whether Foyle is working.

Here's the supporting notebook
https://gist.github.com/jlewi/31857545ec62b36d2949ccd904918d53
  • Loading branch information
jlewi authored Oct 2, 2024
1 parent 238e04f commit 527e171
Show file tree
Hide file tree
Showing 2 changed files with 73 additions and 0 deletions.
24 changes: 24 additions & 0 deletions app/pkg/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,11 @@ func (a *Agent) StreamGenerate(ctx context.Context, stream *connect.BidiStream[v
return
}

if dropResponse(response) {
log.V(logs.Debug).Info("Dropping response", zap.Object("response", response))
continue
}

log.V(logs.Debug).Info("Sending response", zap.Object("response", response))
if err := stream.Send(response); err != nil {
log.Error(err, "Failed to send response")
Expand Down Expand Up @@ -564,6 +569,11 @@ func postProcessBlocks(blocks []*v1alpha1.Block) ([]*v1alpha1.Block, error) {
if isOutputTag(block.Contents) {
continue
}

// If the block is empty filter it out.
if strings.TrimSpace(block.Contents) == "" {
continue
}
results = append(results, block)
return results, nil
}
Expand Down Expand Up @@ -607,3 +617,17 @@ func shouldTrigger(doc *v1alpha1.Doc, selectedIndex int32) bool {
selectedCell := doc.Blocks[selectedIndex]
return selectedCell.GetKind() == v1alpha1.BlockKind_MARKUP
}

// dropResponse returns true if the response should be dropped rather than being sent to the client.
// The reason for doing this is because if a previous generation generated a "good" response we don't want
// to overwrite it with this one
func dropResponse(response *v1alpha1.StreamGenerateResponse) bool {
if response == nil {
return true
}
// We don't want to send empty cells because that will cause the client to remove any previously suggested cells
if len(response.Cells) == 0 {
return true
}
return false
}
49 changes: 49 additions & 0 deletions app/pkg/agent/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,16 @@ func Test_PostProcessBlocks(t *testing.T) {
},
expected: []*v1alpha1.Block{},
},
{
name: "whitespace-only-block",
blocks: []*v1alpha1.Block{
{
Kind: v1alpha1.BlockKind_CODE,
Contents: " ",
},
},
expected: []*v1alpha1.Block{},
},
}

for _, c := range cases {
Expand All @@ -375,3 +385,42 @@ func Test_PostProcessBlocks(t *testing.T) {
})
}
}

func Test_dropResponse(t *testing.T) {
type testCase struct {
name string
response *v1alpha1.StreamGenerateResponse
expected bool
}

cases := []testCase{
{
name: "basic",
response: &v1alpha1.StreamGenerateResponse{
Cells: []*parserv1.Cell{
{
Kind: parserv1.CellKind_CELL_KIND_CODE,
Value: "print('Hello, World!')",
},
},
},
expected: false,
},
{
name: "empty",
response: &v1alpha1.StreamGenerateResponse{
Cells: []*parserv1.Cell{},
},
expected: true,
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
result := dropResponse(c.response)
if result != c.expected {
t.Errorf("Expected %v; got %v", c.expected, result)
}
})
}
}

0 comments on commit 527e171

Please sign in to comment.