[Fluid AI] Update ai-collab client API to transform TreeEdit types into a consumable version for UI visualizations #23117

chentong7 · 2024-11-15T23:52:13Z

Description

Currently, the only way to get a set of objects for visualizing each change an LLM makes to your tree using the aiCollab library is for users to use the sharedTreeDiff utility that was created for the implicit strategy separately. This utility is not perfect, likely does not cover many edge cases and is fickle. However, when using the explicit strategy we maintain an edit log internally that is the source truth. Currently, we don't return the editLog because the TreeEdit type of the edit log is not simple for users the consume and visualize changes on their UI.

The purpose of this ticket is to update the explicit strategy's entry point function generateTreeEdits by taking the editLog, which is an array of TreeEdit, and transforming it into a new set of objects "diffs" that can be easily used for UI visualizations and return it along with the success/fail response.

Test

/workspaces/FluidFramework/node_modules/.bin/mocha ./../../../dist/test/ai-collab/jobBoardBenchmark.spec.js --no-timeouts --exit

spec.js:54
  AI Job Listings App Benchmark
spec.js:54
    - Create a new Job with the title 'QA tester' and add a candidate named 'John Doe', who is only available on mondays and tuesdays, to the job.
spec.js:66
  0 passing (3ms)
base.js:411
  1 pending

…to a consumable version for UI visualizations

github-actions · 2024-11-16T00:02:03Z

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> [email protected] ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> [email protected] serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> [email protected] check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  171745 links
    1640 destination URLs
    1840 URLs ignored
       0 warnings
       0 errors

msfluid-bot

Code Coverage Summary

↓ packages.framework.ai-collab.src:
Line Coverage Change: -2.67% Branch Coverage Change: No change

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	0.00%	0.00%	→ No change
Line Coverage	35.59%	32.92%	↓ -2.67%

↓ packages.framework.ai-collab.src.explicit-strategy:
Line Coverage Change: -0.40% Branch Coverage Change: No change

Metric Name	Baseline coverage	PR coverage	Coverage Diff
Branch Coverage	77.98%	77.98%	→ No change
Line Coverage	77.60%	77.20%	↓ -0.40%

Baseline commit: 0b6d14f
Baseline build: 307935
Happy Coding!!

Code coverage comparison check failed!!
More Details: Readme

Skip This Check!!

What to do if the code coverage check fails:

Ideally, add more tests to increase the code coverage for the package(s) whose code-coverage regressed.
If a regression is causing the build to fail and is due to removal of tests, removal of code with lots of tests or any other valid reason, there is a checkbox further up in this comment that determines if the code coverage check should fail the build or not. You can check the box and trigger the build again. The test coverage analysis will still be done, but it will not fail the build if a regression is detected.
Unchecking the checkbox and triggering another build should go back to failing the build if a test-coverage regression is detected.
You can check which lines are covered or not covered by your tests with these steps:
- Go to the PR ADO build.
- Click on the link to see its published artifacts. You will see an artifact named codeCoverageAnalysis, which you can expand to reach to a particular source file's coverage html which will show which lines are covered/not covered by your tests.
- You can also run different kind of tests locally with :coverage tests commands to find out the coverage.

alexvy86

Besides the comments below, a couple of things:

It would be nice to see how the React changes look in the running app.
Not sure I get what the Test section in the PR description is supposed to convey. 0 passing tests, 1 skipped test?

alexvy86 · 2024-11-18T15:02:05Z

examples/apps/ai-collab/src/app/page.tsx

@@ -32,6 +32,7 @@ import { useSharedTreeRerender } from "@/useSharedTreeRerender";
 // Uncomment the import line that corresponds to the server you want to use
 // import { createContainer, loadContainer, postAttach, containerIdFromUrl } from "./spe"; // eslint-disable-line import/order
 import { createContainer, loadContainer, postAttach, containerIdFromUrl } from "./tinylicious"; // eslint-disable-line import/order
+// import { DiffViewer } from "@/components/DiffViewer";


alexvy86 · 2024-11-18T15:04:06Z

