feat: Add pluggable services architecture in lfx and comprehensive testing

[ogabrielluiz]

Introduce a pluggable services architecture with detailed documentation and an example configuration file. Add unit and integration tests for various services, including LocalStorageService, TelemetryService, and VariableService, ensuring robust coverage and error handling. Implement service registration via decorators to enhance service discovery and management.

Summary by CodeRabbit

• New Features

  • Introduces a pluggable services system with discovery via configuration, decorators, and entry points.
  • Adds minimal built-in services: Telemetry, Tracing, and Variable (in-memory with env fallback).
  • Enhances Storage service lifecycle with teardown support.
  • Supports configuration via lfx.toml or pyproject.toml for service overrides.

• Documentation

  • New guide detailing pluggable services, configuration, discovery, and troubleshooting.
  • README updates: Pluggable Services overview and flattened component access.
  • Adds lfx.toml example for common service mappings.

• Tests

  • Comprehensive unit and integration tests covering discovery, overrides, DI, lifecycle, and edge cases.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

Introduces a pluggable services system with decorator, config, and entry-point discovery; updates ServiceManager for class registration, discovery orchestration, and DI; adds telemetry, tracing, and variable minimal services; adjusts local storage inheritance/lifecycle; exports registration API; adds example config and extensive unit tests; expands docs.

Changes

Cohort / File(s)	Summary
Documentation src/lfx/PLUGGABLE_SERVICES.md, src/lfx/README.md, src/lfx/lfx.toml.example	Adds pluggable services guide, README sections, and example lfx.toml mapping service keys to implementations, including locations and precedence.
Service Manager & Registry Core src/lfx/src/lfx/services/manager.py, src/lfx/src/lfx/services/registry.py, src/lfx/src/lfx/services/__init__.py	Implements multi-source discovery (entry points, decorators, config), class registration path, DI via annotations, new NoServiceRegisteredError, discovery orchestration, and exports register_service.
Telemetry Services src/lfx/src/lfx/services/telemetry/__init__.py, .../telemetry/base.py, .../telemetry/service.py	Adds abstract telemetry base and a minimal logging-only TelemetryService; exposes module exports.
Tracing Services src/lfx/src/lfx/services/tracing/base.py, .../tracing/service.py	Adds abstract tracing base and refactors TracingService to a minimal structured tracing API with async context and logging.
Variable Service src/lfx/src/lfx/services/variable/__init__.py, .../variable/service.py	Introduces in-memory VariableService with env fallback; exposes module exports.
Storage Service Adjustment src/lfx/src/lfx/services/storage/local.py	Updates LocalStorageService to inherit Service, adds name attribute and async teardown; initializes bases explicitly.
Unit Tests — Decorator & Edge Cases src/lfx/tests/unit/services/test_decorator_registration.py, .../test_edge_cases.py	Tests decorator registration, overrides, settings protection, lifecycle, DI edge cases, circular deps, config parsing robustness, concurrency.
Unit Tests — Integration & Manager src/lfx/tests/unit/services/test_integration.py, .../test_service_manager.py	Integration coverage for discovery precedence, overrides, DI, config sources, lifecycle, error handling, and real-world interactions.
Unit Tests — Minimal Services src/lfx/tests/unit/services/test_minimal_services.py	Verifies minimal storage, telemetry, tracing, variable services behavior and teardown; includes integration scenarios.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant App as Application
  participant SM as ServiceManager
  participant EP as Entry Points
  participant CFG as Config Files
  participant REG as Decorator Registry
  participant FAC as Built-in Factories

  User->>App: request service(ServiceType)
  App->>SM: get(service_type)
  alt First-time discovery
    SM->>SM: discover_plugins()
    SM->>EP: _discover_from_entry_points()
    EP-->>SM: register classes
    SM->>REG: load decorator registrations
    REG-->>SM: register classes
    SM->>CFG: _discover_from_config(lfx.toml/pyproject.toml)
    CFG-->>SM: register classes from dotted paths
  end
  alt Class registered for type
    SM->>SM: _create_service_from_class()
  else Factory available
    SM->>FAC: _create_service_from_factory()
  else None
    SM-->>App: NoServiceRegisteredError
  end
  SM-->>App: service instance (cached)

