Skip to content

chore: add pyrightconfig.json and fix all pyright errors#448

Merged
Aureliolo merged 2 commits intomainfrom
chore/pyright-config-and-fixes
Mar 15, 2026
Merged

chore: add pyrightconfig.json and fix all pyright errors#448
Aureliolo merged 2 commits intomainfrom
chore/pyright-config-and-fixes

Conversation

@Aureliolo
Copy link
Copy Markdown
Owner

Summary

  • Add pyrightconfig.json with standard mode and suppressed noise rules for Pydantic/structlog/pytest (reduces 1648 false positives to 0)
  • Fix all 17 actionable pyright findings across 8 source files:
    • middleware.py: pyright-specific ignore for ASGI send() type mismatch
    • loader.py: use getattr for YAMLError.problem_mark (avoids missing attr on base class)
    • mem0/adapter.py: pyright ignore for _client attr assignment
    • org/store.py: initialize escaped before conditional use
    • presets.py: extract validation loop into function to avoid unbound variable warnings
    • mcp/__init__.py: add TYPE_CHECKING imports for lazy-loaded __all__ symbols
    • docker_sandbox.py: explicit aiodocker.containers import + pyright ignores
    • test_factory.py: initialize side_effect with None + assert narrowing

Context

Pyright (used as LSP) produced 1648 errors due to no Pydantic plugin (unlike mypy). The project already has mypy strict + Pydantic plugin in CI as the authoritative type checker. This PR configures pyright as a useful editor companion by suppressing dynamic-library noise while keeping real bug detection.

Test plan

  • pyright src/ tests/ — 0 errors, 0 warnings
  • mypy src/ tests/ — 0 issues in 974 files
  • pytest tests/ -m unit -n auto — 7868 passed
  • ruff check — all passed

Add pyright configuration with standard mode and suppressed noise
rules (unknown types from Pydantic/structlog/pytest). Fix all 17
actionable pyright findings:

- middleware: pyright-specific ignore for ASGI send() type mismatch
- loader: use getattr for YAMLError.problem_mark (no type stubs)
- mem0 adapter: pyright ignore for _client attr assignment
- org/store: initialize escaped variable before conditional use
- presets: extract validation loop into function to avoid unbound vars
- mcp/__init__: add TYPE_CHECKING imports for lazy-loaded symbols
- docker_sandbox: explicit aiodocker.containers import + pyright ignores
- test_factory: initialize side_effect with None + assert narrowing
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 15, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 15, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 35a9c597-4a9e-485a-a99c-f0b24261beff

📥 Commits

Reviewing files that changed from the base of the PR and between 38d1ca2 and c8bff28.

📒 Files selected for processing (3)
  • src/synthorg/observability/events/template.py
  • src/synthorg/templates/presets.py
  • tests/unit/memory/test_factory.py
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Test (Python 3.14)
  • GitHub Check: Build Backend
  • GitHub Check: Build Web
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: No from __future__ import annotations — Python 3.14 has PEP 649 native lazy annotations
Use except A, B: syntax (no parentheses) instead of except (A, B): — PEP 758 syntax enforced by ruff on Python 3.14
All public functions must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Use NotBlankStr from core.types for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Use @computed_field in Pydantic models for derived values instead of storing and validating redundant fields
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations (e.g., multiple tool invocations, parallel agent calls) instead of bare create_task
Line length: 88 characters (enforced by ruff)
Functions must be < 50 lines; files must be < 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate input at system boundaries (user input, external APIs, config files)
Create new objects instead of mutating existing ones (immutability principle)

Files:

  • src/synthorg/observability/events/template.py
  • tests/unit/memory/test_factory.py
  • src/synthorg/templates/presets.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Every module with business logic must import from synthorg.observability import get_logger and define logger = get_logger(__name__)
Never use import logging, logging.getLogger(), or print() in application code — use the synthorg logger instead
Always use event name constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider); import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT
Always log with structured kwargs: logger.info(EVENT, key=value) — never use old-style formatting logger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
Use DEBUG level for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do NOT need logging
Never implement retry logic in driver subclasses or calling code — all provider calls go through BaseCompletionProvider which applies retry + rate limiting automatically
Set RetryConfig and RateLimiterConfig per-provider in ProviderConfig; retryable errors are RateLimitError, ProviderTimeoutError, ProviderConnectionError, ProviderInternalError
Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state
For dict/list fields in frozen Pydantic models, use copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use Pydantic v2 conventions: BaseModel, model_validator, computed_field, ConfigDict

