Repository navigation
Refresh browser automation docs for current navigation flow - #1344
Closed
lawrencecchen wants to merge 5 commits into
Closed
lawrencecchen wants to merge 5 commits into
lawrencecchen wants to merge 5 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 54cf37fef2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughAdded navigation guidance and a troubleshooting section to the Browser Automation docs page; also added corresponding localization keys across many language message files. (≤50 words) Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![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
🧹 Nitpick comments (1)
web/app/[locale]/docs/browser-automation/page.tsx (1)
280-282: Re-verify URL after recovery navigation.In the recovery flow, add a second URL check after Line 280 so the snippet fully follows the “verify before wait/snapshot” pattern.
Suggested snippet tweak
cmux browser surface:2 navigate https://example.com +cmux browser surface:2 get url cmux browser surface:2 wait --load-state complete --timeout-ms 15000 cmux browser surface:2 snapshot --interactive --compact🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/app/`[locale]/docs/browser-automation/page.tsx around lines 280 - 282, After the recovery navigation step, add a second URL verification so the snippet follows the “verify before wait/snapshot” pattern: after the existing "cmux browser surface:2 navigate https://example.com" recovery call, insert another URL check (i.e., re-verify the target URL) before calling "cmux browser surface:2 wait --load-state complete --timeout-ms 15000" and "cmux browser surface:2 snapshot --interactive --compact" to ensure the page is confirmed loaded on the expected URL prior to waiting and snapshotting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/messages/en.json`:
- Line 519: The documentation string under the JSON key "navigationDesc" uses
both "navigate" and "goto" inconsistently; update that value so command naming
matches the canonical command set (either replace "goto" with "navigate" or
explicitly mention "`goto` (alias of `navigate`)") so the text consistently
references the documented command name used elsewhere (navigationDesc).
In `@web/messages/ja.json`:
- Line 518: The sentence in navigationDesc currently mentions both
"navigateまたはgoto", causing inconsistency with the rest of the docs where the
primary command name is "navigate"; update navigationDesc to use the same
canonical command name ("navigate") everywhere (or alternatively add "goto" to
the main command list if you intend to keep both), ensuring references to open,
open-split, wait, snapshot remain unchanged and the phrasing matches the command
list and samples.
---
Nitpick comments:
In `@web/app/`[locale]/docs/browser-automation/page.tsx:
- Around line 280-282: After the recovery navigation step, add a second URL
verification so the snippet follows the “verify before wait/snapshot” pattern:
after the existing "cmux browser surface:2 navigate https://example.com"
recovery call, insert another URL check (i.e., re-verify the target URL) before
calling "cmux browser surface:2 wait --load-state complete --timeout-ms 15000"
and "cmux browser surface:2 snapshot --interactive --compact" to ensure the page
is confirmed loaded on the expected URL prior to waiting and snapshotting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fbb72aaf-2101-41ab-b143-5e28a829aa96
📒 Files selected for processing (3)
web/app/[locale]/docs/browser-automation/page.tsxweb/messages/en.jsonweb/messages/ja.json
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/messages/ko.json (1)
539-539: Use one consistent term forsurfacein Korean copy.Line 539 switches to “표면” while the rest of this section uses “서피스”, which may read as two different concepts.
✏️ Suggested wording tweak
- "navigationDesc": "open 또는 open-split을 사용해 새 브라우저 표면을 만들고 URL을 로드하세요. 이미 표면이 있고 페이지를 바꾸고 싶다면 navigate 또는 goto를 사용하세요. 어느 쪽이든 기다리거나 스냅샷을 찍기 전에 먼저 URL을 확인하세요.", + "navigationDesc": "open 또는 open-split을 사용해 새 브라우저 서피스를 만들고 URL을 로드하세요. 이미 서피스가 있고 페이지를 바꾸고 싶다면 navigate 또는 goto를 사용하세요. 어느 쪽이든 기다리거나 스냅샷을 찍기 전에 먼저 URL을 확인하세요.",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/messages/ko.json` at line 539, The translation for key "navigationDesc" uses "표면" while the rest of the section uses "서피스"; update the string value for navigationDesc to replace "표면" with "서피스" so terminology is consistent across the file (modify the value associated with the navigationDesc key).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/messages/de.json`:
- Around line 539-543: The German locale uses informal "du" in the provided
strings (navigationDesc, patternNavigateDesc, troubleBlankPageDesc) which
conflicts with the project's formal "Sie" register; update these keys to formal
phrasing (e.g., change "Verwende" → "Verwenden Sie", "du" → "Sie", "prüfe" →
"prüfen Sie", "du möchtest" → "Sie möchten", etc.) and ensure verbs and sentence
structure match formal tone consistently across navigationDesc,
patternNavigateDesc, troubleBlankPageDesc (and any nearby strings that refer to
the same actions).
---
Nitpick comments:
In `@web/messages/ko.json`:
- Line 539: The translation for key "navigationDesc" uses "표면" while the rest of
the section uses "서피스"; update the string value for navigationDesc to replace
"표면" with "서피스" so terminology is consistent across the file (modify the value
associated with the navigationDesc key).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da8c0728-17bb-4379-9adc-d0a8626525f8
📒 Files selected for processing (17)
web/messages/ar.jsonweb/messages/bs.jsonweb/messages/da.jsonweb/messages/de.jsonweb/messages/es.jsonweb/messages/fr.jsonweb/messages/it.jsonweb/messages/km.jsonweb/messages/ko.jsonweb/messages/no.jsonweb/messages/pl.jsonweb/messages/pt-BR.jsonweb/messages/ru.jsonweb/messages/th.jsonweb/messages/tr.jsonweb/messages/zh-CN.jsonweb/messages/zh-TW.json
…-browser-automation-docs
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
openvsnavigate, blank-page recovery, andjs_errorfallbacksTesting
jq empty web/messages/en.json web/messages/ja.jsonIssues
Summary by cubic
Refreshes the browser automation docs to match the get‑url‑first navigation loop, clarify when to use
open/open-splitvsnavigate, and add clear recovery steps for blank pages andjs_error. Improves examples, adds Troubleshooting, and updates i18n across all locales (with German wording cleanups); closes #1335.get urlbefore wait/snapshot and updates the debug flow; clarifiesopen/open-splitvsnavigate.js_error.Written for commit c4c8ac9. Summary will update on new commits.
Summary by CodeRabbit