Loading

sequenceDiagram
  autonumber
  participant SM as ServiceManager
  participant Impl as ServiceClass
  note over SM: Dependency Injection on init
  SM->>SM: inspect __init__ annotations
  loop For each parameter
    alt param is Service subclass type
      SM->>SM: get(param_service_type)
      SM-->>SM: inject dependency
    else optional/has default
      SM-->>SM: use default/None
    else non-service required
      SM-->>SM: raise TypeError
    end
  end
  SM->>Impl: instantiate with resolved deps
  Impl-->>SM: instance (ready or sets ready)

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~75 minutes

Suggested labels

lgtm

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 77.14% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (6 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title concisely and accurately summarizes the primary additions: the pluggable services architecture and comprehensive testing in the lfx package. It’s clear, short, and directly reflects the main changes without unnecessary details or noise.
Test Coverage For New Implementations	✅ Passed	The PR introduces a substantial pluggable-services architecture and accompanies it with an extensive suite of new tests covering decorator registration, edge cases, integration scenarios, and minimal service implementations; the tests live in files named with the project’s test_*.py convention and exercise the newly added telemetry, tracing, variable, and storage services, as well as the service manager’s discovery and override semantics. Regression-style scenarios are included for failure paths (invalid configs, circular dependencies, unresolvable injections), and both unit-level and integration-level behaviors are validated. Overall, the tests meaningfully exercise the new functionality rather than serving as placeholders, so the custom coverage requirement is satisfied.
Test Quality And Coverage	✅ Passed	The PR adds a broad suite of pytest-based unit and integration tests that exercise the newly introduced pluggable service architecture, covering decorator registration, discovery via configuration, entry-point fallbacks, dependency injection, lifecycle, and error handling; these tests validate real behavior (not just smoke) and include async cases correctly annotated with pytest.mark.asyncio. Minimal service implementations (storage, telemetry, tracing, variable) are thoroughly tested for expected functionality, readiness, teardown, and environment interactions, ensuring the main features work as intended. The test modules follow the project’s backend testing conventions and include both success paths and failure scenarios (e.g., invalid configs, circular dependencies, unresolvable services), demonstrating comprehensive coverage for the new functionality.
Test File Naming And Structure	✅ Passed	All newly added backend tests follow the test_*.py naming convention, rely on pytest fixtures, and present descriptive function names that capture the behavior under test. The suites are grouped logically (decorator registration, edge cases, integration flows, minimal services, service manager) with appropriate setup and teardown via fixtures and asynchronous cleanup, and they exercise both positive paths and error conditions, including circular dependencies, invalid registrations, and override semantics. Integration-oriented coverage is provided within test_integration.py, and despite residing in the unit services directory it is clearly labeled through filename, fixture naming, and comprehensive scenario coverage, satisfying the requirement that such tests be clearly marked and organized. Overall, the test files demonstrate thorough scenario coverage across happy paths and edge cases with observable verification using public APIs, so the check passes.
Excessive Mock Usage Warning	✅ Passed	After reviewing the newly added and modified test modules, I found no usage of mocks or patching utilities; the tests instantiate real service implementations, interact with actual ServiceManager instances, and rely on genuine behavior rather than simulated dependencies, aligning well with the custom check’s criteria against excessive mocking.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch pluggable-service-layer

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[ogabrielluiz]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

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)

src/lfx/src/lfx/services/manager.py (1)