Files:

  • src/synthorg/observability/events/template.py
  • src/synthorg/templates/presets.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Use @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, @pytest.mark.slow to categorize tests
Maintain 80% minimum code coverage (enforced in CI)
Each test must complete within 30 seconds (timeout enforcement)
Always include -n auto when running pytest via uv run python -m pytest — never run tests sequentially (pytest-xdist parallelism)
Prefer @pytest.mark.parametrize for testing similar cases
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names like example-provider, example-large-001, test-provider, test-small-001, or size aliases (large/medium/small)
Use Hypothesis for property-based testing with @given + @settings decorators; control profiles via HYPOTHESIS_PROFILE env var (ci for 200 examples, dev for 1000 examples)

Files:

  • tests/unit/memory/test_factory.py
🧠 Learnings (5)
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising.

Applied to files:

  • src/synthorg/templates/presets.py
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to src/synthorg/**/*.py : All error paths must log at WARNING or ERROR with context before raising

Applied to files:

  • src/synthorg/templates/presets.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: use `except A, B:` (no parentheses) — ruff enforces this on Python 3.14.

Applied to files:

  • src/synthorg/templates/presets.py
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to **/*.py : Use `except A, B:` syntax (no parentheses) instead of `except (A, B):` — PEP 758 syntax enforced by ruff on Python 3.14

Applied to files:

  • src/synthorg/templates/presets.py
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to **/*.py : Handle errors explicitly; never silently swallow exceptions

Applied to files:

  • src/synthorg/templates/presets.py
🔇 Additional comments (4)
tests/unit/memory/test_factory.py (1)

95-100: Good Pyright-safe narrowing in test setup.

The ValidationError | None initialization plus assert side_effect is not None is a clean way to narrow the type before using side_effect in patch(...), and the updated inline comment is now accurate.

src/synthorg/observability/events/template.py (1)

16-16: LGTM!

The new constant follows the established naming convention and type annotation pattern. Correctly placed alongside the related TEMPLATE_PERSONALITY_PRESET_UNKNOWN constant.

src/synthorg/templates/presets.py (2)

15-18: LGTM!

Import follows the established pattern and guideline to import event constants directly from synthorg.observability.events.<domain>.


432-447: LGTM!

The validation function correctly implements fail-fast import-time validation with proper observability:

  • Logs warning with TEMPLATE_PERSONALITY_PRESET_INVALID and structured context before raising
  • Uses exception chaining (from exc) for traceability
  • Cleans up the helper from module namespace after use

