Wegent/improve startup and fix chat flash #303
Conversation
|
Warning Rate limit exceeded@Micro66 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 57 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 (2)
WalkthroughMigrates dependency management from pip/requirements.txt to uv/pyproject.toml, bumps Python to 3.10, adds per-component start.sh scripts and usage docs, implements task‑sharing (model/schema/service/endpoints), refactors frontend message UI and streaming behavior, and updates CI/tooling to invoke tools via uv. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (Browser)
participant Frontend
participant API as Backend API
participant Service as SharedTaskService
participant DB as Database
rect rgb(220,240,255)
Note over Frontend,API: Create Share Link
User->>Frontend: Click "Share"
Frontend->>API: POST /{task_id}/share
API->>Service: create_share_link(task_id, user_id)
Service->>DB: verify ownership, create SharedTask record, generate token
Service-->>API: {share_url, share_token}
API-->>Frontend: TaskShareResponse
Frontend->>User: Show share modal
end
rect rgb(240,220,255)
Note over Frontend,API: Join Shared Task
User->>Frontend: Submit share_token (+team_id?)
Frontend->>API: POST /share/join
API->>Service: join_shared_task(token, user_id, team_id,...)
Service->>DB: decode token, fetch original task
Service->>Service: _copy_task_with_subtasks(...)
Service->>DB: create copied task & SharedTask record
Service-->>API: return copied task id
API-->>Frontend: JoinSharedTaskResponse
Frontend->>User: Navigate to copied task
end
rect rgb(240,255,220)
Note over Frontend,API: Public View
User->>Frontend: Open public share URL
Frontend->>API: GET /share/public?token=...
API->>Service: get_public_shared_task(token)
Service->>DB: fetch original task & public-friendly attachments
Service-->>API: PublicSharedTaskResponse
API-->>Frontend: Render read-only view
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
wegent-cli/setup.py (1)
19-23: Upgrade requests to address multiple security vulnerabilities.While click 8.0.0 and PyYAML 5.4.0 are secure (each fixed critical vulnerabilities upon release), requests 2.25.0 is affected by multiple CVEs including CVE-2023-32681 (Proxy-Authorization header leakage on redirects) and CVE-2024-6345. Upgrade requests to at least 2.32.x or the latest stable version.
docs/en/guides/developer/testing.md (1)
232-249: Test matrix Python versions do not align with CI configuration. Backend matrix (3.10/3.11) is correctly documented, and test-executor and test-shared (both 3.10) match CI. However, test-executor-manager is incorrectly documented as Python 3.10 when the actual CI workflow uses Python 3.11 (.github/workflows/test.yml, line 100). Update the documentation to reflect the correct Python 3.11 version for test-executor-manager tests..github/workflows/e2e-tests.yml (1)
52-56: Replaceactions/setup-pythonwithastral-sh/setup-uvfor proper cache support.Line 55 specifies
cache: 'pip', butactions/setup-pythondoes not support caching foruv. The built-in cache insetup-pythononly works with pip, pipenv, and poetry—not uv. Since the workflow usesuv sync(line 63), the cache configuration here is ineffective.Use the official uv GitHub Action instead:
- name: Set up uv uses: astral-sh/setup-uv@v5 with: enable-cache: true - name: Install backend dependencies working-directory: backend run: uv syncThis automatically handles uv's cache directories and provides proper cache hit/miss behavior.
🧹 Nitpick comments (8)
executor/pyproject.toml (1)
29-34: Missingpytestin dependencies for test configuration.The
[tool.pytest.ini_options]section is configured, butpytestis not listed in the dependencies. Consider adding a dev dependency group for testing tools.[tool.pytest.ini_options] testpaths = ["tests"] python_files = ["test_*.py"] python_classes = ["Test*"] python_functions = ["test_*"] addopts = "-v --tb=short" + +[project.optional-dependencies] +dev = [ + "pytest>=7.0.0", + "pytest-asyncio>=0.21.0", + "pytest-mock>=3.10.0", +]backend/pyproject.toml (2)
5-28: Verify [build-system] configuration.The
pyproject.tomlhas project metadata and tool configurations but lacks a[build-system]table. Depending on your build and packaging setup, this may be required for proper project packaging and build isolation.Consider adding:
[build-system] requires = ["setuptools>=68.0", "wheel"] build-backend = "setuptools.build_meta"Or if using uv exclusively:
[build-system] requires = ["hatchling"] build-backend = "hatchling.build"
11-28: Review dependency pinning strategy.The dependencies use inconsistent version specifications: some are pinned with
>=X.Y.Zwhile others include patch versions or range constraints. Consider establishing a consistent pinning strategy (e.g., all>=X.Y.Zfor flexibility vs. stricter pins for stability).docs/en/guides/developer/setup.md (1)
256-271: Executor Manager setup may be incomplete.The executor_manager setup shows
uv syncbut doesn't demonstrate how to run the development server. The backend section includes both installation and server startup; consider adding equivalent instructions for executor_manager for consistency.Consider adding:
#### Configure Environment Variables ```bash # Set environment variables export TASK_API_DOMAIN=http://localhost:8000 export CALLBACK_HOST=http://localhost:8001 export MAX_CONCURRENT_TASKS=5 export EXECUTOR_IMAGE=ghcr.io/wecode-ai/wegent-executor:1.0.13 export EXECUTOR_WORKSPACE=${HOME}/wecode-botRun Development Server
# Run with Python directly python main.py # Or use uv to run uv run python main.py</blockquote></details> <details> <summary>docs/zh/guides/developer/setup.md (1)</summary><blockquote> `142-142`: **uv installation introduced without context on first use.** Line 142 and 256 introduce `uv sync` but the Chinese guide doesn't explain what `uv` is or how it differs from pip. Readers may be confused if they're unfamiliar with the tool. Consider adding a brief explanation before the first `uv sync` command, or referencing a glossary explaining uv as the new dependency manager. Also applies to: 256-256 </blockquote></details> <details> <summary>docker/backend/Dockerfile (1)</summary><blockquote> `28-34`: **Worker count hardcoded without configurability.** Line 34 sets `--workers 4` in the CMD. For small deployments or testing (e.g., docker-compose dev setup), this may be excessive and wasteful. Consider making workers configurable via environment variable with a sensible default. ```diff -CMD ["sh", "-c", "uvicorn app.main:app --host 0.0.0.0 --port ${PORT} --workers 4"] +ENV WORKERS=4 +CMD ["sh", "-c", "uvicorn app.main:app --host 0.0.0.0 --port ${PORT} --workers ${WORKERS}"]docker/executor_manager/Dockerfile (1)
8-13: Add version pinning to uv installation for reproducible builds.The uv installation script on line 9 is fetched without version pinning, which can lead to non-reproducible Docker builds if the upstream script changes. Consider pinning the uv version explicitly.
Example:
-RUN curl -LsSf https://astral.sh/uv/install.sh | sh && \ - export PATH="/root/.local/bin:${PATH}" && \ - uv --version +RUN curl -LsSf https://astral.sh/uv/install.sh | sh && \ + export PATH="/root/.local/bin:${PATH}" && \ + uv --version && \ + uv self update-check --disableAlternatively, use a base image that already includes uv if available.
executor_manager/START_SCRIPT_USAGE.md (1)
45-60: Add language specifications to code blocks.Several fenced code blocks are missing language specifications, which helps with syntax highlighting and clarity. Consider adding language identifiers to improve documentation readability.
Examples of lines needing language specification:
- Line 45:
```→```textor```bash- Line 66: Similar
This aligns with markdown best practices and improves rendering in various documentation viewers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
executor/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (40)
.github/workflows/e2e-tests.yml(2 hunks).github/workflows/test.yml(6 hunks)AGENTS.md(2 hunks)CONTRIBUTING.md(5 hunks)README.md(2 hunks)README_zh.md(2 hunks)backend/README.md(3 hunks)backend/START_SCRIPT_USAGE.md(1 hunks)backend/pyproject.toml(2 hunks)backend/requirements.txt(0 hunks)backend/start.sh(1 hunks)docker/backend/Dockerfile(1 hunks)docker/executor/Dockerfile(2 hunks)docker/executor_manager/Dockerfile(1 hunks)docs/en/concepts/architecture.md(1 hunks)docs/en/faq.md(1 hunks)docs/en/getting-started/installation.md(4 hunks)docs/en/guides/developer/setup.md(4 hunks)docs/en/guides/developer/testing.md(3 hunks)docs/en/troubleshooting.md(1 hunks)docs/zh/concepts/architecture.md(1 hunks)docs/zh/faq.md(1 hunks)docs/zh/getting-started/installation.md(4 hunks)docs/zh/guides/developer/setup.md(4 hunks)docs/zh/guides/developer/testing.md(3 hunks)docs/zh/troubleshooting.md(1 hunks)executor/pyproject.toml(1 hunks)executor/requirements.txt(0 hunks)executor/start.sh(1 hunks)executor_manager/README.md(2 hunks)executor_manager/README_zh.md(1 hunks)executor_manager/START_SCRIPT_USAGE.md(1 hunks)executor_manager/pyproject.toml(1 hunks)executor_manager/requirements.txt(0 hunks)executor_manager/start.sh(1 hunks)frontend/.env.local(1 hunks)frontend/START_SCRIPT_USAGE.md(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(1 hunks)frontend/start.sh(1 hunks)wegent-cli/setup.py(2 hunks)
💤 Files with no reviewable changes (3)
- executor/requirements.txt
- executor_manager/requirements.txt
- backend/requirements.txt
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
wegent-cli/setup.pyfrontend/src/features/tasks/components/ChatArea.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
wegent-cli/setup.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/features/tasks/components/ChatArea.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/features/tasks/components/ChatArea.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/ChatArea.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/tasks/components/ChatArea.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/tasks/components/ChatArea.tsx
🧠 Learnings (5)
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Frontend MUST only use `NEXT_PUBLIC_*` environment variables for client-safe values
Applied to files:
frontend/.env.local
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/app/schemas/**/*.py : Backend Pydantic schemas MUST be created in `app/schemas/` directory
Applied to files:
docs/en/guides/developer/setup.md
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Update AGENTS.md when adding new modules, commands, workflows, changing code style guidelines, updating dependencies or architecture patterns
Applied to files:
AGENTS.md
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/app/services/**/*.py : Backend business logic MUST be placed in `app/services/` directory
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/alembic/versions/*.py : Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Applied to files:
backend/README.md
🧬 Code graph analysis (1)
executor/start.sh (2)
backend/start.sh (3)
cleanup(13-19)validate_port(107-120)check_port(127-161)executor_manager/start.sh (3)
cleanup(13-19)validate_port(93-106)check_port(111-145)
🪛 actionlint (1.7.9)
.github/workflows/e2e-tests.yml
60-60: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/test.yml
46-46: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
89-89: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
132-132: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 markdownlint-cli2 (0.18.1)
executor_manager/START_SCRIPT_USAGE.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
119-119: Bare URL used
(MD034, no-bare-urls)
120-120: Bare URL used
(MD034, no-bare-urls)
132-132: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
140-140: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
178-178: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
frontend/START_SCRIPT_USAGE.md
40-40: 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)
108-108: Bare URL used
(MD034, no-bare-urls)
109-109: Bare URL used
(MD034, no-bare-urls)
257-257: Bare URL used
(MD034, no-bare-urls)
backend/START_SCRIPT_USAGE.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
118-118: Bare URL used
(MD034, no-bare-urls)
119-119: Bare URL used
(MD034, no-bare-urls)
122-122: Multiple headings with the same content
(MD024, no-duplicate-heading)
backend/README.md
74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
executor/start.sh
[warning] 104-104: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 220-220: Declare and assign separately to avoid masking return values.
(SC2155)
executor_manager/start.sh
[warning] 120-120: Declare and assign separately to avoid masking return values.
(SC2155)
frontend/start.sh
[warning] 99-99: Declare and assign separately to avoid masking return values.
(SC2155)
backend/start.sh
[warning] 136-136: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 254-254: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ 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). (2)
- GitHub Check: Test wegent CLI Integration
- GitHub Check: e2e-tests
🔇 Additional comments (31)
wegent-cli/setup.py (2)
38-38: LGTM! Python 3.12 classifier added.Adding the Python 3.12 classifier is consistent with the
python_requires=">=3.10"setting and properly declares support for the latest stable Python version.
18-18: Python 3.10+ requirement is correctly set and consistent across all modules.The
python_requires=">=3.10"aligns with the classifiers (3.10, 3.11, 3.12) and matches the requirement inbackend/pyproject.toml,executor/pyproject.toml, andexecutor_manager/pyproject.toml.docs/en/concepts/architecture.md (1)
352-352: LGTM!The Python version requirement update to 3.10+ is consistent with the project-wide migration reflected in the pyproject.toml files.
docs/zh/faq.md (1)
216-218: LGTM!The Python version updates for backend and executor are consistent with the project-wide migration to Python 3.10+.
executor/pyproject.toml (1)
5-27: LGTM on project configuration.The pyproject.toml structure follows modern Python packaging standards with appropriate metadata and dependency declarations.
docs/zh/guides/developer/testing.md (2)
232-247: LGTM!The CI/CD matrix version updates (Python 3.10/3.11 for backend, 3.10 for executor-manager and shared) are consistent with the project-wide Python version bump.
460-460: LGTM!The troubleshooting instruction correctly reflects the migration from
pip installtouv syncfor dependency management.frontend/src/features/tasks/components/ChatArea.tsx (2)
489-503: Good fix for the chat flash issue.The approach of waiting for
refreshSelectedTaskDetailto complete before callingcontextResetStreamcorrectly prevents the flash of empty content. Thefinallyblock ensures cleanup happens regardless of success or failure.The 100ms delay is a pragmatic workaround for the URL change propagation timing. Consider making this configurable or using a more deterministic approach (e.g., listening for a state change event) if this proves unreliable under varying network conditions.
504-516: LGTM on the follow-up message path.The await pattern for
refreshSelectedTaskDetail(false)before clearing the stream is correct and consistent with the new task path handling. The try/catch/finally structure ensures proper cleanup.frontend/.env.local (1)
8-8: Backend port configuration must be updated to match port 9000.The frontend API URL was changed to
http://localhost:9000, but the backend is still configured to listen on port8000in bothdocker-compose.ymlandbackend/start.sh. Update the backend service port to9000or revert the frontend change to use port8000to ensure the services can communicate properly.⛔ Skipped due to learnings
Learnt from: CR Repo: wecode-ai/Wegent PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-01T06:44:24.499Z Learning: Frontend MUST only use `NEXT_PUBLIC_*` environment variables for client-safe valuesdocs/en/faq.md (1)
216-216: ✅ Python version requirements updated consistently.The Python requirement bump to 3.10+ is properly reflected in both backend and executor documentation. This aligns with the pyproject.toml changes and overall PR objectives.
Also applies to: 218-218
backend/README.md (1)
115-124: Clarify venv activation workflow after uv sync.The manual setup mixes
uv sync(which manages its own virtual environment) with explicit venv activation. Clarify whether venv activation is necessary or if uv commands run directly in the uv-managed environment.Suggest clarifying:
# After uv sync, dependencies are installed in uv's managed venv # You can either: # Option 1: Use uv directly (recommended) uv run uvicorn app.main:app --host 0.0.0.0 --port 8000 --reload # Option 2: Activate uv's venv explicitly (if needed) source .venv/bin/activate uvicorn app.main:app --host 0.0.0.0 --port 8000 --reloadexecutor_manager/pyproject.toml (1)
6-6: ✅ Python version requirement downgraded to 3.10.The minimum Python version is properly lowered from 3.11 to 3.10, aligning with the backend and overall PR objectives for version consistency.
docs/zh/concepts/architecture.md (1)
352-352: ✅ Python version requirement updated in architecture docs.Chinese documentation properly reflects the Python 3.10+ requirement, maintaining consistency with English documentation and backend configuration.
docs/en/guides/developer/setup.md (1)
13-13: ✅ Python version prerequisites updated.The minimum Python 3.10+ requirement is clearly stated and aligns across all documentation sections and configuration files.
README_zh.md (1)
237-238: ✅ Quick start updated to use start.sh workflow.The backend quick start properly prioritizes the one-click
start.shscript with a commented manual alternative usinguv syncanduvicorn. This aligns with the project's shift to start script-based workflows.CONTRIBUTING.md (2)
357-357: Test matrix Python versions expanded appropriately. Backend now tests 3.10, 3.11, 3.12 (providing good forward compatibility coverage). Aligns with bumped minimum version requirement.
80-81: Verify thatbackend/start.sh,executor/start.sh, andexecutor_manager/start.shexist and are executable.Lines 80-81 and 91-92 reference
./start.shscripts for starting services, with manual alternatives provided as fallbacks. If these scripts don't exist, are outdated, or lack proper permissions, contributors will encounter failures when following the setup instructions.Confirm these scripts are present in their respective directories, have executable permissions, and that their content correctly executes the intended startup procedures (typically wrapping
uv sync && source .venv/bin/activateor equivalent setup).AGENTS.md (1)
59-63: All start.sh scripts referenced in AGENTS.md exist and have valid syntax; no action needed.The backend/start.sh, executor/start.sh, and executor_manager/start.sh scripts at lines 59–63 and 76–81 are present and syntactically valid. The documentation is consistent with the actual implementation.
docker/backend/Dockerfile (1)
17-19: No action required—backend/pyproject.toml exists with all necessary dependencies properly declared.The file contains all major backend dependencies referenced in the Dockerfile: FastAPI (>=0.109.1), SQLAlchemy (>=2.0.25), MySQL drivers (mysql-connector-python >=8.3.0, pymysql >=1.1.0), plus supporting libraries (alembic, redis, pydantic, uvicorn, etc.). The TOML structure is valid and the uv pip install command on line 19 will correctly resolve these dependencies.
docker/executor/Dockerfile (1)
33-33: No action needed - pyproject.toml dependency migration is complete.The executor/requirements.txt file has been successfully removed and all dependencies are properly defined in executor/pyproject.toml with version constraints. No lingering references to requirements.txt exist in the executor directory.
README.md (1)
6-10: Clear and helpful documentation updates.The Python version bump is consistent with the migration, and the Claude badge addition provides context. The startup instructions effectively guide users to the new
./start.shapproach while retaining the manual alternative as a fallback. The commented manual command properly references the uv workflow.executor_manager/README_zh.md (1)
5-43: Well-structured quick-start documentation.The restructured quick-start section effectively promotes the one-click startup script while providing clear custom configuration examples and port validation guidance. The automated steps are comprehensive and user-friendly.
.github/workflows/e2e-tests.yml (1)
58-63: Approve uv installation sequence.The uv installation and sourcing of
.cargo/envis correctly sequenced. The transition from pip-based to uv-based dependency management is properly implemented in the workflow.backend/START_SCRIPT_USAGE.md (1)
1-207: Comprehensive and well-structured backend startup documentation.The guide provides clear examples, troubleshooting, and FAQ sections that will help users get started quickly. The organization mirrors the executor_manager documentation, creating consistency across the project.
frontend/START_SCRIPT_USAGE.md (1)
1-290: Well-organized frontend startup documentation.The guide effectively covers quick start, custom configuration, troubleshooting, and integration scenarios. The content is clear and comprehensive, with helpful examples for connecting to both local and remote backends.
executor_manager/start.sh (4)
110-145: Consider portability of port-checking utility.The script uses
lsofto check if a port is in use (line 115), butlsofmay not be available on all systems or minimal container environments. This could cause the script to fail silently iflsofis not installed, rather than providing a clear error message.Consider adding a fallback check or validating
lsofavailability. For example:check_port() { local port=$1 local name=$2 + if ! command -v lsof &> /dev/null; then + echo -e "${YELLOW}Warning: lsof not available, skipping port check${NC}" + return 0 + fi + if lsof -Pi :$port -sTCP:LISTEN -t >/dev/null 2>&1; thenAlternatively, consider using a more portable approach like
netstat(if available on target systems) or accepting the port check limitation gracefully.
228-254: Approve dependency installation sequence.The uv dependency sync logic is correct. Creating a virtual environment with a specified Python version and then syncing dependencies via
uv sync --pythonis the proper approach. The error handling for missingpyproject.tomland failed syncs is appropriate.
257-313: Approve comprehensive Docker and environment setup.The Docker validation, network creation, and host IP detection logic is thorough and handles multiple OS scenarios (macOS, Linux, Windows). The fallback to
host.docker.internalfor unsupported systems is a reasonable defensive approach.
318-332: Review virtual environment activation approach.Line 331 sources the virtual environment activation script but silently suppresses errors with
2>/dev/null || true. This could mask legitimate activation failures. If activation is essential for the uvicorn command to work correctly, consider whether silent failure is appropriate.Consider whether explicit error handling would be more appropriate:
-source .venv/bin/activate 2>/dev/null || true +if [ -f ".venv/bin/activate" ]; then + source .venv/bin/activate +else + echo -e "${YELLOW}Warning: Virtual environment not found${NC}" +fiThis ensures the script will still run (uvicorn should work with the PATH set by uv), but documents the assumption explicitly.
.github/workflows/test.yml (1)
168-172: test-shared dependency installation is appropriate without uv.The
test-sharedjob correctly uses pip for direct dependency installation. Unlike other backend jobs that useuv sync(which requires apyproject.toml), the shared module does not have a project configuration file. Migrating to uv would first require addingpyproject.tomlto the shared module, which is outside the scope of workflow configuration. The current pip-based approach is suitable for this module.Likely an incorrect or invalid review comment.
| ## FAQ | ||
| ## FAQ | ||
|
|
There was a problem hiding this comment.
Remove duplicate FAQ section heading.
The FAQ section heading appears twice on consecutive lines (122-123), with an extra blank line between them. Remove one instance to clean up the documentation structure.
Apply this diff:
## FAQ
-## FAQ
-
+
### Q: How to stop the service?📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## FAQ | |
| ## FAQ | |
| ## FAQ | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
122-122: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 Prompt for AI Agents
In backend/START_SCRIPT_USAGE.md around lines 121 to 123, there is a duplicated
"## FAQ" heading on consecutive lines with an extra blank line; remove the
redundant heading (and the extra blank line if present) so only a single "##
FAQ" remains to clean up the document structure.
| # Parse command line arguments | ||
| # Parse command line arguments |
There was a problem hiding this comment.
Remove duplicate comment.
Lines 46 and 47 both contain "# Parse command line arguments". Remove the duplicate.
- # Parse command line arguments
- # Parse command line arguments
- while [[ $# -gt 0 ]]; do
+ # Parse command line arguments
+ while [[ $# -gt 0 ]]; do📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Parse command line arguments | |
| # Parse command line arguments | |
| # Parse command line arguments | |
| while [[ $# -gt 0 ]]; do |
🤖 Prompt for AI Agents
In backend/start.sh around lines 46 to 47, there are two identical comment lines
"# Parse command line arguments"; remove the duplicate so only a single instance
of that comment remains (delete the second/extra line).
| echo "" | ||
|
|
||
| # Check for suspended jobs | ||
| local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Declare variable assignment separately to avoid masking return values.
Line 136 combines variable assignment with command substitution. This masks the return value, preventing proper error detection. Declare and assign in separate statements.
- local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)
+ local suspended_jobs
+ suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) | |
| local suspended_jobs | |
| suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 136-136: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In backend/start.sh around line 136, the command substitution and variable
assignment are combined which masks the command's exit status; run the jobs
pipeline first (capture its output into a temporary variable or directly to a
variable), then check the command's exit status ($?) before using the value —
i.e., perform the pipeline in one statement, inspect its exit code, and only
then assign or use the captured output so failures are detectable and handled
(avoid combining assignment and command execution that hides return values).
| # Force clear any virtual environment variables | ||
| unset VIRTUAL_ENV | ||
| unset PYTHONHOME | ||
| export PATH=$(echo $PATH | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//') |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Simplify PATH manipulation and separate variable declaration.
Line 254 combines PATH manipulation with variable assignment, masking errors. The multi-step pipeline is also fragile. Separate the declaration and quote variables for robustness.
- # Force clear any virtual environment variables
- unset VIRTUAL_ENV
- unset PYTHONHOME
- export PATH=$(echo $PATH | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//')
+ # Force clear any virtual environment variables
+ unset VIRTUAL_ENV
+ unset PYTHONHOME
+ local new_path
+ new_path=$(echo "$PATH" | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//')
+ export PATH="$new_path"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Force clear any virtual environment variables | |
| unset VIRTUAL_ENV | |
| unset PYTHONHOME | |
| export PATH=$(echo $PATH | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//') | |
| # Force clear any virtual environment variables | |
| unset VIRTUAL_ENV | |
| unset PYTHONHOME | |
| new_path=$(echo "$PATH" | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//') | |
| export PATH="$new_path" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 254-254: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In backend/start.sh around lines 251 to 254, the current one‑liner that rewrites
PATH is fragile and hides errors by combining manipulation and export; replace
it by computing a new variable (e.g. CLEAN_PATH) that removes any entries ending
with '/venv/bin' or '/.venv/bin' using a simpler, robust method (e.g. iterate
PATH entries or use awk) then export PATH by assigning using the CLEAN_PATH
variable; ensure you quote variables when expanding (e.g. export
PATH="$CLEAN_PATH") and trim any trailing colon before export.
| # Step 5: Check database connection | ||
| echo -e "${BLUE}[5/7] Checking database connection...${NC}" | ||
| DB_USER=$(grep DATABASE_URL .env | cut -d'/' -f3 | cut -d':' -f1) | ||
| DB_NAME=$(grep DATABASE_URL .env | cut -d'/' -f4) | ||
|
|
||
| if command -v mysql &> /dev/null; then | ||
| if mysql -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" -e "SELECT 1" &> /dev/null; then | ||
| echo -e "${GREEN}✓ Database connection successful${NC}" | ||
|
|
||
| # Check if database exists | ||
| if mysql -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" -e "USE $DB_NAME" &> /dev/null; then | ||
| echo -e "${GREEN}✓ Database '$DB_NAME' exists${NC}" | ||
| else | ||
| echo -e "${YELLOW}Warning: Database '$DB_NAME' does not exist${NC}" | ||
| echo -e "${YELLOW}Creating database...${NC}" | ||
| mysql -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" -e "CREATE DATABASE IF NOT EXISTS $DB_NAME CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;" | ||
| echo -e "${GREEN}✓ Database created${NC}" | ||
| fi | ||
| else | ||
| echo -e "${YELLOW}Warning: Cannot connect to MySQL database${NC}" | ||
| echo -e "${YELLOW}Please ensure MySQL is running on port $DB_PORT${NC}" | ||
| echo -e "${YELLOW}Database migrations will run automatically on first startup (development mode)${NC}" | ||
| fi | ||
| else | ||
| echo -e "${YELLOW}Warning: mysql client not found, skipping database check${NC}" | ||
| echo -e "${YELLOW}Please ensure MySQL is running and accessible${NC}" | ||
| fi | ||
| echo "" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check DATABASE_URL format in .env.example
if [ -f "backend/.env.example" ]; then
echo "=== DATABASE_URL format in .env.example ==="
grep "DATABASE_URL" backend/.env.example
fi
# Also check if there's an .env file
if [ -f "backend/.env" ]; then
echo "=== DATABASE_URL format in .env ==="
grep "DATABASE_URL" backend/.env | head -1
fiRepository: wecode-ai/Wegent
Length of output: 172
🏁 Script executed:
# Get the exact lines 315-342 from backend/start.sh to see password handling
if [ -f "backend/start.sh" ]; then
echo "=== Lines 315-342 of backend/start.sh ==="
sed -n '315,342p' backend/start.sh
fiRepository: wecode-ai/Wegent
Length of output: 1534
🏁 Script executed:
# Search for all mysql command invocations in the start.sh to see password handling
echo "=== All mysql command invocations in backend/start.sh ==="
grep -n "mysql " backend/start.sh | head -20Repository: wecode-ai/Wegent
Length of output: 599
Database connectivity check lacks password handling.
The database connection attempts on lines 321, 325, and 330 do not pass the password from DATABASE_URL. While DB_USER and DB_NAME are extracted from the .env file, the password is never parsed or provided to the mysql commands. Given the standard DATABASE_URL format (mysql+pymysql://user:password@host:port/database), the script should extract and use the password with the -p flag, otherwise connections will fail silently when MySQL requires authentication.
🤖 Prompt for AI Agents
In backend/start.sh around lines 315 to 342, the mysql client invocations do not
include the database password parsed from DATABASE_URL; extract the password
component from DATABASE_URL (handle URL-encoded characters), store it in a
DB_PASS variable, and pass it to each mysql command invocation using the mysql
client's password option (ensuring it is quoted and only supplied when
non-empty); also avoid printing the password to logs and handle the case of an
empty password by omitting the password flag so authentication behavior remains
correct.
| echo "" | ||
|
|
||
| # Check for suspended jobs | ||
| local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Declare variable assignment separately to avoid masking return values.
Line 104 combines variable assignment with command substitution. This masks the return value, preventing proper error detection if grep fails. Declare and assign in separate statements.
- local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)
+ local suspended_jobs
+ suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) | |
| local suspended_jobs | |
| suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 104-104: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In executor/start.sh around line 104, the current single-line assignment using
command substitution masks the command's exit status; change it so the pipeline
is executed first and its exit status checked, then assign the variable
separately (or enable pipefail and inspect PIPESTATUS), e.g. run the jobs/grep
pipeline, check its exit code, and only then set suspended_jobs to the command
output (or to empty) based on that check so failures from grep are not silently
ignored.
| unset VIRTUAL_ENV | ||
| unset PYTHONHOME | ||
| export PATH=$(echo $PATH | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//') |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Simplify PATH manipulation and separate variable declaration.
Line 220 combines PATH manipulation with variable assignment, masking errors from the pipeline. Additionally, the multi-step pipeline (tr → grep → tr → sed) is fragile and difficult to debug. Consider separating the declaration and using more explicit, robust logic.
- unset VIRTUAL_ENV
- unset PYTHONHOME
- export PATH=$(echo $PATH | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//')
+ unset VIRTUAL_ENV
+ unset PYTHONHOME
+ local new_path
+ new_path=$(echo "$PATH" | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//')
+ export PATH="$new_path"Additionally, quote $PATH to prevent word splitting and use a variable for the intermediate result.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| unset VIRTUAL_ENV | |
| unset PYTHONHOME | |
| export PATH=$(echo $PATH | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//') | |
| unset VIRTUAL_ENV | |
| unset PYTHONHOME | |
| new_path=$(echo "$PATH" | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//') | |
| export PATH="$new_path" |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 220-220: Declare and assign separately to avoid masking return values.
(SC2155)
| echo "" | ||
|
|
||
| # Check for suspended jobs | ||
| local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Declare variable assignment separately to avoid masking return values.
Line 99 combines variable assignment with command substitution. This masks the return value of the grep command, preventing proper error detection. Declare and assign in separate statements.
- local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)
+ local suspended_jobs
+ suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) | |
| local suspended_jobs | |
| suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) |
🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 99-99: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
In frontend/start.sh around line 99, the variable assignment combines
declaration with command substitution which masks the pipeline's exit status;
instead, declare the variable on its own line, run the pipeline separately so
you can capture and check its exit code (e.g. run the jobs | grep ... pipeline,
check $? or capture into a temp var), then assign the final value to
suspended_jobs based on that result so failures are detectable.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/app/api/endpoints/adapter/tasks.py (2)
48-49: Add missing return type hint tocall_chat_shell_cancel.Per coding guidelines, all functions must include type hints. This function is missing the return type annotation.
Apply this diff to add the return type:
-async def call_chat_shell_cancel(subtask_id: int, partial_content: str = ""): +async def call_chat_shell_cancel(subtask_id: int, partial_content: str = "") -> None:
188-193: Add missing return type hint tocancel_taskendpoint.Per coding guidelines, all functions must include type hints. This async endpoint handler is missing a return type annotation.
Apply this diff to add the return type:
-async def cancel_task( +async def cancel_task( task_id: int, background_tasks: BackgroundTasks, current_user: User = Depends(security.get_current_user), db: Session = Depends(get_db), -): +) -> dict:backend/app/api/endpoints/adapter/chat.py (1)
520-541: Avoid logging fullmodel_config/data_sources(likely to contain secrets)
model_configanddata_sourcesalmost certainly contain sensitive values (e.g., API keys indefault_headers), but they are logged atINFO:
logger.info(f"Data sources for header processing: {data_sources}")logger.info(f"Streaming chat for model_config={model_config}")This risks leaking credentials into logs.
Consider redacting or omitting sensitive fields, for example:
- logger.info(f"Raw default headers before processing: {raw_default_headers}") - logger.info(f"Data sources for header processing: {data_sources}") + logger.info("Raw default headers before processing: %s", list(raw_default_headers.keys())) + logger.debug("Data sources for header processing (redacted)") @@ - logger.info(f"Streaming chat for model_config={model_config}") + safe_model_config = {**model_config, "api_key": "***redacted***"} + logger.info("Streaming chat for model_config (redacted): %s", safe_model_config)Adjust the exact redaction strategy as needed, but avoid emitting raw secrets to logs.
🧹 Nitpick comments (22)
scripts/hooks/run-module-tests.sh (1)
109-110: Remove duplicate comment block.Lines 109–110 both contain "# Check each module for changes and run tests". Consolidate to a single comment.
-# Check each module for changes and run tests -# Check each module for changes and run tests +# Check each module for changes and run testsscripts/hooks/ai-push-gate.sh (2)
282-283: Remove duplicate comment block.Lines 282–283 both contain "# Python syntax check". Consolidate to a single comment.
- # Python syntax check - # Python syntax check + # Python syntax check
286-286: Clarify file redirection pattern (SC2188).The pattern
> "$TEMP_DIR/syntax.log"on its own line is valid but non-idiomatic. For clarity and to satisfy shellcheck, prepend a no-op command like:ortrue.- > "$TEMP_DIR/syntax.log" # Clear/create the file + : > "$TEMP_DIR/syntax.log" # Clear/create the fileApply the same fix at line 365.
Also applies to: 365-365
backend/app/api/endpoints/adapter/tasks.py (3)
283-285: Uselogging.exceptionin exception handlers to preserve stack traces.When logging errors within an exception handler, use
logging.exception()instead oflogging.error()to automatically capture and include the traceback. This aids debugging in production.Additionally, simplify the f-string by removing the redundant
str()call—f-strings coerce exceptions to strings by default.Apply this diff:
except Exception as e: - logger.error( - f"Failed to update Chat Shell task {task_id} status: {str(e)}" + logger.exception( + f"Failed to update Chat Shell task {task_id} status: {e}" )
315-317: Uselogging.exceptionin exception handlers to preserve stack traces.When logging errors within an exception handler, use
logging.exception()instead oflogging.error()to automatically capture and include the traceback. This aids debugging in production.Additionally, simplify the f-string by removing the redundant
str()call—f-strings coerce exceptions to strings by default.Apply this diff:
except Exception as e: - logger.error( - f"Failed to update task {task_id} status to CANCELLING: {str(e)}" + logger.exception( + f"Failed to update task {task_id} status to CANCELLING: {e}" )
188-325: Consider decomposingcancel_taskinto smaller helper functions.This endpoint handler is 138 lines, significantly exceeding the preferred 50-line maximum. The logic combines task verification, type detection, and two distinct cancellation flows (Chat Shell vs executor). Extracting helper functions would improve readability, testability, and maintainability.
For example:
_verify_task_permissions()→ Handle ownership and final-state checks_determine_task_type()→ Extract Chat Shell detection logic_cancel_chat_shell_task()→ Encapsulate Chat Shell-specific flow_cancel_executor_task()→ Encapsulate executor-specific flowThis refactoring can be deferred if higher-priority work takes precedence.
backend/app/services/chat/base.py (1)
24-27: Annotate shared client state and DIRECT_CHAT_SHELL_TYPES for clearer typingLogic for the shared
httpx.AsyncClientand base class is sound. To align with Ruff (RUF012) and your type-hinting guidelines, consider marking the mutable class attribute as aClassVar:-from typing import Optional +from typing import ClassVar, Optional @@ class ChatServiceBase: @@ - # Shell types that support direct chat (bypass executor) - DIRECT_CHAT_SHELL_TYPES = ["Chat"] + # Shell types that support direct chat (bypass executor) + DIRECT_CHAT_SHELL_TYPES: ClassVar[list[str]] = ["Chat"]This keeps the behavior identical while making intent explicit for type checkers.
Also applies to: 78-87
backend/app/services/adapters/task_kinds.py (1)
613-632: Attachments dict construction looks good; consider tightening model_id extractionThe new attachment-to-dict mapping for subtasks is clear and safe, including the enum/value fallback for
status. This should serialize attachments cleanly for API consumers.For
model_id, the current expression:model_id = task_crd.metadata.labels and task_crd.metadata.labels.get("modelId")can produce
{}ifmetadata.labelsis an empty dict. For slightly cleaner typing and downstream handling, consider:- model_id = task_crd.metadata.labels and task_crd.metadata.labels.get("modelId") + model_id = (task_crd.metadata.labels or {}).get("modelId")so
model_idis alwaysstr | None.Also applies to: 1073-1073
backend/app/services/attachment/attachment_service.py (1)
35-60: DocumentParser integration and status handling look correct; enhance parse error loggingThe upload flow correctly:
- Validates extension and size via
DocumentParser.- Persists an attachment with placeholder fields and
UPLOADING/PARSINGstatuses.- Updates parsed fields and marks
READY, or marksFAILEDand commits before re-raisingDocumentParseError.To improve debuggability of parser failures, consider switching the parse-error log to include a traceback:
- except DocumentParseError as e: - logger.error(f"Document parsing failed for attachment {attachment.id}: {e}") + except DocumentParseError as e: + logger.exception( + "Document parsing failed for attachment %s: %s", + attachment.id, + e, + )Behavior stays the same, but you get stack traces in logs for hard-to-reproduce parse issues.
Also applies to: 71-76, 80-101, 108-127
backend/app/services/chat/chat_service.py (4)
26-31: Remove duplicate logger initialization
loggeris initialized twice:logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)You can safely drop one of these lines.
420-457: Tighten type hints for default_headers and reduce duplication in vision-support logicIn the provider helpers,
default_headersis optional but annotated as non-optional:async def _call_openai_streaming( ..., default_headers: Dict[str, Any] = None, ) -> AsyncGenerator[str, None]:Same pattern exists in
_call_claude_streamingand_call_gemini_streaming. To satisfy Ruff (RUF013) and make intent explicit:-from typing import Any, AsyncGenerator, Dict, List, Optional, Union +from typing import Any, AsyncGenerator, Dict, List, Optional, Union @@ - async def _call_openai_streaming( + async def _call_openai_streaming( ..., - default_headers: Dict[str, Any] = None, + default_headers: Dict[str, Any] | None = None, @@ - async def _call_claude_streaming( + async def _call_claude_streaming( ..., - default_headers: Dict[str, Any] = None, + default_headers: Dict[str, Any] | None = None, @@ - async def _call_gemini_streaming( + async def _call_gemini_streaming( ..., - default_headers: Dict[str, Any] = None, + default_headers: Dict[str, Any] | None = None,Additionally, the
supports_visiondomain list is duplicated between the OpenAI streaming and cancellation variants; consider extracting it to a shared constant (e.g.,VISION_SUPPORTED_DOMAINS) to avoid drift.These changes are purely structural and don’t alter behavior.
Also applies to: 626-663, 726-771
227-239: Broadened exception handlers are acceptable, but consider richer loggingThe generic
except Exceptionblocks around streaming and status updates are defensively fine in this layer, but they currently uselogger.error(..., exc_info=True)or simple f-strings. For easier production debugging you might:
- Prefer
logger.exception("...")in truly unexpected branches (e.g., the generic catch inchat_streamand in the sync update/save helpers).- Optionally include a structured error identifier in the log message for correlation.
No behavior change is required, this would just improve observability.
Also applies to: 1135-1139, 1257-1259, 1295-1299
1305-1369: Confirm whether_handle_client_disconnectis actually wired in
_handle_client_disconnectimplements a clear strategy for partial responses (save content, mark as COMPLETED withincomplete=True, update history, clean Redis), but this method is not referenced withinchat_service.py. If it is not invoked from any API layer:
- Either call it from the generator when you detect client disconnects, or
- Remove or comment it as unused to avoid confusion.
If other modules use it, consider adding a brief comment/docstring note mentioning the external call site.
backend/app/services/chat/session_manager.py (1)
86-113: Session manager behavior looks solid; consider minor typing and logging refinementsThe Redis-backed chat history, cancellation flags, and streaming cache behavior all look correct and resilient to failures (errors are logged and callers get safe fallbacks). Two optional cleanups:
- Explicit Optional for
expire(RUF013):- async def save_streaming_content( - self, subtask_id: int, content: str, expire: int = None - ) -> bool: + async def save_streaming_content( + self, subtask_id: int, content: str, expire: int | None = None + ) -> bool:
- Richer exception logging in a few generic
except Exceptionblocks (save_chat_history,save_streaming_content,get_streaming_content,delete_streaming_content,publish_streaming_chunk,publish_streaming_done,subscribe_streaming_channel) by switching tologger.exception(...)where you want stack traces.Behavior would remain the same while improving type clarity and observability.
Also applies to: 344-369, 390-393, 407-414, 444-447, 476-479
backend/app/services/chat/__init__.py (1)
12-20: Sort__all__to satisfy Ruff/isort-style expectationsRuff flags
__all__as unsorted; reordering alphabetically keeps tooling green without changing behavior.__all__ = [ - "chat_service", - "session_manager", - "get_model_config_for_bot", + "chat_service", + "get_model_config_for_bot", + "session_manager", ]backend/app/api/endpoints/adapter/attachments.py (2)
78-90: Tighten generic exception handling and logging inupload_attachmentThe new broad
except Exceptionhandlers are reasonable at the API boundary, but can be made clearer and more informative while satisfying the static-analysis hints.Consider:
- try: - binary_data = await file.read() - except Exception as e: - logger.error(f"Error reading uploaded file: {e}") - raise HTTPException(status_code=400, detail="Failed to read uploaded file") + try: + binary_data = await file.read() + except Exception as e: + logger.exception("Error reading uploaded file") + raise HTTPException( + status_code=400, + detail="Failed to read uploaded file", + ) from e @@ - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - except DocumentParseError as e: - raise HTTPException(status_code=400, detail=str(e)) - except Exception as e: - logger.error(f"Error uploading attachment: {e}", exc_info=True) - raise HTTPException(status_code=500, detail="Failed to upload attachment") + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + except DocumentParseError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + except Exception as e: + logger.exception("Error uploading attachment") + raise HTTPException( + status_code=500, + detail="Failed to upload attachment", + ) from eIf you want to keep the broad catch-all, you may also choose to silence BLE001 with an inline
# noqa: BLE001comment.Also applies to: 118-124
53-77: Consider adding explicit return type hints to endpoint functionsTo fully align with the Python coding guidelines (type hints for all functions), you could add concrete return types to the FastAPI endpoints here (e.g.,
-> AttachmentResponse,-> AttachmentDetailResponse,-> Response,-> dict | AttachmentDetailResponse | None).As per coding guidelines, Python code should be fully type-annotated.
backend/app/api/endpoints/adapter/chat.py (2)
99-130: Clean up unused parameter/variable and duplicate imports (Ruff hints)There are a few small lint issues that are easy to fix:
_should_use_direct_chat'suser_idparameter is unused.bot_crdis computed but never used.jsonandStreamingResponseare imported twice instream_chat.Suggested diff:
-def _should_use_direct_chat(db: Session, team: Kind, user_id: int) -> bool: +def _should_use_direct_chat(db: Session, team: Kind) -> bool: @@ - if not _should_use_direct_chat(db, team, current_user.id): + if not _should_use_direct_chat(db, team): @@ - supports_direct_chat = _should_use_direct_chat(db, team, current_user.id) + supports_direct_chat = _should_use_direct_chat(db, team, current_user.id) + # If you do not need `current_user.id` here, you can also drop it and call + # `_should_use_direct_chat(db, team)` consistently. @@ - bot_crd = Bot.model_validate(bot.json) - bot_json = bot.json or {} + bot_json = bot.json or {} @@ - import json - - from fastapi.responses import StreamingResponse + import json @@ - import json - - from fastapi.responses import StreamingResponse + # `json` and `StreamingResponse` are already imported above in this functionDepending on whether you intend to use
current_user.idinside_should_use_direct_chatlater, either drop the param entirely or add a usage check; right now it is unused.Also applies to: 375-378, 497-501, 544-547
347-374: Add explicit return type hints for FastAPI endpointsTo better align with the “type hints for all functions” guideline, you can add return type annotations for the main chat endpoints, e.g.:
stream_chat(...) -> StreamingResponsecancel_chat(...) -> dictget_streaming_content(...) -> dictresume_stream(...) -> StreamingResponseAs per coding guidelines, endpoint functions should be explicitly typed.
Also applies to: 905-924, 1006-1022, 1075-1092
backend/app/services/attachment/parser.py (1)
97-152:DocumentParser.parsecentralization looks good; consider preserving original exception causeThe new
parsedispatcher cleanly routes by extension and validates text length in one place; the overall structure is solid and matches the configured limits.To preserve the original traceback and address Ruff’s B904 hint, you can chain the underlying exception when wrapping it in
DocumentParseError:- except DocumentParseError: - raise - except Exception as e: - logger.error(f"Error parsing document: {e}", exc_info=True) - raise DocumentParseError(f"Failed to parse document: {str(e)}") + except DocumentParseError: + raise + except Exception as e: + logger.error("Error parsing document", exc_info=True) + raise DocumentParseError("Failed to parse document") from eYou can apply the same pattern to the helper methods if you want consistent exception chaining across all parsers.
executor/pyproject.toml (1)
29-43: Consider adding linting/type-checking to dev dependencies (optional).The dev dependencies focus on testing (pytest suite), but do not include linting or type-checking tools (black, isort, mypy, ruff). While these may be installed separately, it's conventional to include them in [dependency-groups].dev for consistency with the migration from requirements.txt.
If linting/type-checking is part of your development workflow, consider adding them to the dev group:
[dependency-groups] dev = [ "pytest>=9.0.1", "pytest-asyncio>=1.3.0", "pytest-cov>=7.0.0", "pytest-mock>=3.15.1", "pytest-httpx>=0.36.0", + "black>=24.1.0", + "isort>=5.13.0", + "mypy>=1.8.0", ]backend/pyproject.toml (1)
36-43: Consider adding linting/type-checking to dev dependencies (optional).Development dependencies match the executor service and cover testing well. As noted for executor, linting and type-checking tools (black, isort, mypy, ruff) are not included, though conventional practice would add them to [dependency-groups].dev for CI/pre-commit integration.
If desired, mirror the executor enhancement by adding these tools:
[dependency-groups] dev = [ "pytest>=9.0.1", "pytest-asyncio>=1.3.0", "pytest-cov>=7.0.0", "pytest-mock>=3.15.1", "pytest-httpx>=0.36.0", + "black>=24.1.0", + "isort>=5.13.0", + "mypy>=1.8.0", ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
backend/uv.lockis excluded by!**/*.lockexecutor/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
backend/app/api/api.py(1 hunks)backend/app/api/endpoints/adapter/attachments.py(11 hunks)backend/app/api/endpoints/adapter/chat.py(46 hunks)backend/app/api/endpoints/adapter/models.py(1 hunks)backend/app/api/endpoints/adapter/tasks.py(7 hunks)backend/app/core/config.py(0 hunks)backend/app/main.py(2 hunks)backend/app/models/subtask_attachment.py(5 hunks)backend/app/services/adapters/task_kinds.py(2 hunks)backend/app/services/adapters/team_kinds.py(4 hunks)backend/app/services/attachment/__init__.py(1 hunks)backend/app/services/attachment/attachment_service.py(8 hunks)backend/app/services/attachment/parser.py(12 hunks)backend/app/services/chat/__init__.py(1 hunks)backend/app/services/chat/base.py(3 hunks)backend/app/services/chat/chat_service.py(34 hunks)backend/app/services/chat/model_resolver.py(17 hunks)backend/app/services/chat/session_manager.py(9 hunks)backend/pyproject.toml(2 hunks)executor/agents/agno/mcp_manager.py(1 hunks)executor/pyproject.toml(1 hunks)scripts/hooks/ai-push-gate.sh(7 hunks)scripts/hooks/run-module-tests.sh(2 hunks)
💤 Files with no reviewable changes (1)
- backend/app/core/config.py
✅ Files skipped from review due to trivial changes (2)
- backend/app/api/endpoints/adapter/models.py
- executor/agents/agno/mcp_manager.py
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
backend/app/main.pybackend/app/api/endpoints/adapter/attachments.pybackend/app/services/attachment/parser.pybackend/app/services/adapters/task_kinds.pybackend/app/services/chat/__init__.pybackend/app/services/attachment/__init__.pybackend/app/models/subtask_attachment.pybackend/app/services/adapters/team_kinds.pybackend/app/services/chat/session_manager.pybackend/app/api/api.pybackend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.pybackend/app/services/attachment/attachment_service.pybackend/app/services/chat/base.pybackend/app/services/chat/chat_service.pybackend/app/services/chat/model_resolver.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/app/main.pybackend/app/api/endpoints/adapter/attachments.pybackend/app/services/attachment/parser.pybackend/app/services/adapters/task_kinds.pybackend/app/services/chat/__init__.pybackend/app/services/attachment/__init__.pybackend/app/models/subtask_attachment.pybackend/app/services/adapters/team_kinds.pybackend/app/services/chat/session_manager.pybackend/app/api/api.pybackend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.pybackend/app/services/attachment/attachment_service.pybackend/app/services/chat/base.pybackend/app/services/chat/chat_service.pybackend/app/services/chat/model_resolver.py
**/backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend API route handlers MUST be created in
app/api/directory
Files:
backend/app/api/endpoints/adapter/attachments.pybackend/app/api/api.pybackend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.py
**/backend/app/services/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend business logic MUST be placed in
app/services/directory
Files:
backend/app/services/attachment/parser.pybackend/app/services/adapters/task_kinds.pybackend/app/services/chat/__init__.pybackend/app/services/attachment/__init__.pybackend/app/services/adapters/team_kinds.pybackend/app/services/chat/session_manager.pybackend/app/services/attachment/attachment_service.pybackend/app/services/chat/base.pybackend/app/services/chat/chat_service.pybackend/app/services/chat/model_resolver.py
**/backend/app/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend SQLAlchemy models MUST be created in
app/models/directory
Files:
backend/app/models/subtask_attachment.py
🧠 Learnings (1)
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/app/api/**/*.py : Backend API route handlers MUST be created in `app/api/` directory
Applied to files:
backend/app/api/api.py
🧬 Code graph analysis (8)
backend/app/main.py (1)
backend/app/services/chat/base.py (1)
close_http_client(61-75)
backend/app/services/attachment/parser.py (1)
frontend/src/apis/attachments.ts (1)
SUPPORTED_EXTENSIONS(44-61)
backend/app/services/attachment/__init__.py (1)
backend/app/services/attachment/parser.py (1)
DocumentParser(31-395)
backend/app/services/adapters/team_kinds.py (3)
frontend/src/types/api.ts (1)
Shell(84-93)backend/app/schemas/kind.py (1)
Shell(129-136)backend/app/models/public_shell.py (1)
PublicShell(19-38)
backend/app/services/chat/session_manager.py (1)
backend/app/api/endpoints/adapter/chat.py (1)
get_streaming_content(1001-1066)
backend/app/api/endpoints/adapter/chat.py (5)
backend/app/services/chat/base.py (2)
ChatServiceBase(78-99)is_direct_chat_shell(89-99)backend/app/models/subtask.py (2)
Subtask(31-72)SubtaskStatus(17-23)backend/app/services/adapters/task_kinds.py (2)
create_task_id(881-941)validate_task_id(943-966)backend/app/services/chat/model_resolver.py (1)
get_bot_system_prompt(461-507)backend/app/services/chat/session_manager.py (2)
get_streaming_content(371-393)subscribe_streaming_channel(480-502)
backend/app/api/endpoints/adapter/tasks.py (4)
backend/app/services/chat/session_manager.py (1)
cancel_stream(249-278)backend/app/schemas/kind.py (1)
Task(320-327)backend/app/models/subtask.py (1)
SubtaskStatus(17-23)backend/app/schemas/subtask.py (1)
SubtaskStatus(25-31)
backend/app/services/chat/model_resolver.py (3)
shared/utils/crypto.py (1)
decrypt_api_key(222-239)backend/app/models/kind.py (1)
Kind(25-42)backend/app/schemas/kind.py (1)
Ghost(47-54)
🪛 Ruff (0.14.7)
backend/app/api/endpoints/adapter/attachments.py
88-88: Do not catch blind exception: Exception
(BLE001)
89-89: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
90-90: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
backend/app/services/attachment/parser.py
151-151: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
151-151: Avoid specifying long messages outside the exception class
(TRY003)
151-151: Use explicit conversion flag
Replace with conversion flag
(RUF010)
163-163: Abstract raise to an inner function
(TRY301)
163-163: Avoid specifying long messages outside the exception class
(TRY003)
177-177: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
177-177: Avoid specifying long messages outside the exception class
(TRY003)
177-177: Use explicit conversion flag
Replace with conversion flag
(RUF010)
215-215: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
215-215: Avoid specifying long messages outside the exception class
(TRY003)
215-215: Use explicit conversion flag
Replace with conversion flag
(RUF010)
221-223: Abstract raise to an inner function
(TRY301)
221-223: Avoid specifying long messages outside the exception class
(TRY003)
257-257: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
257-257: Avoid specifying long messages outside the exception class
(TRY003)
257-257: Use explicit conversion flag
Replace with conversion flag
(RUF010)
263-265: Abstract raise to an inner function
(TRY301)
263-265: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
295-295: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Use explicit conversion flag
Replace with conversion flag
(RUF010)
317-317: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
317-317: Avoid specifying long messages outside the exception class
(TRY003)
317-317: Use explicit conversion flag
Replace with conversion flag
(RUF010)
351-353: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
351-353: Avoid specifying long messages outside the exception class
(TRY003)
352-352: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/services/chat/__init__.py
16-20: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
backend/app/services/chat/session_manager.py
80-80: Consider moving this statement to an else block
(TRY300)
82-82: Do not catch blind exception: Exception
(BLE001)
83-83: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
114-114: Do not catch blind exception: Exception
(BLE001)
115-115: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
135-135: Do not catch blind exception: Exception
(BLE001)
136-136: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
182-182: Do not catch blind exception: Exception
(BLE001)
183-183: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
200-200: Do not catch blind exception: Exception
(BLE001)
201-201: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
244-244: Do not catch blind exception: Exception
(BLE001)
267-267: Do not catch blind exception: Exception
(BLE001)
268-270: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
299-299: Do not catch blind exception: Exception
(BLE001)
345-345: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
366-368: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
390-392: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
410-410: Do not catch blind exception: Exception
(BLE001)
411-413: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
444-446: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
476-476: Do not catch blind exception: Exception
(BLE001)
477-477: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
499-501: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backend/app/api/endpoints/adapter/chat.py
99-99: Unused function argument: user_id
(ARG001)
497-497: Local variable bot_crd is assigned to but never used
Remove assignment to unused variable bot_crd
(F841)
544-544: Redefinition of unused json from line 375
Remove definition: json
(F811)
546-546: Redefinition of unused StreamingResponse from line 377
Remove definition: StreamingResponse
(F811)
backend/app/api/endpoints/adapter/tasks.py
283-285: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
284-284: Use explicit conversion flag
Replace with conversion flag
(RUF010)
315-317: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
316-316: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/services/attachment/attachment_service.py
66-69: Avoid specifying long messages outside the exception class
(TRY003)
75-75: Avoid specifying long messages outside the exception class
(TRY003)
121-121: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backend/app/services/chat/base.py
86-86: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
backend/app/services/chat/chat_service.py
129-129: Do not catch blind exception: Exception
(BLE001)
207-207: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
217-217: Use explicit conversion flag
Replace with conversion flag
(RUF010)
218-218: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
328-330: Avoid specifying long messages outside the exception class
(TRY003)
369-371: Avoid specifying long messages outside the exception class
(TRY003)
427-427: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
529-529: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
733-733: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
1135-1135: Do not catch blind exception: Exception
(BLE001)
1136-1136: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1188-1188: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1257-1257: Do not catch blind exception: Exception
(BLE001)
1258-1258: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1295-1295: Do not catch blind exception: Exception
(BLE001)
1296-1298: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backend/app/services/chat/model_resolver.py
184-184: Do not catch blind exception: Exception
(BLE001)
216-216: Do not catch blind exception: Exception
(BLE001)
290-290: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Avoid specifying long messages outside the exception class
(TRY003)
385-385: f-string without any placeholders
Remove extraneous f prefix
(F541)
404-404: f-string without any placeholders
Remove extraneous f prefix
(F541)
416-416: Do not catch blind exception: Exception
(BLE001)
449-449: Do not catch blind exception: Exception
(BLE001)
🪛 Shellcheck (0.11.0)
scripts/hooks/ai-push-gate.sh
[warning] 286-286: This redirection doesn't have a command. Move to its command (or use 'true' as no-op).
(SC2188)
⏰ 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). (2)
- GitHub Check: Test wegent CLI Integration
- GitHub Check: e2e-tests
🔇 Additional comments (11)
scripts/hooks/run-module-tests.sh (1)
114-114: Approve uv run migration for pytest invocations.The migration from direct
pytesttouv run pytestis consistent with the PR's shift to uv-based tooling. Command structure and flags remain unchanged, ensuring no behavioral impact.Also applies to: 126-126, 132-132, 138-138
scripts/hooks/ai-push-gate.sh (1)
289-289: Approve uv run migration for all Python tool invocations.The migration from direct tool invocations (
black,isort,python,pytest) touv runwrappers is consistent with the PR's shift to uv-based tooling and maintains command semantics. Error handling and logging paths remain intact.Also applies to: 307-307, 326-326, 347-347, 368-368, 403-403, 437-437
backend/app/services/adapters/team_kinds.py (2)
224-226: LGTM - Formatting improvement.The
.order_by()call is now wrapped across multiple lines, which is consistent with Black formatter's line length limit (88 characters). This improves readability without changing functionality.
854-886: LGTM - Improved readability with blank lines.The added blank lines (at lines 854, 867, 874, 886) provide better visual separation between logical sections:
- After variable initialization
- After the user's custom shell query
- Before the public shell import
- After the public shell lookup
This follows PEP 8 conventions for using blank lines to indicate logical sections within functions.
backend/app/main.py (1)
107-115: Good integration of chat HTTP client with app lifecycleInitializing the shared chat HTTP client on startup and closing it on shutdown is a clean way to manage connection pooling and resource cleanup. The imports inside the event handlers avoid circulars and keep the lifecycle well-scoped.
Also applies to: 207-216
backend/app/services/attachment/__init__.py (1)
5-12: Attachment package exports are clear and minimalRe-exporting
DocumentParserand the singletonattachment_servicevia__all__makes the attachment API easy to consume and keeps the module surface focused.backend/app/api/api.py (1)
32-34: Router reformatting is semantically neutralThe multiline
include_routerforattachments.routerpreserves the same prefix/tags and keeps API wiring unchanged; location underapp/api/aligns with the API guidelines.backend/app/models/subtask_attachment.py (1)
21-26: New long-text and image fields look consistent with attachment parsingUsing
LongTextTypeforextracted_textandimage_base64, plus addingtext_lengthand aFAILEDstatus, aligns well with the enhanced parser and attachment flows. Just ensure migrations match these schema changes and that API/clients handle the newFAILEDstate appropriately.Also applies to: 28-35, 62-76, 68-76
backend/pyproject.toml (2)
73-73: Tool configurations look good; alembic skip is correct.Line 73 appropriately adds "alembic" to isort's skip list to preserve auto-generated migration files. Lines 75–80 define pytest configuration matching the executor service (consistent tooling across services ✓).
Also applies to: 75-80
5-34: Verify Python 3.10+ compatibility and dependency version consistency.The backend requires Python 3.10+, consistent with the executor service. Version constraints align appropriately:
- cryptography: backend >=42.0.4, executor >=46.0.3 (executor stricter; no conflict)
- requests: both >=2.31.0 (consistent)
- fastapi: both >=0.109.1 (consistent)
All listed packages support Python 3.10+. Note that other dependencies also differ between services (sqlalchemy, httpx, uvicorn versions), which is expected given their distinct responsibilities. Verify version constraints align with your testing matrix before deployment.
executor/pyproject.toml (1)
1-43: All dependencies in executor/pyproject.toml support Python 3.10+.The project explicitly documents Python 3.10+ support in README.md, and all 20 packages (15 production + 5 dev) are compatible with Python 3.10+. The presence of uv.lock confirms that dependencies are pinned to known-compatible versions. The requires-python constraint is appropriate.
| # Validate file size before processing | ||
| if not DocumentParser.validate_file_size(len(binary_data)): | ||
| max_size_mb = DocumentParser.MAX_FILE_SIZE / (1024 * 1024) | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail=f"File size exceeds maximum limit ({max_size_mb} MB)" | ||
| status_code=400, | ||
| detail=f"File size exceeds maximum limit ({max_size_mb} MB)", | ||
| ) |
There was a problem hiding this comment.
Fix reference to removed DocumentParser.MAX_FILE_SIZE
DocumentParser now exposes get_max_file_size() instead of a MAX_FILE_SIZE attribute. As written, the error path will raise AttributeError whenever a file is too large, turning a user-facing 400 into a 500.
- # Validate file size before processing
- if not DocumentParser.validate_file_size(len(binary_data)):
- max_size_mb = DocumentParser.MAX_FILE_SIZE / (1024 * 1024)
+ # Validate file size before processing
+ if not DocumentParser.validate_file_size(len(binary_data)):
+ max_size_mb = DocumentParser.get_max_file_size() / (1024 * 1024)
raise HTTPException(
status_code=400,
detail=f"File size exceeds maximum limit ({max_size_mb} MB)",
)🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/attachments.py around lines 92 to 98, the
error path references the removed DocumentParser.MAX_FILE_SIZE constant which
will raise AttributeError; replace that reference with a call to
DocumentParser.get_max_file_size() (assumed to return bytes) and compute
max_size_mb = DocumentParser.get_max_file_size() / (1024 * 1024) so the 400
response shows the correct maximum size without causing a server error.
| # 3. Subscribe to Redis Pub/Sub for real-time updates | ||
| redis_client, pubsub = await session_manager.subscribe_streaming_channel(subtask_id) | ||
| redis_client, pubsub = await session_manager.subscribe_streaming_channel( | ||
| subtask_id | ||
| ) | ||
| if not pubsub: | ||
| # No pub/sub available - check if stream is still active | ||
| # If not, the stream might have completed while we were connecting | ||
| logger.warning(f"Could not subscribe to streaming channel for subtask {subtask_id}") | ||
|
|
||
| logger.warning( | ||
| f"Could not subscribe to streaming channel for subtask {subtask_id}" | ||
| ) | ||
|
|
||
| # Re-check subtask status | ||
| db.refresh(subtask) | ||
| if subtask.status == SubtaskStatus.COMPLETED: | ||
| final_content = "" | ||
| if subtask.result: | ||
| final_content = subtask.result.get("value", "") | ||
|
|
||
| # Send any remaining content | ||
| if current_offset < len(final_content): | ||
| remaining = final_content[current_offset:] | ||
| yield f"data: {json.dumps({'offset': current_offset, 'content': remaining, 'done': False})}\n\n" | ||
| current_offset = len(final_content) | ||
|
|
||
| yield f"data: {json.dumps({'offset': current_offset, 'content': '', 'done': True, 'result': subtask.result})}\n\n" | ||
| else: | ||
| yield f"data: {json.dumps({'error': 'Stream not available'})}\n\n" | ||
| return | ||
|
|
There was a problem hiding this comment.
Guard Redis pubsub cleanup when subscription fails
If session_manager.subscribe_streaming_channel returns (None, None), the if not pubsub: branches correctly handle the missing subscription, but the finally blocks still call await pubsub.unsubscribe() and await pubsub.close(), causing an AttributeError on None.
Update both resume generators to guard pubsub in the cleanup:
- finally:
- # Cleanup: unsubscribe and close client
- await pubsub.unsubscribe()
- await pubsub.close()
- if redis_client:
- await redis_client.close()
+ finally:
+ # Cleanup: unsubscribe and close client
+ if pubsub:
+ await pubsub.unsubscribe()
+ await pubsub.close()
+ if redis_client:
+ await redis_client.close()Apply the same pattern in both _handle_resume_stream and resume_stream so error paths do not raise new exceptions while trying to clean up.
Also applies to: 820-826, 1138-1152, 1220-1226
🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/chat.py around lines 707 to 735, the
cleanup code may call await pubsub.unsubscribe() and await pubsub.close() even
when subscribe_streaming_channel returned (None, None), raising AttributeError;
update the finally/cleanup sections to check if pubsub is not None before
calling unsubscribe() or close(); apply the same guard pattern to the other
affected ranges (lines ~820-826, ~1138-1152, and ~1220-1226) so cleanup only
runs when pubsub exists.
| if not default_headers: | ||
| # settings.EXECUTOR_ENV is a JSON string, need to parse it first | ||
| try: | ||
| executor_env = json.loads(settings.EXECUTOR_ENV) if settings.EXECUTOR_ENV else {} | ||
| executor_env = ( | ||
| json.loads(settings.EXECUTOR_ENV) if settings.EXECUTOR_ENV else {} | ||
| ) | ||
| default_headers = executor_env.get("DEFAULT_HEADERS", {}) | ||
| except (json.JSONDecodeError, TypeError): | ||
| logger.warning(f"[model_resolver] Failed to parse settings.EXECUTOR_ENV as JSON") | ||
| logger.warning( | ||
| f"[model_resolver] Failed to parse settings.EXECUTOR_ENV as JSON" | ||
| ) | ||
| default_headers = {} | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l backend/app/services/chat/model_resolver.pyRepository: wecode-ai/Wegent
Length of output: 107
🏁 Script executed:
sed -n '390,425p' backend/app/services/chat/model_resolver.pyRepository: wecode-ai/Wegent
Length of output: 1471
🏁 Script executed:
rg -A 5 -B 5 "DEFAULT_HEADERS" backend/app/services/chat/model_resolver.pyRepository: wecode-ai/Wegent
Length of output: 3876
🏁 Script executed:
sed -n '460,480p' backend/app/services/chat/model_resolver.pyRepository: wecode-ai/Wegent
Length of output: 638
🏁 Script executed:
rg -n "logger.warning.*default_headers" backend/app/services/chat/model_resolver.pyRepository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
rg -n "logger.warning.*DEFAULT_HEADERS" backend/app/services/chat/model_resolver.pyRepository: wecode-ai/Wegent
Length of output: 152
🏁 Script executed:
sed -n '413,425p' backend/app/services/chat/model_resolver.pyRepository: wecode-ai/Wegent
Length of output: 512
🏁 Script executed:
rg -n "logger\.(warning|error|info).*\{.*default_headers" backend/app/services/chat/model_resolver.pyRepository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
sed -n '175,190p' backend/app/services/chat/model_resolver.pyRepository: wecode-ai/Wegent
Length of output: 689
Avoid logging full DEFAULT_HEADERS to prevent leaking secrets
Line 417 logs the entire DEFAULT_HEADERS value when JSON parsing fails, which can leak sensitive headers like Authorization:
except Exception:
logger.warning(
f"Failed to parse DEFAULT_HEADERS as JSON: {default_headers}"
)Since DEFAULT_HEADERS can contain Authorization or other sensitive headers, this log exposes secrets. The rest of the code logs only keys (keys={list(default_headers.keys())}), which is safer. Use this pattern instead:
except Exception:
logger.warning(
"[model_resolver] Failed to parse DEFAULT_HEADERS as JSON; value type=%s",
type(default_headers).__name__,
)This maintains diagnostics while avoiding secret exposure.
🧰 Tools
🪛 Ruff (0.14.7)
404-404: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In backend/app/services/chat/model_resolver.py around lines 395 to 407, the
except block currently logs the full DEFAULT_HEADERS value which can leak
sensitive header contents; change the exception handling to avoid logging header
values by instead logging safe diagnostics (for example the type of
default_headers or just its keys), e.g. in the except clause call logger.warning
with a message like "[model_resolver] Failed to parse DEFAULT_HEADERS as JSON;
value type=%s" and pass type(default_headers).__name__ (or use
keys=list(default_headers.keys()) if it's a dict) so no header values (like
Authorization) are written to logs.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
frontend/src/features/tasks/components/MessageBubble.tsx (3)
398-404: Minor regex quirk:澄清问题?makes题optional.The pattern
澄清问题?will match both澄清问题and澄清问, which may not be intentional. If you want to match both澄清问题(clarification questions) and澄清(clarification), consider using(?:澄清问题|澄清)or澄清(?:问题)?instead.That said, the flexible header detection approach is a good improvement for supporting multiple input formats.
521-524: Add debug logging to the catch block.The empty catch block silently swallows all errors. While continuing to parse other questions is the right approach, consider logging the error in development mode to help diagnose parsing issues:
- } catch { - // Continue parsing other questions even if one fails - continue; + } catch (error) { + // Continue parsing other questions even if one fails + if (process.env.NODE_ENV === 'development') { + console.warn('Failed to parse clarification question:', error); + } + continue; }
540-545: Consider extracting duplicate code block regex logic.The code block extraction logic (lines 540-545) is duplicated from
parseMarkdownClarification(lines 391-396). Consider extracting this into a shared helper function to improve maintainability:const extractFromCodeBlock = (content: string): string => { const codeBlockRegex = /```(?:markdown)?\s*\n([\s\S]+?)\n```/ const match = content.match(codeBlockRegex) return match ? match[1] : content }frontend/src/utils/pdf-generator.ts (3)
639-643: Single-column tables won't be detected.The regex pattern requires at least two separator cells due to
(\|[\s]*:?-+:?[\s]*)+, meaning single-column markdown tables (e.g.,|---|) won't be recognized. This edge case is uncommon but worth noting.If single-column table support is desired:
- return /^\|?[\s]*:?-+:?[\s]*(\|[\s]*:?-+:?[\s]*)+\|?$/.test(trimmed); + return /^\|?[\s]*:?-+:?[\s]*(\|[\s]*:?-+:?[\s]*)*\|?$/.test(trimmed);
648-656: Potential false positive detection for lines with pipe characters.Lines containing pipe characters in regular text (e.g.,
"A | B description") may be incorrectly identified as table rows. While the context-based parsing (inTableflag) mitigates this during actual rendering, theisTableRowfunction itself could be more precise.Consider adding stricter validation, such as requiring the line to start or end with a pipe:
function isTableRow(line: string): boolean { const trimmed = line.trim(); - // Table row must contain at least one pipe character - // and should have content (not just pipes) - if (!trimmed.includes('|')) return false; + // Table row should start or end with pipe for better accuracy + if (!trimmed.startsWith('|') && !trimmed.endsWith('|')) return false; // Check if it has actual content between pipes const cells = parseTableCells(trimmed); return cells.length >= 1 && cells.some(cell => cell.length > 0); }
1125-1169: Consider table header repetition on page breaks for better UX.The table rendering correctly handles page breaks, but when a table spans multiple pages, the header row is not repeated on subsequent pages. This could make it harder to understand data in long tables.
If header repetition is desired, modify the page break handling:
// Check if we need a new page for this row if (checkNewPage(rowInfo.height, pageNum)) { - // After page break, we might want to redraw header (optional) - // For now, just continue with the row + // Redraw header on new page for better readability + const newHeaderInfo = calculateRowHeight(headers, true); + drawRow(headers, yPosition, newHeaderInfo.height, newHeaderInfo.cellLines, true, true, false); + yPosition += newHeaderInfo.height; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
backend/uv.lockis excluded by!**/*.lockexecutor_manager/uv.lockis excluded by!**/*.lockfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (7)
.github/workflows/test.yml(7 hunks)backend/pyproject.toml(2 hunks)executor_manager/pyproject.toml(2 hunks)frontend/package.json(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(1 hunks)frontend/src/features/tasks/components/MessageBubble.tsx(2 hunks)frontend/src/utils/pdf-generator.ts(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- executor_manager/pyproject.toml
- frontend/src/features/tasks/components/ChatArea.tsx
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/utils/pdf-generator.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/utils/pdf-generator.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/utils/pdf-generator.ts
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/MessageBubble.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/tasks/components/MessageBubble.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/tasks/components/MessageBubble.tsx
🧬 Code graph analysis (1)
frontend/src/features/tasks/components/MessageBubble.tsx (1)
frontend/src/types/api.ts (2)
ClarificationData(347-350)FinalPromptData(365-368)
⏰ 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: e2e-tests
🔇 Additional comments (18)
frontend/src/features/tasks/components/MessageBubble.tsx (1)
387-533: Good resilient parsing implementation.The flexible parsing approach with try-catch blocks, fallback option formats, and support for various header styles is well-designed for handling varied AI-generated markdown content. The code handles edge cases gracefully by continuing to parse subsequent questions even if individual ones fail.
frontend/src/utils/pdf-generator.ts (7)
611-623: LGTM!The type definitions for table support are well-structured. The alignment type constraint and optional fields are appropriately defined.
628-634: LGTM!The cell parsing logic correctly handles leading/trailing pipes and trims whitespace from each cell.
661-671: LGTM!The alignment parsing correctly interprets markdown table alignment syntax (
:---,---:,:---:).
676-702: LGTM!The table detection logic appropriately prioritizes table separators over horizontal rules, and the context-aware parsing handles the state correctly.
1562-1565: LGTM!Proper cleanup of pending table state at end of content ensures tables are rendered even when they're the last element in a message.
1318-1321: LGTM!Correctly delegates table rendering to
renderMessagewhich has the necessary multi-line context to construct complete tables.
804-805: LGTM!The table line height constant is appropriately added for consistent spacing in table rendering.
.github/workflows/test.yml (7)
36-49: Approve uv installation anduv sync --devpattern intest-backend.The uv installation via the official Astral script, followed by cargo environment sourcing and
uv sync --dev, is correct and consistent with uv best practices. Cache key hashing onbackend/pyproject.tomlensures proper invalidation on dependency changes.
25-25: Approve Python version matrix for backend tests.Testing on both Python 3.10 and 3.11 (line 25) is good practice for compatibility assurance. Aligns well with the new minimum requirement of Python 3.10.
300-302: Approve uv installation intest-wegent-cli-integrationbackend setup.The backend installation in the integration test job now correctly uses uv (lines 300–302), ensuring consistency with the backend's pyproject.toml-based setup. Good alignment with the PR's dependency-management migration.
83-101: Approve uv setup and cache strategy intest-executorjob.The uv-based installation pattern, cache hashing on
executor/pyproject.toml, and use ofuv run pytestare all correct and align with the migration objectives. Theexecutor/pyproject.tomlfile exists and is properly configured.
126-147: Approve uv setup and cache strategy intest-executor-managerjob.The uv-based installation pattern, cache key on
executor_manager/pyproject.toml, anduv run pytestwrapper are correct. Theexecutor_manager/pyproject.tomlfile exists with proper TOML formatting and contains expected configuration sections.
217-243:test-wegent-cliusespipby design while other backend services useuv.The
test-wegent-clijob installs dependencies viapip install -r requirements.txt(lines 241–243). Unlike other backend services (app,agents,executors), wegent-cli does not have apyproject.tomland remains requirements.txt-based. This is an intentional design choice and is not part of the dependency-management migration in this PR. No changes needed.
156-179: The review comment is incorrect. Verification shows that theshareddirectory has neither apyproject.tomlnor arequirements.txtfile, meaning it is not structured as a Python package. Consequently, thetest-sharedjob's use ofpip installfor test dependencies is the appropriate approach, not an inconsistency. The backend, executor, and executor-manager services useuvbecause they each have their ownpyproject.tomlfiles;sharedlacks this structure and should remain pip-based. No changes are required.Likely an incorrect or invalid review comment.
backend/pyproject.toml (3)
76-81: Pytest configuration is appropriate for the current backend test suite.The
[tool.pytest.ini_options]settings correctly reflect the test structure: 9 test files organized undertests/directory match thetest_*.pypattern, and the standard class/function naming conventions are in place. Aconftest.pyfile is present for shared fixtures, and the basic configuration is sufficient without additional marker registration or plugin setup.
37-44: Verify uv correctly interprets the[dependency-groups]section during dependency resolution.The
[dependency-groups]section uses PEP 735 syntax (approved October 2024), which uv officially supports. Ensure your CI/build pipeline correctly resolves and installs dev dependencies withuv sync --dev.
5-35: Fix pydantic version constraint—2.5.3 does not exist on PyPI.The pydantic dependency specifies
pydantic>=2.5.3, but only pydantic 2.5.2 (and earlier 2.5.x releases) exist on PyPI. Update the constraint topydantic>=2.5.2or use a more stable version pin likepydantic~=2.5.0. All other key dependencies (cryptography, sqlalchemy, fastapi) support Python 3.10+ without issues.⛔ Skipped due to learnings
Learnt from: CR Repo: wecode-ai/Wegent PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-01T06:44:24.499Z Learning: Applies to **/backend/alembic/versions/*.py : Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
| "@testing-library/react": "^16.0.0", | ||
| "@testing-library/user-event": "^14.5.0", | ||
| "@types/jest": "^29.5.0", | ||
| "@types/jspdf": "^1.3.3", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@types/jspdf version 1.3.3 compatibility jspdf 3.0.4
💡 Result:
Short answer: They are not compatible. @types/jspdf@1.3.3 is the old DefinitelyTyped stub for much earlier jsPDF releases; modern jspdf (v3.x) ships its own TypeScript definitions and uses a different API/import shape. Remove the @types package and use the built-in types (import { jsPDF } from 'jspdf') or upgrade your code to the v3 API. [1][2][3]
References:
- @types/jspdf (v1.3.3) npm entry. [1]
- Note that jspdf provides its own TypeScript definitions (so @types is not needed). [2]
- jspdf package / release info (3.x). [3]
🌐 Web query:
@types/jspdf latest version npm registry
💡 Result:
The latest published version on the npm registry is 2.0.0 (package is marked deprecated). [1][2]
Sources:
[1] npm registry page for @types/jspdf.
[2] jsDelivr / package info for @types/jspdf.
🌐 Web query:
jspdf 3.0.4 TypeScript type definitions support
💡 Result:
Yes — jsPDF 3.0.4 ships with its own TypeScript definitions (you do not need @types/jspdf). Import with the named export, e.g.:
import { jsPDF } from "jspdf";
(See the jsPDF README which states it “offers a typings file” and that @types/jspdf is a stub/deprecated.) [1][2]
Sources:
[1] jsPDF GitHub README (installation / TypeScript mention).
[2] @types/jspdf package note (stub — jspdf provides its own types).
Remove @types/jspdf—it is incompatible with jspdf@3.0.4 and deprecated.
The @types/jspdf@1.3.3 package is a stub from DefinitelyTyped targeting much older jsPDF releases and is now marked as deprecated. jspdf@3.0.4 ships with its own TypeScript type definitions built-in, making @types/jspdf redundant and potentially conflicting. Remove this dependency and use jspdf's native types via import { jsPDF } from "jspdf".
🤖 Prompt for AI Agents
In frontend/package.json around line 71, remove the deprecated and incompatible
dependency "@types/jspdf": "^1.3.3" because jspdf@3.0.4 includes built-in
TypeScript definitions; delete that entry from package.json, run npm/yarn
install to update lockfile, and update imports to use the bundled types (e.g.,
import { jsPDF } from "jspdf") where needed.
| } else if (parsedLine.type === 'tableRow' && parsedLine.tableCells) { | ||
| if (!inTable) { | ||
| // This might be a header row, wait for separator | ||
| // Don't render yet, just continue | ||
| } else { | ||
| // This is a data row | ||
| tableRows.push(parsedLine.tableCells); | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential content loss for table header rows not followed by separator.
When a line is detected as tableRow but inTable is false, the code doesn't store or render it. If the subsequent line is not a table separator (e.g., the user types a line with pipes that isn't actually a table), this content will be silently dropped.
Consider rendering the line as a paragraph if no separator follows:
} else if (parsedLine.type === 'tableRow' && parsedLine.tableCells) {
if (!inTable) {
- // This might be a header row, wait for separator
- // Don't render yet, just continue
+ // This might be a header row, wait for separator
+ // Store it temporarily in case next line is not a separator
+ // If next line is not a separator, we'll render it as paragraph
+ const nextLine = i + 1 < lines.length ? lines[i + 1] : '';
+ if (!isTableSeparator(nextLine.trim())) {
+ // Not a table, render as paragraph
+ renderMarkdownLine({ type: 'paragraph', content: parsedLine.content }, pageNum);
+ }
+ // If next line IS separator, it will be handled in next iteration
} else {
// This is a data row
tableRows.push(parsedLine.tableCells);
}
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/utils/pdf-generator.ts around lines 1539 to 1547, a parsedLine
with type 'tableRow' encountered when inTable is false is currently ignored
which can drop content; modify logic to buffer that first tableRow as a
potential header (e.g., headerCandidate), then when the next line is parsed
check if it is a table separator to start the table and include the buffered
header; if the next line is not a separator, flush the buffered headerCandidate
as a normal paragraph (or text block) before continuing so no user content is
silently lost.
Micro66
force-pushed
the
wegent/improve-startup-and-fix-chat-flash
branch
from
48aa1f8 to
06b6b04
Compare
* fix: fix unread status * refactor: backend isort&black
#302) * feat: Added a shared task model and updated user relationships and configurations. * feat: 分享页面移除侧边栏
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (15)
frontend/package.json (1)
71-71: Remove deprecated and incompatible@types/jspdfpackage.As noted in the previous review,
@types/jspdf@1.3.3is incompatible withjspdf@3.0.4(line 50), which ships with its own TypeScript definitions. This dependency should be removed.docs/zh/getting-started/installation.md (1)
208-208: Add uv installation instructions.As noted in the previous review, the document instructs users to run
uv syncbut does not explain how to install theuvtool. Add installation instructions before this line.backend/app/api/endpoints/adapter/chat.py (1)
820-826: Guard pubsub cleanup against None to prevent AttributeError.When
subscribe_streaming_channelreturns(None, None)due to subscription failure, thefinallyblocks at lines 820-826 and 1220-1226 will attempt to callawait pubsub.unsubscribe()andawait pubsub.close()onNone, causing anAttributeError.Apply this diff to guard the cleanup:
finally: # Cleanup: unsubscribe and close client - await pubsub.unsubscribe() - await pubsub.close() + if pubsub: + await pubsub.unsubscribe() + await pubsub.close() if redis_client: await redis_client.close()Apply the same pattern at both locations (lines ~820-826 and ~1220-1226).
Also applies to: 1220-1226
docs/zh/troubleshooting.md (1)
532-541: Incorrect uv command syntax remains unfixed.The
-rflag inuv pip installexpects a requirements file, notpyproject.toml. The correct approach for pyproject.toml-based projects is to useuv sync.Based on the past review comment, apply this fix:
# 2. 使用镜像源 -uv pip install --index-url https://pypi.tuna.tsinghua.edu.cn/simple -r pyproject.toml +uv sync --index-url https://pypi.tuna.tsinghua.edu.cn/simple # 3. 分别安装依赖 -uv pip install --no-deps -r pyproject.toml -uv pip install <specific-package> +uv pip install --no-deps <specific-package> +uv pip install <another-specific-package> # 4. 使用 conda（如果 pip 失败） conda create -n wegent python=3.10 conda activate wegent uv syncexecutor_manager/README_zh.md (1)
15-15: Fix Python version requirement to match project minimum.The documentation states Python 3.8+ is required, but the PR objectives and project minimum have been bumped to Python 3.10+. While a past review flagged this and marked it as addressed, the current code still shows 3.8+.
Apply this diff:
-- 检查 Python 版本（需要 3.8+） +- 检查 Python 版本（需要 3.10+）backend/app/services/chat/model_resolver.py (1)
381-421: Stop logging rawDEFAULT_HEADERSand clean up unnecessary f‑stringsThe
exceptblock for JSON parsing still logs the fullDEFAULT_HEADERSvalue:except Exception: logger.warning( f"Failed to parse DEFAULT_HEADERS as JSON: {default_headers}" )Because
DEFAULT_HEADERScan carry auth headers (e.g.Authorization), this leaks secrets to logs and should be removed. You already use a safer “keys only” logging pattern elsewhere.Additionally, the following f‑strings have no interpolation and trigger Ruff F541:
logger.info( f"[model_resolver] api_key contains placeholder, resolving from env..." ) ... logger.warning( f"[model_resolver] Failed to parse settings.EXECUTOR_ENV as JSON" )They can be plain strings.
Suggested change:
@@ - if api_key and "${" in api_key: - logger.info( - f"[model_resolver] api_key contains placeholder, resolving from env..." - ) + if api_key and "${" in api_key: + logger.info( + "[model_resolver] api_key contains placeholder, resolving from env..." + ) @@ - try: - default_headers = json.loads(default_headers) - except Exception: - logger.warning( - f"Failed to parse DEFAULT_HEADERS as JSON: {default_headers}" - ) - default_headers = {} + try: + default_headers = json.loads(default_headers) + except Exception: + # Avoid logging header values to prevent leaking secrets + logger.warning( + "[model_resolver] Failed to parse DEFAULT_HEADERS as JSON; " + "value type=%s", + type(default_headers).__name__, + ) + default_headers = {} @@ - if not default_headers: - # settings.EXECUTOR_ENV is a JSON string, need to parse it first - try: - executor_env = ( - json.loads(settings.EXECUTOR_ENV) if settings.EXECUTOR_ENV else {} - ) + if not default_headers: + # settings.EXECUTOR_ENV is a JSON string, need to parse it first + try: + executor_env = ( + json.loads(settings.EXECUTOR_ENV) if settings.EXECUTOR_ENV else {} + ) default_headers = executor_env.get("DEFAULT_HEADERS", {}) - except (json.JSONDecodeError, TypeError): - logger.warning( - f"[model_resolver] Failed to parse settings.EXECUTOR_ENV as JSON" - ) + except (json.JSONDecodeError, TypeError): + logger.warning( + "[model_resolver] Failed to parse settings.EXECUTOR_ENV as JSON" + ) default_headers = {}This keeps diagnostics while avoiding header value leakage and fixes the F541 warnings.
Also applies to: 422-434
frontend/start.sh (1)
89-122: Splitsuspended_jobsdeclaration and assignment (SC2155)In
check_port,suspended_jobsis declared and assigned in one line:local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)ShellCheck flags this (SC2155) because it can mask the pipeline’s exit status. It’s safer to declare, then assign:
- # Check for suspended jobs - local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) + # Check for suspended jobs + local suspended_jobs + suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)Behavior stays the same while removing the warning.
executor/start.sh (1)
94-127: Fixsuspended_jobsand PATH handling per ShellCheck (SC2155)Two small ShellCheck SC2155 issues remain:
suspended_jobsdeclaration + assignment# Check for suspended jobs local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)Split declaration and assignment:
- # Check for suspended jobs - local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) + # Check for suspended jobs + local suspended_jobs + suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)
- PATH manipulation in a single export
unset VIRTUAL_ENV unset PYTHONHOME export PATH=$(echo $PATH | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//')Better to build a new value first and quote
$PATH:-# Force clear any virtual environment variables -unset VIRTUAL_ENV -unset PYTHONHOME -export PATH=$(echo $PATH | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//') +# Force clear any virtual environment variables +unset VIRTUAL_ENV +unset PYTHONHOME +local new_path +new_path=$(echo "$PATH" | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//') +export PATH="$new_path"This preserves behavior while addressing the warnings.
Also applies to: 217-221
backend/start.sh (2)
46-48: Clean up duplicate comment and ShellCheck SC2155 issuesThere are a few easy cleanups:
- Remove duplicate “Parse command line arguments” comment
-# Parse command line arguments -# Parse command line arguments -while [[ $# -gt 0 ]]; do +# Parse command line arguments +while [[ $# -gt 0 ]]; do
- Split
suspended_jobsdeclaration and assignment (SC2155)# Check for suspended jobs local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)Refactor to:
- # Check for suspended jobs - local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) + # Check for suspended jobs + local suspended_jobs + suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)
- Avoid in‑place PATH rewrite in a single export (SC2155 + quoting)
unset VIRTUAL_ENV unset PYTHONHOME export PATH=$(echo $PATH | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//')Safer pattern:
-# Force clear any virtual environment variables -unset VIRTUAL_ENV -unset PYTHONHOME -export PATH=$(echo $PATH | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//') +# Force clear any virtual environment variables +unset VIRTUAL_ENV +unset PYTHONHOME +local new_path +new_path=$(echo "$PATH" | tr ':' '\n' | grep -v '/venv/bin' | grep -v '/.venv/bin' | tr '\n' ':' | sed 's/:$//') +export PATH="$new_path"These changes keep behavior intact while aligning with ShellCheck.
Also applies to: 126-160, 251-255
315-337: Database connectivity check ignores password and can hang or always failThe MySQL connectivity check derives
DB_USERandDB_NAMEfromDATABASE_URL, but never parses or passes the password:DB_USER=$(grep DATABASE_URL .env | cut -d'/' -f3 | cut -d':' -f1) DB_NAME=$(grep DATABASE_URL .env | cut -d'/' -f4) if command -v mysql &> /dev/null; then if mysql -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" -e "SELECT 1" &> /dev/null; then ... if mysql -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" -e "USE $DB_NAME" &> /dev/null; then ... else ... mysql -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" -e "CREATE DATABASE IF NOT EXISTS $DB_NAME ..."If MySQL requires a password (which is typical, and matches the
root:123456pattern in.env.example), these commands will:
- Prompt for a password and block the script, or
- Fail the connectivity check even though the configured
DATABASE_URLis valid.To avoid interactive prompts and get an accurate check, you should also extract the password from
DATABASE_URLand pass it non‑interactively (or skip the check when a password is present).One possible approach, keeping everything in shell and avoiding logging the password:
-DB_USER=$(grep DATABASE_URL .env | cut -d'/' -f3 | cut -d':' -f1) -DB_NAME=$(grep DATABASE_URL .env | cut -d'/' -f4) +DB_URL=$(grep '^DATABASE_URL=' .env | head -1 | cut -d'=' -f2-) +DB_USER=$(printf '%s\n' "$DB_URL" | sed -E 's|^[^:]+://([^:]+):[^@]+@.*$|\1|') +DB_PASS=$(printf '%s\n' "$DB_URL" | sed -E 's|^[^:]+://[^:]+:([^@]+)@.*$|\1|') +DB_NAME=$(printf '%s\n' "$DB_URL" | sed -E 's|^.*/([^/?]+).*$|\1|') @@ -if command -v mysql &> /dev/null; then - if mysql -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" -e "SELECT 1" &> /dev/null; then +if command -v mysql &> /dev/null; then + MYSQL_AUTH=(-h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER") + if [ -n "$DB_PASS" ]; then + MYSQL_AUTH+=("-p$DB_PASS") + fi + + if mysql "${MYSQL_AUTH[@]}" -e "SELECT 1" &> /dev/null; then @@ - if mysql -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" -e "USE $DB_NAME" &> /dev/null; then + if mysql "${MYSQL_AUTH[@]}" -e "USE $DB_NAME" &> /dev/null; then @@ - mysql -h "$DB_HOST" -P "$DB_PORT" -u "$DB_USER" -e "CREATE DATABASE IF NOT EXISTS $DB_NAME CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;" + mysql "${MYSQL_AUTH[@]}" -e "CREATE DATABASE IF NOT EXISTS $DB_NAME CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci;"This keeps the password out of logs, avoids interactive prompts, and makes the connectivity check respect the actual
DATABASE_URL.backend/app/api/endpoints/adapter/attachments.py (1)
92-98: Fix reference to removedDocumentParser.MAX_FILE_SIZE.
DocumentParsernow exposesget_max_file_size()instead of aMAX_FILE_SIZEattribute. This will raiseAttributeErrorwhen a file exceeds the size limit.Apply this diff to fix the issue:
# Validate file size before processing if not DocumentParser.validate_file_size(len(binary_data)): - max_size_mb = DocumentParser.MAX_FILE_SIZE / (1024 * 1024) + max_size_mb = DocumentParser.get_max_file_size() / (1024 * 1024) raise HTTPException( status_code=400, detail=f"File size exceeds maximum limit ({max_size_mb} MB)", )docker/backend/Dockerfile (1)
10-15: uv installation lacks error handling and version pinning.The uv installation via
curl | shdoesn't include version pinning or explicit error handling, which can lead to non-deterministic builds and silent failures.Consider this improvement:
-# Install uv -RUN curl -LsSf https://astral.sh/uv/install.sh | sh && \ - export PATH="/root/.local/bin:${PATH}" && \ - uv --version +# Install uv with version pinning +ENV UV_VERSION=0.5.11 +RUN curl -fSL https://astral.sh/uv/install.sh -o /tmp/install-uv.sh && \ + sh /tmp/install-uv.sh && \ + rm /tmp/install-uv.sh && \ + export PATH="/root/.local/bin:${PATH}" && \ + uv --version | grep -q "${UV_VERSION}" || (echo "UV version mismatch" && exit 1)docker/executor_manager/Dockerfile (1)
17-17: Fix incorrect uv command syntax.The command
uv pip install --system --no-cache -r pyproject.tomluses the-rflag, which is invalid forpyproject.tomlfiles. The-rflag is designed for requirements.txt files. Forpyproject.toml, useuv syncinstead, which properly reads dependencies from the project configuration and lockfile.-RUN cd /app/executor_manager && uv pip install --system --no-cache -r pyproject.toml +RUN cd /app/executor_manager && uv sync --frozenNote: The
--frozenflag ensures the installation uses the existing lockfile without updating it, which is appropriate for Docker builds.Based on learnings from past reviews, this was previously flagged but remains unfixed.
backend/START_SCRIPT_USAGE.md (1)
121-123: Remove duplicate FAQ section heading.The FAQ section heading appears twice on consecutive lines (121-122), creating a documentation structure issue.
Apply this diff:
## FAQ -## FAQ - + ### Q: How to stop the service?executor_manager/README.md (1)
15-15: Update minimum Python version requirement to match enforcement.The documentation states "3.8+ required" but the executor_manager/start.sh script enforces Python 3.10+. This was previously flagged and marked as addressed, but the fix appears to have been reverted or lost.
Apply this diff:
The script will automatically: -- Check Python version (3.8+ required) +- Check Python version (3.10+ required) - Install uv if not presentBased on learnings from past reviews, ensure this aligns with the actual version check in start.sh.
🧹 Nitpick comments (20)
backend/app/services/chat/base.py (1)
86-86: Annotate class attribute withClassVar.The
DIRECT_CHAT_SHELL_TYPESlist is a class-level constant that should be annotated withtyping.ClassVarto distinguish it from instance attributes.Apply this diff:
+from typing import ClassVar, Optional -from typing import Optional - DIRECT_CHAT_SHELL_TYPES = ["Chat"] + DIRECT_CHAT_SHELL_TYPES: ClassVar[list[str]] = ["Chat"]backend/app/api/endpoints/adapter/tasks.py (2)
283-285: Preferlogging.exceptionin exception handlers.When logging from within an
exceptblock, uselogging.exceptioninstead oflogging.errorto automatically include the exception traceback.Apply this diff:
- logger.error( - f"Failed to update Chat Shell task {task_id} status: {str(e)}" - ) + logger.exception( + f"Failed to update Chat Shell task {task_id} status: {e!r}" + )
315-317: Preferlogging.exceptionin exception handlers.When logging from within an
exceptblock, uselogging.exceptioninstead oflogging.errorto automatically include the exception traceback.Apply this diff:
- logger.error( - f"Failed to update task {task_id} status to CANCELLING: {str(e)}" - ) + logger.exception( + f"Failed to update task {task_id} status to CANCELLING: {e!r}" + )backend/app/services/chat/__init__.py (1)
16-20: Consider sorting__all__alphabetically.For consistency with isort conventions,
__all__exports could be alphabetically sorted.Apply this diff:
__all__ = [ "chat_service", + "get_model_config_for_bot", "session_manager", - "get_model_config_for_bot", ]backend/app/api/endpoints/adapter/chat.py (3)
99-99: Consider if user_id parameter is necessary.Static analysis flags
user_idas potentially unused in_get_shell_type. While it's used to filter user shells (line 63), you may want to verify if this parameter is always needed or if the function could be simplified.Based on static analysis hints.
497-501: Remove unused variable assignment.
bot_crdis assigned at line 497 but never used. Consider removing this assignment to reduce noise.- bot_crd = Bot.model_validate(bot.json) bot_json = bot.json or {} bot_spec = bot_json.get("spec", {}) agent_config = bot_spec.get("agent_config", {})Based on static analysis hints.
544-546: Avoid redefinition of imports within function scope.
jsonandStreamingResponseare imported at lines 375 and 377, then redefined at lines 544 and 546. While not breaking, consider moving these imports to the top of the function or removing the redundant imports.Based on static analysis hints.
backend/app/services/chat/chat_service.py (1)
26-41: Minor cleanups: duplicate logger andOptionaltyping fordefault_headersTwo small improvements that will quiet Ruff and slightly tidy the module:
- Duplicate logger assignment
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)The second line is redundant and can be dropped:
-logger = logging.getLogger(__name__) -logger = logging.getLogger(__name__) +logger = logging.getLogger(__name__)
- Implicit Optional type for
default_headers(RUF013)These signatures use
Dict[str, Any] = None, which is flagged by Ruff:async def _call_openai_streaming( ..., default_headers: Dict[str, Any] = None ) -> AsyncGenerator[str, None]: ... async def _call_claude_streaming( ..., default_headers: Dict[str, Any] = None ) -> AsyncGenerator[str, None]: ... async def _call_gemini_streaming( ..., default_headers: Dict[str, Any] = None ) -> AsyncGenerator[str, None]:Given the project requires Python ≥3.10, you can use
| None:- async def _call_openai_streaming( - self, - client: httpx.AsyncClient, - api_key: str, - base_url: str, - model_id: str, - messages: List[Dict[str, str]], - default_headers: Dict[str, Any] = None, - ) -> AsyncGenerator[str, None]: + async def _call_openai_streaming( + self, + client: httpx.AsyncClient, + api_key: str, + base_url: str, + model_id: str, + messages: List[Dict[str, str]], + default_headers: Dict[str, Any] | None = None, + ) -> AsyncGenerator[str, None]:and similarly for
_call_claude_streaming,_call_gemini_streaming, and the corresponding*_with_cancelmethods wheredefault_headersis allowed to be absent.These are non‑functional but bring the file in line with Ruff and the typing guidelines.
Also applies to: 420-444, 522-549, 626-651, 726-771, 861-897, 987-1014
frontend/START_SCRIPT_USAGE.md (1)
40-53: Add languages to code fences and avoid bare URLs (markdownlint)markdownlint is flagging:
- Fenced blocks without a language (MD040)
- Bare URLs (MD034)
You can fix this by annotating the fences and wrapping URLs. For example:
-``` +```text Usage: ./start.sh [OPTIONS] @@ -``` +```text @@ -``` +```bash lsof -i :3000 @@ -```env +```env NEXT_PUBLIC_API_URL=http://localhost:8000 @@ -**Solution**: Install Node.js 18 or higher from https://nodejs.org/ +**Solution**: Install Node.js 18 or higher from <https://nodejs.org/>Apply similar language tags (
bash,env, ortext) to the remaining unlabeled fences, and wrap any other bare URLs in<...>or full markdown links to clear the lints.Also applies to: 59-72, 115-121, 131-142, 176-179, 191-194
backend/app/services/chat/session_manager.py (1)
344-369: Tightenexpiretype hints to avoid implicitOptional(RUF013)In
save_streaming_contentyou have:async def save_streaming_content( self, subtask_id: int, content: str, expire: int = None ) -> bool:Ruff flags
expire: int = Noneas an implicitOptional(RUF013). Given the rest of the file already usesOptional[...], and the project is on Python ≥3.10, you can make this explicit:- async def save_streaming_content( - self, subtask_id: int, content: str, expire: int = None - ) -> bool: + async def save_streaming_content( + self, subtask_id: int, content: str, expire: int | None = None + ) -> bool:This keeps semantics identical while satisfying the typing guideline. Apply the same pattern anywhere else you introduce
param: T = Nonein new code.Also applies to: 449-452
frontend/src/features/tasks/components/TaskListSection.tsx (1)
294-298: Consider removing debug logging before production deployment.The console.log statement provides useful diagnostics during development but may be verbose in production. Consider using a debug flag or removing it once the timestamp-based unread logic is stable.
Apply this diff if you'd like to remove the logging:
- const unread = isTaskUnread(task); - console.log( - `[TaskListSection] shouldShowStatusIcon for task ${task.id}: status=${task.status}, isUnread=${unread}, completed_at=${task.completed_at}, updated_at=${task.updated_at}` - ); - return unread; + return isTaskUnread(task);executor_manager/START_SCRIPT_USAGE.md (2)
45-60: Consider adding language identifiers to code blocks.Several fenced code blocks lack language identifiers, which impacts syntax highlighting and accessibility. While not critical, adding them improves documentation quality.
For example, at line 45:
-``` +```text Usage: ./start.sh [OPTIONS]Similar improvements can be made at lines 66, 132, 140, and 178. --- `119-120`: **Consider wrapping bare URLs in angle brackets.** Bare URLs on lines 119-120 could be wrapped in angle brackets for better Markdown compliance. Apply this pattern: ```diff -- **Executor Manager**: http://localhost:8001 -- **API Documentation**: http://localhost:8001/docs +- **Executor Manager**: <http://localhost:8001> +- **API Documentation**: <http://localhost:8001/docs>backend/app/services/attachment/attachment_service.py (1)
120-125: Consider usinglogging.exceptionfor error logging with exception info.When logging an error with exception context,
logging.exceptionis preferred overlogging.errorwithexc_info=True. It automatically includes the traceback and is more concise.Apply this diff:
except DocumentParseError as e: - logger.error(f"Document parsing failed for attachment {attachment.id}: {e}") + logger.exception(f"Document parsing failed for attachment {attachment.id}: {e}") attachment.status = AttachmentStatus.FAILED attachment.error_message = str(e) db.commit() raisescripts/hooks/ai-push-gate.sh (1)
282-303: LGTM! Consistent uv-based command execution.The changes properly wrap Python commands with
uv run, aligning with the repository-wide migration to uv-based tooling. The syntax check logic is correctly implemented.Optional: On line 286, the bare redirection triggers a shellcheck warning. While valid, it could be more explicit:
- > "$TEMP_DIR/syntax.log" # Clear/create the file + : > "$TEMP_DIR/syntax.log" # Clear/create the fileThe
:(colon) command is a no-op that makes the intent clearer to shellcheck..github/workflows/test.yml (1)
36-57: LGTM! Systematic migration to uv-based workflows.The changes consistently update the backend test job to use uv for dependency management:
- Cache paths and keys properly updated
- Installation flow follows uv best practices (install uv → source cargo env → uv sync)
- Test execution correctly uses
uv run pytestThis aligns with the repository-wide migration to pyproject.toml-based dependency management.
Optional: Address shellcheck warnings about unquoted variables for improved robustness:
run: | curl -LsSf https://astral.sh/uv/install.sh | sh - source $HOME/.cargo/env + source "$HOME/.cargo/env" uv sync --devApply similar quoting to other jobs (lines 92, 135 in install steps, and lines 56, 100, 146 in test steps).
executor_manager/start.sh (3)
17-17: Improve portability of the xargs command.The
-rflag forxargsis GNU-specific and not available on macOS/BSD systems. This could cause the cleanup function to fail on non-Linux systems.Apply this diff to improve portability:
- jobs -p | xargs -r kill 2>/dev/null || true + jobs -p | xargs kill 2>/dev/null || trueThe
-rflag prevents xargs from running if there's no input, but the|| truealready handles the empty case gracefully.
120-120: Consider separating declaration and assignment.Based on static analysis, declaring and assigning the variable separately can make error handling more explicit, though the current code with
|| truealready handles failures correctly.Apply this diff to address the static analysis hint:
- local suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true) + local suspended_jobs + suspended_jobs=$(jobs -l | grep -i "suspended" | grep "start.sh" || true)
239-242: Consider more idiomatic error handling.The exit code checks work correctly but could be simplified by using
set -ebehavior or immediate checks.Apply this diff for more idiomatic bash:
- uv venv --python "$PYTHON_EXEC" - if [ $? -ne 0 ]; then + if ! uv venv --python "$PYTHON_EXEC"; then echo -e "${RED}Error: Failed to create virtual environment${NC}" exit 1 fi-uv sync --python "$PYTHON_EXEC" -if [ $? -ne 0 ]; then +if ! uv sync --python "$PYTHON_EXEC"; then echo -e "${RED}Error: Failed to install dependencies${NC}" exit 1 fiAlso applies to: 249-252
backend/app/services/attachment/parser.py (1)
147-151: Improve exception handling pattern.Based on static analysis, the exception re-raising could follow Python best practices by using exception chaining with
fromto preserve the original traceback.Apply this diff:
except DocumentParseError: raise except Exception as e: logger.error(f"Error parsing document: {e}", exc_info=True) - raise DocumentParseError(f"Failed to parse document: {str(e)}") + raise DocumentParseError(f"Failed to parse document: {e}") from eThis preserves the exception chain and is more idiomatic Python. The same pattern should be applied to other parse methods in this file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
backend/uv.lockis excluded by!**/*.lockexecutor/uv.lockis excluded by!**/*.lockexecutor_manager/uv.lockis excluded by!**/*.lockfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (67)
.github/workflows/e2e-tests.yml(2 hunks).github/workflows/test.yml(7 hunks)AGENTS.md(2 hunks)CONTRIBUTING.md(5 hunks)README.md(2 hunks)README_zh.md(2 hunks)backend/README.md(3 hunks)backend/START_SCRIPT_USAGE.md(1 hunks)backend/app/api/api.py(1 hunks)backend/app/api/endpoints/adapter/attachments.py(11 hunks)backend/app/api/endpoints/adapter/chat.py(46 hunks)backend/app/api/endpoints/adapter/models.py(1 hunks)backend/app/api/endpoints/adapter/tasks.py(7 hunks)backend/app/core/config.py(0 hunks)backend/app/main.py(2 hunks)backend/app/models/subtask_attachment.py(5 hunks)backend/app/schemas/task.py(1 hunks)backend/app/services/adapters/task_kinds.py(4 hunks)backend/app/services/adapters/team_kinds.py(4 hunks)backend/app/services/attachment/__init__.py(1 hunks)backend/app/services/attachment/attachment_service.py(8 hunks)backend/app/services/attachment/parser.py(12 hunks)backend/app/services/chat/__init__.py(1 hunks)backend/app/services/chat/base.py(3 hunks)backend/app/services/chat/chat_service.py(34 hunks)backend/app/services/chat/model_resolver.py(17 hunks)backend/app/services/chat/session_manager.py(9 hunks)backend/pyproject.toml(2 hunks)backend/start.sh(1 hunks)docker/backend/Dockerfile(1 hunks)docker/executor/Dockerfile(2 hunks)docker/executor_manager/Dockerfile(1 hunks)docs/en/concepts/architecture.md(1 hunks)docs/en/faq.md(1 hunks)docs/en/getting-started/installation.md(4 hunks)docs/en/guides/developer/setup.md(4 hunks)docs/en/guides/developer/testing.md(3 hunks)docs/en/troubleshooting.md(1 hunks)docs/zh/concepts/architecture.md(1 hunks)docs/zh/faq.md(1 hunks)docs/zh/getting-started/installation.md(4 hunks)docs/zh/guides/developer/setup.md(4 hunks)docs/zh/guides/developer/testing.md(3 hunks)docs/zh/troubleshooting.md(1 hunks)executor/agents/agno/mcp_manager.py(1 hunks)executor/pyproject.toml(1 hunks)executor/start.sh(1 hunks)executor_manager/README.md(2 hunks)executor_manager/README_zh.md(1 hunks)executor_manager/START_SCRIPT_USAGE.md(1 hunks)executor_manager/pyproject.toml(2 hunks)executor_manager/start.sh(1 hunks)frontend/.env.local(1 hunks)frontend/START_SCRIPT_USAGE.md(1 hunks)frontend/package.json(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(6 hunks)frontend/src/features/tasks/components/MessageBubble.tsx(3 hunks)frontend/src/features/tasks/components/MessagesArea.tsx(2 hunks)frontend/src/features/tasks/components/TaskListSection.tsx(1 hunks)frontend/src/features/tasks/components/TaskSidebar.tsx(3 hunks)frontend/src/features/tasks/contexts/taskContext.tsx(1 hunks)frontend/src/types/api.ts(1 hunks)frontend/src/utils/taskViewStatus.ts(2 hunks)frontend/start.sh(1 hunks)scripts/hooks/ai-push-gate.sh(7 hunks)scripts/hooks/run-module-tests.sh(2 hunks)wegent-cli/setup.py(2 hunks)
💤 Files with no reviewable changes (1)
- backend/app/core/config.py
🚧 Files skipped from review as they are similar to previous changes (22)
- docs/en/getting-started/installation.md
- README_zh.md
- docs/zh/concepts/architecture.md
- docs/en/concepts/architecture.md
- backend/app/services/attachment/init.py
- README.md
- docs/en/guides/developer/testing.md
- docs/en/faq.md
- docs/zh/guides/developer/testing.md
- executor/pyproject.toml
- docs/zh/faq.md
- docker/executor/Dockerfile
- executor/agents/agno/mcp_manager.py
- docs/zh/guides/developer/setup.md
- backend/pyproject.toml
- frontend/.env.local
- backend/app/api/api.py
- docs/en/troubleshooting.md
- backend/app/api/endpoints/adapter/models.py
- wegent-cli/setup.py
- AGENTS.md
- backend/app/services/adapters/team_kinds.py
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
backend/app/schemas/task.pyfrontend/src/features/tasks/components/TaskListSection.tsxfrontend/src/types/api.tsfrontend/src/features/tasks/components/TaskSidebar.tsxfrontend/src/utils/taskViewStatus.tsfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsxbackend/app/services/adapters/task_kinds.pybackend/app/models/subtask_attachment.pybackend/app/main.pybackend/app/services/chat/__init__.pybackend/app/services/chat/chat_service.pyfrontend/src/features/tasks/components/MessageBubble.tsxbackend/app/services/attachment/attachment_service.pyfrontend/src/features/tasks/contexts/taskContext.tsxbackend/app/services/attachment/parser.pybackend/app/services/chat/session_manager.pybackend/app/api/endpoints/adapter/attachments.pybackend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.pybackend/app/services/chat/base.pybackend/app/services/chat/model_resolver.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/app/schemas/task.pybackend/app/services/adapters/task_kinds.pybackend/app/models/subtask_attachment.pybackend/app/main.pybackend/app/services/chat/__init__.pybackend/app/services/chat/chat_service.pybackend/app/services/attachment/attachment_service.pybackend/app/services/attachment/parser.pybackend/app/services/chat/session_manager.pybackend/app/api/endpoints/adapter/attachments.pybackend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.pybackend/app/services/chat/base.pybackend/app/services/chat/model_resolver.py
**/backend/app/schemas/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Pydantic schemas MUST be created in
app/schemas/directory
Files:
backend/app/schemas/task.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/features/tasks/components/TaskListSection.tsxfrontend/src/types/api.tsfrontend/src/features/tasks/components/TaskSidebar.tsxfrontend/src/utils/taskViewStatus.tsfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/features/tasks/contexts/taskContext.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/features/tasks/components/TaskListSection.tsxfrontend/src/types/api.tsfrontend/src/features/tasks/components/TaskSidebar.tsxfrontend/src/utils/taskViewStatus.tsfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/features/tasks/contexts/taskContext.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/TaskListSection.tsxfrontend/src/features/tasks/components/TaskSidebar.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/MessageBubble.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/tasks/components/TaskListSection.tsxfrontend/src/features/tasks/components/TaskSidebar.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/features/tasks/contexts/taskContext.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/tasks/components/TaskListSection.tsxfrontend/src/features/tasks/components/TaskSidebar.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/features/tasks/contexts/taskContext.tsx
**/types/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
TypeScript type definitions MUST be organized in
src/types/directory
Files:
frontend/src/types/api.ts
**/backend/app/services/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend business logic MUST be placed in
app/services/directory
Files:
backend/app/services/adapters/task_kinds.pybackend/app/services/chat/__init__.pybackend/app/services/chat/chat_service.pybackend/app/services/attachment/attachment_service.pybackend/app/services/attachment/parser.pybackend/app/services/chat/session_manager.pybackend/app/services/chat/base.pybackend/app/services/chat/model_resolver.py
**/backend/app/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend SQLAlchemy models MUST be created in
app/models/directory
Files:
backend/app/models/subtask_attachment.py
**/backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend API route handlers MUST be created in
app/api/directory
Files:
backend/app/api/endpoints/adapter/attachments.pybackend/app/api/endpoints/adapter/chat.pybackend/app/api/endpoints/adapter/tasks.py
🧠 Learnings (3)
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/app/services/**/*.py : Backend business logic MUST be placed in `app/services/` directory
Applied to files:
CONTRIBUTING.md
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/app/schemas/**/*.py : Backend Pydantic schemas MUST be created in `app/schemas/` directory
Applied to files:
docs/en/guides/developer/setup.md
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/alembic/versions/*.py : Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Applied to files:
backend/README.md
🧬 Code graph analysis (14)
frontend/src/features/tasks/components/TaskListSection.tsx (1)
frontend/src/utils/taskViewStatus.ts (1)
isTaskUnread(96-126)
frontend/src/features/tasks/components/TaskSidebar.tsx (1)
frontend/src/utils/taskViewStatus.ts (1)
getUnreadCount(153-155)
frontend/src/utils/taskViewStatus.ts (2)
backend/app/schemas/task.py (1)
TaskStatus(16-23)frontend/src/types/api.ts (2)
TaskStatus(129-136)Task(266-287)
backend/app/models/subtask_attachment.py (2)
frontend/src/types/api.ts (1)
AttachmentStatus(398-398)frontend/src/apis/attachments.ts (1)
AttachmentStatus(17-17)
backend/app/main.py (1)
backend/app/services/chat/base.py (1)
close_http_client(61-75)
backend/app/services/chat/chat_service.py (3)
backend/app/services/chat/session_manager.py (9)
register_stream(223-247)get_chat_history(56-84)is_cancelled(304-336)publish_streaming_chunk(420-447)save_streaming_content(344-369)delete_streaming_content(395-414)publish_streaming_done(449-478)unregister_stream(280-302)append_user_and_assistant_messages(139-184)backend/app/models/subtask.py (3)
Subtask(31-72)SubtaskStatus(17-23)SubtaskRole(26-28)backend/app/models/kind.py (1)
Kind(25-42)
backend/app/services/attachment/attachment_service.py (2)
backend/app/models/subtask_attachment.py (2)
AttachmentStatus(28-34)SubtaskAttachment(37-96)backend/app/services/attachment/parser.py (5)
DocumentParseError(25-28)is_supported_extension(76-78)validate_file_size(88-90)get_mime_type(81-85)parse(97-151)
frontend/src/features/tasks/contexts/taskContext.tsx (1)
frontend/src/utils/taskViewStatus.ts (1)
markTaskAsViewed(62-83)
backend/app/services/attachment/parser.py (1)
frontend/src/apis/attachments.ts (1)
SUPPORTED_EXTENSIONS(44-61)
backend/app/services/chat/session_manager.py (1)
backend/app/api/endpoints/adapter/chat.py (1)
get_streaming_content(1001-1066)
backend/app/api/endpoints/adapter/attachments.py (2)
backend/app/services/attachment/parser.py (2)
DocumentParser(31-395)validate_file_size(88-90)backend/app/services/attachment/attachment_service.py (3)
upload_attachment(35-135)delete_attachment(216-252)get_attachment_by_subtask(163-182)
backend/app/api/endpoints/adapter/chat.py (4)
backend/app/services/chat/base.py (2)
ChatServiceBase(78-99)is_direct_chat_shell(89-99)backend/app/services/adapters/task_kinds.py (2)
create_task_id(885-945)validate_task_id(947-970)backend/app/services/chat/model_resolver.py (1)
get_bot_system_prompt(461-507)backend/app/services/chat/session_manager.py (2)
get_streaming_content(371-393)subscribe_streaming_channel(480-502)
backend/app/api/endpoints/adapter/tasks.py (5)
backend/app/services/chat/session_manager.py (1)
cancel_stream(249-278)backend/app/schemas/kind.py (1)
Task(320-327)backend/app/core/cache.py (1)
get(44-61)backend/app/models/subtask.py (1)
SubtaskStatus(17-23)backend/app/schemas/subtask.py (1)
SubtaskStatus(25-31)
backend/app/services/chat/model_resolver.py (3)
shared/utils/crypto.py (1)
decrypt_api_key(222-239)backend/app/models/kind.py (1)
Kind(25-42)backend/app/schemas/kind.py (1)
Ghost(47-54)
🪛 actionlint (1.7.9)
.github/workflows/e2e-tests.yml
60-60: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/test.yml
46-46: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
55-55: shellcheck reported issue in this script: SC2086:info:1:8: Double quote to prevent globbing and word splitting
(shellcheck)
90-90: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
99-99: shellcheck reported issue in this script: SC2086:info:1:8: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:2:8: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 markdownlint-cli2 (0.18.1)
backend/START_SCRIPT_USAGE.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
65-65: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
118-118: Bare URL used
(MD034, no-bare-urls)
119-119: Bare URL used
(MD034, no-bare-urls)
122-122: Multiple headings with the same content
(MD024, no-duplicate-heading)
backend/README.md
74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
executor_manager/START_SCRIPT_USAGE.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
119-119: Bare URL used
(MD034, no-bare-urls)
120-120: Bare URL used
(MD034, no-bare-urls)
132-132: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
140-140: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
178-178: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
frontend/START_SCRIPT_USAGE.md
45-45: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
66-66: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
119-119: Bare URL used
(MD034, no-bare-urls)
120-120: Bare URL used
(MD034, no-bare-urls)
132-132: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
140-140: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
178-178: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.7)
backend/app/services/chat/__init__.py
16-20: __all__ is not sorted
Apply an isort-style sorting to __all__
(RUF022)
backend/app/services/chat/chat_service.py
129-129: Do not catch blind exception: Exception
(BLE001)
207-207: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
217-217: Use explicit conversion flag
Replace with conversion flag
(RUF010)
218-218: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
328-330: Avoid specifying long messages outside the exception class
(TRY003)
369-371: Avoid specifying long messages outside the exception class
(TRY003)
427-427: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
529-529: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
733-733: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
1135-1135: Do not catch blind exception: Exception
(BLE001)
1136-1136: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1188-1188: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1257-1257: Do not catch blind exception: Exception
(BLE001)
1258-1258: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
1295-1295: Do not catch blind exception: Exception
(BLE001)
1296-1298: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backend/app/services/attachment/attachment_service.py
66-69: Avoid specifying long messages outside the exception class
(TRY003)
75-75: Avoid specifying long messages outside the exception class
(TRY003)
121-121: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backend/app/services/attachment/parser.py
151-151: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
151-151: Avoid specifying long messages outside the exception class
(TRY003)
151-151: Use explicit conversion flag
Replace with conversion flag
(RUF010)
163-163: Abstract raise to an inner function
(TRY301)
163-163: Avoid specifying long messages outside the exception class
(TRY003)
177-177: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
177-177: Avoid specifying long messages outside the exception class
(TRY003)
177-177: Use explicit conversion flag
Replace with conversion flag
(RUF010)
215-215: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
215-215: Avoid specifying long messages outside the exception class
(TRY003)
215-215: Use explicit conversion flag
Replace with conversion flag
(RUF010)
221-223: Abstract raise to an inner function
(TRY301)
221-223: Avoid specifying long messages outside the exception class
(TRY003)
257-257: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
257-257: Avoid specifying long messages outside the exception class
(TRY003)
257-257: Use explicit conversion flag
Replace with conversion flag
(RUF010)
263-265: Abstract raise to an inner function
(TRY301)
263-265: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
295-295: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Use explicit conversion flag
Replace with conversion flag
(RUF010)
317-317: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
317-317: Avoid specifying long messages outside the exception class
(TRY003)
317-317: Use explicit conversion flag
Replace with conversion flag
(RUF010)
351-353: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
351-353: Avoid specifying long messages outside the exception class
(TRY003)
352-352: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/services/chat/session_manager.py
80-80: Consider moving this statement to an else block
(TRY300)
82-82: Do not catch blind exception: Exception
(BLE001)
83-83: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
114-114: Do not catch blind exception: Exception
(BLE001)
115-115: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
135-135: Do not catch blind exception: Exception
(BLE001)
136-136: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
182-182: Do not catch blind exception: Exception
(BLE001)
183-183: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
200-200: Do not catch blind exception: Exception
(BLE001)
201-201: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
244-244: Do not catch blind exception: Exception
(BLE001)
267-267: Do not catch blind exception: Exception
(BLE001)
268-270: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
299-299: Do not catch blind exception: Exception
(BLE001)
345-345: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
366-368: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
390-392: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
410-410: Do not catch blind exception: Exception
(BLE001)
411-413: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
444-446: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
476-476: Do not catch blind exception: Exception
(BLE001)
477-477: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
499-501: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backend/app/api/endpoints/adapter/attachments.py
88-88: Do not catch blind exception: Exception
(BLE001)
89-89: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
90-90: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
backend/app/api/endpoints/adapter/chat.py
99-99: Unused function argument: user_id
(ARG001)
497-497: Local variable bot_crd is assigned to but never used
Remove assignment to unused variable bot_crd
(F841)
544-544: Redefinition of unused json from line 375
Remove definition: json
(F811)
546-546: Redefinition of unused StreamingResponse from line 377
Remove definition: StreamingResponse
(F811)
backend/app/api/endpoints/adapter/tasks.py
283-285: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
284-284: Use explicit conversion flag
Replace with conversion flag
(RUF010)
315-317: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
316-316: Use explicit conversion flag
Replace with conversion flag
(RUF010)
backend/app/services/chat/base.py
86-86: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
backend/app/services/chat/model_resolver.py
184-184: Do not catch blind exception: Exception
(BLE001)
216-216: Do not catch blind exception: Exception
(BLE001)
290-290: Avoid specifying long messages outside the exception class
(TRY003)
295-295: Avoid specifying long messages outside the exception class
(TRY003)
385-385: f-string without any placeholders
Remove extraneous f prefix
(F541)
404-404: f-string without any placeholders
Remove extraneous f prefix
(F541)
416-416: Do not catch blind exception: Exception
(BLE001)
449-449: Do not catch blind exception: Exception
(BLE001)
🪛 Shellcheck (0.11.0)
scripts/hooks/ai-push-gate.sh
[warning] 286-286: This redirection doesn't have a command. Move to its command (or use 'true' as no-op).
(SC2188)
executor/start.sh
[warning] 104-104: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 220-220: Declare and assign separately to avoid masking return values.
(SC2155)
backend/start.sh
[warning] 136-136: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 254-254: Declare and assign separately to avoid masking return values.
(SC2155)
executor_manager/start.sh
[warning] 120-120: Declare and assign separately to avoid masking return values.
(SC2155)
frontend/start.sh
[warning] 99-99: Declare and assign separately to avoid masking return values.
(SC2155)
| semaphore = _get_chat_semaphore() | ||
|
|
||
| async def generate() -> AsyncGenerator[str, None]: | ||
| acquired = False | ||
| # Register this stream for cancellation support (async for Redis) | ||
| cancel_event = await session_manager.register_stream(subtask_id) | ||
|
|
||
| # Track accumulated response for incremental saving | ||
| full_response = "" | ||
| client_disconnected = False | ||
|
|
||
| try: | ||
| # Try to acquire semaphore with timeout | ||
| try: | ||
| acquired = await asyncio.wait_for( | ||
| semaphore.acquire(), | ||
| timeout=5.0 | ||
| ) | ||
| acquired = await asyncio.wait_for(semaphore.acquire(), timeout=5.0) | ||
| except asyncio.TimeoutError: | ||
| error_msg = "Too many concurrent chat requests, please try again later" | ||
| error_msg = ( | ||
| "Too many concurrent chat requests, please try again later" | ||
| ) | ||
| yield f"data: {json.dumps({'error': error_msg})}\n\n" | ||
| await self._update_subtask_status(subtask_id, "FAILED", error=error_msg) | ||
| await self._update_subtask_status( | ||
| subtask_id, "FAILED", error=error_msg | ||
| ) | ||
| return | ||
|
|
||
| # Update status to RUNNING | ||
| await self._update_subtask_status(subtask_id, "RUNNING") | ||
|
|
||
| # Get chat history | ||
| history = await session_manager.get_chat_history(task_id) | ||
|
|
||
| # Build messages list | ||
| messages = self._build_messages(history, message, system_prompt) | ||
|
|
||
| # Call LLM API and stream response with cancellation support | ||
| cancelled = False | ||
|
|
||
| # Incremental save timing | ||
| last_redis_save = time.time() | ||
| last_db_save = time.time() | ||
| redis_interval = settings.STREAMING_REDIS_SAVE_INTERVAL | ||
| db_interval = settings.STREAMING_DB_SAVE_INTERVAL | ||
|
|
||
| async for chunk in self._call_llm_streaming_with_cancel( | ||
| model_config, messages, subtask_id, cancel_event | ||
| ): | ||
| # Check if cancelled (local event or Redis flag) | ||
| if cancel_event.is_set() or await session_manager.is_cancelled(subtask_id): | ||
| if cancel_event.is_set() or await session_manager.is_cancelled( | ||
| subtask_id | ||
| ): | ||
| cancelled = True |
There was a problem hiding this comment.
Cancellation path can be misclassified as normal completion
chat_stream tracks a local cancelled flag, but it is only set inside the loop body:
async for chunk in self._call_llm_streaming_with_cancel(...):
if cancel_event.is_set() or await session_manager.is_cancelled(subtask_id):
cancelled = True
break
...The inner _call_llm_streaming_with_cancel also stops early when cancel_event is set. In that case the async for terminates without another iteration, so the body isn’t executed and cancelled remains False. The code then falls into the “normal completion” branch:
if cancelled:
...
else:
# Normal completion path: append history, mark COMPLETED, publish done, etc.This means an explicit user cancel can be treated as a normal completion, conflicting with the comment:
# User explicitly cancelled - the cancel endpoint handles saving
and potentially double‑saving history / status.
You can ensure cancellation is detected even when the inner generator exits early by checking the flag once after the loop:
@@
- async for chunk in self._call_llm_streaming_with_cancel(
- model_config, messages, subtask_id, cancel_event
- ):
+ async for chunk in self._call_llm_streaming_with_cancel(
+ model_config, messages, subtask_id, cancel_event
+ ):
# Check if cancelled (local event or Redis flag)
if cancel_event.is_set() or await session_manager.is_cancelled(
subtask_id
):
cancelled = True
break
@@
- # Handle different completion scenarios
- if cancelled:
+ # Handle different completion scenarios
+ if cancelled or cancel_event.is_set() or await session_manager.is_cancelled(
+ subtask_id
+ ):
+ cancelled = True
# User explicitly cancelled - the cancel endpoint handles saving
if not client_disconnected:
yield f"data: {json.dumps({'content': '', 'done': True, 'cancelled': True})}\n\n"
else:
# Normal completion (or client disconnected but stream finished)
...This keeps the intended “cancel endpoint owns persistence” behavior and avoids marking cancelled subtasks as COMPLETED by this path.
Also applies to: 155-181
🤖 Prompt for AI Agents
In backend/app/services/chat/chat_service.py around lines 72 to 122 (and also
apply same fix to 155-181), the loop-local cancelled flag can remain False if
the inner streaming generator exits early due to cancel_event being set, causing
the code to treat an explicit cancellation as normal completion; after the async
for loop, explicitly re-check cancellation by querying cancel_event.is_set() or
awaiting session_manager.is_cancelled(subtask_id) and set cancelled = True if
so, then follow the cancelled branch (so the cancel endpoint remains responsible
for persistence) before executing the normal-completion logic.
| # Or manually run: | ||
| alembic upgrade head | ||
| ``` | ||
| ``` |
There was a problem hiding this comment.
Remove empty code fence.
Line 74 contains an orphaned closing code fence that should be removed. This breaks markdown rendering and was flagged by static analysis.
Apply this diff:
-```Note: This issue was previously flagged but appears to still be present.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
74-74: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In backend/README.md around line 74 there is an orphaned closing code fence
("```") that should be removed; open the file, delete that lone closing fence on
line 74 so there is no unmatched code block terminator, save the file, and run
markdown lint/preview to confirm the document renders correctly.
|
|
||
| # Install dependencies | ||
| pip install -r requirements.txt | ||
| uv sync |
There was a problem hiding this comment.
Add uv installation instructions.
The document instructs developers to run uv sync (lines 142, 256) but does not explain how to install the uv tool. Add installation instructions in the Prerequisites section or before first usage.
Consider adding a section like:
#### Install uv
# macOS/Linux
curl -LsSf https://astral.sh/uv/install.sh | sh
# Windows
powershell -c "irm https://astral.sh/uv/install.ps1 | iex"
# Verify installation
uv --versionAlso applies to: 256-256
🤖 Prompt for AI Agents
In docs/en/guides/developer/setup.md around lines 142 and 256, the guide calls
out running "uv sync" but never explains how to install the uv CLI; add a short
"Install uv" subsection in the Prerequisites or immediately before the first uv
usage that includes platform-specific install commands (macOS/Linux curl
installer, Windows PowerShell installer) and a verification step (uv --version),
and update the table-of-contents or headings if needed so readers can find the
install instructions before the first uv invocation.
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
backend/app/api/endpoints/adapter/chat.py (1)
861-866: Guard Redis pubsub cleanup when subscription fails.If
subscribe_streaming_channelreturns(None, None), thefinallyblock will call methods onNone, causing anAttributeError. This was noted in a previous review.Apply the same guard pattern in both
_handle_resume_stream(lines 861-866) andresume_stream(lines 1261-1266):finally: # Cleanup: unsubscribe and close client - await pubsub.unsubscribe() - await pubsub.close() + if pubsub: + await pubsub.unsubscribe() + await pubsub.close() if redis_client: await redis_client.close()
🧹 Nitpick comments (14)
docs/TASK_SHARING_FEATURE.md (1)
39-61: Tighten Markdown style: add languages to fenced blocks and avoid bold-as-headingThe content is great; a few small formatting tweaks will keep markdownlint happy and improve readability:
- Specify a language for fenced code blocks (MD040)
Blocks like the flow diagram, sample URLs, ASCII UIs, and JSON/HTTP snippets currently use bare ``` fences. Consider:
- For flow/ASCII diagrams:
-``` +```text 用户 A 分享任务 ↓ ... -``` +```
- For URLs and HTTP endpoints:
-``` -http://localhost:3000/shared/task?token=test123dEA%3D%3D -``` +```text +http://localhost:3000/shared/task?token=test123dEA%3D%3D +```
- For API examples:
-``` -POST /api/tasks/{task_id}/share -``` +```http +POST /api/tasks/{task_id}/share +```
- Replace emphasized “headings” with real headings (MD036)
Lines like the API section titles are currently written as bold text, e.g.:**1. 生成分享链接**Prefer true headings instead:
-**1. 生成分享链接** +#### 1. 生成分享链接Apply the same pattern to the other emphasized section titles (
2. 获取分享信息…,3. 获取公开分享任务…, etc.).These small adjustments should clear the markdownlint warnings while keeping the document structure intact.
Also applies to: 71-73, 137-159, 197-209, 247-337, 408-475
frontend/src/features/tasks/components/TaskShareModal.tsx (2)
31-53: Clipboard fallback uses deprecated API but handles errors gracefully.The
document.execCommand('copy')fallback is deprecated but still functional in current browsers. The implementation is acceptable for broad browser compatibility. Consider adding error handling for the fallback path as well, sinceexecCommandcan also fail silently.} catch { // Fallback to traditional method if clipboard API is not available - const textArea = document.createElement('textarea'); - textArea.value = shareUrl; - document.body.appendChild(textArea); - textArea.select(); - document.execCommand('copy'); - document.body.removeChild(textArea); - toast({ - title: t('shared_task.link_copied'), - description: t('shared_task.link_copied_desc'), - }); - onClose(); + try { + const textArea = document.createElement('textarea'); + textArea.value = shareUrl; + document.body.appendChild(textArea); + textArea.select(); + document.execCommand('copy'); + document.body.removeChild(textArea); + toast({ + title: t('shared_task.link_copied'), + description: t('shared_task.link_copied_desc'), + }); + onClose(); + } catch { + toast({ + variant: 'destructive', + title: t('messages.copy_failed'), + }); + } }
78-78: Redundant empty div for spacing.This empty div creates spacing that's already handled by
mt-2on the next Alert. Consider removing it.<Alert variant="default" className="text-sm"> <AlertDescription>{t('shared_task.share_link_info')}</AlertDescription> </Alert> - <div className="mt-2"></div> <Alert variant="default" className="text-sm mt-2">frontend/src/features/tasks/components/TaskShareHandler.tsx (1)
52-52: Unused loading state variable.The
_isLoadingstate is set but never used in the UI. Consider either:
- Using it to show a loading indicator while fetching share info
- Removing it if not needed
If you want to show loading state, add a loading indicator:
if (_isLoading) { return <LoadingSpinner />; }Or remove the unused state:
- const [_isLoading, setIsLoading] = useState(false);and update the
setIsLoadingcalls accordingly.frontend/src/features/tasks/components/ChatArea.tsx (1)
541-555: Consider a more robust approach than a fixed 100ms delay.The 100ms delay is a timing-based workaround that may not be reliable under varying conditions (slow devices, heavy load). While it addresses the immediate flash issue, consider using an event-based approach where TaskParamSync signals completion, or at minimum, document why 100ms was chosen.
That said, the error handling with
try/catch/finallyis correct and ensures proper cleanup.frontend/src/app/shared/task/page.tsx (1)
72-85: Error message matching via string includes is fragile.The error-to-i18n mapping relies on checking if error messages contain specific substrings. If the backend changes error text (e.g., from "Invalid share token" to "Token is invalid"), this will break. Consider having the backend return error codes that can be mapped more reliably.
frontend/src/features/tasks/components/MessagesArea.tsx (1)
707-714: Consider using i18n for the fallback task title.The fallback
'Untitled Task'is hardcoded in English. For consistency with the localized UI, consider using a translation key liket('shared_task.untitled_task').<TaskShareModal visible={showShareModal} onClose={() => setShowShareModal(false)} - taskTitle={selectedTaskDetail?.title || 'Untitled Task'} + taskTitle={selectedTaskDetail?.title || t('shared_task.untitled_task')} shareUrl={shareUrl} />backend/app/api/endpoints/adapter/tasks.py (1)
291-293: Uselogging.exceptionfor better stack trace capture in exception handlers.When logging errors within exception handlers,
logging.exceptionautomatically includes the stack trace, making debugging easier.except Exception as e: - logger.error( - f"Failed to update Chat Shell task {task_id} status: {str(e)}" - ) + logger.exception( + "Failed to update Chat Shell task %s status", task_id + )And similarly for line 322-324:
except Exception as e: - logger.error( - f"Failed to update task {task_id} status to CANCELLING: {str(e)}" - ) + logger.exception( + "Failed to update task %s status to CANCELLING", task_id + )Also applies to: 322-324
backend/app/models/shared_task.py (2)
9-9: Remove unused importString.The
Stringtype is imported but not used in this model.from sqlalchemy import ( Boolean, Column, DateTime, ForeignKey, Integer, - String, UniqueConstraint, )
53-56: Consider using timezone-aware datetime for future compatibility.
datetime.utcnowis deprecated since Python 3.12. Consider usingdatetime.now(timezone.utc)for timezone-aware timestamps, though this should be done consistently across the codebase.-from datetime import datetime +from datetime import datetime, timezone # In the Column definitions: - created_at = Column(DateTime, default=datetime.utcnow, nullable=False) + created_at = Column(DateTime, default=lambda: datetime.now(timezone.utc), nullable=False) updated_at = Column( - DateTime, default=datetime.utcnow, onupdate=datetime.utcnow, nullable=False + DateTime, default=lambda: datetime.now(timezone.utc), onupdate=lambda: datetime.now(timezone.utc), nullable=False )backend/app/services/shared_task.py (1)
530-538: Chain exceptions properly for better debugging context.When re-raising
HTTPExceptionfrom caught exceptions, useraise ... from Noneto suppress the original exception context (if intentional) orraise ... from errto preserve it.try: user_id = int(user_id_str) task_id = int(task_id_str) except ValueError: - raise HTTPException(status_code=400, detail="Invalid share link format") + raise HTTPException(status_code=400, detail="Invalid share link format") from None except HTTPException: raise except Exception: - raise HTTPException(status_code=400, detail="Invalid share link format") + raise HTTPException(status_code=400, detail="Invalid share link format") from Nonebackend/app/schemas/shared_task.py (1)
33-33: Minor:Optional[bool] = Falsecould be simplified tobool = False.Since the field has a default value, it's never truly
Nonefrom a consumer perspective. However, this is a stylistic choice and the current form is valid.- force_override_bot_model: Optional[bool] = False + force_override_bot_model: bool = Falseexecutor_manager/pyproject.toml (2)
16-16: Use version constraints that prevent unintended breaking changes.The version constraint
httpx>=0.27.0is permissive and allows breaking changes (e.g.,0.28.0). Consider using a more conservative constraint likehttpx>=0.27.0,<0.28.0to avoid unexpected behavior from minor/patch version upgrades.Apply this diff:
- "httpx>=0.27.0", + "httpx>=0.27.0,<0.28.0",
25-32: Version constraints for dev dependencies can be tightened for better reproducibility.All listed test dependencies—pytest 9.0.1, pytest-asyncio 1.3.0, pytest-cov 7.0.0, pytest-mock 3.15.1, and pytest-httpx 0.36.0—are confirmed compatible with Python 3.10. However, the current loose constraints (
>=) risk unexpected breaking changes across major versions. Consider tightening to the next major boundary (e.g.,pytest>=9.0.1,<10.0.0) to improve reproducibility while still allowing patch updates:dev = [ - "pytest>=9.0.1", - "pytest-asyncio>=1.3.0", - "pytest-cov>=7.0.0", - "pytest-mock>=3.15.1", - "pytest-httpx>=0.36.0", + "pytest>=9.0.1,<10.0.0", + "pytest-asyncio>=1.3.0,<2.0.0", + "pytest-cov>=7.0.0,<8.0.0", + "pytest-mock>=3.15.1,<4.0.0", + "pytest-httpx>=0.36.0,<1.0.0", ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
executor_manager/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py(1 hunks)backend/app/api/endpoints/adapter/chat.py(45 hunks)backend/app/api/endpoints/adapter/tasks.py(10 hunks)backend/app/core/config.py(1 hunks)backend/app/models/shared_task.py(1 hunks)backend/app/models/user.py(1 hunks)backend/app/schemas/shared_task.py(1 hunks)backend/app/services/adapters/task_kinds.py(5 hunks)backend/app/services/shared_task.py(1 hunks)docs/TASK_SHARING_FEATURE.md(1 hunks)executor_manager/pyproject.toml(2 hunks)frontend/e2e/tests/settings-github.spec.ts(1 hunks)frontend/src/apis/tasks.ts(2 hunks)frontend/src/app/(tasks)/chat/page.tsx(7 hunks)frontend/src/app/(tasks)/code/page.tsx(3 hunks)frontend/src/app/shared/task/page.tsx(1 hunks)frontend/src/features/common/AuthGuard.tsx(1 hunks)frontend/src/features/tasks/components/ChatArea.tsx(9 hunks)frontend/src/features/tasks/components/MessageBubble.tsx(3 hunks)frontend/src/features/tasks/components/MessagesArea.tsx(8 hunks)frontend/src/features/tasks/components/PublicTaskSidebar.tsx(1 hunks)frontend/src/features/tasks/components/TaskShareHandler.tsx(1 hunks)frontend/src/features/tasks/components/TaskShareModal.tsx(1 hunks)frontend/src/i18n/locales/en/chat.json(1 hunks)frontend/src/i18n/locales/en/common.json(1 hunks)frontend/src/i18n/locales/zh-CN/chat.json(1 hunks)frontend/src/i18n/locales/zh-CN/common.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/e2e/tests/settings-github.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/features/tasks/components/MessageBubble.tsx
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
backend/app/models/user.pyfrontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/code/page.tsxfrontend/src/app/(tasks)/chat/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/common/AuthGuard.tsxbackend/app/services/adapters/task_kinds.pybackend/app/models/shared_task.pybackend/app/schemas/shared_task.pyfrontend/src/features/tasks/components/ChatArea.tsxbackend/app/services/shared_task.pybackend/app/core/config.pybackend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.pyfrontend/src/apis/tasks.tsbackend/app/api/endpoints/adapter/tasks.pybackend/app/api/endpoints/adapter/chat.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/app/models/user.pybackend/app/services/adapters/task_kinds.pybackend/app/models/shared_task.pybackend/app/schemas/shared_task.pybackend/app/services/shared_task.pybackend/app/core/config.pybackend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.pybackend/app/api/endpoints/adapter/tasks.pybackend/app/api/endpoints/adapter/chat.py
**/backend/app/models/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend SQLAlchemy models MUST be created in
app/models/directory
Files:
backend/app/models/user.pybackend/app/models/shared_task.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/code/page.tsxfrontend/src/app/(tasks)/chat/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/common/AuthGuard.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/apis/tasks.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/code/page.tsxfrontend/src/app/(tasks)/chat/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/common/AuthGuard.tsxfrontend/src/features/tasks/components/ChatArea.tsxfrontend/src/apis/tasks.ts
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/tasks/components/ChatArea.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/code/page.tsxfrontend/src/app/(tasks)/chat/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/common/AuthGuard.tsxfrontend/src/features/tasks/components/ChatArea.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/tasks/components/PublicTaskSidebar.tsxfrontend/src/features/tasks/components/TaskShareModal.tsxfrontend/src/app/shared/task/page.tsxfrontend/src/app/(tasks)/code/page.tsxfrontend/src/app/(tasks)/chat/page.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/TaskShareHandler.tsxfrontend/src/features/common/AuthGuard.tsxfrontend/src/features/tasks/components/ChatArea.tsx
**/backend/app/services/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend business logic MUST be placed in
app/services/directory
Files:
backend/app/services/adapters/task_kinds.pybackend/app/services/shared_task.py
**/backend/app/schemas/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Pydantic schemas MUST be created in
app/schemas/directory
Files:
backend/app/schemas/shared_task.py
**/backend/alembic/versions/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Files:
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py
**/backend/app/api/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend API route handlers MUST be created in
app/api/directory
Files:
backend/app/api/endpoints/adapter/tasks.pybackend/app/api/endpoints/adapter/chat.py
🧠 Learnings (2)
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/app/schemas/**/*.py : Backend Pydantic schemas MUST be created in `app/schemas/` directory
Applied to files:
backend/app/schemas/shared_task.py
📚 Learning: 2025-12-01T06:44:24.499Z
Learnt from: CR
Repo: wecode-ai/Wegent PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-01T06:44:24.499Z
Learning: Applies to **/backend/alembic/versions/*.py : Backend Alembic database migrations MUST be reviewed before applying, especially auto-generated migrations
Applied to files:
backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py
🧬 Code graph analysis (8)
frontend/src/features/tasks/components/PublicTaskSidebar.tsx (2)
frontend/src/components/ui/button.tsx (1)
Button(55-55)frontend/e2e/utils/auth.ts (1)
isLoggedIn(94-110)
frontend/src/features/tasks/components/TaskShareModal.tsx (3)
frontend/src/hooks/use-toast.ts (2)
useToast(189-189)toast(189-189)frontend/src/components/ui/alert.tsx (2)
Alert(57-57)AlertDescription(57-57)frontend/src/components/ui/button.tsx (1)
Button(55-55)
frontend/src/app/shared/task/page.tsx (2)
frontend/src/apis/tasks.ts (1)
PublicSharedTaskResponse(132-138)frontend/src/apis/user.ts (1)
getToken(54-59)
frontend/src/app/(tasks)/chat/page.tsx (3)
frontend/src/features/tasks/contexts/taskContext.tsx (1)
useTaskContext(317-323)frontend/src/features/common/UserContext.tsx (1)
useUser(148-148)frontend/src/features/tasks/components/TaskShareHandler.tsx (1)
TaskShareHandler(43-403)
frontend/src/features/tasks/components/MessagesArea.tsx (2)
frontend/src/apis/tasks.ts (1)
taskApis(142-273)frontend/src/features/tasks/components/TaskShareModal.tsx (1)
TaskShareModal(22-98)
frontend/src/features/tasks/components/TaskShareHandler.tsx (2)
frontend/src/apis/tasks.ts (1)
TaskShareInfo(91-96)frontend/src/features/tasks/components/ModelSelector.tsx (3)
allBotsHavePredefinedModel(83-100)DEFAULT_MODEL_NAME(40-40)ModelSelector(102-606)
backend/app/schemas/shared_task.py (1)
frontend/src/apis/tasks.ts (7)
TaskShareInfo(91-96)TaskShareResponse(86-89)JoinSharedTaskRequest(98-103)JoinSharedTaskResponse(105-108)PublicAttachmentData(110-119)PublicSubtaskData(121-130)PublicSharedTaskResponse(132-138)
backend/app/api/endpoints/adapter/chat.py (4)
backend/app/models/subtask.py (3)
Subtask(31-72)SubtaskStatus(17-23)SubtaskRole(26-28)backend/app/services/adapters/task_kinds.py (2)
create_task_id(892-952)validate_task_id(954-977)backend/app/services/chat/session_manager.py (5)
get_chat_history(56-84)save_chat_history(86-116)get_streaming_content(371-393)subscribe_streaming_channel(480-502)cancel_stream(249-278)backend/app/models/subtask_attachment.py (1)
AttachmentStatus(28-34)
🪛 markdownlint-cli2 (0.18.1)
docs/TASK_SHARING_FEATURE.md
39-39: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
71-71: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
137-137: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
138-138: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
147-147: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
148-148: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
159-159: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
160-160: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
197-197: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
198-198: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
247-247: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
259-259: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
318-318: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
408-408: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
440-440: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
459-459: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Ruff (0.14.7)
backend/app/services/adapters/task_kinds.py
101-102: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
110-111: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
backend/app/services/shared_task.py
90-90: Do not catch blind exception: Exception
(BLE001)
131-131: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
141-141: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
164-164: Do not catch blind exception: Exception
(BLE001)
181-181: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
198-198: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
231-231: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
262-262: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
404-404: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
433-433: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
485-485: Avoid equality comparisons to True; use SharedTask.is_active: for truth checks
Replace with SharedTask.is_active
(E712)
500-500: Avoid equality comparisons to True; use SharedTask.is_active: for truth checks
Replace with SharedTask.is_active
(E712)
526-526: Abstract raise to an inner function
(TRY301)
534-534: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
537-537: Do not catch blind exception: Exception
(BLE001)
538-538: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
547-547: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
559-559: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
backend/app/api/endpoints/adapter/tasks.py
291-293: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
292-292: Use explicit conversion flag
Replace with conversion flag
(RUF010)
322-324: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
323-323: Use explicit conversion flag
Replace with conversion flag
(RUF010)
338-338: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
339-339: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
361-361: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
373-373: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
386-386: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
387-387: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
403-403: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
420-420: Avoid equality comparisons to True; use Kind.is_active: for truth checks
Replace with Kind.is_active
(E712)
backend/app/api/endpoints/adapter/chat.py
99-99: Unused function argument: user_id
(ARG001)
538-538: Local variable bot_crd is assigned to but never used
Remove assignment to unused variable bot_crd
(F841)
585-585: Redefinition of unused json from line 416
Remove definition: json
(F811)
587-587: Redefinition of unused StreamingResponse from line 418
Remove definition: StreamingResponse
(F811)
🔇 Additional comments (37)
backend/app/core/config.py (1)
86-88: The review comment is incorrect. Both usages ofTEAM_SHARE_BASE_URLproperly compose URLs with the/chatbase path:
shared_task.pyline 171:f"{base_url}/shared/task?token={share_token}"→http://localhost:3000/chat/shared/task?token=...shared_team.pyline 169:f"{settings.TEAM_SHARE_BASE_URL}?{settings.TEAM_SHARE_QUERY_PARAM}={share_token}"→http://localhost:3000/chat?teamShare=...No double paths or path-stripping issues exist. The code correctly appends subpaths and query parameters to the base URL.
Likely an incorrect or invalid review comment.
frontend/src/features/common/AuthGuard.tsx (1)
25-35: Allowing/shared/taskas a public route looks correctAdding
/shared/tasktoallowedPathscleanly exempts the public shared-task page from auth redirects, while still enforcing auth elsewhere. No issues from a routing or guard perspective.backend/app/services/adapters/task_kinds.py (3)
335-347: Includingcompleted_atin lite task results is consistent and low‑riskInitializing
completed_at = Noneand populating it fromtask_crd.status.completedAt(when present) then exposing it in the lite task dict adds useful information without changing existing behavior for tasks lacking this field. The broadexcept:and lack of DB fallback forcompleted_atmatch existing timestamp parsing patterns in this file, so this is acceptable for now.Also applies to: 433-433
627-643: Attachment mapping structure is clear and robustThe new
attachments_listconstruction:
- Safely checks
hasattr(subtask, "attachments")and truthiness before iterating.- Normalizes
statusto a primitive viaattachment.status.valuewhen present.- Returns a clean list of simple dicts suitable for JSON responses.
This avoids leaking ORM objects and should work even if
attachmentsis lazily loaded. Looks good.
1084-1084:model_idextraction cleanup is fineCollapsing
model_idextraction into a single line keeps semantics unchanged while reducing noise. No issues here.frontend/src/i18n/locales/en/common.json (1)
544-603: Newcopy_commandstring undershellsis fineThe added
"copy_command": "Copy command"key is consistent with the rest of theshellssection and respects the English‑only guideline for comments and labels.frontend/src/i18n/locales/en/chat.json (1)
41-48:shared_tasktranslations for chat are consistent and ready to useThe new
shared_taskgroup under the chat namespace cleanly covers share button label, loading state, empty selection, and error copy. No interpolation is used here, so the simple strings are safe and consistent with the English‑only guideline.frontend/src/i18n/locales/zh-CN/chat.json (1)
41-48: zh-CNshared_tasktranslations mirror the English set correctlyThe added
shared_taskblock matches the English keys and provides appropriate Chinese copy for sharing states (sharing, empty selection, failure, retry). This keeps the chat sharing UI bilingual and consistent.backend/app/models/user.py (1)
28-31:shared_tasksrelationship wiring looks correctThe new relationship:
shared_tasks = relationship( "SharedTask", foreign_keys="[SharedTask.user_id]", back_populates="user" )is an appropriate way to expose user‑side access to shared tasks, assuming
SharedTask.useris defined withback_populates="shared_tasks"anduser_idFKs intousers.id. Using the string form forforeign_keysis fine for deferred mapper configuration.frontend/src/app/(tasks)/code/page.tsx (1)
57-63: Share button plumbing for Code page is wired correctly
- Local
shareButtonstate plushandleShareButtonRendergivesChatAreaa clean hook to supply a share button.- Rendering
{shareButton}insideTopNavigationintegrates it into the existing header without affecting other controls.- Passing
onShareButtonRender={handleShareButtonRender}toChatAreamirrors the chat page pattern, so behavior should be consistent across modes.No issues with state, hooks, or layout based on these changes.
Also applies to: 176-192, 203-209
frontend/src/app/(tasks)/chat/page.tsx (2)
46-54: Consider clearing localStorage before navigation to avoid race conditions.The current flow reads
pendingTaskSharefrom localStorage, removes it, then navigates. If navigation fails or is slow, this is fine. However, if this component re-renders before navigation completes (unlikely but possible with React 18's concurrent features), the token is already cleared.The implementation looks correct for the intended use case. Just ensure the page that sets
pendingTaskShareonly does so right before redirecting to the login page.
114-116: Good use of Suspense for search params handling.The
TaskShareHandlercomponent usesuseSearchParams, which correctly requires a Suspense boundary in Next.js App Router.frontend/src/features/tasks/components/PublicTaskSidebar.tsx (1)
24-109: Well-structured component with good UX patterns.The component properly:
- Uses functional component with hooks as per coding guidelines
- Leverages i18n for all user-facing text
- Provides clear visual hierarchy with login CTAs
- Handles both logged-in and logged-out states appropriately
backend/app/api/endpoints/adapter/chat.py (1)
343-384: Good implementation of Redis chat history initialization for shared tasks.The logic correctly:
- Checks if Redis history is empty before rebuilding
- Sorts subtasks by message_id for correct ordering
- Only includes completed subtasks with valid content
- Handles both user prompts and assistant results appropriately
This ensures shared/copied tasks maintain conversation context.
frontend/src/features/tasks/components/TaskShareHandler.tsx (2)
86-125: Good async data fetching pattern with parallel requests.The
useEffectcorrectly:
- Fetches share info and teams in parallel using
Promise.all- Handles errors with user-friendly toast notifications
- Cleans up URL parameters on error
- Auto-selects the first team for better UX
259-364: Well-implemented team selector with search functionality.The team selection UI:
- Uses accessible combobox pattern with proper ARIA attributes
- Includes search filtering via
CommandInput- Handles empty states gracefully
- Shows visual feedback for selected team
frontend/src/features/tasks/components/ChatArea.tsx (3)
46-56: LGTM! New prop for share button rendering.The
onShareButtonRendercallback prop is correctly typed and destructured. This enables parent components to render the share button in their navigation area, following a clean composition pattern.
865-877: LGTM! Layout adjustments and prop forwarding.The increased max-width (
max-w-5xl) provides more reading space for messages. TheonShareButtonRenderprop is correctly forwarded toMessagesArea, enabling the share button to be rendered in the parent's TopNavigation.
1275-1275: LGTM! Bottom controls layout adjustment.Removing
mb-2and addingjustify-betweenallows proper spacing for the export PDF button positioning. The layout change maintains visual consistency.frontend/src/apis/tasks.ts (3)
85-138: LGTM! Well-structured type definitions for task sharing.The types are comprehensive and cover all aspects of the sharing flow:
TaskShareResponsefor share link generationTaskShareInfofor share metadataJoinSharedTaskRequest/Responsefor copying shared tasksPublicAttachmentData,PublicSubtaskData,PublicSharedTaskResponsefor public task viewingType definitions are clear and self-documenting with appropriate optional fields.
247-272: LGTM! Public endpoint correctly bypasses auth interceptor.Using native
fetchinstead ofapiClientis the correct approach for unauthenticated endpoints. The error handling properly extracts the error message from JSON responses when available, with a sensible fallback to raw text.One minor consideration: there's no request timeout, but this is consistent with other API calls in this file.
233-238: No changes needed. The endpoint is correctly implemented.The backend endpoint at
/tasks/share/infois truly public (no authentication decorator) and explicitly documented as not requiring authentication. TheapiClientdoes not enforce authentication—it conditionally adds the Authorization header only if a token exists (line 30:...(token && { Authorization: \Bearer ${token}` })). Unauthenticated users without a token will send requests without the header, which is acceptable for public endpoints. UsingapiClienthere is correct and consistent with similar public endpoints likegetTeamShareInfo`.frontend/src/app/shared/task/page.tsx (3)
37-53: LGTM! Login state detection and user fetching.The
getToken()check correctly determines login state. The user fetch in useEffect is properly guarded byisLoggedInand handles errors gracefully without breaking the page.
114-178: LGTM! Comprehensive message conversion logic.The
convertSubtaskToMessagefunction properly handles:
- Attachment format conversion with appropriate type casting
- User vs AI message differentiation
- Various result formats (object with value, plain string, null)
- Fallback content for different statuses (completed, failed, processing)
The separator string construction (
'$' + '{$$}$') is intentionally split to trigger markdown rendering in MessageBubble, as documented in the comment.
253-338: LGTM! Well-structured public task page with proper responsive design.The page correctly:
- Uses CSS variables for colors (
bg-base,text-text-primary,bg-surface)- Follows mobile-first approach with
sm:breakpoints (lines 270-271)- Uses functional component with hooks per coding guidelines
- Provides clear read-only notice and CTA for login/continue flow
- Wraps content in Suspense for proper loading states
frontend/src/i18n/locales/zh-CN/common.json (1)
605-655: LGTM! Comprehensive translation keys for task sharing feature.The
shared_tasktranslations cover all aspects of the sharing flow:
- Success/error states with descriptive messages
- Handler flow translations for the copy/join process
- Login prompts and CTA text
The keys are consistently named and provide appropriate Chinese translations.
frontend/src/features/tasks/components/MessagesArea.tsx (3)
560-565: Verify parent memoizesonShareButtonRenderto prevent unnecessary re-renders.This useEffect calls
onShareButtonRender(shareButton)on every change. If the parent doesn't memoize the callback, this could cause unnecessary re-renders. InChatArea.tsx,onShareButtonRenderis passed directly from props without memoization. Ensure the ultimate parent stabilizes this callback.
576-603: LGTM! Improved message key stability.The composite key pattern (
${msg.type}-${msg.subtaskId}) correctly handles cases where user and AI messages might share the same subtaskId. The fallback key (msg-${index}-${msg.timestamp}) handles edge cases without subtaskId.This prevents React duplicate key warnings and ensures stable component identity across re-renders.
207-241: LGTM! Well-implemented share task functionality.The share flow is correctly implemented:
- State management for modal visibility, URL, and loading state
handleShareTaskwrapped inuseCallbackwith proper dependencies- Error handling with descriptive toast messages
- Loading state prevents double-clicks
The translation keys (
shared_task.*) are properly available in the'chat'namespace used by this component.backend/app/api/endpoints/adapter/tasks.py (1)
383-436: LGTM on the join_shared_task logic.The team validation properly checks ownership and falls back to the user's first active team. The error handling for missing teams is appropriate.
Note: The
== Truecomparisons in SQLAlchemy queries (lines 403, 420) are intentional for correct SQL generation. WhileKind.is_active.is_(True)is the more idiomatic SQLAlchemy style, the current approach works correctly.backend/app/models/shared_task.py (1)
17-70: Well-structured ORM model with proper relationships and constraints.The model correctly defines:
- Foreign key relationships with CASCADE delete
- Unique constraint to prevent duplicate task copies
- Proper relationship specifications with explicit
foreign_keys- Useful
__repr__for debuggingbackend/app/services/shared_task.py (4)
39-42: Security note: Static IV in AES-CBC reduces confidentiality guarantees.Using a fixed IV means identical plaintexts produce identical ciphertexts, allowing observers to detect when the same user shares the same task. For share links, this is likely acceptable since:
- The data (user_id#task_id) isn't highly sensitive
- Deterministic tokens may be intentional (same link for repeated shares)
If stronger confidentiality is needed, consider using a random IV prepended to the ciphertext, or switch to AES-GCM which provides authenticated encryption.
Is the deterministic token behavior (same share link for the same task) intentional?
352-368: Consider memory implications when copying large attachments.Copying
binary_data(line 360) loads the entire file content into memory. For tasks with many large attachments, this could cause memory pressure.If large attachments are expected, consider:
- Streaming the copy operation
- Referencing the original attachment data instead of duplicating
- Adding a size limit check before copying
What's the expected maximum attachment size? If attachments are typically small (< 10MB), this approach is fine.
375-479: Solid join logic with proper edge case handling.The method correctly:
- Prevents users from copying their own tasks
- Handles reactivation of previously inactive share records
- Validates the original task still exists before copying
- Avoids unique constraint violations by reusing existing records
589-606: Good practice: Excluding sensitive binary data from public response.The public attachment format correctly excludes
binary_dataandimage_base64, only exposing metadata. This prevents unnecessary data exposure and reduces response size.backend/app/schemas/shared_task.py (1)
11-101: Schemas align well with frontend types; datetime serialization is handled by Pydantic.All schema definitions match the frontend TypeScript interfaces. The
datetimefields in Python will be serialized to ISO 8601 strings by FastAPI/Pydantic, matching the frontend'sstringtype expectations.The schemas follow the coding guidelines by being placed in
app/schemas/directory with proper type hints and docstrings. Based on learnings, this is the correct location for Pydantic schemas.executor_manager/pyproject.toml (1)
6-6: Remove this comment—the Python version change claim is unsupported.The review assumes a downgrade from
>=3.11to>=3.10, but git history showsrequires-python = ">=3.10"was set in the initial commit and has not changed. No evidence of a version downgrade exists in this PR.Likely an incorrect or invalid review comment.
| # revision identifiers, used by Alembic. | ||
| revision: str = '2b3c4d5e6f7g' | ||
| down_revision: Union[str, Sequence[str], None] = 'add_subtask_attachments' | ||
| branch_labels: Union[str, Sequence[str], None] = None | ||
| depends_on: Union[str, Sequence[str], None] = None |
There was a problem hiding this comment.
Non-standard revision ID format may cause issues.
The revision ID 2b3c4d5e6f7g contains the character 'g' which is not a valid hexadecimal digit. Alembic typically uses hex-only revision IDs. Consider regenerating with a standard ID like 2b3c4d5e6f70.
🤖 Prompt for AI Agents
In backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py around lines
14 to 18, the migration's revision ID '2b3c4d5e6f7g' contains a non-hex
character ('g') which can break Alembic; replace it with a valid
hexadecimal-only revision string (for example '2b3c4d5e6f70' or regenerate the
migration to produce a proper hex id) and update any references (down_revision
or elsewhere) consistently to the new id so Alembic recognizes the revision
chain.
| CREATE TABLE IF NOT EXISTS shared_tasks ( | ||
| id INT NOT NULL AUTO_INCREMENT COMMENT '主键ID', | ||
| user_id INT NOT NULL DEFAULT 0 COMMENT '当前用户ID', | ||
| original_user_id INT NOT NULL DEFAULT 0 COMMENT '原始任务所有者用户ID', | ||
| original_task_id INT NOT NULL DEFAULT 0 COMMENT '原始任务ID', | ||
| copied_task_id INT NOT NULL DEFAULT 0 COMMENT '复制后的任务ID', | ||
| is_active BOOLEAN NOT NULL DEFAULT TRUE COMMENT '是否激活', | ||
| created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT '创建时间', | ||
| updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP COMMENT '更新时间', | ||
| PRIMARY KEY (id), | ||
| KEY idx_shared_tasks_id (id), | ||
| KEY idx_shared_tasks_user_id (user_id), | ||
| KEY idx_shared_tasks_original_user_id (original_user_id), | ||
| KEY idx_shared_tasks_original_task_id (original_task_id), | ||
| KEY idx_shared_tasks_copied_task_id (copied_task_id), | ||
| UNIQUE KEY uniq_user_original_task (user_id, original_task_id) | ||
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 |
There was a problem hiding this comment.
Missing foreign key constraints and redundant index.
Several issues with the table definition:
-
Redundant index:
idx_shared_tasks_idon theidcolumn is unnecessary sinceidis already the PRIMARY KEY (which creates an implicit index). -
Missing foreign key constraints: The columns
user_id,original_user_id,original_task_id, andcopied_task_idshould have foreign key constraints to ensure referential integrity. -
DEFAULT 0 for FK columns: Using
DEFAULT 0for foreign key columns is risky as 0 is unlikely to be a valid ID, leading to orphaned records if constraints are later added.
Consider adding foreign key constraints:
PRIMARY KEY (id),
- KEY idx_shared_tasks_id (id),
KEY idx_shared_tasks_user_id (user_id),
KEY idx_shared_tasks_original_user_id (original_user_id),
KEY idx_shared_tasks_original_task_id (original_task_id),
KEY idx_shared_tasks_copied_task_id (copied_task_id),
- UNIQUE KEY uniq_user_original_task (user_id, original_task_id)
+ UNIQUE KEY uniq_user_original_task (user_id, original_task_id),
+ CONSTRAINT fk_shared_tasks_user FOREIGN KEY (user_id) REFERENCES users(id),
+ CONSTRAINT fk_shared_tasks_original_user FOREIGN KEY (original_user_id) REFERENCES users(id),
+ CONSTRAINT fk_shared_tasks_original_task FOREIGN KEY (original_task_id) REFERENCES kinds(id),
+ CONSTRAINT fk_shared_tasks_copied_task FOREIGN KEY (copied_task_id) REFERENCES kinds(id)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4Also remove DEFAULT 0 from the FK columns and make them properly nullable or required based on business logic. Based on learnings, migrations require careful review.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CREATE TABLE IF NOT EXISTS shared_tasks ( | |
| id INT NOT NULL AUTO_INCREMENT COMMENT '主键ID', | |
| user_id INT NOT NULL DEFAULT 0 COMMENT '当前用户ID', | |
| original_user_id INT NOT NULL DEFAULT 0 COMMENT '原始任务所有者用户ID', | |
| original_task_id INT NOT NULL DEFAULT 0 COMMENT '原始任务ID', | |
| copied_task_id INT NOT NULL DEFAULT 0 COMMENT '复制后的任务ID', | |
| is_active BOOLEAN NOT NULL DEFAULT TRUE COMMENT '是否激活', | |
| created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT '创建时间', | |
| updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP COMMENT '更新时间', | |
| PRIMARY KEY (id), | |
| KEY idx_shared_tasks_id (id), | |
| KEY idx_shared_tasks_user_id (user_id), | |
| KEY idx_shared_tasks_original_user_id (original_user_id), | |
| KEY idx_shared_tasks_original_task_id (original_task_id), | |
| KEY idx_shared_tasks_copied_task_id (copied_task_id), | |
| UNIQUE KEY uniq_user_original_task (user_id, original_task_id) | |
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 | |
| CREATE TABLE IF NOT EXISTS shared_tasks ( | |
| id INT NOT NULL AUTO_INCREMENT COMMENT '主键ID', | |
| user_id INT NOT NULL DEFAULT 0 COMMENT '当前用户ID', | |
| original_user_id INT NOT NULL DEFAULT 0 COMMENT '原始任务所有者用户ID', | |
| original_task_id INT NOT NULL DEFAULT 0 COMMENT '原始任务ID', | |
| copied_task_id INT NOT NULL DEFAULT 0 COMMENT '复制后的任务ID', | |
| is_active BOOLEAN NOT NULL DEFAULT TRUE COMMENT '是否激活', | |
| created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP COMMENT '创建时间', | |
| updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP COMMENT '更新时间', | |
| PRIMARY KEY (id), | |
| KEY idx_shared_tasks_user_id (user_id), | |
| KEY idx_shared_tasks_original_user_id (original_user_id), | |
| KEY idx_shared_tasks_original_task_id (original_task_id), | |
| KEY idx_shared_tasks_copied_task_id (copied_task_id), | |
| UNIQUE KEY uniq_user_original_task (user_id, original_task_id), | |
| CONSTRAINT fk_shared_tasks_user FOREIGN KEY (user_id) REFERENCES users(id), | |
| CONSTRAINT fk_shared_tasks_original_user FOREIGN KEY (original_user_id) REFERENCES users(id), | |
| CONSTRAINT fk_shared_tasks_original_task FOREIGN KEY (original_task_id) REFERENCES kinds(id), | |
| CONSTRAINT fk_shared_tasks_copied_task FOREIGN KEY (copied_task_id) REFERENCES kinds(id) | |
| ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 |
🤖 Prompt for AI Agents
In backend/alembic/versions/2b3c4d5e6f7g_add_shared_tasks_table.py around lines
26 to 42, remove the redundant idx_shared_tasks_id (PRIMARY KEY already indexes
id), stop using DEFAULT 0 on foreign key columns (user_id, original_user_id,
original_task_id, copied_task_id) and instead make them either NOT NULL or NULL
per business rules, and add proper FOREIGN KEY constraints referencing the users
and tasks tables (e.g., FOREIGN KEY (user_id) REFERENCES users(id) and similar
for original_user_id, original_task_id, copied_task_id) with explicit ON
DELETE/ON UPDATE policies (CASCADE or RESTRICT as appropriate); ensure any
needed non-unique indexes remain for query performance (but not duplicating the
primary key) and update migration to handle existing rows before adding
non-null/foreign-key constraints.
|
|
||
| # Build data_sources for placeholder replacement in DEFAULT_HEADERS | ||
| # This mirrors the executor's member_builder.py logic | ||
| bot_crd = Bot.model_validate(bot.json) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Unused variable bot_crd.
The variable bot_crd is assigned but never used. Remove the assignment or use the variable.
- bot_crd = Bot.model_validate(bot.json)
bot_json = bot.json or {}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bot_crd = Bot.model_validate(bot.json) | |
| bot_json = bot.json or {} |
🧰 Tools
🪛 Ruff (0.14.7)
538-538: Local variable bot_crd is assigned to but never used
Remove assignment to unused variable bot_crd
(F841)
🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/chat.py around line 538, the variable
`bot_crd` is assigned from Bot.model_validate(bot.json) but never used; remove
the unused assignment or use the validated model. Fix by either calling
Bot.model_validate(bot.json) without assigning its result (if you only need
validation) or replace subsequent uses of `bot` with `bot_crd` (or use `bot_crd`
where the validated model is required) and delete the unused variable if not
needed.
| import json | ||
|
|
||
| from fastapi.responses import StreamingResponse |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Duplicate imports of json and StreamingResponse.
These imports are already present at lines 416-418. Remove the duplicates.
# Create streaming response with task_id and subtask_id in first message
- import json
-
- from fastapi.responses import StreamingResponse
-
async def generate_with_ids():📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import json | |
| from fastapi.responses import StreamingResponse | |
| # Create streaming response with task_id and subtask_id in first message | |
| async def generate_with_ids(): |
🧰 Tools
🪛 Ruff (0.14.7)
585-585: Redefinition of unused json from line 416
Remove definition: json
(F811)
587-587: Redefinition of unused StreamingResponse from line 418
Remove definition: StreamingResponse
(F811)
🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/chat.py around lines 585 to 587, the
imports "import json" and "from fastapi.responses import StreamingResponse" are
duplicated (they already exist at lines 416–418); remove these duplicate import
statements at 585–587 so each module is imported only once to avoid redundancy
and potential linter errors.
| pass # Not JSON, treat as regular chunk | ||
| pass # Not JSON, treat as regular chunk |
There was a problem hiding this comment.
Duplicate pass statement.
There are two consecutive pass statements which is redundant.
except json.JSONDecodeError:
pass # Not JSON, treat as regular chunk
- pass # Not JSON, treat as regular chunk🤖 Prompt for AI Agents
In backend/app/api/endpoints/adapter/chat.py around lines 810 to 811, there are
two identical consecutive "pass" statements which is redundant; remove the
duplicate so only a single "pass" remains (or remove both if the surrounding
control flow no longer requires a noop) to eliminate the redundant line and keep
the intent clear.
| @router.get("/share/info", response_model=TaskShareInfo) | ||
| def get_task_share_info( | ||
| share_token: str = Query(..., description="Share token from URL"), | ||
| db: Session = Depends(get_db), | ||
| ): | ||
| """ | ||
| Get task share information from share token. | ||
| This endpoint doesn't require authentication, so anyone with the link can view. | ||
| """ | ||
| return shared_task_service.get_share_info(db=db, share_token=share_token) |
There was a problem hiding this comment.
Critical: Route ordering issue — /share/info will be incorrectly matched by /{task_id}.
The route GET /{task_id} is defined at line 159, before GET /share/info (line 358) and GET /share/public (line 370). FastAPI matches routes in definition order, so requests to /share/info will be captured by /{task_id} with task_id="share", causing validation errors since "share" is not an integer.
Move the share-related GET routes before the /{task_id} route, or restructure the paths:
+@router.get("/share/info", response_model=TaskShareInfo)
+def get_task_share_info(
+ share_token: str = Query(..., description="Share token from URL"),
+ db: Session = Depends(get_db),
+):
+ ...
+
+@router.get("/share/public", response_model=PublicSharedTaskResponse)
+def get_public_shared_task(
+ token: str = Query(..., description="Share token from URL"),
+ db: Session = Depends(get_db),
+):
+ ...
+
@router.get("/{task_id}", response_model=TaskDetail)
def get_task(
...Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.7)
361-361: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
| expire_hours = settings.APPEND_CHAT_TASK_EXPIRE_HOURS | ||
| # Check if task is expired | ||
| task_type = ( | ||
| task_crd.metadata.labels | ||
| and task_crd.metadata.labels.get("taskType") | ||
| or "chat" | ||
| ) | ||
| # Only check expiration for code tasks, chat tasks have no expiration | ||
| if task_type == "code": | ||
| expire_hours = settings.APPEND_CODE_TASK_EXPIRE_HOURS | ||
| if ( | ||
| datetime.now() - existing_task.updated_at | ||
| ).total_seconds() > expire_hours * 3600: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail=f"{task_type} task has expired. You can only append tasks within {expire_hours} hours after last update.", | ||
| ) | ||
|
|
||
| task_shell_source = ( | ||
| task_crd.chat_shell.labels | ||
| and task_crd.chat_shell.labels.get("source") | ||
| or None | ||
| ) | ||
| if task_shell_source != "chat_shell": | ||
| if ( | ||
| datetime.now() - existing_task.updated_at | ||
| ).total_seconds() > expire_hours * 3600: | ||
| raise HTTPException( | ||
| status_code=400, | ||
| detail=f"{task_type} task has expired. You can only append tasks within {expire_hours} hours after last update.", | ||
| ) |
There was a problem hiding this comment.
Guard access to chat_shell.labels to avoid crashes and align the expiration comment with behavior
The new expiration logic introduces two concerns:
- Potential runtime error
task_shell_source = (
task_crd.chat_shell.labels
and task_crd.chat_shell.labels.get("source")
or None
)If task_crd.chat_shell is None (or missing), task_crd.chat_shell.labels will raise an AttributeError. Given that tasks created by this service don’t populate chat_shell in task_json, this is a realistic scenario.
- Comment vs. behavior mismatch
The comment says:
Only check expiration for code tasks, chat tasks have no expiration
But the code still applies expiration to non‑code tasks (using APPEND_CHAT_TASK_EXPIRE_HOURS) whenever task_shell_source != "chat_shell". That’s stricter than the comment suggests.
You can make this safer and clearer by:
- Safely reading metadata labels.
- Safely reading
chat_shell.labels. - Updating the comment to match logic (skipping expiration only when
source == "chat_shell").
Example patch:
- expire_hours = settings.APPEND_CHAT_TASK_EXPIRE_HOURS
- # Check if task is expired
- task_type = (
- task_crd.metadata.labels
- and task_crd.metadata.labels.get("taskType")
- or "chat"
- )
- # Only check expiration for code tasks, chat tasks have no expiration
- if task_type == "code":
- expire_hours = settings.APPEND_CODE_TASK_EXPIRE_HOURS
-
- task_shell_source = (
- task_crd.chat_shell.labels
- and task_crd.chat_shell.labels.get("source")
- or None
- )
- if task_shell_source != "chat_shell":
+ expire_hours = settings.APPEND_CHAT_TASK_EXPIRE_HOURS
+
+ # Determine task type and expiration window (code vs chat)
+ metadata_labels = getattr(task_crd.metadata, "labels", {}) or {}
+ task_type = metadata_labels.get("taskType") or "chat"
+ if task_type == "code":
+ expire_hours = settings.APPEND_CODE_TASK_EXPIRE_HOURS
+
+ # Only enforce expiration for tasks not coming from chat shell
+ chat_shell_labels = getattr(getattr(task_crd, "chat_shell", None), "labels", {}) or {}
+ task_shell_source = chat_shell_labels.get("source")
+ if task_shell_source != "chat_shell":
if (
datetime.now() - existing_task.updated_at
).total_seconds() > expire_hours * 3600:
raise HTTPException(
status_code=400,
detail=f"{task_type} task has expired. You can only append tasks within {expire_hours} hours after last update.",
)This both prevents AttributeError and makes operator precedence explicit, addressing the Ruff RUF021 hint at the same time.
🧰 Tools
🪛 Ruff (0.14.7)
101-102: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
110-111: Parenthesize a and b expressions when chaining and and or together, to make the precedence clear
Parenthesize the and subexpression
(RUF021)
🤖 Prompt for AI Agents
In backend/app/services/adapters/task_kinds.py around lines 98 to 121, guard
accesses to metadata and chat_shell labels and make the comment reflect actual
logic: read task_type safely (e.g., use task_crd.metadata and
task_crd.metadata.labels and .get("taskType") or default "chat"), read
task_shell_source only if task_crd.chat_shell is not None and has labels (e.g.,
guard via task_crd.chat_shell and task_crd.chat_shell.labels and .get("source")
or None), keep expire_hours selection (use APPEND_CODE_TASK_EXPIRE_HOURS when
task_type == "code"), and update the comment to state that expiration is skipped
only when source == "chat_shell"; use explicit boolean checks rather than
chained and/or to avoid precedence issues and AttributeError.
| "kubernetes==29.0.0", | ||
| "pydantic==2.11.2", | ||
| "pyjwt==2.8.0", | ||
| "httpx>=0.27.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the pyproject.toml file to see the current dependencies
cat -n executor_manager/pyproject.toml | head -30Repository: wecode-ai/Wegent
Length of output: 917
🏁 Script executed:
# Search for httpx usage in the codebase
rg "import httpx|from httpx" --type pythonRepository: wecode-ai/Wegent
Length of output: 146
🏁 Script executed:
# Search for requests usage in the codebase
rg "import requests|from requests" --type pythonRepository: wecode-ai/Wegent
Length of output: 146
🏁 Script executed:
# Search for requests-oauthlib usage
rg "import requests_oauthlib|from requests_oauthlib|requests-oauthlib" --type pythonRepository: wecode-ai/Wegent
Length of output: 146
🏁 Script executed:
# Look for any migration notes or TODOs related to HTTP libraries
rg "TODO|FIXME|migrate|migration" --type python -i | grep -i "http\|request"Repository: wecode-ai/Wegent
Length of output: 146
🏁 Script executed:
# Check git history to see if httpx was recently added
git log --oneline --all -- executor_manager/pyproject.toml | head -10Repository: wecode-ai/Wegent
Length of output: 200
🏁 Script executed:
# Find Python files in executor_manager that might use these libraries
find executor_manager -name "*.py" -type f | head -20Repository: wecode-ai/Wegent
Length of output: 846
🏁 Script executed:
# Search more efficiently by looking at specific file types and patterns
fd "\.py$" executor_manager | xargs grep -l "httpx\|requests" 2>/dev/null | head -20Repository: wecode-ai/Wegent
Length of output: 280
🏁 Script executed:
# Try a simpler pattern search for httpx specifically
fd "\.py$" executor_manager -x grep -l "httpx" {} \; 2>/dev/nullRepository: wecode-ai/Wegent
Length of output: 141
🏁 Script executed:
# Try a simpler pattern search for requests specifically
fd "\.py$" executor_manager -x grep -l "requests" {} \; 2>/dev/nullRepository: wecode-ai/Wegent
Length of output: 280
🏁 Script executed:
# Check how httpx and requests are used in executor.py
cat executor_manager/executors/docker/executor.py | grep -A 5 "import httpx\|import requests\|from httpx\|from requests"Repository: wecode-ai/Wegent
Length of output: 293
🏁 Script executed:
# Check how httpx is used in routers.py
cat executor_manager/routers/routers.py | grep -A 5 "import httpx\|import requests\|from httpx\|from requests"Repository: wecode-ai/Wegent
Length of output: 305
🏁 Script executed:
# Check task_api_client.py (likely a key file for HTTP requests)
cat executor_manager/clients/task_api_client.py | head -50Repository: wecode-ai/Wegent
Length of output: 1292
🏁 Script executed:
# Check github_app.py for requests-oauthlib usage
cat executor_manager/github/github_app.py | head -50Repository: wecode-ai/Wegent
Length of output: 1725
🏁 Script executed:
# Check all httpx calls in executor.py
grep -n "httpx\." executor_manager/executors/docker/executor.pyRepository: wecode-ai/Wegent
Length of output: 118
🏁 Script executed:
# Check all requests calls in executor.py
grep -n "requests\." executor_manager/executors/docker/executor.pyRepository: wecode-ai/Wegent
Length of output: 371
🏁 Script executed:
# Get the full context around httpx usage in routers.py
grep -n "httpx\." executor_manager/routers/routers.py | head -10Repository: wecode-ai/Wegent
Length of output: 125
🏁 Script executed:
# Check requests usage in task_api_client.py
grep -n "requests\." executor_manager/clients/task_api_client.py | head -15Repository: wecode-ai/Wegent
Length of output: 427
🏁 Script executed:
# Check if requests-oauthlib is actually imported/used
rg "requests_oauthlib\|requests-oauthlib" --type pythonRepository: wecode-ai/Wegent
Length of output: 90
🏁 Script executed:
# Get more context around the executor.py httpx and requests usage
sed -n '770,790p' executor_manager/executors/docker/executor.pyRepository: wecode-ai/Wegent
Length of output: 825
🏁 Script executed:
# Check around line 177 where requests is used in executor.py
sed -n '170,190p' executor_manager/executors/docker/executor.pyRepository: wecode-ai/Wegent
Length of output: 1038
🏁 Script executed:
# Look at the class initialization to understand self.requests
sed -n '1,100p' executor_manager/executors/docker/executor.py | grep -A 10 "__init__"Repository: wecode-ai/Wegent
Length of output: 523
🏁 Script executed:
# Check if requests-oauthlib is actually used (different search approach)
grep -r "requests_oauthlib\|RequestsOAuthlib" executor_manager --include="*.py"Repository: wecode-ai/Wegent
Length of output: 42
🏁 Script executed:
# Also check for OAuth or oauthlib usage
grep -r "oauth" executor_manager --include="*.py" -i | head -20Repository: wecode-ai/Wegent
Length of output: 42
Document the rationale for using both httpx and requests, and remove unused dependency.
The project uses both httpx and requests for distinct purposes: httpx handles async HTTP calls (validation stage reporting in executor.py and routers.py), while requests handles synchronous container API communication. However, requests-oauthlib (line 20) appears unused in the codebase and should be removed if no longer needed. Consider adding a comment in the code or documentation explaining why multiple HTTP libraries coexist to prevent future refactoring confusion.
🤖 Prompt for AI Agents
In executor_manager/pyproject.toml around line 16, the review found that both
httpx and requests are used for different purposes but requests-oauthlib (line
20) appears unused; remove the unused requests-oauthlib dependency from
pyproject.toml and run a quick repo-wide search to confirm it isn't referenced
elsewhere, and add a short comment in the README or a top-level module (or in
executor.py/routers.py) documenting why httpx is used for async validation-stage
calls and requests for synchronous container API calls to prevent future
confusion.
| <Image | ||
| src="/weibo-logo.png" | ||
| alt="Weibo Logo" | ||
| width={20} | ||
| height={20} | ||
| className="object-container" | ||
| /> |
There was a problem hiding this comment.
Typo in Tailwind class name.
object-container is not a valid Tailwind CSS class. The correct class for object-fit: contain is object-contain.
<Image
src="/weibo-logo.png"
alt="Weibo Logo"
width={20}
height={20}
- className="object-container"
+ className="object-contain"
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Image | |
| src="/weibo-logo.png" | |
| alt="Weibo Logo" | |
| width={20} | |
| height={20} | |
| className="object-container" | |
| /> | |
| <Image | |
| src="/weibo-logo.png" | |
| alt="Weibo Logo" | |
| width={20} | |
| height={20} | |
| className="object-contain" | |
| /> |
🤖 Prompt for AI Agents
In frontend/src/features/tasks/components/PublicTaskSidebar.tsx around lines 38
to 44, the Image uses an invalid Tailwind class "object-container"; replace it
with the correct Tailwind utility "object-contain" to apply object-fit: contain
(i.e., update the className value accordingly so the image fits as intended).
| "shared_task": { | ||
| "continue_chat": "Continue Chat", | ||
| "login_to_continue": "Login to Continue", | ||
| "read_only_view": "Read-only view", | ||
| "login_prompt": "Login to copy this task to your workspace and continue the conversation.", | ||
| "continue_prompt": "Click the button above to copy this task to your workspace and continue the conversation.", | ||
| "want_to_continue": "Want to continue this conversation?", | ||
| "copy_and_chat": "Copy this task to your workspace and keep chatting", | ||
| "shared_task": "Shared Task", | ||
| "shared_by": "Shared by", | ||
| "read_only_notice": "This is a read-only shared conversation. To copy it to your tasks and continue the conversation, please login", | ||
| "error_invalid_link": "Invalid share link", | ||
| "error_invalid_link_desc": "The share link format is invalid. Please check the link and try again.", | ||
| "error_task_deleted": "Task No Longer Available", | ||
| "error_task_deleted_desc": "This shared task has been deleted by the owner and is no longer available.", | ||
| "error_load_failed": "Failed to Load", | ||
| "error_load_failed_desc": "Unable to load the shared task. Please try again later.", | ||
| "go_home": "Go to Chat", | ||
| "share_success_title": "Share Task Successfully", | ||
| "share_success_message": "Your task {taskTitle} has been shared successfully!", | ||
| "share_success_message_prefix": "Your task", | ||
| "share_success_message_suffix": "has been shared successfully!", | ||
| "share_link_info": "Anyone with this link can view the conversation history of this task.", | ||
| "share_continue_info": "When others open the link, they can copy this task to their own task list and continue the conversation.", | ||
| "copy_link": "Copy Link", | ||
| "link_copied": "Share link copied!", | ||
| "link_copied_desc": "You can now share this link with others", | ||
| "share_task": "Share Task", | ||
| "sharing": "Sharing...", | ||
| "no_task_selected": "No task selected", | ||
| "no_task_selected_desc": "Please select a task to share", | ||
| "share_failed": "Failed to share task", | ||
| "share_failed_desc": "Please try again", | ||
| "handler_load_failed": "Failed to load shared task information", | ||
| "handler_select_team": "Please select a team", | ||
| "handler_copy_success": "Task copied successfully!", | ||
| "handler_copy_success_desc": "has been added to your task list", | ||
| "handler_self_task_title": "This is your own task", | ||
| "handler_self_task_desc": "You cannot copy your own shared task", | ||
| "handler_modal_title": "Shared Task", | ||
| "handler_is_your_own_task": "is your own task. You cannot copy it.", | ||
| "handler_shared_by": "shared the task", | ||
| "handler_with_you": "with you", | ||
| "handler_copy_description": "Copying this task will add", | ||
| "handler_copy_description_suffix": "and all its conversation history to your task list. You can then continue the conversation.", | ||
| "handler_select_team_label": "Select Team", | ||
| "handler_no_teams": "No teams available", | ||
| "handler_create_team_hint": "You need to create at least one team to copy this task.", | ||
| "handler_copying": "Copying...", | ||
| "handler_copy_to_tasks": "Continue chat" | ||
| } |
There was a problem hiding this comment.
Align interpolation placeholder style with existing i18n usage
The new shared_task block looks comprehensive and reads well. One detail to fix for interpolation consistency:
- Existing strings in this file use double curly braces for variables, e.g.
"share_success_message": "Share your {{teamName}} with your friends!". - The new string uses a single‑brace placeholder:
"share_success_message": "Your task {taskTitle} has been shared successfully!"If your i18n layer expects {{varName}} (as other keys suggest), this will render literally {taskTitle} instead of substituting the title.
Recommend updating it to:
- "share_success_message": "Your task {taskTitle} has been shared successfully!",
+ "share_success_message": "Your task {{taskTitle}} has been shared successfully!",The rest of the shared_task keys look good and consistent with the feature behavior.
🤖 Prompt for AI Agents
In frontend/src/i18n/locales/en/common.json around lines 604 to 654, the
share_success_message uses a single-brace placeholder ("Your task {taskTitle}
has been shared successfully!") which is inconsistent with the file's
double-curly interpolation convention and will not be substituted; change the
placeholder to use double curly braces ({{taskTitle}}) so the i18n layer
interpolates the task title correctly.
- Fix flash of empty state during task refresh by keeping messages visible when subtasks exist - Move streaming message rendering to use MessageBubble component for consistency - Show copy button for user messages at all times (not just on hover) - Enable copy/download buttons during streaming instead of hiding them - Remove unnecessary group hover effects that caused UI inconsistency - Keep export PDF button visible but disabled during streaming - Simplify message bubble spacing and remove redundant wrapper divs
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)
frontend/src/features/tasks/components/MessageBubble.tsx (1)
666-712: Redundant conditional check inrenderRecoveredContent.The outer
ifon line 666 already verifiesmsg.recoveredContentis truthy, making the inner ternary condition on line 670 (msg.recoveredContent ? ... : ...) always take the truthy branch. The else branch (lines 707-712) is dead code.const renderRecoveredContent = () => { if (!msg.recoveredContent || msg.subtaskStatus !== 'RUNNING') return null; return ( <div className="space-y-2"> - {msg.recoveredContent ? ( - <> - <MarkdownEditor.Markdown - source={msg.recoveredContent} - style={{ background: 'transparent' }} - wrapperElement={{ 'data-color-mode': theme }} - components={{ - a: ({ href, children, ...props }) => ( - <a href={href} target="_blank" rel="noopener noreferrer" {...props}> - {children} - </a> - ), - }} - /> - {/* Show copy and download buttons during streaming */} - <BubbleTools - contentToCopy={msg.recoveredContent} - tools={[ - { - key: 'download', - title: t('messages.download') || 'Download', - icon: <Download className="h-4 w-4 text-text-muted" />, - onClick: () => { - const blob = new Blob([msg.recoveredContent || ''], { - type: 'text/plain;charset=utf-8', - }); - const url = URL.createObjectURL(url); - const a = document.createElement('a'); - a.href = url; - a.download = 'message.md'; - a.click(); - URL.revokeObjectURL(url); - }, - }, - ]} - /> - </> - ) : ( - <div className="flex items-center gap-2 text-text-muted"> - <span className="animate-pulse">●</span> - <span className="text-sm">{t('messages.thinking') || 'Thinking...'}</span> - </div> - )} + <MarkdownEditor.Markdown + source={msg.recoveredContent} + style={{ background: 'transparent' }} + wrapperElement={{ 'data-color-mode': theme }} + components={{ + a: ({ href, children, ...props }) => ( + <a href={href} target="_blank" rel="noopener noreferrer" {...props}> + {children} + </a> + ), + }} + /> + {/* Show copy and download buttons during streaming */} + <BubbleTools + contentToCopy={msg.recoveredContent} + tools={[ + { + key: 'download', + title: t('messages.download') || 'Download', + icon: <Download className="h-4 w-4 text-text-muted" />, + onClick: () => { + const blob = new Blob([msg.recoveredContent || ''], { + type: 'text/plain;charset=utf-8', + }); + const url = URL.createObjectURL(blob); + const a = document.createElement('a'); + a.href = url; + a.download = 'message.md'; + a.click(); + URL.revokeObjectURL(url); + }, + }, + ]} + /> </div> ); };
🧹 Nitpick comments (2)
frontend/src/features/tasks/components/ChatArea.tsx (1)
551-565: Magic delay value should be documented or extracted.The 100ms delay on line 556 is a timing workaround for the URL change to be processed. Consider extracting this to a named constant and adding a brief comment explaining why this specific value was chosen.
+// Delay to allow TaskParamSync to process URL change before refreshing task detail +const URL_CHANGE_PROCESSING_DELAY_MS = 100; + // ...inside onComplete callback: - await new Promise(resolve => setTimeout(resolve, 100)); + await new Promise(resolve => setTimeout(resolve, URL_CHANGE_PROCESSING_DELAY_MS));frontend/src/features/tasks/components/MessagesArea.tsx (1)
553-570: Copy button missing visual feedback on success.The copy button for pending user messages doesn't provide visual feedback (like the checkmark in
CopyButtoncomponent). Consider reusing theCopyButtoncomponent fromMessageBubble.tsxfor consistency.+import { CopyButton } from './MessageBubble'; // or extract CopyButton to a shared component {/* Copy button for pending user message */} <div className="absolute bottom-2 left-2 flex items-center gap-1 z-10"> - <Button - variant="ghost" - size="icon" - onClick={async () => { - try { - await navigator.clipboard.writeText(pendingUserMessage); - } catch (err) { - console.error('Failed to copy text: ', err); - } - }} - className="h-8 w-8 hover:bg-muted opacity-100" - title="Copy" - > - <Copy className="h-4 w-4 text-text-muted" /> - </Button> + <CopyButton content={pendingUserMessage} className="h-8 w-8 hover:bg-muted opacity-100" /> </div>Note: The
CopyButtoncomponent inMessageBubble.tsxis not currently exported. You would need to either export it or extract it to a shared location.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/features/tasks/components/ChatArea.tsx(11 hunks)frontend/src/features/tasks/components/MessageBubble.tsx(6 hunks)frontend/src/features/tasks/components/MessagesArea.tsx(8 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/tasks/components/MessageBubble.tsxfrontend/src/features/tasks/components/MessagesArea.tsxfrontend/src/features/tasks/components/ChatArea.tsx
🧬 Code graph analysis (1)
frontend/src/features/tasks/components/ChatArea.tsx (1)
frontend/src/features/tasks/components/ExportPdfButton.tsx (1)
ExportPdfButton(54-396)
🔇 Additional comments (11)
frontend/src/features/tasks/components/MessageBubble.tsx (2)
745-750: LGTM! CopyButton always visible for user messages.This is a good UX improvement that ensures users can always copy their messages. The implementation is clean with appropriate styling.
155-157: Simplified bubble styling looks good.The style consolidation is cleaner - user messages get full styling while AI messages get minimal styling. This aligns with the PR objective to fix chat flash by reducing visual complexity.
frontend/src/features/tasks/components/ChatArea.tsx (4)
208-222: Good fix for the chat flash issue.This logic change correctly prevents the "flash of empty state" by ensuring the messages view stays visible when a task has subtasks, even during refresh. The comment clearly explains the intent.
566-578: LGTM! Proper async handling for follow-up messages.The await pattern with try/catch/finally ensures stream content is only cleared after task detail refresh completes, preventing the flash of empty content. Good defensive coding.
1307-1317: Export button correctly disabled during streaming.The
disabled={isStreaming}prop ensures users can't export incomplete content while streaming is in progress, which is the correct behavior.
46-56: NewonShareButtonRenderprop properly propagated.The prop is correctly typed and passed through to MessagesArea for rendering the share button in parent components.
frontend/src/features/tasks/components/MessagesArea.tsx (5)
497-501: Good improvement to message key generation.Combining
typeandsubtaskIdprevents key collisions when user and AI messages share the same subtask ID. The fallback toindex-timestamphandles edge cases properly.
133-158: LGTM! Task sharing handler is well implemented.The
handleShareTaskcallback has proper validation, error handling, and loading state management. TheuseCallbackwith correct dependencies prevents unnecessary re-renders.
576-600: Streaming message now uses MessageBubble for consistency.Using
MessageBubblefor streaming content instead of a custom implementation ensures consistent rendering behavior across all message types. This is a good refactor.
486-491: Container rendering condition expanded to prevent layout shift.The condition now includes
selectedTaskDetail?.idto render the container even when loading, which helps prevent layout shift. This aligns with the PR objective to fix chat flash.
458-482: The shareButton memoization is effective as configured.The
toastfunction is a module-level export fromuse-toast.tsand returns a stable reference across renders. Thetfunction fromreact-i18nextis also stable. Bothtoastandtare correctly included in thehandleShareTaskuseCallback dependencies, ensuring the memoization chain works as intended. Whiletappears redundantly in the shareButton useMemo dependencies (already included viahandleShareTask), this does not affect correctness and can remain for clarity.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/services/shared_task.py (1)
520-538: Improve exception handling with proper exception chaining.The current exception handling catches a bare
Exceptionand re-raisesHTTPExceptionwithout preserving the original exception context. This makes debugging harder when decryption or parsing fails.Apply this diff to improve exception handling:
def get_public_shared_task( self, db: Session, share_token: str ) -> PublicSharedTaskResponse: """Get public shared task data (no authentication required)""" # First try to decode the token format (without database check) try: decoded_token = urllib.parse.unquote(share_token) share_data_str = self._aes_decrypt(decoded_token) if not share_data_str or "#" not in share_data_str: - raise HTTPException(status_code=400, detail="Invalid share link format") + raise HTTPException(status_code=400, detail="Invalid share link format") from None # Parse user_id and task_id user_id_str, task_id_str = share_data_str.split("#", 1) try: user_id = int(user_id_str) task_id = int(task_id_str) except ValueError: - raise HTTPException(status_code=400, detail="Invalid share link format") + raise HTTPException(status_code=400, detail="Invalid share link format") from None except HTTPException: raise - except Exception: - raise HTTPException(status_code=400, detail="Invalid share link format") + except Exception as e: + logger.warning("Failed to decode share token: %s", e) + raise HTTPException(status_code=400, detail="Invalid share link format") from None
🧹 Nitpick comments (3)
backend/app/services/shared_task.py (1)
243-373: Consider breaking down long methods for better maintainability.Several methods significantly exceed the 50-line guideline from your coding standards:
_copy_task_with_subtasks: ~131 linesjoin_shared_task: ~105 linesget_public_shared_task: ~112 linesWhile the logic is clear, extracting helper methods would improve readability and testability. For example,
_copy_task_with_subtaskscould extract subtask copying logic, andget_public_shared_taskcould extract the token decoding and subtask formatting logic.Based on coding guidelines, which specify "Python functions SHOULD NOT exceed 50 lines (preferred maximum)".
Also applies to: 375-479, 516-627
frontend/src/features/tasks/components/MessagesArea.tsx (2)
457-482: Consider documenting the stability requirement foronShareButtonRender.The share button is correctly memoized and passed to the parent. However, if
onShareButtonRenderis not stable (e.g., not wrapped inuseCallbackin the parent), the effect will run on every render. Consider adding a JSDoc comment noting thatonShareButtonRendershould be stable.Add documentation to the prop:
/** Callback to render share button in parent component (e.g., TopNavigation) */ + /** Note: This callback should be stable (wrapped in useCallback) to prevent unnecessary re-renders */ onShareButtonRender?: (button: React.ReactNode) => void;
539-540: Consider mobile-first responsive width for user messages.The
max-w-[75%]works across all screen sizes, but for better mobile UX, consider a mobile-first approach:- <div className="flex max-w-[75%] w-auto flex-col gap-3 items-end"> + <div className="flex max-w-[90%] sm:max-w-[75%] w-auto flex-col gap-3 items-end">This gives more horizontal space on mobile while maintaining the current width on larger screens.
As per coding guidelines, frontend responsive design must follow mobile-first approach.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/app/api/endpoints/adapter/chat.py(2 hunks)backend/app/services/shared_task.py(7 hunks)frontend/src/features/tasks/components/MessagesArea.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/api/endpoints/adapter/chat.py
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
All code comments, inline comments, block comments, docstrings, TODO/FIXME annotations, and type hints descriptions MUST be written in English
Files:
frontend/src/features/tasks/components/MessagesArea.tsxbackend/app/services/shared_task.py
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: TypeScript code MUST use strict mode with type checking enabled
TypeScript/React code MUST use Prettier formatter with single quotes, no semicolons
TypeScript/React code MUST pass ESLint with Next.js configuration
React component names MUST use PascalCase convention
Files:
frontend/src/features/tasks/components/MessagesArea.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: React components MUST use functional components with hooks instead of class-based components
Useconstoverlet, never usevarin TypeScript/JavaScript code
Files:
frontend/src/features/tasks/components/MessagesArea.tsx
**/components/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
React component files MUST use kebab-case naming convention
Files:
frontend/src/features/tasks/components/MessagesArea.tsx
**/src/**/*.{tsx,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Frontend Tailwind CSS MUST use provided CSS variables for color system (e.g., --color-bg-base, --color-text-primary, --color-primary)
Files:
frontend/src/features/tasks/components/MessagesArea.tsx
**/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/src/**/*.{tsx,jsx}: Frontend responsive design MUST follow mobile-first approach with Tailwind breakpoints
Frontend React forms MUST use react-hook-form with zod validation schema
Frontend components MUST use shadcn/ui component library fromfrontend/src/components/ui/
Files:
frontend/src/features/tasks/components/MessagesArea.tsx
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Python code MUST be PEP 8 compliant with Black formatter (line length: 88) and isort for import organization
Python code MUST include type hints for all functions and variables
Python functions SHOULD NOT exceed 50 lines (preferred maximum)
Python functions and classes MUST have descriptive names and public functions/classes MUST include docstrings
Python code MUST extract magic numbers to named constants
Files:
backend/app/services/shared_task.py
**/backend/app/services/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Backend business logic MUST be placed in
app/services/directory
Files:
backend/app/services/shared_task.py
🧬 Code graph analysis (1)
backend/app/services/shared_task.py (3)
backend/app/models/kind.py (1)
Kind(25-42)backend/app/schemas/shared_task.py (1)
SharedTaskInDB(53-65)backend/app/models/user.py (1)
User(14-40)
🪛 Ruff (0.14.7)
backend/app/services/shared_task.py
526-526: Abstract raise to an inner function
(TRY301)
534-534: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
537-537: Do not catch blind exception: Exception
(BLE001)
538-538: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
559-559: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
⏰ 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). (2)
- GitHub Check: Test wegent CLI Integration
- GitHub Check: e2e-tests
🔇 Additional comments (7)
backend/app/services/shared_task.py (3)
243-251: LGTM! Explicit database session parameter improves clarity.The addition of explicit
db: Sessionparameter improves type safety and makes the method signature clearer.
375-383: LGTM! Consistent explicit database session parameters.The addition of explicit
db: Sessionparameters across these methods improves consistency and type safety throughout the service.Also applies to: 481-481
599-603: LGTM! Robust status handling.The defensive enum/string handling with
hasattrcheck is a good practice that prevents attribute errors when the status field can be either an enum or a string.frontend/src/features/tasks/components/MessagesArea.tsx (4)
17-17: LGTM: Import cleanup is consistent with UI simplification.The removal of unused imports (
Bot,Download,MarkdownEditor) aligns with the deletion ofBubbleToolsand heavy Markdown UI mentioned in the summary.
98-98: LGTM: Well-designed callback prop for share button composition.The optional
onShareButtonRendercallback enables flexible parent-controlled rendering of the share button, which is a good composition pattern.
486-491: LGTM: Improved layout stability by showing container when task is selected.The updated condition prevents layout shift by rendering the messages container whenever a task is selected, even if loading. This improves UX.
576-600: Verify the correct usage ofrecoveredContentfor active streaming.The synthetic streaming message sets both
content(line 584) andrecoveredContent(line 588) with different values, whileisRecovered: false(line 589). This is semantically confusing:
contentusesstreamingContentdirectlyrecoveredContentusesdisplayContent(the typewriter-processed version)isRecoveredisfalse, suggesting this is not recovered contentPlease verify that
MessageBubblecorrectly handles this case. IfMessageBubbleprioritizesrecoveredContentwhen set, thecontentfield is unused. If it ignoresrecoveredContentwhenisRecoveredis false, then line 588 does nothing.Consider clarifying the field naming or logic:
// Option 1: If MessageBubble uses recoveredContent for the typewriter effect msg={{ type: 'ai', content: `\${$$}$${streamingContent || ''}`, timestamp: Date.now(), botName: selectedTeam?.name || t('messages.bot') || 'Bot', subtaskStatus: 'RUNNING', displayContent: displayContent, // Rename for clarity isRecovered: false, isIncomplete: true, // Should be true while streaming }} // Option 2: If MessageBubble should use content directly msg={{ type: 'ai', content: `\${$$}$${displayContent || ''}`, // Use typewriter effect directly timestamp: Date.now(), botName: selectedTeam?.name || t('messages.bot') || 'Bot', subtaskStatus: 'RUNNING', // Remove recoveredContent if not needed isRecovered: false, isIncomplete: true, // Should be true while streaming }}Also,
isIncompleteshould likely betruewhile actively streaming.
| .filter(User.id == user_id, User.is_active == True) | ||
| .first() | ||
| ) | ||
| user = db.query(User).filter(User.id == user_id, User.is_active == True).first() |
There was a problem hiding this comment.
Use truthy check instead of == True comparison.
The equality comparison to True is not Pythonic. SQLAlchemy supports direct truthy checks in filter conditions.
Apply this diff:
- user = db.query(User).filter(User.id == user_id, User.is_active == True).first()
+ user = db.query(User).filter(User.id == user_id, User.is_active).first()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| user = db.query(User).filter(User.id == user_id, User.is_active == True).first() | |
| user = db.query(User).filter(User.id == user_id, User.is_active).first() |
🧰 Tools
🪛 Ruff (0.14.7)
559-559: Avoid equality comparisons to True; use User.is_active: for truth checks
Replace with User.is_active
(E712)
🤖 Prompt for AI Agents
In backend/app/services/shared_task.py around line 559, the filter currently
uses an explicit equality check "User.is_active == True"; replace that with a
direct truthy filter (e.g., use User.is_active) so SQLAlchemy performs the
truthiness check correctly and the code is more Pythonic.
| {/* Copy button for pending user message */} | ||
| <div className="absolute bottom-2 left-2 flex items-center gap-1 z-10"> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={async () => { | ||
| try { | ||
| await navigator.clipboard.writeText(pendingUserMessage); | ||
| } catch (err) { | ||
| console.error('Failed to copy text: ', err); | ||
| } | ||
| }} | ||
| className="h-8 w-8 hover:bg-muted opacity-100" | ||
| title="Copy" | ||
| > | ||
| <Copy className="h-4 w-4 text-text-muted" /> | ||
| </Button> | ||
| </div> |
There was a problem hiding this comment.
Add user feedback when copying pending message.
The copy button lacks user feedback. Users won't know if the copy operation succeeded or failed without visual confirmation.
Add a toast notification to confirm the copy action:
<div className="absolute bottom-2 left-2 flex items-center gap-1 z-10">
<Button
variant="ghost"
size="icon"
onClick={async () => {
try {
await navigator.clipboard.writeText(pendingUserMessage);
+ toast({
+ title: t('messages.copied'),
+ description: t('messages.copied_to_clipboard'),
+ });
} catch (err) {
console.error('Failed to copy text: ', err);
+ toast({
+ variant: 'destructive',
+ title: t('messages.copy_failed'),
+ description: (err as Error)?.message,
+ });
}
}}
className="h-8 w-8 hover:bg-muted opacity-100"
title="Copy"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {/* Copy button for pending user message */} | |
| <div className="absolute bottom-2 left-2 flex items-center gap-1 z-10"> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| onClick={async () => { | |
| try { | |
| await navigator.clipboard.writeText(pendingUserMessage); | |
| } catch (err) { | |
| console.error('Failed to copy text: ', err); | |
| } | |
| }} | |
| className="h-8 w-8 hover:bg-muted opacity-100" | |
| title="Copy" | |
| > | |
| <Copy className="h-4 w-4 text-text-muted" /> | |
| </Button> | |
| </div> | |
| {/* Copy button for pending user message */} | |
| <div className="absolute bottom-2 left-2 flex items-center gap-1 z-10"> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| onClick={async () => { | |
| try { | |
| await navigator.clipboard.writeText(pendingUserMessage); | |
| toast({ | |
| title: t('messages.copied'), | |
| description: t('messages.copied_to_clipboard'), | |
| }); | |
| } catch (err) { | |
| console.error('Failed to copy text: ', err); | |
| toast({ | |
| variant: 'destructive', | |
| title: t('messages.copy_failed'), | |
| description: (err as Error)?.message, | |
| }); | |
| } | |
| }} | |
| className="h-8 w-8 hover:bg-muted opacity-100" | |
| title="Copy" | |
| > | |
| <Copy className="h-4 w-4 text-text-muted" /> | |
| </Button> | |
| </div> |
# Conflicts: # frontend/src/features/tasks/components/ChatArea.tsx # frontend/src/features/tasks/components/MessagesArea.tsx
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/e2e-tests.yml (1)
55-56: Useastral-sh/setup-uvaction instead ofsetup-pythonfor uv caching.The workflow currently uses
setup-pythonwithcache: 'pip', which is designed for pip workflows. While it technically works withcache-dependency-path: backend/pyproject.toml, the official recommendation for uv-based projects is to use theastral-sh/setup-uvaction, which provides built-in cache handling optimized for uv, including automatic cache pruning and proper uv cache directory management.Replace the setup-python step with
astral-sh/setup-uv@v1(or later) to align with uv best practices and ensure efficient caching behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/e2e-tests.yml(3 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). (2)
- GitHub Check: Test wegent CLI Integration
- GitHub Check: e2e-tests
🔇 Additional comments (2)
.github/workflows/e2e-tests.yml (2)
61-63: uv installation sequence is correct.The installation of uv via curl, followed by sourcing
$HOME/.cargo/envand runninguv sync, is the correct approach for uv-based workflows. The environment setup ensures uv is available in the same shell context.
99-100: Backend startup with uv is properly configured.The use of
uv run uvicornis correct, and sourcing$HOME/.cargo/envin this step is necessary since GitHub Actions provides a fresh shell environment for eachrun:block. PYTHONPATH is correctly set to support the 'shared' module import.Please verify that
backend/pyproject.tomlexists and contains all necessary dependencies that were previously inrequirements.txt.
Summary by CodeRabbit
New Features
UI Improvements
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.