-
Notifications
You must be signed in to change notification settings - Fork 0
Add implementation plan for API and document graph feature #1
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
Conversation
Summary of ChangesHello @tommcd, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a detailed implementation plan that will guide the future development of the project. It outlines two major tracks: significant improvements to the core API of the existing bookmarks-to-Markdown tool and the introduction of an entirely new document graph feature. The plan provides a structured approach with clear objectives, architectural considerations, and phased milestones, ensuring a well-defined path for upcoming work. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a comprehensive implementation plan for core API refactoring and a new document graph feature. The plan is well-structured, detailed, and provides a clear roadmap for future development. My review includes a couple of suggestions to enhance the completeness of the plan, specifically by defining all proposed terms and by explicitly including security considerations in the architecture. Overall, this is an excellent planning document that sets a strong foundation for the upcoming work.
| - Provide a declarative configuration/DSL for authoring and rendering graph-based documents. | ||
| - Offer rendering backends (Markdown/HTML) that can materialize slices or unified outputs with provenance. | ||
|
|
||
| ### Proposed Architecture |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very thorough architectural proposal. One aspect that could be explicitly included is security. Since the system will parse various documents and render them (potentially to HTML), addressing security from the design stage is crucial. I suggest adding a dedicated point under this section for security considerations, which could cover:
- Input sanitization to prevent injection attacks.
- Safe HTML rendering to mitigate Cross-Site Scripting (XSS) risks.
- Policies for handling external resources and links.
|
|
||
| ### Proposed Architecture | ||
| 1. **Core data structures** | ||
| - Immutable graph representation where nodes are content blocks (sections, paragraphs, snippets) and edges are typed (`contains`, `includes`, `refers`, `variant-of`). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list of typed edges includes variant-of, but its semantics are not defined later in the document alongside contains, includes, and refers (line 69). To ensure clarity in the plan, it would be beneficial to add a definition for what this edge type represents. For instance, does it signify a translation, a summary, or another form of content derivation?
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a comprehensive implementation plan document that outlines future development work for the mdtk (Markdown Toolkit) project. The plan proposes two parallel tracks: refactoring the existing bookmarks converter into a more modular library API, and introducing a new document graph feature that enables advanced document manipulation through splitting, transclusion, and graph-based rendering.
Key Changes
- Detailed architecture and milestones for refactoring the bookmarks converter into reusable parsing, selection, and rendering layers
- Specification of a new document graph feature with transclusion semantics, query capabilities, and multiple rendering backends
- Quality gates, testing expectations, and governance structure for tracking progress
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@codex generated this PR for the wrong repo, it should have been for doctk. |
|
I don’t have access to the |
Summary
Testing
Codex Task