Note: The parenthesized except (ValidationError, TypeError) syntax is acknowledged as intentionally deferred per PR objectives (ruff doesn't support PEP 758 yet).


📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved YAML error handling for non-standard exceptions.
    • Enhanced text filtering in memory queries with proper escaping for special characters.
  • New Features

    • Added an observability event constant for invalid personality presets.
    • Centralized import-time validation of personality presets (behavior preserved).
  • Chores

    • Added Pyright configuration and applied type-checking accommodations and import-time optimizations.
  • Tests

    • Minor type-narrowing tweak in unit tests.

Walkthrough

Adds Pyright configuration and multiple small, safety/type-checking changes: type-ignore annotations, type-only imports, safer YAML error handling, SQL LIKE escaping, import-time preset validation refactor, a new observability constant, and a minor test type-narrowing adjustment.

Changes

Cohort / File(s) Summary
Pyright config
pyrightconfig.json
New Pyright configuration targeting src/ and tests/, Python 3.14, venv settings, and adjusted diagnostic rules.
Type-checking workarounds & annotations
src/synthorg/api/middleware.py, src/synthorg/memory/backends/mem0/adapter.py, src/synthorg/tools/sandbox/docker_sandbox.py
Added # pyright: ignore[...] annotations to silence Pyright complaints around ASGI send, client attribute assignment, and aiodocker container operations; no runtime behavior changes.
Type-only imports
src/synthorg/tools/mcp/__init__.py
Introduced if TYPE_CHECKING: block with type-only imports to avoid heavy/cyclic runtime imports.
YAML error handling
src/synthorg/config/loader.py
Replaced direct access to problem_mark with getattr to safely extract line/column from YAML exceptions when present.
SQL LIKE escaping
src/synthorg/memory/org/store.py
Added local escaped variable and proper escaping for backslashes, %, and _, using ESCAPE '\\' and adjusted query parameters for text filtering and ranking.
Preset validation refactor
src/synthorg/templates/presets.py
Moved import-time PERSONALITY_PRESETS validation into a private _validate_presets() helper that logs and raises ValueError on invalid entries; invoked at import.
Observability constant
src/synthorg/observability/events/template.py
Added TEMPLATE_PERSONALITY_PRESET_INVALID: Final[str] = "template.personality_preset.invalid".
Test type narrowing
tests/unit/memory/test_factory.py
Introduced `side_effect: ValidationError
Manifests
pyproject.toml, requirements.txt
Updated manifests referenced by some changes (no substantive dependency edits in diff).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding pyrightconfig.json and fixing pyright errors, which is the primary focus of the changeset.
Description check ✅ Passed The description provides a comprehensive overview of the changes, including the purpose, specific fixes across multiple files, context about pyright configuration, and test results.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/pyright-config-and-fixes
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch chore/pyright-config-and-fixes
📝 Coding Plan
  • Generate coding plan for human review comments

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request integrates Pyright as a complementary type checker to the existing MyPy setup. It establishes a pyrightconfig.json to configure Pyright for a useful developer experience by filtering out known false positives from dynamic libraries, particularly Pydantic, while maintaining strictness for other type checks. Subsequently, it addresses and resolves all 17 identified Pyright type errors across various files, ensuring a clean Pyright run.

Highlights

  • Pyright Configuration: Configured Pyright with a new pyrightconfig.json file, setting typeCheckingMode to standard and suppressing specific noise rules for Pydantic, structlog, and pytest, which reduced 1648 false positives to zero.
  • Pyright Error Resolution: Fixed all 17 actionable Pyright findings across 8 source files to achieve a clean Pyright run.
  • Specific Code Adjustments: Implemented targeted fixes including adding Pyright-specific ignore comments for ASGI send() and attribute access issues, using getattr for YAMLError.problem_mark, initializing variables before conditional use, refactoring a validation loop into a function, adding TYPE_CHECKING imports for lazy-loaded symbols, and explicit type narrowing in a test.
Changelog
  • pyrightconfig.json
    • Added a new configuration file for Pyright.
  • src/synthorg/api/middleware.py
    • Added a Pyright ignore comment.
  • src/synthorg/config/loader.py
    • Modified YAML error handling to use getattr.
  • src/synthorg/memory/backends/mem0/adapter.py
    • Added a Pyright ignore comment.
  • src/synthorg/memory/org/store.py
    • Initialized a variable to prevent potential unbound errors.
  • src/synthorg/templates/presets.py
    • Encapsulated preset validation logic within a function.
  • src/synthorg/tools/mcp/init.py
    • Added conditional TYPE_CHECKING imports.
  • src/synthorg/tools/sandbox/docker_sandbox.py
    • Added an explicit import and Pyright ignore comments.
  • tests/unit/memory/test_factory.py
    • Updated a test case to explicitly initialize a variable and add an assertion for type narrowing.
Activity
  • No specific human activity (comments, reviews, progress updates) was mentioned in the pull request description.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@Aureliolo Aureliolo temporarily deployed to cloudflare-preview March 15, 2026 16:34 — with GitHub Actions Inactive
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a pyrightconfig.json to improve the developer experience by enabling pyright as an in-editor type checker, while keeping mypy as the authoritative checker for CI. The changes also include fixes for all actionable errors reported by pyright, which improves overall code quality and type safety. My review found one issue in the new configuration file regarding the specified Python version. The rest of the changes are solid improvements.

{
"include": ["src/", "tests/"],
"exclude": [".venv/", "build/", "dist/", ".pytest_cache/", ".hypothesis/"],
"pythonVersion": "3.14",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The pythonVersion is set to "3.14", which is not a released or planned version of Python. This could lead to unexpected behavior from pyright as it might not interpret this version correctly. It's recommended to use the actual Python version the project is targeting, for example "3.12" or "3.13", to ensure accurate type checking based on a real language version.

Suggested change
"pythonVersion": "3.14",
"pythonVersion": "3.13",

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 15, 2026

Greptile Summary

This PR introduces pyrightconfig.json to make pyright usable as an editor LSP companion alongside the authoritative mypy+Pydantic-plugin CI checker, and fixes 17 actionable pyright findings across 8 source files with minimal, targeted changes.

Key changes:

  • pyrightconfig.json: Enables standard mode and suppresses Pydantic/structlog dynamic-typing noise; reportCallIssue: false is broader than the other suppressions and could silently hide real call-site errors outside Pydantic code.
  • config/loader.py: getattr replaces hasattr + direct access for YAMLError.problem_mark — a genuine quality improvement beyond just satisfying pyright.
  • templates/presets.py: Import-time preset validation refactored into a _validate_presets() function — cleaner than the previous conditional del of loop variables.
  • tools/mcp/__init__.py: TYPE_CHECKING block added to satisfy pyright for __all__ symbols; runtime lazy-import via __getattr__ is unchanged and correct.
  • tools/sandbox/docker_sandbox.py: Explicit import aiodocker.containers ensures the submodule is available; four targeted pyright ignores added.
  • tests/unit/memory/test_factory.py: side_effect initialised to None with an assert for type narrowing; the assert comment "always fails" is misleading — it should read "always passes/holds".

Confidence Score: 4/5

  • Safe to merge with one minor follow-up: consider scoping reportCallIssue suppression or documenting why the global disable is safe.
  • All logic changes are small and correct — escaped initialisation is never consumed with the wrong default, the getattr pattern for YAMLError is strictly safer, and the TYPE_CHECKING pattern in mcp/init.py is compatible with the existing getattr lazy loader. The only non-trivial concern is the global reportCallIssue: false in pyrightconfig.json, which is broader than the Pydantic-noise suppressions and could hide real call-site bugs; but this is an editor-only config that does not affect CI (mypy remains authoritative). The misleading test comment is a minor readability issue.
  • pyrightconfig.json — the reportCallIssue: false suppression deserves a brief comment explaining why it is safe to disable globally.

Important Files Changed

Filename Overview
pyrightconfig.json New pyright config with standard mode; globally disables reportCallIssue which is broader than Pydantic-specific noise and could hide real call-site errors.
src/synthorg/templates/presets.py Validation loop extracted to _validate_presets() and deleted after call — cleaner than the previous conditional del of loop variables.
src/synthorg/tools/mcp/init.py TYPE_CHECKING block added to satisfy pyright for all symbols; at runtime the existing getattr lazy-import mechanism continues to handle these correctly.
src/synthorg/tools/sandbox/docker_sandbox.py Explicit aiodocker.containers import ensures submodule availability; four pyright ignores added for known aiodocker attribute-access patterns.
tests/unit/memory/test_factory.py Adds side_effect initialisation to None and an assert for pyright narrowing; assert comment says "always fails" but should say "always passes/holds" — misleading for future readers.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: tests/unit/memory/test_factory.py
Line: 100

Comment:
**Misleading "always fails" comment**

The comment says `# always fails` on an `assert` statement, which typically means the assertion would raise `AssertionError` and the test would fail. The opposite is true: `_Dummy(x="not-an-int")` *always* raises `ValidationError`, so `side_effect` is always set and this assertion *always passes*. Consider rewording to avoid confusion for future readers.

```suggestion
        assert side_effect is not None  # always set — narrows type for pyright
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: pyrightconfig.json
Line: 12

Comment:
**`reportCallIssue: false` is broad**

`reportCallIssue` covers any error related to calling a non-callable or passing arguments to a call expression — it's not limited to Pydantic-dynamic patterns. Disabling it globally could silently hide real call-site bugs in non-Pydantic code throughout `src/` and `tests/`. The PR description justifies the other `reportUnknown*` suppressions as Pydantic noise, but `reportCallIssue` is harder to attribute purely to Pydantic. Consider using per-file or per-line `# pyright: ignore[reportCallIssue]` ignores at the handful of actual offending sites, or at minimum document here why this global suppression is safe.

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: 38d1ca2

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 15, 2026

Codecov Report

❌ Patch coverage is 80.95238% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.67%. Comparing base (fad9353) to head (c8bff28).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/synthorg/templates/presets.py 60.00% 4 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #448   +/-   ##
=======================================
  Coverage   93.67%   93.67%           
=======================================
  Files         469      469           
  Lines       22213    22219    +6     
  Branches     2144     2143    -1     
=======================================
+ Hits        20808    20814    +6     
- Misses       1094     1095    +1     
+ Partials      311      310    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

except ValidationError as ve:
side_effect: ValidationError = ve
side_effect = ve
assert side_effect is not None # always fails, narrows type for pyright
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misleading "always fails" comment

The comment says # always fails on an assert statement, which typically means the assertion would raise AssertionError and the test would fail. The opposite is true: _Dummy(x="not-an-int") always raises ValidationError, so side_effect is always set and this assertion always passes. Consider rewording to avoid confusion for future readers.

Suggested change
assert side_effect is not None # always fails, narrows type for pyright
assert side_effect is not None # always set — narrows type for pyright
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/unit/memory/test_factory.py
Line: 100

Comment:
**Misleading "always fails" comment**

The comment says `# always fails` on an `assert` statement, which typically means the assertion would raise `AssertionError` and the test would fail. The opposite is true: `_Dummy(x="not-an-int")` *always* raises `ValidationError`, so `side_effect` is always set and this assertion *always passes*. Consider rewording to avoid confusion for future readers.

```suggestion
        assert side_effect is not None  # always set — narrows type for pyright
```

How can I resolve this? If you propose a fix, please make it concise.

"reportUnknownMemberType": false,
"reportUnknownVariableType": false,
"reportUnknownParameterType": false,
"reportUnknownLambdaType": false,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reportCallIssue: false is broad

reportCallIssue covers any error related to calling a non-callable or passing arguments to a call expression — it's not limited to Pydantic-dynamic patterns. Disabling it globally could silently hide real call-site bugs in non-Pydantic code throughout src/ and tests/. The PR description justifies the other reportUnknown* suppressions as Pydantic noise, but reportCallIssue is harder to attribute purely to Pydantic. Consider using per-file or per-line # pyright: ignore[reportCallIssue] ignores at the handful of actual offending sites, or at minimum document here why this global suppression is safe.

Prompt To Fix With AI
This is a comment left during a code review.
Path: pyrightconfig.json
Line: 12

Comment:
**`reportCallIssue: false` is broad**

`reportCallIssue` covers any error related to calling a non-callable or passing arguments to a call expression — it's not limited to Pydantic-dynamic patterns. Disabling it globally could silently hide real call-site bugs in non-Pydantic code throughout `src/` and `tests/`. The PR description justifies the other `reportUnknown*` suppressions as Pydantic noise, but `reportCallIssue` is harder to attribute purely to Pydantic. Consider using per-file or per-line `# pyright: ignore[reportCallIssue]` ignores at the handful of actual offending sites, or at minimum document here why this global suppression is safe.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@pyrightconfig.json`:
- Line 13: The pyright config currently disables reportCallIssue by setting
"reportCallIssue": false which silences all call-site type mismatch warnings; to
avoid hiding real issues either re-enable reportCallIssue (set it back to true)
and address any noisy Pydantic/structlog false positives with targeted inline
ignores (use "# pyright: ignore[reportCallIssue]" on the specific functions or
call sites), or keep it disabled for editor ergonomics but add a short
repo-level comment explaining that mypy strict in CI is authoritative and to
revert to per-line ignores if real type errors are observed; reference the
"reportCallIssue" setting in pyrightconfig.json when making this change.

In `@src/synthorg/templates/presets.py`:
- Around line 435-437: The except block handling ValidationError/TypeError when
validating personality presets should log the failure with context before
raising: add a logger.warning or logger.error call (including the preset name
and exception info) inside the except that references the same variables (e.g.,
name, exc, msg) used in the block so observability is preserved before raising
the ValueError in the code path that handles personality preset validation.
- Line 435: Replace the tuple-style exception clause "except (ValidationError,
TypeError) as exc:" with the PEP 758-compliant syntax without parentheses:
"except ValidationError, TypeError as exc:"; update the except line where
ValidationError and TypeError are caught (keeping the exception variable name
exc) so ruff on Python 3.14 accepts it.