packages/framework/ai-collab/src/test/ai-collab/jobBoardBenchmark.spec.ts

@@ -342,6 +343,7 @@ describe.skip("AI Job Listings App Benchmark", () => {
 		}
 		const foundJohnDoe = createJohnDoeCandidateTask.data;
 		assert(foundJohnDoe !== undefined);
+		assert(response.diffs !== undefined);


Can we make this test better by asserting what the diffs look like? Or are they non-deterministic?

@chentong7

Shouldn't we unskip the whole test suite (it's currently skipped in line 223)

Error: 401 You didn't provide an API key. You need to provide your API key in an Authorization header using Bearer auth (i.e. Authorization: Bearer YOUR_KEY), or as the password field (with blank username) if you're accessing the API from your browser and are prompted for a username and password. You can obtain an API key from https://platform.openai.com/account/api-keys

This is what I get when I run the test. I'd like to spend some time in tmrw's parking lot to ramp up on some AI-collab testing library / concepts 🤯 .

Reading through the suite, and the fact that it requires an API_KEY, this test is not a unit test, is more of an integration/e2e test. The code lives here now because here's where everything started, but it needs to move to a different location (potentially the e2e tests package?) later. So it should remain skipped as long as it's here.

If we can extract the mapping of EditLog to Diff to a separate function we can use unit tests instead of this integration test. It isn't quite deterministic given we haven't set the temperature value on the model, and even when its set to be the most deterministic, technically it can give a different response.

alexvy86 · 2024-11-18T15:05:16Z

packages/framework/ai-collab/src/aiCollabApi.ts

+
+/**
+ * The GenerateTreeEditsResponse interface defines the structure of the response object
+ *
+ * @alpha
+ */
+export interface GenerateTreeEditsResponse {
+	status: "success" | "failure" | "partial-failure";
+	errorMessage?: string;
+	tokensUsed: TokenUsage;
+	diffs?: Diff[];
+}


Not sure if we really need a new type; should we just add the diffs to the two existing response types we already have?

I agree we should be reusing the existing types here, not seeing value from this new type and we're exposing the internal naming scheme that doesn't match with the public api surface

jikim-msft · 2024-11-18T23:46:24Z

Adding 21569 in description would help 😄 .

jikim-msft · 2024-11-19T00:47:04Z

When running the example app, I think the LLM agent isn't returning any response (possibly because of missing API_KEY from the above comment?).

alexvy86 · 2024-11-19T14:33:58Z

When running the example app, I think the LLM agent isn't returning any response (possibly because of missing API_KEY from the above comment?).

Yeah, in order for the AI collab to work you need an OpenAI API key for a paid account.

seanimam · 2024-11-19T20:59:45Z

packages/framework/ai-collab/src/aiCollabApi.ts

+
+/**
+ * The GenerateTreeEditsResponse interface defines the structure of the response object
+ *
+ * @alpha
+ */
+export interface GenerateTreeEditsResponse {
+	status: "success" | "failure" | "partial-failure";
+	errorMessage?: string;
+	tokensUsed: TokenUsage;
+	diffs?: Diff[];
+}


I agree we should be reusing the existing types here, not seeing value from this new type and we're exposing the internal naming scheme that doesn't match with the public api surface

seanimam · 2024-11-19T21:03:37Z

packages/framework/ai-collab/src/explicit-strategy/index.ts

+	const diffs: Diff[] = editLog.map((log, index) => ({
+		id: `diff-${index}`,
+		// eslint-disable-next-line @typescript-eslint/strict-boolean-expressions
+		type: log.error ? "error" : "edit",
+		description: log.error ?? log.edit.explanation ?? "No description available",
+	}));


Recommend making this a separate function. You will be able to unit test it as well this way.

What does the id here represent? This should be the id of the tree node that was edited, which the log object should have. Additionally, these diff's need more information. We need to know what type of edit was this, for example:

Is it a modification of an existing object? If so, what fields were modified and what is the new values if you can provide that?

is it an array operation? For example, the insertion of a new object, deletion, move? If so, what index was it inserted/moved/removed from?

seanimam · 2024-11-19T21:09:49Z

