Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notebook support #540

Merged
merged 26 commits into from
Jun 13, 2024
Merged

Notebook support #540

merged 26 commits into from
Jun 13, 2024

Conversation

pelikhan
Copy link
Member

@pelikhan pelikhan commented Jun 12, 2024

  • A new Python feature has been added to the devcontainer configuration in .devcontainer/devcontainer.json.
  • A new file demo/genaisrc/poem.genaibook has been created. It appears to contain a script for generating a one-sentence poem.
  • Changes have been made to the demo/web-app-basic-linux/summary_and_diagrams.md file, including the addition of a new JavaScript script for generating a one-sentence poem.
  • A new file docs/src/content/docs/getting-started/your-first-genai-script.mdx has been created. It appears to contain documentation for creating a GenAI script.
  • Some changes have been made to the TypeScript files in the packages/core/src directory. These changes seem to be related to the handling of file types and indentation.
  • A new file packages/sample/src/jupytest.ipynb has been created. It appears to be a Jupyter notebook file.
  • A new file packages/sample/src/node.nnb has been created. It appears to be a notebook file for Node.js.
  • A new file packages/sample/src/poem.genaibook has been created. It appears to contain a script for generating a one-sentence poem.
  • Some changes have been made to the packages/sample/genaisrc/pr-review-commit.genai.js file. These changes seem to be related to the guidelines for reviewing pull request commits.

generated by pr-describe

@pelikhan
Copy link
Member Author

/genai-describe /genai-review

Copy link

LGTM 🚀

The changes in the pull request include:

  • Removal of a project getter from the Fragment class in ast.ts.
  • Importing dedent from a new file indent.ts instead of ts-dedent in chat.ts and promptdom.ts.
  • Addition of new MIME types for Markdown and YAML in constants.ts.
  • Creation of a new indent.ts file that exports a function to indent text and re-exports ts-dedent as dedent.
  • Exporting the new indent module from index.ts.
  • Addition of a details function in markdown.ts to create a markdown details section.
  • Addition of a new notebook.ts file in the vscode package, which seems to add support for interactive notebooks in the extension.
  • Activation of the new notebook feature in extension.ts.
  • Addition of a notebook option in the AIRequestOptions interface and corresponding changes in state.ts to handle notebook requests.

All changes seem to be well implemented and should not introduce any functional issues. 👍

generated by pr-review

}

return combined
}

Choose a reason for hiding this comment

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

The function getBetweenCellsWhitespace is defined but never used. Consider removing it if it's not needed.

generated by pr-review-commit unused_function


```md title="output" wrap
Beneath the silver moon's gentle gaze, the tranquil sea whispers secrets to the shore.
```

Choose a reason for hiding this comment

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

The wrap attribute in the markdown code block is unnecessary and not standard markdown syntax.

generated by pr-docs-review-commit unnecessary_wrap_attribute

indentation
)
}
} else {

Choose a reason for hiding this comment

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

The code is deeply nested which makes it hard to read and understand. Consider refactoring to reduce the level of nesting. 🔄

generated by pr-review-commit deep_nesting

}

return cells
}

Choose a reason for hiding this comment

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

The function parseMarkdown is quite long and does a lot of things. Consider breaking it down into smaller, more manageable functions. This will improve readability and maintainability. 📚

generated by pr-review-commit long_function

The execution of the GenAIScript generates a prompt (and more)
that gets sent to the LLM model.

The ` $``...`` ` template string function formats and write the string to the prompt; which gets sent to the LLM.

Choose a reason for hiding this comment

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

The template string function is incorrectly represented as $``...```. It should be ${...}` to correctly denote a template literal in JavaScript.

generated by pr-docs-review-commit template_literal


```js title="poem.genai.js"
$`Write a one sentence poem.`
```

Choose a reason for hiding this comment

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

The code block starting on line 50 should specify the language for syntax highlighting. It should be js instead of js title="poem.genai.js".

generated by pr-docs-review-commit code_fence_info_string


```md title="🤖" wrap
In the quiet dusk, the willow whispers secrets to the passing breeze.
```

Choose a reason for hiding this comment

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

The code block starting on line 54 should specify the language for syntax highlighting. It should be md instead of md title="🤖" wrap. Additionally, the "wrap" option is not standard and may not be recognized by the renderer.

generated by pr-docs-review-commit code_fence_info_string


get project() {
return this.file.project
}
}

Choose a reason for hiding this comment

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

The project getter method has been removed from the Fragment class but there is no replacement provided. If other parts of the codebase are using this method, this will cause a breakage.

generated by pr-review-commit unused_code

@@ -28,6 +27,7 @@ import { fenceMD } from "./markdown"
import { YAMLStringify } from "./yaml"
import { estimateChatTokens } from "./tokens"
import { createChatTurnGenerationContext } from "./runpromptcontext"
import { dedent } from "./indent"

Choose a reason for hiding this comment

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

The import of dedent has been changed from ts-dedent to ./indent. If the implementation of dedent in ./indent is different from ts-dedent, this could potentially introduce bugs.

generated by pr-review-commit import_change

@@ -350,7 +367,7 @@ temp/
} else if (connectionToken.type === "localai") await startLocalAI()

Choose a reason for hiding this comment

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

The conditional logic for setting cliInfo in the genOptions object is quite complex and hard to understand. Consider simplifying this logic or breaking it down into smaller functions for better readability and maintainability.

generated by pr-review-commit conditional_complexity


```md title="🤖" wrap
In the quiet dusk, the willow whispers secrets to the passing breeze.
```

Choose a reason for hiding this comment

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

The code fence for the markdown block is incorrectly formatted; it should not include the title and wrap annotations.

generated by pr-docs-review-commit incorrect_code_fence

startPos: [0, 0],
endPos: stringToPos(""),
}
await state.requestAI({

Choose a reason for hiding this comment

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

Missing import for PromptScript.

generated by pr-review-commit missing_import

endPos: stringToPos(""),
}
await state.requestAI({
template,

Choose a reason for hiding this comment

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

Missing import for PromptParameters.

generated by pr-review-commit missing_import

@pelikhan pelikhan merged commit 301f4f8 into main Jun 13, 2024
9 checks passed
@pelikhan pelikhan deleted the notebook branch June 13, 2024 17:32
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.

1 participant