In `@tests/unit/memory/test_factory.py`:
- Line 100: The inline comment on the assertion for side_effect is incorrect: it
says "always fails" but the assertion actually always passes because
_Dummy(x="not-an-int") raises ValidationError and assigns side_effect; update or
remove the comment to accurately reflect this (e.g., "always passes, narrows
type for pyright" or simply remove the comment) where side_effect is asserted
after calling _Dummy and handling ValidationError.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 223a8c5f-8cc3-4ef3-b127-9a9d08fd76ce

📥 Commits

Reviewing files that changed from the base of the PR and between fad9353 and 38d1ca2.

📒 Files selected for processing (9)
  • pyrightconfig.json
  • src/synthorg/api/middleware.py
  • src/synthorg/config/loader.py
  • src/synthorg/memory/backends/mem0/adapter.py
  • src/synthorg/memory/org/store.py
  • src/synthorg/templates/presets.py
  • src/synthorg/tools/mcp/__init__.py
  • src/synthorg/tools/sandbox/docker_sandbox.py
  • tests/unit/memory/test_factory.py
📜 Review details
⏰ 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). (6)
  • GitHub Check: Deploy Preview
  • GitHub Check: Test (Python 3.14)
  • GitHub Check: Build Web
  • GitHub Check: Build Backend
  • GitHub Check: Greptile Review
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.py: No from __future__ import annotations — Python 3.14 has PEP 649 native lazy annotations
Use except A, B: syntax (no parentheses) instead of except (A, B): — PEP 758 syntax enforced by ruff on Python 3.14
All public functions must have type hints; mypy strict mode is enforced
Use Google-style docstrings on all public classes and functions (enforced by ruff D rules)
Use NotBlankStr from core.types for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators
Use @computed_field in Pydantic models for derived values instead of storing and validating redundant fields
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations (e.g., multiple tool invocations, parallel agent calls) instead of bare create_task
Line length: 88 characters (enforced by ruff)
Functions must be < 50 lines; files must be < 800 lines
Handle errors explicitly; never silently swallow exceptions
Validate input at system boundaries (user input, external APIs, config files)
Create new objects instead of mutating existing ones (immutability principle)

