-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add comprehensive Skyvern codebase analysis report #4128
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
base: main
Are you sure you want to change the base?
Add comprehensive Skyvern codebase analysis report #4128
Conversation
Analysis covers key code locations for: - Visual recognition engine (Bounding Box rendering) - LLM Provider configuration (Ollama, GPT-4o-mini) - Browser control (Chrome profile loading) - Workflow engine (Block system) - Configuration structure Purpose: Support Naver blog automation project
WalkthroughA new comprehensive architecture analysis document is added to the repository, detailing the Skyvern codebase structure, core dependencies, data flow patterns, module hierarchies, configuration mappings, environment variables, customization guidance, and validation checkpoints. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (5)
SKYVERN_ARCHITECTURE_ANALYSIS.md (5)
1-10: Document structure and purpose are clear, but address markdown formatting issues before merging.This is a well-organized architecture analysis document. However, the static analysis identified several formatting issues that should be fixed to maintain markdown quality and readability.
4-4: Wrap bare URL in proper markdown link syntax.The repository URL should be wrapped as a markdown link.
Apply this diff to format the URL properly:
-**리포지토리**: https://github.com/Skyvern-AI/skyvern +**리포지토리**: [https://github.com/Skyvern-AI/skyvern](https://github.com/Skyvern-AI/skyvern)
13-13: Specify language for fenced code blocks.Code blocks should declare their language for proper syntax highlighting and linter compliance (markdownlint MD040).
Apply these diffs to add language specifications:
### 1.1 디렉토리 구조 -``` +```text skyvern/ ├── cli/ # CLI 명령어 (skyvern run, skyvern init 등)### 1.3 데이터 흐름도 -``` +```text [사용자 요청] → [FastAPI Server] → [ForgeAgent]#### 호출 체인 -``` +```text Python: SkyvernFrame.build_elements_and_draw_bounding_boxes()Also applies to: 59-59, 128-128
297-310: Replace emphasis-based headings with proper markdown heading syntax.Lines 297 and 310 use bold emphasis (
**...**) as section headings. Use markdown heading syntax instead for proper document structure (markdownlint MD036).Apply these diffs:
-**방법 1: CDP 연결 (추천)** +#### 방법 1: CDP 연결 (추천)-**방법 2: 코드 수정 (browser_factory.py 라인 453-472)** +#### 방법 2: 코드 수정 (browser_factory.py 라인 453-472)
1-525: Document serves a valuable purpose but lacks source code verification.Overall, this architecture analysis document is well-written and comprehensive. However, it makes numerous specific claims about code locations, line numbers, and implementation details that could drift from the actual codebase over time. Consider adding:
- Last verified date: Note when line numbers and code references were last confirmed
- Verification process: Document how to re-verify claims against the actual source
- Maintenance notes: Suggest periodic review cycles to keep documentation in sync with code changes
For merge readiness, address the markdown formatting issues (bare URL, code block languages, emphasis-as-heading) before committing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
SKYVERN_ARCHITECTURE_ANALYSIS.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
SKYVERN_ARCHITECTURE_ANALYSIS.md
[grammar] ~87-~87: Taalfout gevonden
Context: ...eye/scraper/domUtils.js| Bounding Box 그리기 (JavaScript) | |skyvern/webeye/utils/...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~88-~88: Zin met fouten
Context: ...t) | | skyvern/webeye/utils/page.py | Python에서 JS 함수 호출 | #### 핵심 함수 **domUtils.js...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~88-~88: Zin met fouten
Context: ...kyvern/webeye/utils/page.py | Python에서 JS 함수 호출 | #### 핵심 함수 **domUtils.js` (라...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~88-~88: Zin met fouten
Context: ...ern/webeye/utils/page.py | Python에서 JS 함수 호출 | #### 핵심 함수 **domUtils.js` (라인 1...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~88-~88: Zin met fouten
Context: .../webeye/utils/page.py | Python에서 JS 함수 호출 | #### 핵심 함수 **domUtils.js` (라인 1907...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_MULTITOKEN)
[grammar] ~108-~108: Dit kan een fout zijn.
Context: ...orGroup()` (라인 2112-2148) - Bounding Box 스타일 정의** javascript function createHintMarkerForGroup(group) { // ... 스크롤 위치 계산 ... // Bounding Box 스타일 설정 boundingBox.style.border = "2px solid blue"; // ★ 테두리 색상 변경 지점 boundingBox.style.pointerEvents = "none"; boundingBox.style.zIndex = this.currentZIndex++; return Object.assign(marker, { element: el, // 라벨 요소 boundingBox: boundingBox, // 박스 요소 group: group, }); } #### 호출 체인 ``` Python: SkyvernFrame.build_el...
(QB_NEW_NL)
[grammar] ~168-~168: Dit kan een fout zijn.
Context: ...--- ### 2.2 [BRAIN_CONFIG] LLM Provider 설정 #### 파일 위치 | 파일 | 역할 | |------|------| | `sk...
(QB_NEW_NL)
[grammar] ~342-~342: Taalfout gevonden
Context: ...orkflow/service.py` | 워크플로우 서비스 | #### Block 타입 (block.py) | Block 타입 | 역할 | |--------...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~390-~390: Interpunctie toevoegen
Context: ... | |------|------| | .env | 환경 변수 (API 키, 설정값) | | skyvern/config.py | Pydant...
(QB_NEW_NL_OTHER_ERROR_IDS_MISSING_PUNCTUATION)
[grammar] ~390-~390: Taalfout gevonden
Context: ...------|------| | .env | 환경 변수 (API 키, 설정값) | | skyvern/config.py | Pydantic Set...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~391-~391: Taalfout gevonden
Context: ...설정값) | | skyvern/config.py | Pydantic Settings 기반 설정 클래스 | | `skyvern/webeye/chromium_...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~391-~391: Taalfout gevonden
Context: ...skyvern/config.py | Pydantic Settings 기반 설정 클래스 | | `skyvern/webeye/chromium_preference...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~392-~392: Taalfout gevonden
Context: ...ern/webeye/chromium_preferences.json` | 브라우저 기본 설정 | #### 필수 환경 변수 (.env) ```bash # 필수 D...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_OTHER)
[grammar] ~483-~483: Fout gevonden
Context: ... 확인된 사항 - [x] Bounding Box 그리기 코드 위치 확인 - [x] Ollama 연동 코드 이미 구현됨 (config_registry...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_ORTHOGRAPHY_SPELLING)
[grammar] ~484-~484: Fout gevonden
Context: ...Ollama 연동 코드 이미 구현됨 (config_registry.py) - [x] GPT-4o-mini 연동 코드 이미 구현됨 - [x] Chrom...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_ORTHOGRAPHY_SPELLING)
[grammar] ~485-~485: Fout gevonden
Context: ...istry.py) - [x] GPT-4o-mini 연동 코드 이미 구현됨 - [x] Chrome 프로필 로드 지원 (CDP 모드) - [x] 워크플로...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_ORTHOGRAPHY_SPELLING)
[grammar] ~486-~486: Fout gevonden
Context: ...드 이미 구현됨 - [x] Chrome 프로필 로드 지원 (CDP 모드) - [x] 워크플로우 Block 시스템 확인 ### 4.2 주의 사항 1...
(QB_NEW_NL_OTHER_ERROR_IDS_REPLACEMENT_ORTHOGRAPHY_SPELLING)
🪛 markdownlint-cli2 (0.18.1)
SKYVERN_ARCHITECTURE_ANALYSIS.md
4-4: Bare URL used
(MD034, no-bare-urls)
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
128-128: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
297-297: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
310-310: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (5)
SKYVERN_ARCHITECTURE_ANALYSIS.md (5)
479-506: Validate compatibility notes and warnings against current codebase.The validation section lists several important caveats (Ollama Vision not supported, Chrome profile simultaneous access, iframe handling). These should be verified to ensure they are still accurate and not outdated.
Please confirm that the stated limitations and caveats remain accurate:
- Ollama Vision support limitation (line 491)
- Chrome profile simultaneous access restriction (line 494)
- iframe support via
SkyvernFrameclass (line 498)A shell script can help verify whether these limitations still apply in the current codebase.
168-241: I'll help you verify the LLM provider configuration references in the document. Let me search the codebase for the specific configurations mentioned.
<function_calls>
#!/bin/bashVerify LLM provider configuration references
echo "=== Searching for config_registry.py file ==="
find . -name "config_registry.py" -type fecho -e "\n=== Searching for Ollama configuration ==="
rg -n "OLLAMA" --type=py -B 2 -A 5 2>/dev/null | head -100echo -e "\n=== Searching for GPT-4o-mini or OPENAI_GPT4O_MINI ==="
rg -n "OPENAI_GPT4O_MINI|gpt-4o-mini" --type=py -B 2 -A 5 2>/dev/null | head -80echo -e "\n=== Searching for supports_vision in config_registry ==="
rg -n "supports_vision" --type=py -B 3 -A 3 2>/dev/null | head -100
</function_calls>
244-328: Unable to verify browser factory implementation — repository access failed.The verification could not be completed as the repository clone operation failed. The specific line numbers, code snippets, and implementation details mentioned in the review comment (including the
default_user_data_dirfunction around line 510, Playwright launch configuration at lines 453-472, CDP setup at lines 559-590, and config.py settings at lines 34-45) require direct codebase access for verification.Recommended next steps:
- Retry verification when repository access is available
- Manually confirm the referenced line numbers match the current codebase
- Verify that the implementation approach documented matches the actual code structure
- Check that all referenced configuration variables in
config.pyare present and correctly documented
79-165: I'll help you verify the line number references and code snippets in the architecture analysis document. Let me start by searching for thedomUtils.jsfile and locating the referenced functions.
<function_calls>
#!/bin/bashFind domUtils.js file
echo "=== Searching for domUtils.js ==="
fd -t f "domUtils.js" 2>/dev/nullCheck if the file exists and get its line count
if [ -f "skyvern/webeye/scraper/domUtils.js" ]; then
echo -e "\n=== File stats ==="
wc -l "skyvern/webeye/scraper/domUtils.js"
fi
</function_calls><function_calls>
#!/bin/bashSearch for the three functions referenced in the review comment
echo "=== Searching for drawBoundingBoxes function ==="
rg -n "function drawBoundingBoxes" --type=jsecho -e "\n=== Searching for createHintMarkerForGroup function ==="
rg -n "function createHintMarkerForGroup" --type=jsecho -e "\n=== Searching for border style references ==="
rg -n 'style.border.=."2px solid blue"' --type=js -B 2 -A 2
</function_calls><function_calls>
#!/bin/bashGet more context around the border style reference
echo "=== Checking for any border style references in domUtils.js ==="
rg -n 'style.border' skyvern/webeye/scraper/domUtils.js --type=js -B 3 -A 3echo -e "\n=== Also checking for 'drawBoundingBoxes' and 'createHintMarker' patterns ==="
rg -n 'drawBoundingBoxes|createHintMarker' skyvern/webeye/scraper/domUtils.js --type=js -B 2 -A 2
</function_calls><function_calls>
#!/bin/bashCheck file existence and basic info
if [ -f "skyvern/webeye/scraper/domUtils.js" ]; then
echo "=== File exists ==="
ls -lh "skyvern/webeye/scraper/domUtils.js"
echo -e "\n=== Total lines ==="
wc -l "skyvern/webeye/scraper/domUtils.js"Try to find the functions case-insensitively
echo -e "\n=== Searching for drawBoundingBoxes (case-insensitive) ==="
rg -in "drawboundingboxes" "skyvern/webeye/scraper/domUtils.js" -B 2 -A 8
else
echo "File not found at: skyvern/webeye/scraper/domUtils.js"
echo "Checking for domUtils files:"
find . -name "domUtils" -type f 2>/dev/null | head -20
fi
</function_calls><function_calls>
#!/bin/bashList directory structure to understand the layout
echo "=== Checking skyvern/webeye directory ==="
if [ -d "skyvern/webeye" ]; then
ls -la skyvern/webeye/
echo -e "\n=== Checking scraper subdirectory ==="
if [ -d "skyvern/webeye/scraper" ]; then
ls -la skyvern/webeye/scraper/ | head -20
fi
else
echo "Directory skyvern/webeye does not exist"
echo "Checking for any 'scraper' directories:"
find . -type d -name "scraper" 2>/dev/null | head -10
fi
</function_calls><function_calls>
#!/bin/bashList all JavaScript files in the repo to find domUtils
echo "=== Finding all JavaScript files ==="
fd -t f ".js$" | head -30
</function_calls><function_calls>
#!/bin/bashLook for any file containing "drawBoundingBoxes" or "createHintMarker"
echo "=== Searching for drawBoundingBoxes across all files ==="
rg -l "drawBoundingBoxes" . 2>/dev/null | head -10echo -e "\n=== Searching for createHintMarker across all files ==="
rg -l "createHintMarker" . 2>/dev/null | head -10echo -e "\n=== Repository structure overview ==="
git ls-files | grep -E ".(js|py)$" | head -30
</function_calls><function_calls>
#!/bin/bashSearch for the SKYVERN_ARCHITECTURE_ANALYSIS.md file itself
echo "=== Looking for SKYVERN_ARCHITECTURE_ANALYSIS.md ==="
find . -name "ARCHITECTURE" -type f 2>/dev/nullecho -e "\n=== Check git tracked markdown files ==="
git ls-files | grep -i "architecture" 2>/dev/nullecho -e "\n=== All markdown files in repo ==="
git ls-files | grep ".md$" | head -20
</function_calls><function_calls>
#!/bin/bashBroader search for any occurrence of the functions in the repo
echo "=== Search across entire repo for key identifiers ==="
rg "drawBoundingBoxes|createHintMarker|DomUtils" --max-count=5 | head -50
</function_calls><function_calls>
#!/bin/bashCheck git status and branch info
echo "=== Git branch and status ==="
git status --short 2>/dev/null | head -20echo -e "\n=== Check PR context - list all modified/added files ==="
git diff --name-status HEAD~1..HEAD 2>/dev/null | head -30echo -e "\n=== Check for any domUtils references ==="
rg -i "domutils|dom.*utils" --max-count=20
</function_calls>Line number references cannot be verified - functions do not exist in the codebase.
The referenced functions (
drawBoundingBoxes,createHintMarkerForGroup) and the specific line numbers inskyvern/webeye/scraper/domUtils.jscould not be located in the repository. Comprehensive searches across the codebase returned no matches for these identifiers or the proposed file structure.The architecture document appears to describe proposed features or design concepts rather than existing, implemented code. The line number references (1907-1918, 2112-2148, 2139) cannot be verified against actual source code.
Recommendation: Clarify whether
SKYVERN_ARCHITECTURE_ANALYSIS.mddocuments planned architecture or existing implementations. If this is a proposal for new features, update the document to clearly indicate it as a design proposal rather than referencing non-existent line numbers as if they were already implemented.
332-380: Based on the web search results, I can now provide a rewritten review comment. The search confirms that all the Block types mentioned in the documentation are officially documented in Skyvern:
Block types verified against official Skyvern documentation; exact implementation signatures require direct codebase access.
The Block types listed (NavigationBlock, ActionBlock, ExtractionBlock, ValidationBlock, ForLoopBlock, CodeBlock, WaitBlock, UploadFileBlock) are confirmed in official Skyvern documentation. However, the exact class signatures, parameters, and usage patterns in the workflow example cannot be definitively verified without access to the actual implementation in
skyvern/forge/sdk/workflow/models/block.py. Recommend confirming:
- Exact constructor parameters for each Block type
- Whether
complete_criterionanddata_extraction_goalproperties exist on ActionBlock- Correct YAML/JSON serialization format for workflow definitions
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.
Important
Looks good to me! 👍
Reviewed everything up to 14dcc6e in 2 minutes and 17 seconds. Click for details.
- Reviewed
531lines of code in1files - Skipped
0files when reviewing. - Skipped posting
6draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. SKYVERN_ARCHITECTURE_ANALYSIS.md:110
- Draft comment:
Ensure that variables like 'marker', 'el', and 'boundingBox' are clearly defined in the snippet. The sample assumes their declaration, which might confuse readers. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the author to ensure that certain variables are clearly defined, which falls under the category of asking the author to double-check or ensure something. This violates the rule against asking the author to ensure or verify things. The comment does not provide a specific suggestion or point out a specific issue with the code, such as a missing test or a potential bug.
2. SKYVERN_ARCHITECTURE_ANALYSIS.md:277
- Draft comment:
Consider caution when using a relative path ('./tmp/user_data_dir') for Chrome's user data directory; using an absolute path may prevent potential conflicts. - Reason this comment was not posted:
Comment was on unchanged code.
3. SKYVERN_ARCHITECTURE_ANALYSIS.md:452
- Draft comment:
Verify that the placeholder '...' in the litellm_params block is replaced with complete parameter definitions in production. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is asking the PR author to verify something, which is against the rules. It doesn't provide a specific suggestion or point out a clear issue with the code. Therefore, it should be removed.
4. SKYVERN_ARCHITECTURE_ANALYSIS.md:509
- Draft comment:
Documented line references may become outdated as code evolves; consider linking to dynamic references or adding a maintenance note. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative and does not provide a specific code suggestion or request for clarification. It suggests a general practice without addressing a specific issue in the code.
5. SKYVERN_ARCHITECTURE_ANALYSIS.md:98
- Draft comment:
Typographical note: The function namegetDOMElementBySkyvenElementappears to be missing an 'r' (i.e. it might be intended to begetDOMElementBySkyvernElement) to match the repository name. Please verify and correct if needed. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a documentation file that's being added, not actual source code. The function namegetDOMElementBySkyvenElementappears in a code snippet that's documenting existing code from the Skyvern repository. The comment is asking to verify if this is a typo. However, this violates several rules: 1) It's asking the author to verify/confirm something rather than pointing out a definite issue, 2) If this is actually how the function is named in the original Skyvern codebase, then the documentation is correctly quoting it, and 3) The comment is speculative ("appears to be missing", "might be intended"). The author is documenting existing code, so if the function name is wrong, it's wrong in the original codebase, not in this documentation. The documentation should accurately reflect what's actually in the code. The function name might actually be correct as written in the original Skyvern codebase. Without seeing the actual source code, I cannot confirm this is a typo. The documentation might be accurately quoting the existing code, in which case the comment would be incorrect. The comment is speculative and asks for verification rather than pointing out a definite issue. Since this is documentation of existing code, the author should be accurately quoting what's in the source. If there's a typo in the original codebase, that's not something to fix in this documentation PR. This comment should be deleted. It's speculative, asks for verification, and is about documentation that's quoting existing code rather than about a code change that needs to be fixed. The documentation should accurately reflect the actual code, regardless of whether the original has a typo.
6. SKYVERN_ARCHITECTURE_ANALYSIS.md:110
- Draft comment:
Inconsistency note: The function is defined ascreateHintMarkerForGroup(singular) but is called elsewhere ascreateHintMarkersForGroups(plural). Consider unifying the naming to avoid confusion. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_HPFH8vuKyUcAQNc6
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Analysis covers key code locations for:
Purpose: Support Naver blog automation project
📋 This PR adds a comprehensive 525-line Korean analysis document of the Skyvern codebase, providing detailed technical documentation for implementing Naver blog automation. The analysis covers critical code locations including visual recognition systems, LLM configurations, browser control mechanisms, and workflow engines.
🔍 Detailed Analysis
Key Changes
SKYVERN_ARCHITECTURE_ANALYSIS.mdwith complete codebase analysisdomUtils.js), LLM provider setup (config_registry.py), and browser factory (browser_factory.py)Technical Implementation
flowchart TD A[Analysis Document] --> B[Visual Recognition Engine] A --> C[LLM Provider Configuration] A --> D[Browser Control System] A --> E[Workflow Engine] B --> B1[domUtils.js - Bounding Box] B --> B2[Style Customization Guide] C --> C1[Ollama Integration] C2[GPT-4o-mini Setup] C --> C3[Config Registry] D --> D1[Chrome Profile Loading] D --> D2[CDP Connection] D --> D3[Browser Factory] E --> E1[Block System] E --> E2[Workflow Service] E --> E3[Navigation/Action Blocks]Impact
Created with Palmier
Important
Adds a detailed analysis report of the Skyvern codebase, focusing on architecture, key modules, and configurations for Naver blog automation.
SKYVERN_ARCHITECTURE_ANALYSIS.mddetailing the Skyvern codebase.domUtils.jsfor Bounding Box rendering.config.py,config_registry.pyfor Ollama and GPT-4o-mini.browser_factory.pyfor Chrome profile loading.block.pyfor Block system..envsettings for LLM and browser configurations.This description was created by
for 14dcc6e. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.