packages/framework/ai-collab/src/aiCollabApi.ts

+}
+
+/**
+ * The Diff interface defines the structure of the response object


This comment needs a more detailed explanation, this doesn't explain the concept of what a diff is.

Josmithr · 2024-11-20T00:29:18Z

packages/framework/ai-collab/api-report/ai-collab.alpha.api.md

@@ -46,6 +46,16 @@ export function createMergableDiffSeries(diffs: Difference[]): Difference[];
 // @alpha
 export function createMergableIdDiffSeries(oldObject: unknown, diffs: Difference[], idAttributeName: string | number): Difference[];

+// @alpha
+export interface Diff {
+    // (undocumented)


Please be sure to document all new exported members before merging 🙂

Josmithr · 2024-11-20T00:29:55Z

packages/framework/ai-collab/src/aiCollabApi.ts

@@ -182,3 +182,26 @@ export interface TokenLimits {
 	 */
 	readonly outputTokens?: number;
 }
+
+/**
+ * The GenerateTreeEditsResponse interface defines the structure of the response object


Response to what? Would be good to be a bit more descriptive here.

Josmithr · 2024-11-20T00:31:35Z

packages/framework/ai-collab/src/explicit-strategy/index.ts

- * - Primitive root nodes are not supported
- *
- * @internal
+ * The editLog is transformed into an array of Diff objects. Each Diff object includes


"editLog" is something that's defined within the function - the caller of this function likely won't know what it is. We should express the docs here in terms of what the function means to the caller.

chentong7 added 2 commits November 15, 2024 23:16

[Fluid AI] Update ai-collab client API to transform TreeEdit types in…

dea685a

…to a consumable version for UI visualizations

update

0b482c8

chentong7 requested review from alexvy86 and seanimam November 15, 2024 23:52

github-actions bot added base: main PRs targeted against main branch area: examples Changes that focus on our examples area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct public api change Changes to a public API labels Nov 15, 2024

msfluid-bot reviewed Nov 16, 2024

View reviewed changes

alexvy86 reviewed Nov 18, 2024

View reviewed changes

seanimam reviewed Nov 19, 2024

View reviewed changes

Josmithr reviewed Nov 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fluid AI] Update ai-collab client API to transform TreeEdit types into a consumable version for UI visualizations #23117

[Fluid AI] Update ai-collab client API to transform TreeEdit types into a consumable version for UI visualizations #23117

chentong7 commented Nov 15, 2024

github-actions bot commented Nov 16, 2024

msfluid-bot left a comment

alexvy86 left a comment

alexvy86 Nov 18, 2024

alexvy86 Nov 18, 2024

jikim-msft Nov 19, 2024

jikim-msft Nov 19, 2024

alexvy86 Nov 19, 2024

seanimam Nov 19, 2024 •

edited

Loading

alexvy86 Nov 18, 2024

seanimam Nov 19, 2024

jikim-msft commented Nov 18, 2024

jikim-msft commented Nov 19, 2024

alexvy86 commented Nov 19, 2024

seanimam Nov 19, 2024

seanimam Nov 19, 2024

seanimam Nov 19, 2024

Josmithr Nov 20, 2024

Josmithr Nov 20, 2024

Josmithr Nov 20, 2024

[Fluid AI] Update ai-collab client API to transform TreeEdit types into a consumable version for UI visualizations #23117

Are you sure you want to change the base?

[Fluid AI] Update ai-collab client API to transform TreeEdit types into a consumable version for UI visualizations #23117

Conversation

chentong7 commented Nov 15, 2024

Description

Test

github-actions bot commented Nov 16, 2024

linkcheck output

msfluid-bot left a comment

Choose a reason for hiding this comment

Code Coverage Summary

Code coverage comparison check failed!!More Details: Readme

What to do if the code coverage check fails:

alexvy86 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanimam Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jikim-msft commented Nov 18, 2024

jikim-msft commented Nov 19, 2024

alexvy86 commented Nov 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Code coverage comparison check failed!!
More Details: Readme

seanimam Nov 19, 2024 •

edited

Loading