Files:

  • src/synthorg/memory/org/store.py
  • src/synthorg/tools/sandbox/docker_sandbox.py
  • src/synthorg/templates/presets.py
  • src/synthorg/memory/backends/mem0/adapter.py
  • tests/unit/memory/test_factory.py
  • src/synthorg/tools/mcp/__init__.py
  • src/synthorg/config/loader.py
  • src/synthorg/api/middleware.py
src/synthorg/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

src/synthorg/**/*.py: Every module with business logic must import from synthorg.observability import get_logger and define logger = get_logger(__name__)
Never use import logging, logging.getLogger(), or print() in application code — use the synthorg logger instead
Always use event name constants from domain-specific modules under synthorg.observability.events (e.g., PROVIDER_CALL_START from events.provider); import directly: from synthorg.observability.events.<domain> import EVENT_CONSTANT
Always log with structured kwargs: logger.info(EVENT, key=value) — never use old-style formatting logger.info("msg %s", val)
All error paths must log at WARNING or ERROR with context before raising
All state transitions must log at INFO level
Use DEBUG level for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do NOT need logging
Never implement retry logic in driver subclasses or calling code — all provider calls go through BaseCompletionProvider which applies retry + rate limiting automatically
Set RetryConfig and RateLimiterConfig per-provider in ProviderConfig; retryable errors are RateLimitError, ProviderTimeoutError, ProviderConnectionError, ProviderInternalError
Use frozen Pydantic models for config/identity; separate mutable-via-copy models (using model_copy(update=...)) for runtime state
For dict/list fields in frozen Pydantic models, use copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use Pydantic v2 conventions: BaseModel, model_validator, computed_field, ConfigDict

