fix: support rst and adoc knowledge uploads#8255
Conversation
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The set of Markitdown-backed extensions is now duplicated across
select_parser, the file inputacceptattribute, and the icon/color helpers; consider centralizing these extensions in a shared constant or config to keep frontend and backend behavior in sync and avoid drift when formats change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The set of Markitdown-backed extensions is now duplicated across `select_parser`, the file input `accept` attribute, and the icon/color helpers; consider centralizing these extensions in a shared constant or config to keep frontend and backend behavior in sync and avoid drift when formats change.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request adds support for .rst and .adoc file formats to the knowledge base by updating the backend parser selection, frontend file upload filters, and UI components. Feedback suggests replacing .includes() with exact string equality when determining file icons and colors in the frontend to prevent incorrect matches with other file extensions.
| const type = fileType?.toLowerCase() || '' | ||
| if (type.includes('pdf')) return 'mdi-file-pdf-box' | ||
| if (type.includes('epub')) return 'mdi-book-open-page-variant' | ||
| if (type.includes('rst') || type.includes('adoc')) return 'mdi-file-document-outline' |
There was a problem hiding this comment.
Using .includes() for file extension checks is unreliable because it can match substrings in unrelated extensions. For example, a file with a .first extension would incorrectly match the rst check. Since type is the lowercased extension string, exact equality is safer and more predictable.
if (type === 'rst' || type === 'adoc') return 'mdi-file-document-outline'
| const type = fileType?.toLowerCase() || '' | ||
| if (type.includes('pdf')) return 'error' | ||
| if (type.includes('epub')) return 'warning' | ||
| if (type.includes('rst') || type.includes('adoc')) return 'success' |
There was a problem hiding this comment.
Similar to the icon selection logic, using .includes() here is prone to false positives with extensions that happen to contain these substrings (e.g., .first matching rst). Exact string comparison should be used to ensure the correct color is applied.
if (type === 'rst' || type === 'adoc') return 'success'
There was a problem hiding this comment.
Code Review
This pull request adds support for .rst and .adoc file formats to the knowledge base by updating the parser selection logic, UI localization files, and file upload components. Feedback suggests consolidating the icon and color mapping logic in DocumentsTab.vue for these new formats and .txt files to improve maintainability and reduce redundancy.
| if (type.includes('rst') || type.includes('adoc')) return 'mdi-file-document-outline' | ||
| if (type.includes('md') || type.includes('markdown')) return 'mdi-language-markdown' | ||
| if (type.includes('txt')) return 'mdi-file-document-outline' |
There was a problem hiding this comment.
The icon mapping for .rst, .adoc, and .txt files can be consolidated since they all use the same icon (mdi-file-document-outline). This improves maintainability and reduces redundancy in the icon selection logic.
if (type.includes('md') || type.includes('markdown')) return 'mdi-language-markdown'
if (type.includes('rst') || type.includes('adoc') || type.includes('txt')) return 'mdi-file-document-outline'
| if (type.includes('rst') || type.includes('adoc')) return 'success' | ||
| if (type.includes('md')) return 'info' | ||
| if (type.includes('txt')) return 'success' |
There was a problem hiding this comment.
The color mapping for .rst, .adoc, and .txt files can be merged as they all share the 'success' color status. This keeps the mapping logic concise and easier to update in the future.
if (type.includes('md')) return 'info'
if (type.includes('rst') || type.includes('adoc') || type.includes('txt')) return 'success'
Summary
.rstand.adocfiles through the existing MarkItDown parserFixes #8213.
To verify
python -m pytest tests/test_epub_parser.py -qpython -m ruff check astrbot/core/knowledge_base/parsers/util.py tests/test_epub_parser.pypython -m py_compile astrbot/core/knowledge_base/parsers/util.py tests/test_epub_parser.pypnpm install --frozen-lockfileindashboard/pnpm typecheckindashboard/Note: direct
pnpm exec eslint src/views/knowledge-base/components/DocumentsTab.vuecould not find an ESLint config in this checkout, so I did not run the repo'slint --fixscript.Summary by Sourcery
Extend knowledge base text parsing and dashboard upload support to handle additional markup formats.
New Features:
Documentation:
Tests: