feat: Skip extract file content with txt or markdown file ✨#2898
feat: Skip extract file content with txt or markdown file ✨#2898
Conversation
lethemanh
requested review from
JF-Cozy,
doubleface,
rezk2ll and
zatteo
as code owners
|
Warning Rate limit exceeded@lethemanh has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 8 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (12)
WalkthroughReplaces the previous in-panel content extraction with a MIME-aware extractor that reads Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)
59-65: Simplify the MIME type condition.The explicit checks for
text/plainandtext/markdownare redundant sincemime.startsWith('text/')already covers all text/* MIME types including these two.Apply this diff to simplify the condition:
- if ( - mime === 'text/plain' || - mime === 'text/markdown' || - mime.startsWith('text/') - ) { + if (mime.startsWith('text/')) { textContent = await fileBlob.text() } else {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and publish
🔇 Additional comments (1)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)
66-71: RemoveJSON.stringifywrapping—it breaks downstream string consumption.Line 70 wraps
rawTextContentwithJSON.stringify, buttextContentmust be a string for downstream usage:roughTokensEstimation(textContent)calls.lengthon it, andgetSummaryUserPrompt(textContent)interpolates it directly into a template. IfextractTextreturns a string (consistent withfileBlob.text()for text files),JSON.stringifywill add quotes and escape characters, breaking both token estimation and the summarization prompt. Remove theJSON.stringifywrapper and userawTextContentdirectly, or if it's structured data, extract the text property instead.
lethemanh
force-pushed
the
skip-extract-file-content-with-txt-or-markdown-file
branch
from
fac5314 to
c08f695
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
lethemanh
force-pushed
the
skip-extract-file-content-with-txt-or-markdown-file
branch
from
c08f695 to
c14fccf
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/cozy-viewer/src/helpers.js (1)
92-98: Consider simplifying the redundant MIME type checks.The explicit checks for
'text/plain'and'text/markdown'are redundant since they're both covered bynormalizedMime.startsWith('text/'). Unless there's a specific reason to highlight these types, the function could be simplified to just thestartsWithcheck.Apply this diff to simplify:
export const isTextFile = mime => { const normalizedMime = mime?.toLowerCase() || '' - return ( - normalizedMime === 'text/plain' || - normalizedMime === 'text/markdown' || - normalizedMime.startsWith('text/') - ) + return normalizedMime.startsWith('text/') }packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (2)
52-62: Consider adding error handling for JSON.stringify edge cases.While the current implementation should work for most cases,
JSON.stringifycan throw exceptions on circular references or unsupported types (e.g., BigInt). Adding a try-catch or validating the structure could prevent unexpected failures.Apply this diff to add error handling:
const extractFileContent = async (client, fileBlob, file) => { if (isTextFile(file.mime)) { return await fileBlob.text() } const rawTextContent = await extractText(client, fileBlob, { name: file.name, mime: file.mime }) - return rawTextContent ? JSON.stringify(rawTextContent) : '' + try { + return rawTextContent ? JSON.stringify(rawTextContent) : '' + } catch (err) { + logger.error('Error stringifying extracted content:', err) + return '' + } }
64-74: Consider validating file size before extraction for efficiency.The current flow extracts the entire file content before validating its size (Line 79 extracts, Line 81 validates). For very large files, this means unnecessary processing before rejection. Consider checking the file size or blob size earlier in the flow, though this would require estimating tokens from file size rather than actual content length.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx(2 hunks)packages/cozy-viewer/src/helpers.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)
packages/cozy-viewer/src/helpers.js (4)
isTextFile(92-99)isTextFile(92-99)roughTokensEstimation(83-85)roughTokensEstimation(83-85)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and publish
🔇 Additional comments (1)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)
76-101: Good refactoring improves code organization.The extraction of
extractFileContentandvalidateContentSizeinto separate functions improves modularity and makes thesummarizeFilefunction easier to understand. The error handling flow is preserved correctly, and the integration of the MIME-aware content extraction is well-implemented.
| error.code = 'DOCUMENT_TOO_LARGE' | ||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
You should move these functions outside the component if they do not use state (maybe in same helpers?)
packages/cozy-viewer/src/helpers.js
Outdated
| return ( | ||
| normalizedMime === 'text/plain' || | ||
| normalizedMime === 'text/markdown' || | ||
| normalizedMime.startsWith('text/') |
There was a problem hiding this comment.
I think the 2 first conditions have no point 😁
There was a problem hiding this comment.
as Théo said that's dead code.
suggestion:
/**
* Checks whether a MIME type represents a `text/*` media type.
*
* @param {string | undefined} mime - MIME type to check
* @returns {boolean} True if the MIME type starts with `text/`
*/
export const isTextFile = (mime?: string): boolean =>
typeof mime === 'string' && mime.toLowerCase().startsWith('text/')also the name of the function is a bit off, i would suggest isTextMimeType
|
|
||
| const extractFileContent = async (client, fileBlob, file) => { | ||
| if (isTextFile(file.mime)) { | ||
| return await fileBlob.text() |
There was a problem hiding this comment.
| return await fileBlob.text() | |
| return fileBlob.text() |
The await is pointless here
There was a problem hiding this comment.
Without await this function return a promise
| name: file.name, | ||
| mime: file.mime | ||
| }) | ||
| return rawTextContent ? JSON.stringify(rawTextContent) : '' |
There was a problem hiding this comment.
The returned data is not consistent here. In line 54, you return a text, but here you JSON.stringify the content extracted from the AI client, and if it fails, you return a string.
So here the caller needs to guess if the returned data is a string or a serialized object?
This might cause problems in the future.
Also, we silently fail. Is this expected? i would suggest throwing an error and handing it in the caller side
if (!extracted) {
throw new Error(`Failed to extract content from ${file.name}`)
}| const validateContentSize = textContent => { | ||
| const summaryConfig = flag('drive.summary') | ||
| if ( | ||
| summaryConfig?.maxTokens && | ||
| roughTokensEstimation(textContent) > summaryConfig.maxTokens | ||
| ) { | ||
| const error = new Error('DOCUMENT_TOO_LARGE') | ||
| error.code = 'DOCUMENT_TOO_LARGE' | ||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
A few things are wrong with this function:
- it is tightly coupled with flags
- it should not be responsible for config discovery
- that error is ... i don't know what to call it
my suggestions
1 - create an Error for this usecase
export class DocumentTooLargeError extends Error {
readonly code = 'DOCUMENT_TOO_LARGE'
constructor() {
super('DOCUMENT_TOO_LARGE')
}
}2 - the validateContentSize should take only two params: the content and max tokens, and if you don't have any max tokens simply return;
const validateContentSize = (
textContent: string,
maxTokens?: number
): void => {
if (!maxTokens) return
const tokens = roughTokensEstimation(textContent)
if (tokens > maxTokens) {
throw new DocumentTooLargeError()
}
}3 - simplify the call
const { maxTokens } = flag('drive.summary') ?? {}
validateContentSize(textContent, maxTokens)
packages/cozy-viewer/src/helpers.js
Outdated
| return ( | ||
| normalizedMime === 'text/plain' || | ||
| normalizedMime === 'text/markdown' || | ||
| normalizedMime.startsWith('text/') |
There was a problem hiding this comment.
as Théo said that's dead code.
suggestion:
/**
* Checks whether a MIME type represents a `text/*` media type.
*
* @param {string | undefined} mime - MIME type to check
* @returns {boolean} True if the MIME type starts with `text/`
*/
export const isTextFile = (mime?: string): boolean =>
typeof mime === 'string' && mime.toLowerCase().startsWith('text/')also the name of the function is a bit off, i would suggest isTextMimeType
lethemanh
force-pushed
the
skip-extract-file-content-with-txt-or-markdown-file
branch
from
c14fccf to
d4819b2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx(6 hunks)packages/cozy-viewer/src/Panel/AI/helpers.js(1 hunks)packages/cozy-viewer/src/helpers.js(1 hunks)packages/cozy-viewer/src/helpers/ContentExtractionError.js(1 hunks)packages/cozy-viewer/src/helpers/DocumentTooLargeError.js(1 hunks)packages/cozy-viewer/src/locales/en.json(1 hunks)packages/cozy-viewer/src/locales/fr.json(1 hunks)packages/cozy-viewer/src/locales/ru.json(1 hunks)packages/cozy-viewer/src/locales/vi.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cozy-viewer/src/helpers.js
🧰 Additional context used
🧬 Code graph analysis (3)
packages/cozy-viewer/src/helpers/ContentExtractionError.js (1)
packages/playgrounds/src/common/App.jsx (1)
Error(30-32)
packages/cozy-viewer/src/helpers/DocumentTooLargeError.js (1)
packages/playgrounds/src/common/App.jsx (1)
Error(30-32)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)
packages/cozy-viewer/src/Panel/AI/helpers.js (4)
extractFileContent(16-33)extractFileContent(16-33)validateContentSize(41-49)validateContentSize(41-49)
🔇 Additional comments (12)
packages/cozy-viewer/src/locales/fr.json (1)
89-90: LGTM! Clean locale update.The new
extractContenterror key has been properly added to support the new content extraction error handling introduced in this PR.packages/cozy-viewer/src/locales/en.json (1)
89-90: LGTM! Consistent locale update.The English translation for the new
extractContenterror message is clear and aligns with the new error handling flow.packages/cozy-viewer/src/locales/vi.json (1)
89-90: LGTM! Locale update looks good.The Vietnamese translation for the new error message has been properly added.
packages/cozy-viewer/src/helpers/ContentExtractionError.js (1)
1-7: LGTM! Well-structured error class.The
ContentExtractionErrorfollows a clean pattern with consistent error code and name properties. This addresses the feedback from previous reviews about creating dedicated error classes.packages/cozy-viewer/src/helpers/DocumentTooLargeError.js (1)
1-7: LGTM! Consistent error class implementation.The
DocumentTooLargeErrorfollows the same clean pattern asContentExtractionError. This addresses previous review feedback about creating dedicated error types.packages/cozy-viewer/src/locales/ru.json (1)
89-90: LGTM! Locale update is consistent.The Russian translation for the new error message has been properly added.
packages/cozy-viewer/src/Panel/AI/helpers.js (1)
41-49: LGTM! Clean implementation of size validation.The
validateContentSizefunction follows the simplified pattern suggested in previous reviews:
- Early return when maxTokens is not provided
- Clear token estimation and validation logic
- Appropriate error throwing
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (5)
8-8: LGTM! Clean refactoring of extraction logic.The import changes and usage of
extractFileContentproperly externalize the content extraction logic as suggested in previous reviews. The helper is correctly invoked with all required parameters.Also applies to: 24-24, 55-55
57-58: LGTM! Simplified size validation.The size validation logic has been cleanly extracted and simplified as suggested in previous reviews. The
maxTokensis properly retrieved from the flag and passed to the validation helper.
80-95: LGTM! Proper memoization with useCallback.Converting
persistedSummaryto useuseCallbackwithclientas a dependency is correct. This ensures the function reference remains stable between renders whenclientdoesn't change, which is important since it's used in thefetchSummarydependencies.
132-140: LGTM! Comprehensive error handling.The enhanced error handling properly distinguishes between different error types:
DOCUMENT_TOO_LARGEfor oversized documentsCONTENT_EXTRACTION_FAILEDfor extraction failures- Generic fallback for other errors
Each error code correctly maps to the corresponding locale key added in this PR.
151-151: LGTM! Correct dependency array.The
fetchSummarydependencies are complete and accurate:
client,file: used in the summarization flowpersistedSummary: correctly included now that it's memoizedt: used for error message translations
lethemanh
force-pushed
the
skip-extract-file-content-with-txt-or-markdown-file
branch
from
d4819b2 to
55eac76
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)
185-195: Remove outdated PropTypes.The PropTypes defined here (
isLoading,summary,onStop,onSend) don't match the actual props received by the component, which only acceptsclassName. These appear to be leftover from an earlier implementation.🔎 Apply this diff to fix the PropTypes:
AIAssistantPanel.propTypes = { - isLoading: PropTypes.bool, - summary: PropTypes.string, - onStop: PropTypes.func, - onSend: PropTypes.func + className: PropTypes.string } -AIAssistantPanel.defaultProps = { - isLoading: false, - summary: '' -} +AIAssistantPanel.defaultProps = {}
♻️ Duplicate comments (1)
packages/cozy-viewer/src/Panel/AI/helpers.js (1)
16-33: Remove unnecessary JSON.stringify on line 32.The
JSON.stringify(content)on line 32 wraps an already-string value in extra quotes. BothfileBlob.text()andextractTextreturn plain strings, so stringifying them produces'"actual content"'instead of'actual content'. This corrupts the prompt sent to the AI model and inflates token counts.🔎 Apply this diff to return the content directly:
if (!content || content.trim().length === 0) { throw new ContentExtractionError() } - return JSON.stringify(content) + return content }
🧹 Nitpick comments (1)
packages/cozy-viewer/src/locales/en.json (1)
89-90: The JSON structure and error key are correct.The new
extractContenterror message is clear and grammatically correct. For better consistency with other error messages in this section (like line 88's "Please try again"), consider adding user guidance.Optional: Add user action for consistency
- "extractContent": "Failed to extract content from file" + "extractContent": "Failed to extract content from file. Please try again."This would align with the
summaryerror pattern on line 88 and provide a clearer user experience.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx(6 hunks)packages/cozy-viewer/src/Panel/AI/LoadingState.jsx(1 hunks)packages/cozy-viewer/src/Panel/AI/PanelHeader.jsx(1 hunks)packages/cozy-viewer/src/Panel/AI/SummaryContent.jsx(1 hunks)packages/cozy-viewer/src/Panel/AI/helpers.js(1 hunks)packages/cozy-viewer/src/helpers.js(1 hunks)packages/cozy-viewer/src/helpers/ContentExtractionError.js(1 hunks)packages/cozy-viewer/src/helpers/DocumentTooLargeError.js(1 hunks)packages/cozy-viewer/src/locales/en.json(1 hunks)packages/cozy-viewer/src/locales/fr.json(1 hunks)packages/cozy-viewer/src/locales/ru.json(1 hunks)packages/cozy-viewer/src/locales/vi.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/cozy-viewer/src/helpers/ContentExtractionError.js
- packages/cozy-viewer/src/locales/fr.json
- packages/cozy-viewer/src/helpers/DocumentTooLargeError.js
- packages/cozy-viewer/src/locales/ru.json
- packages/cozy-viewer/src/locales/vi.json
🧰 Additional context used
🧬 Code graph analysis (4)
packages/cozy-viewer/src/Panel/AI/LoadingState.jsx (1)
packages/cozy-doctypes/src/contacts/Contact.js (1)
PropTypes(2-2)
packages/cozy-viewer/src/Panel/AI/SummaryContent.jsx (1)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (2)
summary(34-34)error(35-35)
packages/cozy-viewer/src/Panel/AI/helpers.js (4)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (2)
client(29-29)error(35-35)packages/cozy-viewer/src/helpers.js (4)
isTextMimeType(92-93)isTextMimeType(92-93)roughTokensEstimation(83-85)roughTokensEstimation(83-85)packages/cozy-viewer/src/helpers/ContentExtractionError.js (1)
ContentExtractionError(1-7)packages/cozy-viewer/src/helpers/DocumentTooLargeError.js (1)
DocumentTooLargeError(1-7)
packages/cozy-viewer/src/Panel/AI/PanelHeader.jsx (1)
packages/cozy-doctypes/src/contacts/Contact.js (1)
PropTypes(2-2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and publish
🔇 Additional comments (9)
packages/cozy-viewer/src/helpers.js (1)
87-93: LGTM! Clean implementation.The function correctly checks for text MIME types with proper type validation and case-insensitive matching. The implementation addresses the previous review feedback by removing dead code and simplifying the logic.
packages/cozy-viewer/src/Panel/AI/LoadingState.jsx (1)
1-43: LGTM! Well-structured loading component.The component is cleanly implemented with proper PropTypes validation and uses Cozy UI components appropriately. The loading state UI with a progress bar and stop button provides good UX.
packages/cozy-viewer/src/Panel/AI/PanelHeader.jsx (1)
1-28: LGTM! Clean header component with good accessibility.The component is well-structured with proper PropTypes validation and includes appropriate ARIA labels for the close button. Good use of Cozy UI components.
packages/cozy-viewer/src/Panel/AI/SummaryContent.jsx (1)
1-53: LGTM! Well-designed summary content component.The component handles different states (summary, error, loading) appropriately with conditional rendering. PropTypes are correctly defined, and the UI provides clear visual feedback with proper use of Cozy UI components.
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (3)
78-93: LGTM! Proper memoization of async callback.The conversion of
persistedSummarytouseCallbackwith correct dependencies ensures stable function identity and prevents unnecessary re-renders. This addresses the previous review feedback about moving functions outside or properly memoizing them.
53-56: LGTM! Clean content extraction and validation.The refactored flow using
extractFileContentandvalidateContentSizeproperly separates concerns and addresses previous review feedback. The maxTokens configuration is correctly retrieved from the flag, and validation is cleanly delegated to the helper.
161-182: LGTM! Improved component composition.The refactored rendering using
PanelHeader,SummaryContent, andLoadingStatecomponents creates a cleaner, more maintainable structure. The conditional rendering logic is clear and appropriate.packages/cozy-viewer/src/Panel/AI/helpers.js (2)
41-49: LGTM! Clean size validation implementation.The function properly validates content size with early return when no limit is set, uses token estimation correctly, and throws appropriate errors. This implementation matches the suggested approach from previous reviews.
57-65: LGTM! Well-structured error message mapping.The function provides clean error code to translation key mapping with a sensible fallback for unknown errors. Good separation of concerns.
lethemanh
force-pushed
the
skip-extract-file-content-with-txt-or-markdown-file
branch
from
55eac76 to
9f29be0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)
188-198: Remove or correct PropTypes that don't match the component signature.The component only accepts
className(line 27), but PropTypes defineisLoading,summary,onStop, andonSendwhich are not props but internal state. This is misleading for consumers of this component.🔎 Apply this diff to fix the PropTypes:
AIAssistantPanel.propTypes = { - isLoading: PropTypes.bool, - summary: PropTypes.string, - onStop: PropTypes.func, - onSend: PropTypes.func + className: PropTypes.string } - -AIAssistantPanel.defaultProps = { - isLoading: false, - summary: '' -}
🧹 Nitpick comments (1)
packages/cozy-viewer/src/Panel/AI/SummaryContent.jsx (1)
28-34: Consider using Typography's color prop for consistency.The inline style works but could be more consistent with Cozy UI patterns.
🔎 Alternative approach using Typography's built-in color handling:
- <Typography className="u-mb-1"> - {error ? ( - <span style={{ color: 'var(--errorColor)' }}>{error}</span> - ) : ( - summary - )} + <Typography className="u-mb-1" color={error ? 'error' : undefined}> + {error || summary} </Typography>Note: Verify that Typography supports
color="error"in your Cozy UI version.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx(6 hunks)packages/cozy-viewer/src/Panel/AI/LoadingState.jsx(1 hunks)packages/cozy-viewer/src/Panel/AI/PanelHeader.jsx(1 hunks)packages/cozy-viewer/src/Panel/AI/SummaryContent.jsx(1 hunks)packages/cozy-viewer/src/Panel/AI/helpers.js(1 hunks)packages/cozy-viewer/src/helpers.js(1 hunks)packages/cozy-viewer/src/helpers/ContentExtractionError.js(1 hunks)packages/cozy-viewer/src/helpers/DocumentTooLargeError.js(1 hunks)packages/cozy-viewer/src/locales/en.json(1 hunks)packages/cozy-viewer/src/locales/fr.json(1 hunks)packages/cozy-viewer/src/locales/ru.json(1 hunks)packages/cozy-viewer/src/locales/vi.json(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/cozy-viewer/src/Panel/AI/PanelHeader.jsx
- packages/cozy-viewer/src/helpers/DocumentTooLargeError.js
- packages/cozy-viewer/src/locales/en.json
- packages/cozy-viewer/src/Panel/AI/helpers.js
- packages/cozy-viewer/src/locales/fr.json
- packages/cozy-viewer/src/helpers/ContentExtractionError.js
- packages/cozy-viewer/src/locales/vi.json
- packages/cozy-viewer/src/locales/ru.json
- packages/cozy-viewer/src/Panel/AI/LoadingState.jsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: lethemanh
Repo: linagora/cozy-libs PR: 2898
File: packages/cozy-viewer/src/Panel/AI/helpers.js:16-33
Timestamp: 2025-12-18T03:46:08.870Z
Learning: In packages/cozy-viewer/src/Panel/AI/helpers.js, the extractFileContent function uses JSON.stringify to wrap the extracted text content. This is intentional sanitization to escape special characters that can cause failures on the LLM side, as documented in PR #2897.
📚 Learning: 2025-12-18T03:46:08.870Z
Learnt from: lethemanh
Repo: linagora/cozy-libs PR: 2898
File: packages/cozy-viewer/src/Panel/AI/helpers.js:16-33
Timestamp: 2025-12-18T03:46:08.870Z
Learning: In packages/cozy-viewer/src/Panel/AI/helpers.js, the extractFileContent function uses JSON.stringify to wrap the extracted text content. This is intentional sanitization to escape special characters that can cause failures on the LLM side, as documented in PR #2897.
Applied to files:
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsxpackages/cozy-viewer/src/helpers.jspackages/cozy-viewer/src/Panel/AI/SummaryContent.jsx
🧬 Code graph analysis (1)
packages/cozy-viewer/src/Panel/AI/SummaryContent.jsx (2)
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (2)
summary(34-34)error(35-35)packages/cozy-doctypes/src/contacts/Contact.js (1)
PropTypes(2-2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and publish
🔇 Additional comments (3)
packages/cozy-viewer/src/helpers.js (1)
87-93: LGTM!The implementation is clean and addresses the previous review feedback. The function correctly normalizes the MIME type to lowercase and checks for the
text/prefix, with proper type guarding.packages/cozy-viewer/src/Panel/AI/SummaryContent.jsx (1)
11-53: Well-structured component with clear responsibilities.The component properly separates concerns, with clear prop definitions and conditional rendering logic. The use of PropTypes ensures type safety at runtime.
packages/cozy-viewer/src/Panel/AI/AIAssistantPanel.jsx (1)
50-76: Excellent refactoring with improved separation of concerns.The refactoring successfully:
- Delegates content extraction and validation to dedicated helpers
- Uses centralized error messaging via
getErrorMessage- Separates UI concerns into
PanelHeader,SummaryContent, andLoadingStatecomponents- Properly memoizes callbacks with correct dependencies
The code is now more maintainable and testable.
Also applies to: 99-142, 160-185
lethemanh
force-pushed
the
skip-extract-file-content-with-txt-or-markdown-file
branch
from
9f29be0 to
be35042
Compare
3 participants
Summary by CodeRabbit
Bug Fixes
New Features
Refactor
New Components
✏️ Tip: You can customize this high-level summary in your review settings.