225-255: Use the resolved factory instance instead of indexing the dict again
When we fall back to a default factory, we call register_factory(default) which stores it under default.service_class.name (a string such as "storage_service"). Immediately after, we still access self.factories[service_name], where service_name is a ServiceType. If the enum value doesn’t hash-identically to that string (e.g. when using plain Enum instead of StrEnum), this will raise a KeyError or return the wrong factory. We already have the correct factory selected in the local factory variable—use it directly to avoid mismatched key lookups.

-        self.services[service_name] = self.factories[service_name].create(**dependent_services)
+        self.services[service_name] = factory.create(**dependent_services)

🧹 Nitpick comments (8)

src/lfx/src/lfx/services/variable/service.py (1)

18-83: Consider security implications of in-memory variable storage.

Variables are stored in plain memory without encryption. If sensitive data (API keys, secrets) is stored via set_variable, it remains in memory until teardown and could be exposed through memory dumps or debugging.

For production use, consider:

• Adding a warning in the docstring about not storing secrets
• Or implementing memory encryption for sensitive values
• Or directing users to use environment variables for secrets (which are also plain text but at least OS-managed)

Example docstring addition:

     def set_variable(self, name: str, value: str, **kwargs) -> None:  # noqa: ARG002
-        """Set a variable value (in-memory only).
+        """Set a variable value (in-memory only).
+        
+        Warning: Values are stored in plain memory. Do not use for secrets.
+        For sensitive data, use environment variables instead.
 
         Args:
             name: Variable name

src/lfx/src/lfx/services/telemetry/base.py (1)

21-24: Consider removing @abstractmethod from __init__.

Making __init__ abstract is unusual and may be overly restrictive. Subclasses must override __init__ even if the base initialization logic would suffice. Typically, only behavior-defining methods (not initialization) are marked abstract.

If all subclasses need custom initialization, keep it abstract. Otherwise, consider making __init__ concrete:

-    @abstractmethod
     def __init__(self):
         """Initialize the telemetry service."""
         super().__init__()

This allows subclasses to call super().__init__() without being forced to override.

src/lfx/src/lfx/services/registry.py (1)

36-52: Consider more specific exception handling.

The broad except Exception clause (line 47) logs a warning but continues execution, potentially hiding unexpected registration errors. While this prevents decorator application from breaking the code, it can make debugging difficult when registration silently fails.

Consider catching more specific exceptions or providing a way to fail fast in development:

         try:
             from lfx.services.manager import get_service_manager
 
             service_manager = get_service_manager()
             service_manager.register_service_class(service_type, service_class, override=override)
             logger.debug(f"Registered service via decorator: {service_type.value} -> {service_class.__name__}")
         except ValueError:
             # Re-raise ValueError (used for settings service protection)
             raise
+        except (ImportError, AttributeError) as exc:
+            # Expected errors during import/registration
+            logger.warning(f"Failed to register service {service_type.value} from decorator: {exc}")
-        except Exception as exc:  # noqa: BLE001
+        except Exception as exc:
-            logger.warning(f"Failed to register service {service_type.value} from decorator: {exc}")
+            # Unexpected error - log and optionally fail in development
+            logger.error(f"Unexpected error registering service {service_type.value}: {exc}", exc_info=True)
+            # Consider: raise in development mode
 
         return service_class

Alternatively, add an environment variable to control whether to raise or warn on unexpected errors.

src/lfx/PLUGGABLE_SERVICES.md (1)

124-125: Resolve markdownlint MD036 warning

markdownlint flagged the **Important:** line as “emphasis used instead of a heading.” Please switch to a proper heading (for example, ### Important) or rephrase to satisfy the lint rule and keep CI clean.

src/lfx/tests/unit/services/test_edge_cases.py (1)

13-22: Deduplicate the repeated clean_manager fixture

Each test class redefines the same clean_manager fixture. Consider promoting it to a module-level (or conftest) fixture so it’s declared once and reused everywhere. That trims duplication and keeps future updates to the teardown logic in one location.

src/lfx/tests/unit/services/test_integration.py (1)

15-24: Extract the shared clean_manager fixture

Multiple classes in this module redeclare an identical clean_manager fixture. Moving it to a single module-level (or shared) fixture avoids repetition and keeps the teardown logic consistent everywhere.

src/lfx/tests/unit/services/test_minimal_services.py (2)

166-173: Consider using pytest's monkeypatch for environment variables.

The manual try/finally blocks for environment variable cleanup work correctly, but pytest's monkeypatch fixture provides cleaner and more robust test isolation with automatic cleanup.

Example refactor using monkeypatch:

def test_get_from_environment(self, variables, monkeypatch):
    """Test getting variable from environment."""
    monkeypatch.setenv("TEST_ENV_VAR", "env_value")
    value = variables.get_variable("TEST_ENV_VAR")
    assert value == "env_value"
    # Automatic cleanup by monkeypatch

def test_in_memory_overrides_env(self, variables, monkeypatch):
    """Test that in-memory variables override environment."""
    monkeypatch.setenv("TEST_VAR", "env_value")
    variables.set_variable("TEST_VAR", "memory_value")
    value = variables.get_variable("TEST_VAR")
    assert value == "memory_value"
    # Automatic cleanup by monkeypatch

Also applies to: 196-204

---

215-256: Integration tests provide good baseline coverage.

The integration tests effectively verify that minimal services can be initialized, torn down, and work together. The basic coverage is appropriate for minimal service implementations.

For more thorough integration testing, consider verifying the actual interactions. For example, in test_storage_with_tracing, you could verify that tracing logs were recorded:

@pytest.mark.asyncio
async def test_storage_with_tracing(self, tmp_path):
    """Test using storage with tracing."""
    storage = LocalStorageService(data_dir=tmp_path)
    tracing = TracingService()

    # Track that logs were added
    log_count_before = len(tracing._logs) if hasattr(tracing, '_logs') else 0
    
    tracing.add_log("storage_test", {"operation": "save", "flow_id": "123"})
    await storage.save_file("flow_123", "test.txt", b"content")
    tracing.add_log("storage_test", {"operation": "saved", "flow_id": "123"})

    # Verify both storage and tracing worked
    assert await storage.get_file("flow_123", "test.txt") == b"content"
    # Optionally verify logs were recorded (if tracing service exposes this)

Note: This assumes the tracing service exposes a way to verify logs were recorded, which may not be the case for a minimal implementation.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aa96d74 and 83d0568.

📒 Files selected for processing (19)

• src/lfx/PLUGGABLE_SERVICES.md (1 hunks)
• src/lfx/README.md (1 hunks)
• src/lfx/lfx.toml.example (1 hunks)
• src/lfx/src/lfx/services/__init__.py (2 hunks)
• src/lfx/src/lfx/services/manager.py (6 hunks)
• src/lfx/src/lfx/services/registry.py (1 hunks)
• src/lfx/src/lfx/services/storage/local.py (1 hunks)
• src/lfx/src/lfx/services/telemetry/__init__.py (1 hunks)
• src/lfx/src/lfx/services/telemetry/base.py (1 hunks)
• src/lfx/src/lfx/services/telemetry/service.py (1 hunks)
• src/lfx/src/lfx/services/tracing/base.py (1 hunks)
• src/lfx/src/lfx/services/tracing/service.py (1 hunks)
• src/lfx/src/lfx/services/variable/__init__.py (1 hunks)
• src/lfx/src/lfx/services/variable/service.py (1 hunks)
• src/lfx/tests/unit/services/test_decorator_registration.py (1 hunks)
• src/lfx/tests/unit/services/test_edge_cases.py (1 hunks)
• src/lfx/tests/unit/services/test_integration.py (1 hunks)
• src/lfx/tests/unit/services/test_minimal_services.py (1 hunks)
• src/lfx/tests/unit/services/test_service_manager.py (1 hunks)

🧰 Additional context used

🪛 markdownlint-cli2 (0.18.1)

src/lfx/PLUGGABLE_SERVICES.md

158-158: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

210-210: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

220-220: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

---

227-227: Emphasis used instead of a heading

(MD036, no-emphasis-as-heading)

🔇 Additional comments (9)

src/lfx/src/lfx/services/variable/__init__.py (1)

1-5: LGTM!

The module correctly exports the VariableService as part of the public API, following standard Python packaging conventions.

src/lfx/src/lfx/services/telemetry/base.py (1)

26-86: LGTM!

The abstract method definitions provide a clear and comprehensive interface for telemetry service implementations. Type hints, async declarations, and docstrings are all appropriate.

src/lfx/src/lfx/services/__init__.py (1)

12-12: LGTM!

The addition of register_service to the public API correctly exposes the decorator-based registration mechanism, enabling users to register pluggable services.

Also applies to: 27-27

src/lfx/README.md (1)

29-71: LGTM!

The documentation clearly explains the new pluggable services architecture and improved component access patterns. The before/after examples are helpful, and the reference to PLUGGABLE_SERVICES.md provides a clear path for users seeking detailed information.

src/lfx/src/lfx/services/telemetry/__init__.py (1)

1-5: LGTM!

The module correctly exports the TelemetryService as part of the public API, following standard Python packaging conventions.

src/lfx/lfx.toml.example (1)

14-44: Confirm all service paths import correctly from the installed Langflow package. Ensure each module.path:ClassName in src/lfx/lfx.toml.example maps to an existing, importable class in your Langflow environment.

src/lfx/tests/unit/services/test_minimal_services.py (3)

13-87: Comprehensive test coverage for LocalStorageService.

The test suite provides solid coverage of LocalStorageService functionality, including CRUD operations, error handling, and lifecycle management. The use of tmp_path fixture ensures proper test isolation.

---

89-120: Test coverage appropriate for minimal TelemetryService.

The tests appropriately verify that the no-op telemetry service methods complete without errors. For a minimal service implementation, this level of testing is acceptable.

---

122-145: Test coverage appropriate for minimal TracingService.

The tests verify basic functionality of the minimal tracing service. The coverage is suitable for a minimal implementation.

[github-actions]

Frontend Unit Test Coverage Report

Coverage Summary

Lines	Statements	Branches	Functions
	17.49% (4997/28556)	10.83% (2388/22036)	11.59% (724/6244)

Unit Test Results

Tests	Skipped	Failures	Errors	Time
1998	0 💤	0 ❌	0 🔥	26.645s ⏱️

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 34.24%. Comparing base (6b4f946) to head (bdf6ded).

❌ Your project check has failed because the head coverage (40.80%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10111      +/-   ##
==========================================
- Coverage   34.24%   34.24%   -0.01%     
==========================================
  Files        1409     1409              
  Lines       66929    66921       -8     
  Branches     9877     9877              
==========================================
- Hits        22918    22915       -3     
+ Misses      42810    42805       -5     
  Partials     1201     1201              

Flag	Coverage Δ
frontend	16.03% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/lfx/src/lfx/services/manager.py	68.46% <ø> (-0.29%)	⬇️
src/lfx/src/lfx/services/storage/local.py	18.51% <ø> (-1.97%)	⬇️
src/lfx/src/lfx/services/tracing/service.py	0.00% <ø> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mpawlow]

@coderabbitai configuration

[coderabbitai]

Configuration used: Path: .coderabbit.yaml

CodeRabbit configuration

language: en-US
tone_instructions: ''
early_access: true
enable_free_tier: true
inheritance: false
reviews:
  profile: chill
  request_changes_workflow: false
  high_level_summary: true
  high_level_summary_instructions: ''
  high_level_summary_placeholder: '@coderabbitai summary'
  high_level_summary_in_walkthrough: false
  auto_title_placeholder: '@coderabbitai'
  auto_title_instructions: Use conventional commits structure
  review_status: true
  commit_status: true
  fail_commit_status: false
  collapse_walkthrough: false
  changed_files_summary: true
  sequence_diagrams: true
  estimate_code_review_effort: true
  assess_linked_issues: true
  related_issues: true
  related_prs: true
  suggested_labels: true
  labeling_instructions: []
  auto_apply_labels: false
  suggested_reviewers: true
  auto_assign_reviewers: false
  in_progress_fortune: true
  poem: false
  enable_prompt_for_ai_agents: true
  path_filters:
    - '!src/backend/base/langflow/initial_setup/starter_projects'
  path_instructions: []
  abort_on_close: true
  disable_cache: false
  auto_review:
    enabled: true
    auto_incremental_review: false
    ignore_title_keywords: []
    labels: []
    drafts: false
    base_branches: []
    ignore_usernames: []
  finishing_touches:
    docstrings:
      enabled: true
    unit_tests:
      enabled: true
  pre_merge_checks:
    docstrings:
      mode: warning
      threshold: 80
    title:
      mode: warning
      requirements: ''
    description:
      mode: warning
    issue_assessment:
      mode: warning
    custom_checks:
      - mode: error
        name: Test Coverage for New Implementations
        instructions: >
          Check if the PR includes new or updated test files that correspond to
          the code changes:

          1. For new components or functionality, ensure corresponding tests are
          included in the PR

          2. For bug fixes, verify that regression tests are included

          3. For new features, ensure both unit and integration tests are
          present where appropriate

          4. Check that test files follow the project's naming conventions
          (test_*.py for backend, *.test.ts for frontend)

          5. Verify that tests actually test the new functionality, not just
          placeholder tests
      - mode: warning
        name: Test Quality and Coverage
        instructions: >
          Evaluate the quality and comprehensiveness of tests for new
          implementations:

          1. Tests should cover the main functionality being implemented

          2. For async functions, ensure proper async testing patterns are used
          (pytest for backend)

          3. Check that tests are not just smoke tests but actually validate
          behavior

          4. Ensure tests follow the project's testing patterns (pytest for
          backend, Playwright for frontend)

          5. For API endpoints, verify both success and error response testing
      - mode: warning
        name: Test File Naming and Structure
        instructions: >
          Verify that test files follow the correct patterns and structure:

          1. Backend tests: test_*.py with proper pytest structure

          2. Frontend tests: *.test.ts or *.test.tsx using Playwright

          3. Integration tests should be clearly marked and in appropriate
          directories

          4. Test files should have descriptive test function names that explain
          what is being tested

          5. Tests should be organized logically with proper setup and teardown

          6. Consider including edge cases and error conditions for
          comprehensive coverage

          7. Verify tests cover both positive and negative scenarios where
          appropriate
      - mode: warning
        name: Excessive Mock Usage Warning
        instructions: >
          Review test files for excessive use of mocks that may indicate poor
          test design:

          1. Check if tests have too many mock objects that obscure what's
          actually being tested

          2. Warn when mocks are used instead of testing real behavior and
          interactions

          3. Suggest using real objects or test doubles when mocks become
          excessive

          4. Ensure mocks are used appropriately for external dependencies, not
          core logic

          5. Recommend integration tests when unit tests become overly mocked
  tools:
    ast-grep:
      rule_dirs: []
      util_dirs: []
      essential_rules: true
      packages: []
    shellcheck:
      enabled: true
    ruff:
      enabled: true
    markdownlint:
      enabled: true
    github-checks:
      enabled: true
      timeout_ms: 90000
    languagetool:
      enabled: true
      enabled_rules: []
      disabled_rules: []
      enabled_categories: []
      disabled_categories: []
      enabled_only: false
      level: default
    biome:
      enabled: true
    hadolint:
      enabled: true
    swiftlint:
      enabled: true
    phpstan:
      enabled: true
      level: default
    phpmd:
      enabled: true
    phpcs:
      enabled: true
    golangci-lint:
      enabled: true
    yamllint:
      enabled: true
    gitleaks:
      enabled: true
    checkov:
      enabled: true
    detekt:
      enabled: true
    eslint:
      enabled: true
    flake8:
      enabled: true
    fortitudeLint:
      enabled: true
    rubocop:
      enabled: true
    buf:
      enabled: true
    regal:
      enabled: true
    actionlint:
      enabled: true
    pmd:
      enabled: true
    clang:
      enabled: true
    cppcheck:
      enabled: true
    semgrep:
      enabled: true
    circleci:
      enabled: true
    clippy:
      enabled: true
    sqlfluff:
      enabled: true
    prismaLint:
      enabled: true
    pylint:
      enabled: true
    oxc:
      enabled: true
    shopifyThemeCheck:
      enabled: true
    luacheck:
      enabled: true
    brakeman:
      enabled: true
    dotenvLint:
      enabled: true
    htmlhint:
      enabled: true
    checkmake:
      enabled: true
    osvScanner:
      enabled: true