Files:

  • src/synthorg/memory/org/store.py
  • src/synthorg/tools/sandbox/docker_sandbox.py
  • src/synthorg/templates/presets.py
  • src/synthorg/memory/backends/mem0/adapter.py
  • src/synthorg/tools/mcp/__init__.py
  • src/synthorg/config/loader.py
  • src/synthorg/api/middleware.py
tests/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

tests/**/*.py: Use @pytest.mark.unit, @pytest.mark.integration, @pytest.mark.e2e, @pytest.mark.slow to categorize tests
Maintain 80% minimum code coverage (enforced in CI)
Each test must complete within 30 seconds (timeout enforcement)
Always include -n auto when running pytest via uv run python -m pytest — never run tests sequentially (pytest-xdist parallelism)
Prefer @pytest.mark.parametrize for testing similar cases
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names like example-provider, example-large-001, test-provider, test-small-001, or size aliases (large/medium/small)
Use Hypothesis for property-based testing with @given + @settings decorators; control profiles via HYPOTHESIS_PROFILE env var (ci for 200 examples, dev for 1000 examples)

Files:

  • tests/unit/memory/test_factory.py
🧠 Learnings (2)
📚 Learning: 2026-03-15T12:05:56.884Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T12:05:56.884Z
Learning: Applies to docker/**/*.py : Backend Dockerfile must use 3-stage build (builder → setup → distroless runtime), Chainguard Python base, run as non-root (UID 65532), and apply CIS hardening

