Ai collab feedback loop upgrades #23517
Conversation
…onexistant field or invalid type for field
… 'modify' edit tests in agentEditReducer.spec.ts
github-actions
bot
added
area: framework
There was a problem hiding this comment.
Copilot reviewed 5 out of 7 changed files in this pull request and generated no comments.
Files not reviewed (2)
- packages/framework/ai-collab/src/test/explicit-strategy/snapshots/Prompt_Regression_Snapshot_Tests/Editing_System_Prompt_With_Plan_With_Log.snap: Language not supported
- packages/framework/ai-collab/src/test/explicit-strategy/snapshots/Prompt_Regression_Snapshot_Tests/Editing_System_Prompt_With_Plan_With_Log_With_Failed_Edits.snap: Language not supported
| function createInvalidModifyFeedbackMsg( | ||
| modifyEdit: Modify, | ||
| treeNode: TreeNode, | ||
| errorType: "NONEXISTENT_FIELD" | "INVALID_TYPE", |
There was a problem hiding this comment.
NIT:
I know there's only two types of errorType but can we make this an enum or object?
As we expand the usage of createInvalidModifyFeedbackMsg() in other treeEdit.types we should be having more errorTypes besides NONEXISTENT_FIELD or INVALID_TYPE right?
| } | ||
|
|
||
| messageSuffix = ` The node's field you selected for modification \`${modifyEdit.field}\` does not exist in this nodes schema. The set of available fields for this node are: \`[${nodeFieldNames.map((field) => `'${field}'`).join(", ")}]\`.${closestPossibleMatchForFieldMessage}`; | ||
| } else if (errorType === "INVALID_TYPE") { |
There was a problem hiding this comment.
For now, we can make this else.
| "str": "testStr", | ||
| "vectors": [ | ||
| { | ||
| "id": (view.root.vectors[0] as Vector).id, |
There was a problem hiding this comment.
I see id is expected for vectors but not for primitive values such as string or bool. Is this expected? Does idGenerator() generate and embed id to non-primitive nodes?
| ); | ||
| }); | ||
|
|
||
| it("modify edit with non existent field fails", () => { |
There was a problem hiding this comment.
Correct me if I'm wrong, but this test is passing in my local environment.
Also, I think it might be useful to add a test case of a root node with multiple fields of identical Levenshtein distance (so a maybe have fields x1 and x2 and try to modify x3). And verify that the error log has good readability with multiple values.
|
|
||
| for (const candidate of possibleMatches) { | ||
| const distance = levenshteinDistance(input, candidate); | ||
| if (distance < bestDistance) { |
There was a problem hiding this comment.
Doesn't this condition only honor the first-seen candidate and disregard other candidates with same Levenshtein distance?
Can we return an array containing all the candidates and use string concatenation in the createInvalidModifyFeedbackMsg()?
Description
The error messages produced from attempting an invalid modify edit are being used as part of the feedback loop for an LLM so that when it makes an invalid error, it gets a failure message and description as to why it failed. Until now, these error messages were simply what the shared tree library produced, which may be helpful for developers but loses meaningful context for an LLM looking at what it has done in the last via a prompt.
agentEditReducer.ts- Catches errors during themodifyswitch case and replaces the error message with a context rich one.agentEditingReducer.spec.ts- Refactors tests to have a describe block just for various 'modify' tree edit test cases & adds new cases for when the new error messages would be thrown.utils.ts- Adds new helper methods for determining what the closest match for an invalid field would be. This helps with providing a 'did you mean to use the field xxx' messageutils.spec.ts- Adds tests for new helper methods for finding the closest match to a given string.Reviewer Guidance