chat:
  art: true
  auto_reply: true
  integrations:
    jira:
      usage: auto
    linear:
      usage: auto
knowledge_base:
  opt_out: false
  web_search:
    enabled: true
  code_guidelines:
    enabled: true
    filePatterns: []
  learnings:
    scope: auto
  issues:
    scope: auto
  jira:
    usage: auto
    project_keys: []
  linear:
    usage: auto
    team_keys: []
  pull_requests:
    scope: auto
  mcp:
    usage: auto
    disabled_servers: []
code_generation:
  docstrings:
    language: en-US
    path_instructions: []
  unit_tests:
    path_instructions:
      - path: src/backend/tests/**/*.py
        instructions: We already have a client fixture and use pytest
      - path: src/frontend/tests/**.ts
        instructions: We use Playwright
issue_enrichment:
  auto_enrich:
    enabled: false
  planning:
    enabled: true
    auto_planning:
      enabled: true
      labels: []
  labeling:
    labeling_instructions: []
    auto_apply_labels: false

[mpawlow]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[HimavarshaVS]

Seem like we are storing the partial state even before the validation is complete.
can do something like

Suggested change

	service_instance = service_class(**dependencies)
	service_instance = service_class(**dependencies)
	# Validate service has required attributes
	if not hasattr(service_instance, 'name'):
	raise ValueError(f"Service {service_class.__name__} missing 'name' attribute")
	-----

[mpawlow]

• I need to circle back and evaluate this further before making any destabilizing changes
• Not sure if validating properties is sufficient or explicit validation should be done via introducing a validate lifecycle method or deferring validation to the set_ready method

[HimavarshaVS]

few comments to avoid race conditions . But other than that lgtm

[jordanrfrazier]

Looks good to me. Tested a brief script that plugged in a Custom Service. Didn't look at every line of code, but the idea and architecture look good to me and we can discover any small issues as we utilize this.

[jordanrfrazier]

Does this potential race condition exist?

Thread A: 
* get(serviceA) -> lock(serviceA) -> dependsOn(ServiceC) -> _create_service(serviceC) (no serviceC locking)
Thread B:
* get(serviceB) -> lock(serviceB) -> dependsOn(ServiceC) -> _create_service(serviceC) (no serviceC locking)

If so, we can do something like:

+                with self.keyed_lock.lock(dependency_type):
+                    if dependency_type not in self.services:
+                        self._create_service(dependency_type)
+                    dependencies[param_name] = self.services[dependency_type]

[jordanrfrazier]

This does only catch A -> A dependencies, I don't believe it will catch A -> B -> A. The error message does concur with that statement, but we may want to add logic to catch the latter scenarios.