Applied to files:

  • src/synthorg/tools/sandbox/docker_sandbox.py
📚 Learning: 2026-03-15T11:48:14.867Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-15T11:48:14.867Z
Learning: Applies to docker/** : Docker build context: single root .dockerignore (both images build with context: .). Tags: CI tags images with version from pyproject.toml ([tool.commitizen].version), semver, and SHA.

Applied to files:

  • src/synthorg/tools/sandbox/docker_sandbox.py
🧬 Code graph analysis (2)
src/synthorg/templates/presets.py (1)
web/src/api/types.ts (1)
  • PersonalityConfig (267-282)
src/synthorg/tools/mcp/__init__.py (1)
tests/unit/tools/mcp/test_bridge_tool.py (1)
  • bridge_tool (23-31)
🔇 Additional comments (10)
src/synthorg/config/loader.py (1)

119-122: LGTM! Safe attribute access for YAML error marks.

The getattr pattern correctly handles the type-checking concern: yaml.YAMLError is the base class and doesn't guarantee problem_mark, while subclasses like ScannerError and ParserError do. This approach is both type-safe and maintains runtime behavior.

pyrightconfig.json (1)

1-16: Configuration looks appropriate for an editor companion setup.

The configuration correctly targets Python 3.14, uses standard mode, and disables noise from dynamic libraries (Pydantic, structlog, pytest) while keeping actionable checks enabled. This aligns well with the PR's intent to use Pyright alongside mypy in CI.

src/synthorg/tools/sandbox/docker_sandbox.py (2)

14-14: LGTM! Import needed for type annotations.

The explicit aiodocker.containers import is required for the DockerContainer type hints used in method signatures (e.g., _wait_for_exit, _safe_collect_logs, _collect_logs).


357-357: Appropriate use of pyright ignores for aiodocker container operations.

The # pyright: ignore[reportAttributeAccessIssue] comments are applied consistently to all docker.containers.* access points. This is a reasonable workaround for incomplete type stubs in aiodocker, and the pattern is applied uniformly across create, lookup, stop, and remove operations.

Also applies to: 413-413, 562-562, 589-589

src/synthorg/api/middleware.py (1)

156-156: LGTM! Appropriate pyright ignore for ASGI send wrapper.

The Send type expects specific ASGI message types, but the wrapper function receives message: Any for flexibility. The runtime behavior is correct — valid ASGI messages flow through unchanged. The narrowly-scoped reportArgumentType ignore is appropriate for this ASGI typing limitation.

src/synthorg/memory/backends/mem0/adapter.py (1)

177-177: LGTM! Appropriate pyright ignore for protocol assignment.

The Mem0Client protocol defines the method subset used by the adapter, and Memory.from_config() returns an instance that satisfies it. Pyright may struggle with this inference due to the dynamic nature of the mem0 library. The narrowly-scoped ignore is appropriate since mypy handles this correctly in CI.

src/synthorg/tools/mcp/__init__.py (1)

8-9: LGTM! Clean TYPE_CHECKING pattern for lazy-loaded symbols.

The TYPE_CHECKING imports provide static type information for IDEs and type checkers, while __getattr__ handles actual runtime imports. This correctly bridges static typing with the existing lazy-loading mechanism and avoids circular imports.

Also applies to: 20-26

tests/unit/memory/test_factory.py (1)

95-99: Type narrowing approach is appropriate.

The pattern of declaring side_effect: ValidationError | None = None, then assigning in the except block, and finally asserting non-None is a valid way to satisfy pyright's type narrowing while keeping the test logic clear.

src/synthorg/memory/org/store.py (1)

424-439: LGTM! Proper SQL LIKE escaping and variable initialization.

The changes correctly:

  1. Initialize escaped before conditional use (fixes pyright's "possibly unbound" warning)
  2. Escape SQL LIKE wildcards (\, %, _) with backslash
  3. Use ESCAPE '\\' clause for the LIKE pattern
  4. Reuse the escaped value for INSTR ranking within the same if text is not None block

The escaping order is correct (backslash first to avoid double-escaping), and the logic flow ensures escaped holds the proper value when used in the ORDER BY clause.

src/synthorg/templates/presets.py (1)

440-441: Import-time validation invocation and namespace cleanup are good.

Calling _validate_presets() immediately and deleting the helper keeps startup validation explicit while preserving a clean module API surface.

"reportUnknownVariableType": false,
"reportUnknownParameterType": false,
"reportUnknownLambdaType": false,
"reportCallIssue": false,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider the implications of disabling reportCallIssue.

Disabling reportCallIssue suppresses all call-site type mismatches, which could hide legitimate type errors beyond the intended Pydantic/structlog noise. Since mypy strict is the authoritative checker in CI, this may be acceptable for editor ergonomics, but be aware that some genuine issues might be silently skipped in the IDE.

If you find real type errors slipping through, consider re-enabling this and using targeted # pyright: ignore[reportCallIssue] comments on specific false-positive lines instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pyrightconfig.json` at line 13, The pyright config currently disables
reportCallIssue by setting "reportCallIssue": false which silences all call-site
type mismatch warnings; to avoid hiding real issues either re-enable
reportCallIssue (set it back to true) and address any noisy Pydantic/structlog
false positives with targeted inline ignores (use "# pyright:
ignore[reportCallIssue]" on the specific functions or call sites), or keep it
disabled for editor ergonomics but add a short repo-level comment explaining
that mypy strict in CI is authoritative and to revert to per-line ignores if
real type errors are observed; reference the "reportCallIssue" setting in
pyrightconfig.json when making this change.

for name, preset in PERSONALITY_PRESETS.items():
try:
PersonalityConfig(**preset)
except (ValidationError, TypeError) as exc:
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify tuple-style multi-except usages that violate the repository rule.
# Expected: this file/line should appear until fixed.
rg -nP --type=py 'except\s*\([^)]*,[^)]*\)\s*(as\s+\w+)?\s*:' src tests

Repository: Aureliolo/synthorg

Length of output: 11555


Change to PEP 758 except syntax (no parentheses).

Line 435 uses except (ValidationError, TypeError) as exc:, which violates the enforced exception syntax rule. Must use except ValidationError, TypeError as exc: instead per PEP 758 (enforced by ruff on Python 3.14).

Proposed fix
-        except (ValidationError, TypeError) as exc:
+        except ValidationError, TypeError as exc:
📝 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.

Suggested change
except (ValidationError, TypeError) as exc:
except ValidationError, TypeError as exc:
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/synthorg/templates/presets.py` at line 435, Replace the tuple-style
exception clause "except (ValidationError, TypeError) as exc:" with the PEP
758-compliant syntax without parentheses: "except ValidationError, TypeError as
exc:"; update the except line where ValidationError and TypeError are caught
(keeping the exception variable name exc) so ruff on Python 3.14 accepts it.

- Add logging before raise in preset validation (CLAUDE.md requirement)
- Add TEMPLATE_PERSONALITY_PRESET_INVALID event constant
- Fix misleading comment in test_factory.py ("always fails" -> "always set")
- Keep reportCallIssue disabled (393 false positives from Pydantic/Annotated)
- PEP 758 except syntax deferred (ruff doesn't support it yet)
Copy link
Copy Markdown

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aureliolo has reached the 50-review limit for trial accounts. If you'd like to continue receiving code reviews, you can add a payment method here.

@Aureliolo Aureliolo temporarily deployed to cloudflare-preview March 15, 2026 16:53 — with GitHub Actions Inactive
@Aureliolo Aureliolo merged commit f60746a into main Mar 15, 2026
31 of 32 checks passed
@Aureliolo Aureliolo deleted the chore/pyright-config-and-fixes branch March 15, 2026 17:10
@Aureliolo Aureliolo temporarily deployed to cloudflare-preview March 15, 2026 17:10 — with GitHub Actions Inactive
Aureliolo added a commit that referenced this pull request Mar 15, 2026
🤖 I have created a release *beep* *boop*
---


##
[0.2.5](v0.2.4...v0.2.5)
(2026-03-15)


### Features

* default sandbox to enabled, polish CLI output, add sandbox CI build
([#455](#455))
([a4869b6](a4869b6))


### Bug Fixes

* export .intoto.jsonl provenance for OpenSSF Scorecard
([#456](#456))
([2feed09](2feed09))


### Maintenance

* add pyrightconfig.json and fix all pyright errors
([#448](#448))
([f60746a](f60746a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
This was referenced Mar 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant