[Feature] Add Markdown-Aware Text Replacement for Google Docs Action #19049
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdds Markdown-aware replace flow: new app method Changes
Sequence Diagram(s)sequenceDiagram
participant Action as Replace Text Action
participant App as google_docs.app
participant Parser as markdown-parser
participant Docs as Google Docs API
Note over Action: Read enableMarkdown flag
alt enableMarkdown = true
Action->>App: replaceTextWithMarkdown(docId, textToReplace, markdownReplacement, matchCase, tabIds)
App->>Parser: parse(markdownReplacement) → { replacementText, markdownFormatting }
App->>Docs: batchUpdate ReplaceAllText(replacementText, matchCase, tabIds)
Docs-->>App: replace result
alt markdownFormatting exists
App->>Docs: getDocument(docId)
Docs-->>App: updated document
App->>Parser: buildFormattingRequestsForReplacement(markdownFormatting, doc, replacementText)
Parser-->>App: formattingRequests
App->>Docs: batchUpdate formattingRequests
Docs-->>App: formatting applied
end
App-->>Action: return updated document
else enableMarkdown = false
Action->>App: replaceText(docId, textToReplace, containsText, tabsCriteria)
App->>Docs: batchUpdate ReplaceAllText (plain)
Docs-->>App: replace result
App-->>Action: return updated document
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (17)
🧰 Additional context used🧬 Code graph analysis (3)components/google_docs/common/markdown-parser.mjs (1)
components/google_docs/actions/replace-text/replace-text.mjs (2)
components/google_docs/google_docs.app.mjs (1)
⏰ 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). (4)
🔇 Additional comments (4)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
components/google_docs/actions/replace-text/replace-text.mjs(3 hunks)components/google_docs/google_docs.app.mjs(1 hunks)components/google_docs/package.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/google_docs/actions/replace-text/replace-text.mjs (6)
components/google_docs/google_docs.app.mjs (2)
text(302-302)doc(124-126)components/google_docs/actions/append-image/append-image.mjs (1)
doc(39-39)components/google_docs/actions/append-text/append-text.mjs (1)
doc(39-39)components/google_docs/actions/create-document/create-document.mjs (1)
doc(73-73)components/google_docs/actions/replace-image/replace-image.mjs (1)
doc(45-45)components/google_docs/sources/new-or-updated-document/new-or-updated-document.mjs (1)
doc(45-45)
components/google_docs/google_docs.app.mjs (2)
components/google_docs/common/markdown-parser.mjs (10)
parseResult(231-234)formattingRequests(30-30)currentIndex(31-31)currentIndex(132-132)text(46-46)text(73-73)text(140-140)text(187-187)textStyle(282-286)fields(287-291)components/google_docs/actions/replace-text/replace-text.mjs (2)
doc(101-101)text(87-98)
🪛 Biome (2.1.2)
components/google_docs/actions/replace-text/replace-text.mjs
[error] 88-88: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
⏰ 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). (5)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Verify TypeScript components
- GitHub Check: Publish TypeScript components
- GitHub Check: Ensure component commits modify component versions
jcortes
force-pushed
the
google-docs-add-md-aware-text-replacement
branch
2 times, most recently
from
c5a896a to
964d2e1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (16)
components/google_docs/actions/append-image/append-image.mjs(1 hunks)components/google_docs/actions/append-text/append-text.mjs(1 hunks)components/google_docs/actions/create-document-from-template/create-document-from-template.mjs(1 hunks)components/google_docs/actions/create-document/create-document.mjs(1 hunks)components/google_docs/actions/find-document/find-document.mjs(1 hunks)components/google_docs/actions/get-document/get-document.mjs(1 hunks)components/google_docs/actions/get-tab-content/get-tab-content.mjs(1 hunks)components/google_docs/actions/insert-page-break/insert-page-break.mjs(1 hunks)components/google_docs/actions/insert-table/insert-table.mjs(1 hunks)components/google_docs/actions/insert-text/insert-text.mjs(1 hunks)components/google_docs/actions/replace-image/replace-image.mjs(1 hunks)components/google_docs/actions/replace-text/replace-text.mjs(3 hunks)components/google_docs/google_docs.app.mjs(1 hunks)components/google_docs/package.json(1 hunks)components/google_docs/sources/new-document-created/new-document-created.mjs(1 hunks)components/google_docs/sources/new-or-updated-document/new-or-updated-document.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/google_docs/actions/replace-text/replace-text.mjs (2)
components/google_docs/common/markdown-parser.mjs (4)
text(46-46)text(73-73)text(140-140)text(187-187)components/google_docs/google_docs.app.mjs (1)
doc(124-126)
components/google_docs/google_docs.app.mjs (1)
components/google_docs/common/markdown-parser.mjs (4)
parseResult(231-234)formattingRequests(30-30)textStyle(282-286)fields(287-291)
⏰ 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). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
- GitHub Check: pnpm publish
🔇 Additional comments (25)
components/google_docs/actions/insert-text/insert-text.mjs (1)
7-7: Clarify the rationale for the version bump.The version is incremented from 0.0.3 to 0.0.4, but there are no functional changes to this action. Version bumps should correspond to logic changes, bug fixes, or property updates. If this is intentional collateral versioning across all actions in the PR, please confirm. Otherwise, consider reverting the version to 0.0.3 to keep version numbers meaningful.
components/google_docs/actions/get-tab-content/get-tab-content.mjs (1)
7-7: Verify necessity of version bump.This action has no functional changes, yet the version is incremented from "0.0.3" to "0.0.4". Since the get-tab-content action is unrelated to the Markdown-aware text replacement feature being added in this PR, please verify whether this version bump is intentional and necessary, or if it should be reverted to avoid unnecessary noise in the version history.
components/google_docs/actions/find-document/find-document.mjs (1)
17-17: Verify the necessity of this version bump.This action has no functional changes and is unrelated to the markdown text replacement feature introduced in this PR. Version bumps typically reflect changes to the action itself.
Is this version bump intentional? Does it follow the project's versioning policy for actions with no code changes?
components/google_docs/actions/create-document-from-template/create-document-from-template.mjs (1)
16-16: Verify the necessity of this version bump.This file implements document creation from templates and has no functional changes in this PR. The PR's objective is to add Markdown-aware text replacement functionality, which should be in a replace-text action file, not this file.
Please verify:
- Is this version bump intentional for a file unrelated to the main feature?
- Are the core changes (the new
replaceTextWithMarkdownmethod,enableMarkdownoption, and replace-text action modifications) located in other files not provided for review?If the main implementation files are part of this PR, please provide them for review to ensure the Markdown-aware text replacement feature is correctly implemented.
components/google_docs/package.json (1)
3-3: LGTM!The package version bump from 0.6.0 to 0.6.1 is appropriate for the Markdown-aware text replacement feature addition.
components/google_docs/actions/insert-table/insert-table.mjs (1)
7-7: LGTM!Version bump is coordinated with the package update.
components/google_docs/actions/replace-image/replace-image.mjs (1)
7-7: LGTM!Version bump is coordinated with the package update.
components/google_docs/actions/append-text/append-text.mjs (1)
7-7: LGTM!Version bump is coordinated with the package update.
components/google_docs/actions/insert-page-break/insert-page-break.mjs (1)
7-7: LGTM!Version bump is coordinated with the package update.
components/google_docs/sources/new-document-created/new-document-created.mjs (1)
8-8: LGTM!Version bump is coordinated with the package update.
components/google_docs/actions/append-image/append-image.mjs (1)
7-7: LGTM!Version bump is coordinated with the package update.
components/google_docs/sources/new-or-updated-document/new-or-updated-document.mjs (1)
12-12: LGTM!Version bump is coordinated with the package update.
components/google_docs/actions/create-document/create-document.mjs (1)
7-7: LGTM: Version bump aligns with package release.The metadata version increment is consistent with the broader Google Docs integration update that adds Markdown-aware replacement functionality.
components/google_docs/actions/get-document/get-document.mjs (1)
8-8: LGTM: Version bump aligns with package release.components/google_docs/actions/replace-text/replace-text.mjs (4)
6-7: LGTM: Description and version updated appropriately.The description accurately reflects the new Markdown support, and the version increment follows semantic versioning for a new feature.
36-37: LGTM: Prop description clearly explains Markdown capability.
38-44: LGTM: Backward-compatible feature flag.The
enableMarkdownprop is well-documented and defaults tofalse, ensuring existing workflows continue to work unchanged.
66-103: LGTM: TDZ issue from prior review has been resolved.The temporal dead zone issue has been fixed by renaming the request object to
textObject(line 87), eliminating the variable shadowing. The branching logic correctly routes to either the new Markdown-aware method or the original plain-text replacement, and both paths properly await the document retrieval.components/google_docs/google_docs.app.mjs (7)
291-323: Past issue resolved: Now formats all matched occurrences.The prior review concern about formatting only the first occurrence has been addressed. The code correctly iterates through all matches (
matches.forEach, line 305) and generates formatting requests for each one.Note: The broader correctness issue regarding pre-existing text has been flagged separately.
330-348: LGTM: Comprehensive recursive document scanning.The function correctly handles various Google Docs structural elements (paragraphs, tables, sections, page breaks) and recurses appropriately into nested structures, addressing the prior review's concern about scanning beyond top-level paragraphs.
353-380: LGTM: Accurate index tracking through paragraph elements.The function correctly handles text runs and special elements (inline objects, breaks, references), advancing the character index appropriately for each element type according to Google Docs API indexing rules.
386-394: LGTM: Table scanning correctly delegates to row scanning.
399-407: LGTM: Row scanning correctly delegates to cell scanning.
413-420: LGTM: Cell scanning correctly recurses into nested content.
449-513: LGTM: Formatting request builder handles multiple style types correctly.The function properly translates abstract formatting directives into Google Docs API requests for paragraph styles, text styles, and bullet lists. The dynamic field construction for
updateTextStyle(lines 471-495) ensures only specified properties are updated.
jcortes
force-pushed
the
google-docs-add-md-aware-text-replacement
branch
from
964d2e1 to
f9e56aa
Compare
michelle0927
left a comment
michelle0927
left a comment
There was a problem hiding this comment.
Looks good, just one comment/question.
jcortes
force-pushed
the
google-docs-add-md-aware-text-replacement
branch
from
f9e56aa to
3659ce0
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
components/google_docs/google_docs.app.mjs (1)
203-282: Critical: The two-phase algorithm still formats pre-existing replacement text.The critical issue flagged in the previous review remains unresolved. The current implementation:
- Replaces all occurrences of
textToReplacewithreplacementText(lines 240-247)- Searches the updated document for ALL occurrences of
replacementText(line 255)- Applies formatting to every match found (lines 262-268)
This incorrectly formats pre-existing instances of
replacementTextthat were already in the document before replacement.Example:
- Document:
"Remove foo. The word bar already exists."- Replace
"foo"with"**bar**"(bold)- After step 1:
"Remove bar. The word bar already exists."- Step 2:
buildFormattingRequestsForReplacementsearches for"bar"→ finds BOTH- Result: Both "bar" instances are bolded (only the first should be)
The previous review recommended: before replacement, use
documents.getto locate alltextToReplaceranges, then compute the new positions after replacement accounting for length differences, and apply formatting only to those calculated ranges.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
components/google_docs/actions/append-image/append-image.mjs(1 hunks)components/google_docs/actions/append-text/append-text.mjs(1 hunks)components/google_docs/actions/create-document-from-template/create-document-from-template.mjs(1 hunks)components/google_docs/actions/create-document/create-document.mjs(1 hunks)components/google_docs/actions/find-document/find-document.mjs(1 hunks)components/google_docs/actions/get-document/get-document.mjs(1 hunks)components/google_docs/actions/get-tab-content/get-tab-content.mjs(1 hunks)components/google_docs/actions/insert-page-break/insert-page-break.mjs(1 hunks)components/google_docs/actions/insert-table/insert-table.mjs(1 hunks)components/google_docs/actions/insert-text/insert-text.mjs(1 hunks)components/google_docs/actions/replace-image/replace-image.mjs(1 hunks)components/google_docs/actions/replace-text/replace-text.mjs(3 hunks)components/google_docs/common/markdown-parser.mjs(1 hunks)components/google_docs/google_docs.app.mjs(1 hunks)components/google_docs/package.json(1 hunks)components/google_docs/sources/new-document-created/new-document-created.mjs(1 hunks)components/google_docs/sources/new-or-updated-document/new-or-updated-document.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/google_docs/common/markdown-parser.mjs (2)
components/google_docs/actions/replace-text/replace-text.mjs (1)
doc(101-101)components/google_docs/google_docs.app.mjs (3)
doc(124-126)replacementText(213-216)requests(219-234)
components/google_docs/actions/replace-text/replace-text.mjs (3)
components/google_docs/actions/insert-text/insert-text.mjs (1)
text(46-52)components/google_docs/common/markdown-parser.mjs (4)
text(46-46)text(73-73)text(140-140)text(187-187)components/google_docs/google_docs.app.mjs (1)
doc(124-126)
⏰ 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). (4)
- GitHub Check: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (9)
components/google_docs/actions/insert-text/insert-text.mjs (1)
7-7: LGTM! Version bump aligns with the new Markdown feature.The patch version increment is appropriate for this coordinated release that adds Markdown-aware text replacement capabilities to the package.
components/google_docs/actions/insert-table/insert-table.mjs (1)
7-7: LGTM! Consistent version bump.The version increment aligns with the coordinated package release.
components/google_docs/package.json (1)
3-3: LGTM! Package version bump is appropriate.The patch increment from 0.6.0 to 0.6.1 correctly reflects the addition of new Markdown-aware replacement functionality while maintaining backward compatibility.
components/google_docs/actions/replace-image/replace-image.mjs (1)
7-7: LGTM! Version bump is appropriate.components/google_docs/actions/append-text/append-text.mjs (1)
7-7: LGTM! Version bump is consistent with the release.components/google_docs/sources/new-or-updated-document/new-or-updated-document.mjs (1)
12-12: LGTM! Source version bump aligns with package updates.components/google_docs/actions/find-document/find-document.mjs (1)
17-17: LGTM! Version increment is appropriate.components/google_docs/actions/insert-page-break/insert-page-break.mjs (1)
7-7: LGTM! Version bump is consistent.components/google_docs/google_docs.app.mjs (1)
255-259: Note: Unable to verify implementation details in markdown-parser.mjs.The past reviews flagged issues with
buildFormattingRequestsForReplacement:
- Formatting only the first occurrence instead of all replaced instances
- Overlapping match detection causing redundant requests
Since
./common/markdown-parser.mjsis not included in the files under review, I cannot verify whether these issues persist. Please ensure the implementation in that module addresses the concerns raised in previous reviews.
jcortes
force-pushed
the
google-docs-add-md-aware-text-replacement
branch
from
3659ce0 to
7b30327
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
components/google_docs/google_docs.app.mjs (2)
203-282: Critical: Pre-existing replacement text gets incorrectly formatted.This issue persists from the previous review. The current algorithm:
- Replaces all occurrences of
textToReplacewithreplacementText(lines 240-247)- Retrieves the updated document (lines 250-252)
- Searches for ALL occurrences of
replacementTextin the entire document (line 255-259)- Applies Markdown formatting to every match found (lines 262-268)
This incorrectly formats pre-existing instances of
replacementTextthat were already in the document before replacement.Example:
- Document:
"Remove foo. The word bar already exists."- Replace
"foo"with"**bar**"(bold bar)- After replacement:
"Remove bar. The word bar already exists."- Current behavior: Both
"bar"instances get formatted- Expected: Only the first
"bar"(which replaced"foo") should be boldRecommended fix:
- Before replacement, call
this.docs().documents.get()to find alltextToReplaceoccurrences and record their{ startIndex, endIndex }- Execute the
replaceAllTextrequest- Calculate new positions:
newStart = oldStart + cumulativeDelta,newEnd = newStart + replacementText.length(wherecumulativeDeltaaccumulatesreplacementText.length - textToReplace.lengthafter each replacement)- Apply formatting requests only to those calculated ranges
224-224: Nitpick: Redundant fallback expression.The
matchCase || falseexpression is unnecessary since the parameter already has a default value offalse(line 207).Apply this diff:
- matchCase: matchCase || false, + matchCase,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
components/google_docs/actions/append-image/append-image.mjs(1 hunks)components/google_docs/actions/append-text/append-text.mjs(1 hunks)components/google_docs/actions/create-document-from-template/create-document-from-template.mjs(1 hunks)components/google_docs/actions/create-document/create-document.mjs(1 hunks)components/google_docs/actions/find-document/find-document.mjs(1 hunks)components/google_docs/actions/get-document/get-document.mjs(1 hunks)components/google_docs/actions/get-tab-content/get-tab-content.mjs(1 hunks)components/google_docs/actions/insert-page-break/insert-page-break.mjs(1 hunks)components/google_docs/actions/insert-table/insert-table.mjs(1 hunks)components/google_docs/actions/insert-text/insert-text.mjs(1 hunks)components/google_docs/actions/replace-image/replace-image.mjs(1 hunks)components/google_docs/actions/replace-text/replace-text.mjs(3 hunks)components/google_docs/common/markdown-parser.mjs(1 hunks)components/google_docs/google_docs.app.mjs(1 hunks)components/google_docs/package.json(1 hunks)components/google_docs/sources/new-document-created/new-document-created.mjs(1 hunks)components/google_docs/sources/new-or-updated-document/new-or-updated-document.mjs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
components/google_docs/actions/replace-text/replace-text.mjs (5)
components/google_docs/common/markdown-parser.mjs (4)
text(46-46)text(73-73)text(140-140)text(187-187)components/google_docs/actions/append-image/append-image.mjs (1)
doc(39-39)components/google_docs/actions/append-text/append-text.mjs (1)
doc(39-39)components/google_docs/actions/create-document/create-document.mjs (1)
doc(73-73)components/google_docs/google_docs.app.mjs (1)
doc(124-126)
components/google_docs/common/markdown-parser.mjs (1)
components/google_docs/google_docs.app.mjs (3)
doc(124-126)replacementText(213-216)requests(219-234)
components/google_docs/google_docs.app.mjs (1)
components/google_docs/common/markdown-parser.mjs (3)
parseResult(231-234)requests(337-337)formattingRequests(30-30)
⏰ 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). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (14)
components/google_docs/actions/append-text/append-text.mjs (1)
7-7: LGTM: Coordinated version bump.The version increment is appropriate as part of the broader package release that introduces Markdown-aware text replacement functionality.
components/google_docs/actions/insert-page-break/insert-page-break.mjs (1)
7-7: LGTM: Coordinated version bump.The version increment is appropriate as part of the broader package release.
components/google_docs/actions/insert-table/insert-table.mjs (1)
7-7: LGTM: Coordinated version bump.The version increment is appropriate as part of the broader package release.
components/google_docs/actions/create-document/create-document.mjs (1)
7-7: LGTM: Coordinated version bump.The version increment is appropriate as part of the broader package release.
components/google_docs/actions/get-tab-content/get-tab-content.mjs (1)
7-7: LGTM: Coordinated version bump.The version increment is appropriate as part of the broader package release.
components/google_docs/sources/new-document-created/new-document-created.mjs (1)
8-8: LGTM: Coordinated version bump.The version increment is appropriate as part of the broader package release.
components/google_docs/actions/create-document-from-template/create-document-from-template.mjs (1)
16-16: LGTM: Coordinated version bump.The version increment is appropriate as part of the broader package release.
components/google_docs/sources/new-or-updated-document/new-or-updated-document.mjs (1)
12-12: LGTM: Coordinated version bump.The version increment is appropriate as part of the broader package release.
components/google_docs/actions/replace-text/replace-text.mjs (5)
38-44: LGTM: Well-defined Markdown option.The
enableMarkdownprop is properly typed, documented, and defaults tofalseto maintain backward compatibility.
66-74: LGTM: Complete destructuring.All necessary properties are correctly extracted.
76-84: LGTM: Markdown path correctly implemented.The conditional delegates to
replaceTextWithMarkdownwith all required parameters.
85-99: TDZ issue resolved.The variable shadowing that caused a temporal dead zone error has been fixed by renaming the request object to
textObject. The plain-text replacement logic is now correct.
101-102: LGTM: Document retrieval and summary.The document is correctly retrieved with
await, and the summary message references the correctdocId.components/google_docs/google_docs.app.mjs (1)
250-252: Verify tab content is included when tabIds are specified.When
tabIdsis provided (line 208), the replacement occurs in specific tabs. However, the document retrieval at line 250 doesn't setincludeTabsContentflag. This may preventbuildFormattingRequestsForReplacementfrom finding the replaced text in tabs, causing formatting to fail or be skipped for tab replacements.Consider passing
includeTabsContent: true(ortabIds?.length > 0) to ensure tab content is included when needed.Apply this diff:
// Retrieve the updated document to find the exact position of replaced text const updatedDoc = await this.docs().documents.get({ documentId, + includeTabsContent: tabIds?.length > 0, });
jcortes
force-pushed
the
google-docs-add-md-aware-text-replacement
branch
from
7b30327 to
79db7a5
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (8)
components/google_docs/actions/replace-image/replace-image.mjs (1)
7-7: Version increment without code changes.The action version bumps from 0.0.10 to 0.0.11 with no behavioral modifications. This aligns with the package-wide versioning pattern observed across this PR.
components/google_docs/actions/append-text/append-text.mjs (1)
7-7: Version increment without code changes.components/google_docs/actions/insert-page-break/insert-page-break.mjs (1)
7-7: Version increment without code changes.components/google_docs/actions/insert-text/insert-text.mjs (1)
7-7: Version increment without code changes.components/google_docs/sources/new-document-created/new-document-created.mjs (1)
8-8: Version increment without code changes.components/google_docs/sources/new-or-updated-document/new-or-updated-document.mjs (1)
12-12: Version increment without code changes.components/google_docs/google_docs.app.mjs (1)
224-224: Redundant fallback expression.The
matchCase || falseexpression is unnecessary since the parameter already has a default value offalse(line 207).Apply this diff:
- matchCase: matchCase || false, + matchCase,components/google_docs/common/markdown-parser.mjs (1)
338-338: Inconsistent document structure access.Line 338 reads
doc?.data?.body?.content, but this assumes the caller passes the raw Axios response{ data: { body: ... } }. The current caller ingoogle_docs.app.mjs(line 250-252) does callthis.docs().documents.get()directly, which returns this structure. However, elsewhere the codebase usesthis.getDocument()which returnsdatadirectly (see google_docs.app.mjs lines 119-127). This inconsistency creates fragility.Recommended fix: Update
google_docs.app.mjsline 250-252 to usethis.getDocument(documentId)instead of the raw API call, then change line 338 here to:- const bodyContent = doc?.data?.body?.content || []; + const bodyContent = doc?.body?.content || [];This ensures consistent document structure handling across the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (17)
components/google_docs/actions/append-image/append-image.mjs(1 hunks)components/google_docs/actions/append-text/append-text.mjs(1 hunks)components/google_docs/actions/create-document-from-template/create-document-from-template.mjs(1 hunks)components/google_docs/actions/create-document/create-document.mjs(1 hunks)components/google_docs/actions/find-document/find-document.mjs(1 hunks)components/google_docs/actions/get-document/get-document.mjs(1 hunks)components/google_docs/actions/get-tab-content/get-tab-content.mjs(1 hunks)components/google_docs/actions/insert-page-break/insert-page-break.mjs(1 hunks)components/google_docs/actions/insert-table/insert-table.mjs(1 hunks)components/google_docs/actions/insert-text/insert-text.mjs(1 hunks)components/google_docs/actions/replace-image/replace-image.mjs(1 hunks)components/google_docs/actions/replace-text/replace-text.mjs(3 hunks)components/google_docs/common/markdown-parser.mjs(1 hunks)components/google_docs/google_docs.app.mjs(1 hunks)components/google_docs/package.json(1 hunks)components/google_docs/sources/new-document-created/new-document-created.mjs(1 hunks)components/google_docs/sources/new-or-updated-document/new-or-updated-document.mjs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-20T01:01:02.970Z
Learnt from: js07
Repo: PipedreamHQ/pipedream PR: 18744
File: components/slack_v2/actions/send-large-message/send-large-message.mjs:49-64
Timestamp: 2025-10-20T01:01:02.970Z
Learning: In components/slack_v2/actions/send-large-message/send-large-message.mjs, the metadata_event_payload prop is typed as string, so the code only needs to handle string-to-JSON parsing and does not need to handle object inputs.
Applied to files:
components/google_docs/actions/append-text/append-text.mjs
🧬 Code graph analysis (3)
components/google_docs/google_docs.app.mjs (1)
components/google_docs/common/markdown-parser.mjs (3)
parseResult(231-234)requests(337-337)formattingRequests(30-30)
components/google_docs/common/markdown-parser.mjs (1)
components/google_docs/google_docs.app.mjs (3)
doc(124-126)replacementText(213-216)requests(219-234)
components/google_docs/actions/replace-text/replace-text.mjs (1)
components/google_docs/google_docs.app.mjs (1)
doc(124-126)
⏰ 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). (4)
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
🔇 Additional comments (12)
components/google_docs/actions/append-image/append-image.mjs (1)
7-7: LGTM! Version bump is appropriate.The patch version increment is consistent with the coordinated release that introduces Markdown-aware text replacement functionality elsewhere in the codebase.
components/google_docs/package.json (1)
3-3: Version bump follows patch increment for a feature addition.The version increment from 0.6.0 to 0.6.1 is a patch-level bump. Per strict semantic versioning, new features typically warrant a minor version bump (e.g., 0.7.0), while patches are reserved for backward-compatible bug fixes. However, if Pipedream components follow a different versioning convention, this may be acceptable.
Please confirm that patch-level version increments are the intended strategy for feature additions in Pipedream components, or consider whether a minor version bump (0.7.0) would be more appropriate.
components/google_docs/actions/create-document/create-document.mjs (1)
7-7: Version bump on unchanged action—verify necessity.This action's version increments from 0.2.0 to 0.2.1, but the file contains no code changes beyond the version number itself. If this is part of a package-wide versioning strategy (e.g., republishing all components when the package version changes), that's acceptable.
Please confirm that versioning all actions/sources is intentional when only the package or related files change, or consider updating only the components that have behavioral changes to avoid unnecessary version inflation.
components/google_docs/actions/insert-table/insert-table.mjs (1)
7-7: Version bump acknowledged.The version increment aligns with the broader package release cycle introducing Markdown support across Google Docs actions.
components/google_docs/actions/create-document-from-template/create-document-from-template.mjs (1)
16-16: Version bump acknowledged.The version increment aligns with the broader package release cycle introducing Markdown support across Google Docs actions.
components/google_docs/actions/find-document/find-document.mjs (1)
17-17: Version bump acknowledged.The version increment aligns with the broader package release cycle introducing Markdown support across Google Docs actions.
components/google_docs/actions/get-document/get-document.mjs (1)
8-8: Version bump acknowledged.The version increment aligns with the broader package release cycle introducing Markdown support across Google Docs actions.
components/google_docs/actions/get-tab-content/get-tab-content.mjs (1)
7-7: Version bump acknowledged.The version increment aligns with the broader package release cycle introducing Markdown support across Google Docs actions.
components/google_docs/actions/replace-text/replace-text.mjs (3)
6-6: Clear description update.The updated description accurately reflects the new Markdown formatting capability.
38-44: Well-designed Markdown feature flag.The
enableMarkdownprop provides a clear, opt-in mechanism for Markdown parsing with appropriate defaults and user guidance.
76-100: Branching logic is correct.The conditional execution properly delegates to the Markdown-aware path when enabled, otherwise preserving the original plain-text replacement behavior. The TDZ issue flagged in prior reviews has been resolved.
components/google_docs/google_docs.app.mjs (1)
203-283: Markdown replacement method implementation is sound.The two-phase approach (replace then format) correctly leverages the Google Docs API's separate
replaceAllTextand formatting operations. The error handling and conditional formatting application are appropriate.
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
|
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check test reports below for more information:
|
jcortes
force-pushed
the
google-docs-add-md-aware-text-replacement
branch
from
79db7a5 to
8d01e41
Compare
jcortes
force-pushed
the
google-docs-add-md-aware-text-replacement
branch
from
8d01e41 to
dca2052
Compare
|
Hi everyone, all test cases are passed! Ready for release! Test reports
|
WHY
Resolves #19007
Summary by CodeRabbit
New Features
Chores