microsoft / genaiscript

Generative AI Scripting
https://microsoft.github.io/genaiscript/
MIT License
81 stars 22 forks source link

display current trace as explorer tree #554

Closed pelikhan closed 2 weeks ago

pelikhan commented 2 weeks ago
image

generated by pr-describe

github-actions[bot] commented 2 weeks ago

The changes look good overall, they add a new feature to parse and display a trace tree from the markdown content. This is accomplished by adding two functions in markdown.ts to parse the markdown content, and then use the parsed result in newly introduced tracetree.ts to display the tree.

However, there are some potential issues:

  1. In markdown.ts, the function parseDetailsTree does not handle the cases when the markdown syntax format is incorrect, such as mismatching <details> and </details> tags. This may cause issues in runtime.

  2. In tracetree.ts, the TraceTree class does not seem to handle the case when state.aiRequest?.trace?.content is undefined. This might cause the application to crash when the data is not available.

  3. There is no null or undefined check for the element in the getChildren function, which could possibly lead to a runtime error when element is undefined.

To mitigate these concerns, consider adding error handling or checks for undefined values where necessary.

Here is the suggested code fix for getChildren function:

+ getChildren(element?: TraceNode): vscode.ProviderResult<TraceNode[]> {
+    if (!element) return [];
+    if (element === undefined)
+        element = parseDetailsTree(this.state.aiRequest?.trace?.content) || '';
+    if (typeof element === "object") return element?.content || [];
+    return undefined
+}

And a possible improvement for parseDetailsTree:

+ export function parseDetailsTree(text: string): DetailsNode {
+    if (!text || typeof text !== 'string') return { label: "root", content: [] };
+    const stack: DetailsNode[] = [{ label: "root", content: [] }]
+    const lines = text.split("\n")
+    for (let i = 0; i < lines.length; ++i) {
+        const startDetails = /^\s*<details( open)>\s*$/m.exec(lines[i])
+        if (startDetails) {
+            const parent = stack.at(-1)
+            const current: DetailsNode = { label: "", content: [] }
+            parent.content.push(current)
+            stack.push(current)
+            continue
+        }
+        const endDetails = /^\s*<\/details>\s*$/m.exec(lines[i])
+        if (endDetails) {
+            if (stack.length === 1) throw new Error("Mismatching </details> tag at line " + i);
+            stack.pop()
+            continue
+        }
+        const summary = /^\s*<summary>(.*)<\/summary>\s*$/m.exec(lines[i])
+        if (summary) {
+            const current = stack.at(-1)
+            current.label = summary[1]
+            continue
+        }
+
+        const contents = stack.at(-1).content
+        const content = lines[i]
+        const lastContent = contents.at(-1)
+        if (typeof lastContent === "string")
+            contents[contents.length - 1] = lastContent + "\n" + content
+        else contents.push(content)
+    }
+
+    if (stack.length > 1) throw new Error("Missing </details> tag");
+
+    return stack[0]
+}

If these improvements are applied, then LGTM :rocket:.

generated by pr-review

pelikhan commented 2 weeks ago

/genai-describe