feat: add meeting scheduler and event-triggered meetings#397
feat: add meeting scheduler and event-triggered meetings#397Aureliolo wants to merge 13 commits into
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Meeting subsystem: MeetingFrequency enum, participant resolution and registry resolver, MeetingScheduler service (periodic and event-triggered), MeetingController endpoints (list/get/trigger) and lifecycle wiring, observability events/WS types, frontend types/store/UI, tests, and docs updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Scheduler as MeetingScheduler
participant Resolver as RegistryParticipantResolver
participant Registry as AgentRegistryService
participant Orchestrator as MeetingOrchestrator
participant EventPub as EventPublisher
Scheduler->>Scheduler: periodic interval / trigger received
Scheduler->>Resolver: resolve(participant_refs, context)
Resolver->>Registry: list_active / list_by_department / get_by_name
Registry-->>Resolver: agent IDs
Resolver-->>Scheduler: deduplicated participant IDs
Scheduler->>Orchestrator: run_meeting(meeting_type, agenda, participants)
Orchestrator-->>Scheduler: MeetingRecord
Scheduler->>EventPub: publish WS event (meeting.completed)
EventPub-->>Scheduler: ack / error
sequenceDiagram
participant Client as API Client
participant Controller as MeetingController
participant Scheduler as MeetingScheduler
participant Resolver as ParticipantResolver
participant Orchestrator as MeetingOrchestrator
participant EventPub as EventPublisher
Client->>Controller: POST /meetings/trigger {event_name, context}
Controller->>Scheduler: trigger_event(event_name, context)
Scheduler->>Scheduler: find matching meeting types
Scheduler->>Resolver: resolve(...)
Resolver-->>Scheduler: participants
Scheduler->>Orchestrator: run_meeting(...)
Orchestrator-->>Scheduler: MeetingRecord(s)
Scheduler->>EventPub: publish WS events
Scheduler-->>Controller: tuple[MeetingRecord, ...]
Controller-->>Client: ApiResponse[tuple[MeetingRecord, ...]]
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
📝 Coding Plan
Comment |
Summary of ChangesHello, 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 introduces a robust and comprehensive system for managing agent meetings, allowing for both regularly scheduled and dynamically triggered interactions. It establishes the foundational backend services for meeting orchestration and participant management, alongside a user-friendly frontend interface for monitoring and initiating these processes. The changes significantly enhance the platform's ability to coordinate agent activities and provide transparency into their collaborative efforts. Highlights
Changelog
Activity
Using Gemini Code AssistThe 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
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 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
|
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
This PR implements the meeting scheduler service and event-triggered meetings (closes #264). It replaces the stub MeetingLogsPage and meeting controller with fully functional implementations backed by a MeetingScheduler, ParticipantResolver, and MeetingFrequency enum.
Changes:
- Added
MeetingSchedulerservice with periodic asyncio tasks for frequency-based meetings and on-demand event-triggered meetings, plusParticipantResolverprotocol andMeetingFrequencyenum replacingNotBlankStr - Implemented REST API endpoints (
GET /meetings,GET /meetings/{id},POST /meetings/trigger) with filtering, pagination, and input validation, plus WebSocketmeetingschannel - Built out the frontend
MeetingLogsPagewith DataTable, detail sidebar, trigger dialog, Pinia store with WS integration, and full TypeScript types
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ai_company/communication/meeting/scheduler.py | New meeting scheduler with periodic + event-triggered execution |
| src/ai_company/communication/meeting/participant.py | New participant resolver protocol + registry implementation |
| src/ai_company/communication/meeting/frequency.py | New MeetingFrequency enum + seconds conversion |
| src/ai_company/communication/meeting/errors.py | New scheduler error classes |
| src/ai_company/communication/config.py | Changed frequency field from NotBlankStr to MeetingFrequency |
| src/ai_company/api/controllers/meetings.py | Full meeting controller replacing stub |
| src/ai_company/api/state.py | Added meeting_orchestrator and meeting_scheduler to AppState |
| src/ai_company/api/app.py | Wired meeting services into create_app |
| src/ai_company/api/channels.py | Added meetings WS channel |
| src/ai_company/api/ws_models.py | Added meeting WS event types |
| src/ai_company/observability/events/meeting.py | Added scheduler event constants |
| src/ai_company/communication/meeting/init.py | Re-exported new public symbols |
| web/src/views/MeetingLogsPage.vue | Full meeting logs page replacing stub |
| web/src/stores/meetings.ts | New Pinia store for meetings |
| web/src/api/endpoints/meetings.ts | New API client endpoints |
| web/src/api/types.ts | Meeting TypeScript types + WS channel/events |
| web/src/styles/theme.ts | Added meeting status colors |
| tests/unit/communication/meeting/test_scheduler.py | Scheduler tests |
| tests/unit/communication/meeting/test_participant.py | Participant resolver tests |
| tests/unit/communication/meeting/test_frequency.py | Frequency enum tests |
| tests/unit/communication/meeting/test_config_frequency.py | Config frequency coercion tests |
| tests/unit/api/controllers/test_meetings.py | Controller endpoint tests |
| web/src/tests/stores/meetings.test.ts | Store + WS handler tests |
| CLAUDE.md | Updated docs for new modules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| records = orchestrator.get_records() | ||
|
|
||
| for record in records: | ||
| if record.meeting_id == meeting_id: | ||
| return Response( | ||
| content=ApiResponse[MeetingRecord](data=record), | ||
| status_code=200, | ||
| ) | ||
|
|
| body = resp.json() | ||
| assert body["success"] is False | ||
|
|
||
| def test_trigger_endpoint_callsmock_scheduler( |
| cost_tracker=CostTracker(), | ||
| approval_store=ApprovalStore(), | ||
| auth_service=auth_service, | ||
| # No meeting_orchestrator or meetingmock_scheduler |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive meeting scheduler feature, including backend services, API endpoints, and a frontend UI. The implementation is well-structured. However, I've identified a critical syntax error in the new scheduler module that will prevent the application from starting. Additionally, there's a performance concern in the new meetings API and a minor documentation gap that should be addressed to improve maintainability.
| except MemoryError, RecursionError: | ||
| raise |
There was a problem hiding this comment.
The syntax except MemoryError, RecursionError: is from Python 2 and is invalid in Python 3. This will cause a SyntaxError when the module is imported, preventing the application from starting. The correct syntax for catching multiple exceptions in Python 3 is to use a tuple. I noticed the CLAUDE.md file mentions using this syntax, but that information seems to be incorrect for Python 3.
| except MemoryError, RecursionError: | |
| raise | |
| except (MemoryError, RecursionError): | |
| raise |
| for record in records: | ||
| if record.meeting_id == meeting_id: | ||
| return Response( | ||
| content=ApiResponse[MeetingRecord](data=record), | ||
| status_code=200, | ||
| ) |
There was a problem hiding this comment.
The current implementation for get_meeting iterates through all meeting records to find one by its ID. This is a linear scan with O(n) complexity, which will become slow as the number of meetings grows. This lookup should be optimized for better performance and scalability, for example by using a dictionary for storage in MeetingOrchestrator to allow for O(1) lookups.
| leader_id = resolved[0] | ||
| participant_ids = resolved[1:] |
There was a problem hiding this comment.
The logic assumes that the first agent ID in the resolved tuple is the leader. This is an implicit convention that depends on the order of participants in the meeting configuration and the implementation of the ParticipantResolver. For better clarity and maintainability, this convention should be explicitly documented. A simple code comment would suffice.
# The first resolved participant is designated as the meeting leader.
leader_id = resolved[0]
participant_ids = resolved[1:]
Greptile SummaryThis PR delivers a substantial, well-architected feature: a Key issues found:
Confidence Score: 3/5
Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/communication/conftest.py (1)
73-77: 🛠️ Refactor suggestion | 🟠 MajorUpdate factory to use
MeetingFrequencyenum instead of string.The
MeetingTypeConfigFactorystill uses a string literal"daily"forfrequency, whileMeetingTypeConfig.frequencynow expectsMeetingFrequency | None. Although Pydantic'sStrEnumcoercion may handle this, using the enum directly is more explicit and consistent with the fixture update at line 175.♻️ Proposed fix
class MeetingTypeConfigFactory(ModelFactory[MeetingTypeConfig]): __model__ = MeetingTypeConfig - frequency = "daily" + frequency = MeetingFrequency.DAILY trigger = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/communication/conftest.py` around lines 73 - 77, Replace the string literal for frequency in MeetingTypeConfigFactory with the MeetingFrequency enum to match MeetingTypeConfig's type: update the MeetingTypeConfigFactory class (symbol MeetingTypeConfigFactory) to set frequency = MeetingFrequency.DAILY (or the correct enum member) instead of "daily" so tests and fixtures use the explicit MeetingFrequency enum consistent with the MeetingTypeConfig model.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ai_company/api/app.py`:
- Around line 362-363: The meeting_scheduler injected into app state
(meeting_scheduler: MeetingScheduler) is never started or stopped; in the app
startup/lifespan handler where MeetingOrchestrator/meeting_scheduler are wired
into state, call the scheduler's lifecycle methods (e.g.,
meeting_scheduler.start() on startup and meeting_scheduler.stop() or shutdown on
application shutdown), guarding for None and ensuring idempotence; likewise
ensure MeetingOrchestrator is started/stopped if it requires it. Update the
app's startup and shutdown handlers to manage meeting_scheduler (and
meeting_orchestrator if applicable), so periodic schedules are activated on
startup and cleanly stopped on shutdown.
In `@src/ai_company/api/controllers/meetings.py`:
- Around line 72-102: The endpoint list_meetings currently loads all records via
orchestrator.get_records() and filters in-memory, which won't scale; update the
call to pass filter params (status and meeting_type/meeting_type_name) and
pagination args (offset, limit) into orchestrator.get_records so filtering and
slicing occur at the data layer, and then call paginate only if orchestrator
returns an unpaginated full list. Modify the MeetingOrchestrator.get_records
signature to accept optional status: MeetingStatus | None, meeting_type: str |
None, offset: int | PaginationOffset | None, limit: int | PaginationLimit | None
(keeping defaults for backward compatibility) and implement the efficient
query/filtering there; ensure list_meetings uses the new signature and still
returns PaginatedResponse(data=page, pagination=meta) when needed.
- Around line 166-169: The call to scheduler.trigger_event uses a redundant
"data.context or None" which converts empty dicts to None; update the call to
pass data.context directly (i.e., remove the "or None") so
scheduler.trigger_event receives the original dict or None as intended,
referencing the scheduler.trigger_event invocation and the data.context field in
the request object.
- Around line 129-133: The logger.warning call currently uses the
MEETING_SCHEDULER_ERROR constant for a lookup-miss case; change it to a
semantically correct event (either introduce a new MEETING_NOT_FOUND constant
mirroring the style of MEETING_PROTOCOL_NOT_FOUND or use an existing
MEETING_VALIDATION_FAILED) and update the logger.warning invocation in the
GET/lookup flow (the call that passes meeting_id and note="meeting not found")
to use that new/appropriate constant instead of MEETING_SCHEDULER_ERROR; ensure
the new constant follows existing naming and message patterns used for
MEETING_PROTOCOL_NOT_FOUND.
In `@src/ai_company/communication/meeting/participant.py`:
- Around line 123-124: The current branch that handles isinstance(val, list) in
participant.py blindly stringifies every element (val) creating synthetic
participant IDs; instead validate each list item before returning: ensure items
are instances of str and non-empty (e.g., v and v.strip()), discard or raise on
invalid entries per the module policy, and return only the validated list of
participant IDs; update the code path in the function/method that contains the
isinstance(val, list) check to perform this filtering/validation rather than
str() coercion.
In `@src/ai_company/communication/meeting/scheduler.py`:
- Around line 279-280: The code implicitly chooses the first item in resolved as
the meeting leader (leader_id = resolved[0]) and the rest as participant_ids =
resolved[1:], which is not documented; add a brief comment or expand the
enclosing function/class docstring to state this selection strategy (e.g., "The
first resolved participant is designated leader; subsequent entries are
participants") and reference the variables leader_id and participant_ids so
future readers understand the convention.
In `@tests/unit/api/controllers/test_meetings.py`:
- Line 226: There is a typo in the test comment: replace the incorrect
"meetingmock_scheduler" token with "meeting_scheduler" in the comment found in
tests/unit/api/controllers/test_meetings.py (search for the exact string
"meetingmock_scheduler") so the comment reads "No meeting_orchestrator or
meeting_scheduler".
- Around line 182-196: Rename the test function currently named
test_trigger_endpoint_callsmock_scheduler to
test_trigger_endpoint_calls_mock_scheduler in the
tests/unit/api/controllers/test_meetings.py file; update the function definition
(def test_trigger_endpoint_callsmock_scheduler(...): -> def
test_trigger_endpoint_calls_mock_scheduler(...):) and any references to that
symbol (e.g., from other tests or parametrizations) so the test name reads
correctly and discovery still works.
- Around line 63-72: The fixture mock_orchestrator is declared async but
contains no await; change its definition to a synchronous fixture by removing
the async keyword (i.e., make def mock_orchestrator() -> MagicMock), keep the
body as-is using _make_record and MeetingStatus, and leave orch.get_records =
MagicMock(return_value=records) unchanged so callers still receive the preset
records.
In `@tests/unit/communication/meeting/test_config_frequency.py`:
- Around line 21-28: Add the missing "per_sprint_day" case to the parametrized
coercion test so the string "per_sprint_day" maps to
MeetingFrequency.PER_SPRINT_DAY; update the pytest.mark.parametrize tuple in the
test (the parameter list currently containing "daily", "weekly", "bi_weekly",
"monthly") to include ("per_sprint_day", MeetingFrequency.PER_SPRINT_DAY) so the
test covers string-to-enum coercion for the per-sprint-day frequency (refer to
MeetingFrequency and the parametrized test in test_config_frequency.py).
In `@web/src/__tests__/views/MeetingLogsPage.test.ts`:
- Around line 16-31: Tests currently only assert initial state and method
existence for useMeetingStore; add behavioral tests that invoke store methods
and assert state changes. Specifically, mock the API helpers (e.g.,
listMeetings) and write an async test that calls fetchMeetings() and asserts
meetings, total, and loading are updated; mock any create/append API and call
triggerMeeting() to assert new records are appended to meetings; simulate
different websocket payloads and call handleWsEvent() to assert the store
refresh/modify behavior. Use the store factory useMeetingStore() to get a fresh
store instance per test and ensure mocked functions are reset between tests.
In `@web/src/api/types.ts`:
- Around line 577-580: The frontend type TriggerMeetingRequest currently
declares context as Record<string, unknown> which mismatches the backend's
dict[str, str]; update the TriggerMeetingRequest interface so context is typed
as Record<string, string> to enforce string-only values (adjust any callers of
TriggerMeetingRequest to stringify non-string values if needed) and run
typescript checks to ensure compatibility with the backend contract.
In `@web/src/stores/meetings.ts`:
- Around line 81-97: handleWsEvent currently reacts to meeting event_type values
without verifying the event.channel, so events from other channels can trigger
_refreshMeeting; update handleWsEvent (which accepts a WsEvent) to first check
that event.channel === 'meetings' (or falsy-safe equivalent) before processing
meeting.started/meeting.completed/meeting.failed, and only call
_refreshMeeting(meetingId) when the channel guard passes; leave existing payload
validation and meetingId checks intact.
- Around line 64-75: _refreshMeeting updates the meetings list but doesn't
update the currently open selectedMeeting, causing the sidebar/detail view to
display stale data; after obtaining fresh from meetingsApi.getMeeting and after
you replace or append in meetings.value inside _refreshMeeting, check if
selectedMeeting.value?.meeting_id === meetingId and if so assign
selectedMeeting.value = fresh so the detail view stays in sync (ensure you
reference the existing selectedMeeting reactive/state variable and the
_refreshMeeting function when making the change).
---
Outside diff comments:
In `@tests/unit/communication/conftest.py`:
- Around line 73-77: Replace the string literal for frequency in
MeetingTypeConfigFactory with the MeetingFrequency enum to match
MeetingTypeConfig's type: update the MeetingTypeConfigFactory class (symbol
MeetingTypeConfigFactory) to set frequency = MeetingFrequency.DAILY (or the
correct enum member) instead of "daily" so tests and fixtures use the explicit
MeetingFrequency enum consistent with the MeetingTypeConfig model.
🪄 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: 3758fc4d-b295-46e9-b6ee-0c2b0afbb7da
📒 Files selected for processing (29)
CLAUDE.mdsrc/ai_company/api/app.pysrc/ai_company/api/channels.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/api/state.pysrc/ai_company/api/ws_models.pysrc/ai_company/communication/config.pysrc/ai_company/communication/meeting/__init__.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/communication/meeting/frequency.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/communication/meeting/scheduler.pysrc/ai_company/observability/events/meeting.pytests/integration/communication/test_meeting_integration.pytests/unit/api/controllers/test_meetings.pytests/unit/api/test_channels.pytests/unit/communication/conftest.pytests/unit/communication/meeting/test_config_frequency.pytests/unit/communication/meeting/test_frequency.pytests/unit/communication/meeting/test_participant.pytests/unit/communication/meeting/test_scheduler.pytests/unit/communication/test_config.pyweb/src/__tests__/stores/meetings.test.tsweb/src/__tests__/views/MeetingLogsPage.test.tsweb/src/api/endpoints/meetings.tsweb/src/api/types.tsweb/src/stores/meetings.tsweb/src/styles/theme.tsweb/src/views/MeetingLogsPage.vue
📜 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: Cleanup artifacts
- GitHub Check: Build Backend
- GitHub Check: Build Web
- GitHub Check: Greptile Review
- GitHub Check: Test (Python 3.14)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python 3.14+ required; use PEP 649 native lazy annotations (nofrom __future__ import annotations)
Use PEP 758 except syntax:except A, B:(no parentheses) — enforced by ruff on Python 3.14
Add type hints to all public functions; enforce mypy strict mode
Use Google style docstrings required on all public classes and functions — enforced by ruff D rules
Create new objects instead of mutating existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction and MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (model_copy(update=...)) for runtime state that evolves — never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict); use@computed_fieldfor derived values instead of storing redundant fields; use NotBlankStr for all identifier/name fields (including optional and tuple variants)
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (multiple tool invocations, parallel agent calls) — prefer structured concurrency over bare create_task; existing code is being migrated incrementally
Enforce 88 character line length — ruff enforces this
Functions must be less than 50 lines; files must be less than 800 lines
Handle errors explicitly, never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Files:
src/ai_company/api/channels.pysrc/ai_company/communication/meeting/__init__.pytests/unit/communication/meeting/test_frequency.pysrc/ai_company/api/app.pysrc/ai_company/communication/meeting/frequency.pysrc/ai_company/observability/events/meeting.pytests/integration/communication/test_meeting_integration.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/api/ws_models.pysrc/ai_company/communication/meeting/participant.pytests/unit/communication/conftest.pysrc/ai_company/api/state.pytests/unit/communication/meeting/test_participant.pysrc/ai_company/api/controllers/meetings.pytests/unit/communication/meeting/test_scheduler.pytests/unit/communication/meeting/test_config_frequency.pysrc/ai_company/communication/config.pytests/unit/api/test_channels.pytests/unit/api/controllers/test_meetings.pytests/unit/communication/test_config.pysrc/ai_company/communication/meeting/scheduler.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Every module with business logic must import get_logger from ai_company.observability and define logger = get_logger(name); never use import logging or logging.getLogger() or print() in application code
Always use logger variable name (not _logger or log)
Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Use structured kwargs in logging: logger.info(EVENT, key=value) — never 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 logging for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do NOT need logging
Maintain 80% minimum code coverage — enforced in CI
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names: example-provider, example-large-001, example-medium-001, example-small-001, or large/medium/small aliases
Lint Python code with: uv run ruff check src/ tests/
Auto-fix lint issues with: uv run ruff check src/ tests/ --fix
Format code with: uv run ruff format src/ tests/
Type-check with strict mode: uv run mypy src/ tests/
Files:
src/ai_company/api/channels.pysrc/ai_company/communication/meeting/__init__.pysrc/ai_company/api/app.pysrc/ai_company/communication/meeting/frequency.pysrc/ai_company/observability/events/meeting.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/api/ws_models.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/api/state.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/communication/config.pysrc/ai_company/communication/meeting/scheduler.py
src/ai_company/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/ai_company/**/*.py: Retryable errors (is_retryable=True) include: RateLimitError, ProviderTimeoutError, ProviderConnectionError, ProviderInternalError; non-retryable errors raise immediately without retry; RetryExhaustedError signals all retries failed
Rate limiter respects RateLimitError.retry_after from providers — automatically pauses future requests
Files:
src/ai_company/api/channels.pysrc/ai_company/communication/meeting/__init__.pysrc/ai_company/api/app.pysrc/ai_company/communication/meeting/frequency.pysrc/ai_company/observability/events/meeting.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/api/ws_models.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/api/state.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/communication/config.pysrc/ai_company/communication/meeting/scheduler.py
web/**
📄 CodeRabbit inference engine (CLAUDE.md)
web/**: Install frontend deps with: npm --prefix web install
Start web dashboard dev server with: npm --prefix web run dev (http://localhost:5173)
Build production web dashboard with: npm --prefix web run build
Lint web dashboard with: npm --prefix web run lint
Type-check web dashboard with: npm --prefix web run type-check
Test web dashboard with: npm --prefix web run test
Dashboard audit: npm audit (critical + high) runs per-PR via dashboard-audit job in ci.yml
Files:
web/src/__tests__/views/MeetingLogsPage.test.tsweb/src/styles/theme.tsweb/src/api/endpoints/meetings.tsweb/src/views/MeetingLogsPage.vueweb/src/__tests__/stores/meetings.test.tsweb/src/api/types.tsweb/src/stores/meetings.ts
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark tests with@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slow
Use pytest-xdist via -n auto for parallel test execution — ALWAYS include -n auto when running pytest, never run tests sequentially
Prefer@pytest.mark.parametrizefor testing similar cases
Use test-provider, test-small-001, etc. in tests instead of real vendor names
Run unit tests with: uv run pytest tests/ -m unit -n auto
Run integration tests with: uv run pytest tests/ -m integration -n auto
Run e2e tests with: uv run pytest tests/ -m e2e -n auto
Run full test suite with coverage: uv run pytest tests/ -n auto --cov=ai_company --cov-fail-under=80
Files:
tests/unit/communication/meeting/test_frequency.pytests/integration/communication/test_meeting_integration.pytests/unit/communication/conftest.pytests/unit/communication/meeting/test_participant.pytests/unit/communication/meeting/test_scheduler.pytests/unit/communication/meeting/test_config_frequency.pytests/unit/api/test_channels.pytests/unit/api/controllers/test_meetings.pytests/unit/communication/test_config.py
🧠 Learnings (10)
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Applied to files:
src/ai_company/api/app.pysrc/ai_company/observability/events/meeting.pyCLAUDE.mdsrc/ai_company/api/state.py
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to docs/** : Docs source in docs/ (Markdown, built with Zensical); design spec in docs/design/ (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to web/package.json : Web dashboard Node.js 20+; dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, ESLint, vue-tsc)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Every module with business logic must import get_logger from ai_company.observability and define logger = get_logger(__name__); never use import logging or logging.getLogger() or print() in application code
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Use structured kwargs in logging: logger.info(EVENT, key=value) — never logger.info('msg %s', val)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : All state transitions must log at INFO level
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Always use logger variable name (not _logger or log)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Use DEBUG logging for object creation, internal flow, and entry/exit of key functions
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Pure data models, enums, and re-exports do NOT need logging
Applied to files:
CLAUDE.md
🧬 Code graph analysis (18)
web/src/__tests__/views/MeetingLogsPage.test.ts (1)
web/src/stores/meetings.ts (1)
useMeetingStore(12-110)
web/src/styles/theme.ts (1)
web/src/api/types.ts (1)
MeetingStatus(492-498)
web/src/api/endpoints/meetings.ts (2)
web/src/api/types.ts (5)
MeetingFilters(570-575)PaginatedResponse(105-107)MeetingRecord(560-568)ApiResponse(94-96)TriggerMeetingRequest(577-580)web/src/api/client.ts (3)
apiClient(12-16)unwrapPaginated(68-84)unwrap(56-62)
src/ai_company/communication/meeting/__init__.py (4)
src/ai_company/communication/meeting/errors.py (3)
MeetingSchedulerError(30-31)NoParticipantsResolvedError(34-35)SchedulerAlreadyRunningError(38-39)src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)src/ai_company/communication/meeting/participant.py (2)
ParticipantResolver(20-39)RegistryParticipantResolver(42-142)src/ai_company/communication/meeting/scheduler.py (1)
MeetingScheduler(47-364)
tests/unit/communication/meeting/test_frequency.py (1)
src/ai_company/communication/meeting/frequency.py (2)
MeetingFrequency(16-31)frequency_to_seconds(48-57)
src/ai_company/api/app.py (3)
src/ai_company/communication/meeting/orchestrator.py (1)
MeetingOrchestrator(68-457)src/ai_company/communication/meeting/scheduler.py (1)
MeetingScheduler(47-364)src/ai_company/api/state.py (2)
meeting_orchestrator(149-154)meeting_scheduler(157-162)
tests/integration/communication/test_meeting_integration.py (1)
src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)
web/src/__tests__/stores/meetings.test.ts (2)
web/src/api/types.ts (2)
MeetingRecord(560-568)WsEvent(615-620)web/src/stores/meetings.ts (1)
useMeetingStore(12-110)
src/ai_company/communication/meeting/participant.py (1)
src/ai_company/hr/registry.py (3)
list_active(129-138)list_by_department(140-158)get_by_name(113-127)
tests/unit/communication/conftest.py (1)
src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)
src/ai_company/api/state.py (3)
tests/unit/communication/meeting/test_scheduler.py (1)
orchestrator(90-96)src/ai_company/communication/meeting/orchestrator.py (1)
MeetingOrchestrator(68-457)src/ai_company/communication/meeting/scheduler.py (1)
MeetingScheduler(47-364)
tests/unit/communication/meeting/test_participant.py (3)
src/ai_company/communication/meeting/participant.py (3)
RegistryParticipantResolver(42-142)resolve(23-39)resolve(61-102)src/ai_company/engine/parallel_models.py (1)
agent_id(79-81)src/ai_company/hr/registry.py (3)
list_active(129-138)list_by_department(140-158)get_by_name(113-127)
src/ai_company/api/controllers/meetings.py (5)
src/ai_company/api/dto.py (2)
ApiResponse(39-57)PaginatedResponse(76-96)src/ai_company/api/guards.py (1)
require_write_access(56-80)src/ai_company/api/pagination.py (1)
paginate(26-58)src/ai_company/api/state.py (2)
meeting_orchestrator(149-154)meeting_scheduler(157-162)src/ai_company/communication/meeting/scheduler.py (1)
trigger_event(143-176)
src/ai_company/communication/config.py (1)
src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)
tests/unit/api/controllers/test_meetings.py (3)
src/ai_company/api/app.py (1)
create_app(353-493)web/src/api/types.ts (5)
MeetingProtocolType(500-503)MeetingStatus(492-498)MeetingAgenda(511-515)MeetingMinutes(542-558)MeetingRecord(560-568)tests/unit/api/conftest.py (11)
FakePersistenceBackend(397-488)make_auth_headers(614-640)approval_store(676-677)auth_service(652-653)_seed_test_users(723-754)cost_tracker(671-672)get(60-61)get(223-224)get(294-295)get(322-323)get(383-384)
tests/unit/communication/test_config.py (2)
src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)src/ai_company/communication/config.py (2)
MeetingTypeConfig(83-137)MeetingsConfig(140-166)
src/ai_company/communication/meeting/scheduler.py (5)
src/ai_company/communication/meeting/errors.py (2)
NoParticipantsResolvedError(34-35)SchedulerAlreadyRunningError(38-39)src/ai_company/communication/meeting/frequency.py (1)
frequency_to_seconds(48-57)src/ai_company/communication/meeting/orchestrator.py (2)
MeetingOrchestrator(68-457)run_meeting(103-181)src/ai_company/communication/meeting/participant.py (3)
ParticipantResolver(20-39)resolve(23-39)resolve(61-102)src/ai_company/communication/config.py (2)
MeetingsConfig(140-166)MeetingTypeConfig(83-137)
web/src/stores/meetings.ts (3)
web/src/api/types.ts (4)
MeetingRecord(560-568)MeetingFilters(570-575)TriggerMeetingRequest(577-580)WsEvent(615-620)web/src/utils/errors.ts (1)
getErrorMessage(16-66)web/src/api/endpoints/meetings.ts (1)
triggerMeeting(24-29)
🪛 LanguageTool
CLAUDE.md
[style] ~154-~154: A comma is missing here.
Context: ...nder ai_company.observability.events (e.g. PROVIDER_CALL_START from `events.prov...
(EG_NO_COMMA)
🔇 Additional comments (36)
src/ai_company/api/channels.py (1)
16-26: LGTM!The new
CHANNEL_MEETINGSconstant and its inclusion inALL_CHANNELSfollows the established pattern for other channels. This correctly enables the WebSocket channel for meeting-related events.src/ai_company/communication/meeting/__init__.py (1)
28-31: LGTM!The public API surface is correctly expanded with the new scheduler components, error types, frequency enum, and participant resolver. The
__all__list maintains alphabetical ordering and all imports align with the relevant module implementations.Also applies to: 32-32, 43-46, 57-57, 75-75, 85-86, 88-89, 92-92, 95-95
src/ai_company/api/ws_models.py (1)
46-49: LGTM!The new
WsEventTypemembers (MEETING_STARTED,MEETING_COMPLETED,MEETING_FAILED) correctly extend the enum for meeting lifecycle events. The values align with the event format published byMeetingScheduler._publish_meeting_event.src/ai_company/communication/meeting/errors.py (1)
28-39: LGTM!The new exception hierarchy is well-structured:
MeetingSchedulerErroras the base for scheduler-specific errorsNoParticipantsResolvedErrorandSchedulerAlreadyRunningErroras specific error casesThe docstrings are concise and the inheritance chain is appropriate.
src/ai_company/observability/events/meeting.py (1)
47-55: LGTM!The new scheduler lifecycle event constants follow the established naming conventions and are properly typed with
Final[str]. The constants comprehensively cover the scheduler lifecycle events used byMeetingScheduler.tests/unit/communication/conftest.py (1)
31-31: LGTM!The import of
MeetingFrequencyand its usage insample_meeting_typefixture correctly aligns with the updated type signature ofMeetingTypeConfig.frequency.Also applies to: 175-176
src/ai_company/communication/config.py (1)
16-16: LGTM!The type change from
NotBlankStr | NonetoMeetingFrequency | Nonecorrectly enforces the enum constraint. SinceMeetingFrequencyis aStrEnum, Pydantic will handle coercion from YAML string values (e.g.,"daily"→MeetingFrequency.DAILY). Thenoqa: TC001comment is appropriate for the type-checking import.Also applies to: 100-103
tests/unit/api/test_channels.py (1)
10-10: LGTM!The test correctly verifies the new
CHANNEL_MEETINGSconstant is included inALL_CHANNELSand updates the expected count to 7.Also applies to: 27-30
tests/unit/communication/meeting/test_frequency.py (1)
1-56: LGTM!Comprehensive test coverage for
MeetingFrequencyenum parsing andfrequency_to_secondsconversion. Good use of@pytest.mark.parametrizefor all enum members and appropriate error case testing.tests/unit/communication/test_config.py (1)
21-21: LGTM!Clean migration from string literals to
MeetingFrequencyenum values. The updated error match on line 151 correctly reflects Pydantic's enum validation behavior (now validates against enum members rather than string constraints).Also applies to: 101-106, 150-152
tests/integration/communication/test_meeting_integration.py (1)
19-19: LGTM!Correctly updated to use
MeetingFrequency.DAILYenum value, aligning with the public API change toMeetingTypeConfig.frequency.Also applies to: 424-424
CLAUDE.md (1)
104-104: LGTM!Documentation correctly updated to reflect the new meeting scheduler functionality:
- Communication module description includes frequency and participant resolver
- Pinia stores list includes meetings store
- Views list includes MeetingLogsPage
- Event names section includes new scheduler events
Also applies to: 123-123, 126-126, 154-154
src/ai_company/api/state.py (1)
13-16: LGTM!Clean implementation following the established pattern for optional service accessors. The property accessors correctly use
_require_serviceto raise 503 when services are not configured, maintaining consistency with existing services likepersistenceandmessage_bus.Also applies to: 47-48, 68-69, 80-81, 148-162
web/src/styles/theme.ts (1)
3-5: LGTM!Correctly extends the
Statustype union to includeMeetingStatusand adds the two new status color mappings (scheduled,budget_exhausted) that aren't already covered by existing statuses. The color choices are visually appropriate.Also applies to: 60-61
web/src/__tests__/stores/meetings.test.ts (1)
40-117: Good realtime store regression coverage.The WS-path tests on Lines 40-117 cover update, append, malformed payload, and fetch-failure behavior well.
src/ai_company/communication/meeting/frequency.py (1)
16-57: Frequency enum and conversion mapping look solid.The enum/mapping pair is complete and
frequency_to_seconds()is clean and type-safe for scheduler usage.tests/unit/communication/meeting/test_participant.py (1)
26-144: Comprehensive branch coverage for participant resolution.This suite exercises the resolver precedence and dedup behavior thoroughly, including context and fallback paths.
web/src/api/endpoints/meetings.ts (1)
10-29: API endpoint wrappers are consistent and safe.Using shared unwrappers plus
encodeURIComponent()on Line 19 keeps behavior aligned and avoids path issues.tests/unit/communication/meeting/test_scheduler.py (1)
120-361: Scheduler tests cover core lifecycle and failure modes well.The suite validates startup/shutdown semantics, trigger filtering, participant resolution behavior, and best-effort publisher flow.
web/src/views/MeetingLogsPage.vue (5)
34-41: Status options are complete and match backend enum.The status options correctly mirror the
MeetingStatustype defined inweb/src/api/types.ts(lines 492-498), ensuring consistency between the UI and backend.
43-58: WebSocket and data fetching setup looks correct.The lifecycle hook properly:
- Connects WebSocket only if authenticated and not already connected
- Subscribes to the "meetings" channel
- Registers the event handler for real-time updates
- Fetches initial data separately with independent error handling
Good separation of concerns for WS setup vs. data fetching errors.
79-115: Trigger handler has comprehensive result handling.The function properly distinguishes between:
- Success with matches (shows count)
- Success with no matches (info toast)
- Error from store (shows store error message)
- Exception (logs sanitized error, shows generic toast)
The loading state management in the
finallyblock ensures proper cleanup.
117-122: Token formatting helper is clean and handles missing minutes.The fallback to
- / budgetwhen minutes are absent provides a clear visual indication of pending meetings.
259-283: Trigger dialog is accessible and functional.Good practices observed:
- Proper label association with
for="eventName"- Enter key binding for quick submission
- Disabled state when input is empty
- Loading state feedback on button
tests/unit/api/controllers/test_meetings.py (2)
127-148: List and filter tests look correct.The tests properly verify:
- List returns all records (2 items)
- Status filter returns matching record with correct status value
- Meeting type filter returns matching record with correct type name
Good coverage of the filtering functionality.
108-117: No action required —task_engineis not used by meeting endpoints.MeetingController accesses only
meeting_orchestratorandmeeting_schedulerfrom app state; it does not referencetask_engine. All test methods exercise only meeting endpoints, which will not return 503 due to the missingtask_engine. The missing parameter is intentional for this unit test's isolated scope.src/ai_company/api/controllers/meetings.py (2)
28-59: Well-designed request validation with bounded context.The
TriggerMeetingRequestmodel properly:
- Uses
NotBlankStrfor the event name (per coding guidelines)- Bounds context size to prevent abuse (max 20 keys, 256 char keys, 1024 char values)
- Uses frozen model config for immutability
Good defensive design.
141-174: Trigger endpoint is well-implemented.The endpoint:
- Uses
require_write_accessguard appropriately- Logs the trigger event with source attribution
- Returns records in a standard ApiResponse envelope
Good implementation.
web/src/api/types.ts (2)
490-568: Meeting types are comprehensive and well-structured.The types correctly mirror the backend models:
MeetingStatusmatches all 6 status valuesMeetingProtocolTypecovers all protocol variantsMeetingRecordincludes all fields including nullableminutesanderror_messageMeetingMinuteshas complete contribution and action item tracking
584-613: WebSocket types properly extended for meetings.The
WsChannelandWsEventTypeunions now include meeting-related values, enabling real-time updates for meeting lifecycle events.src/ai_company/communication/meeting/scheduler.py (6)
70-83: Clean initialization with proper slot usage.The
__slots__declaration is correct and matches all instance attributes. The optionalevent_publishercallback pattern allows flexible integration with WebSocket infrastructure.
113-127: Consider using TaskGroup for periodic task creation.Per coding guidelines, "Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code." While the current
create_taskapproach works, using a TaskGroup would provide structured concurrency benefits.However, since these tasks are long-running periodic loops (not fan-out/fan-in),
create_taskmay be more appropriate here. The tasks are manually cancelled instop().
129-141: Stop method handles cleanup correctly.The method:
- Early returns if not running
- Cancels all tasks
- Uses
gatherwithreturn_exceptions=Trueto await cancellation- Clears state
Good cleanup pattern.
194-232: Periodic loop has robust error handling.The
_run_periodicmethod:
- Validates that meeting_type has frequency (defensive)
- Sleeps first (avoids immediate execution on start)
- Catches and logs exceptions without terminating the loop
- Exits cleanly on
CancelledErrorGood resilience pattern for long-running background tasks.
333-364: Default agenda builder is straightforward.The static method creates a simple agenda from meeting type name and context. Context keys/values become agenda items, providing basic traceability for event-triggered meetings.
322-323: PEP 758 except syntax is correct.The
except MemoryError, RecursionError:syntax conforms to PEP 758 in Python 3.14+. This simply catches and re-raises the two critical exceptions without logging.> Likely an incorrect or invalid review comment.
| @get() | ||
| async def list_meetings( | ||
| self, | ||
| state: State, # noqa: ARG002 | ||
| state: State, | ||
| offset: PaginationOffset = 0, | ||
| limit: PaginationLimit = 50, | ||
| ) -> PaginatedResponse[object]: | ||
| """List meetings (empty — no repository yet). | ||
| status: MeetingStatus | None = None, | ||
| meeting_type: str | None = None, | ||
| ) -> PaginatedResponse[MeetingRecord]: | ||
| """List meeting records with optional filters. | ||
|
|
||
| Args: | ||
| state: Application state. | ||
| offset: Pagination offset. | ||
| limit: Page size. | ||
| status: Optional status filter. | ||
| meeting_type: Optional meeting type name filter. | ||
|
|
||
| Returns: | ||
| Empty paginated response. | ||
| Paginated meeting records. | ||
| """ | ||
| empty: tuple[object, ...] = () | ||
| page, meta = paginate(empty, offset=offset, limit=limit) | ||
| orchestrator = state.app_state.meeting_orchestrator | ||
| records = orchestrator.get_records() | ||
|
|
||
| if status is not None: | ||
| records = tuple(r for r in records if r.status == status) | ||
| if meeting_type is not None: | ||
| records = tuple(r for r in records if r.meeting_type_name == meeting_type) | ||
|
|
||
| page, meta = paginate(records, offset=offset, limit=limit) | ||
| return PaginatedResponse(data=page, pagination=meta) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
In-memory filtering may not scale.
The list_meetings endpoint fetches all records from the orchestrator and filters in-memory. For a large number of meetings, this could become a performance bottleneck. Consider pushing filters to the orchestrator's get_records method if meeting count grows significantly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/api/controllers/meetings.py` around lines 72 - 102, The
endpoint list_meetings currently loads all records via
orchestrator.get_records() and filters in-memory, which won't scale; update the
call to pass filter params (status and meeting_type/meeting_type_name) and
pagination args (offset, limit) into orchestrator.get_records so filtering and
slicing occur at the data layer, and then call paginate only if orchestrator
returns an unpaginated full list. Modify the MeetingOrchestrator.get_records
signature to accept optional status: MeetingStatus | None, meeting_type: str |
None, offset: int | PaginationOffset | None, limit: int | PaginationLimit | None
(keeping defaults for backward compatibility) and implement the efficient
query/filtering there; ensure list_meetings uses the new signature and still
returns PaginatedResponse(data=page, pagination=meta) when needed.
| it('meeting store initializes with empty state', () => { | ||
| const store = useMeetingStore() | ||
| expect(store.meetings).toEqual([]) | ||
| expect(store.selectedMeeting).toBeNull() | ||
| expect(store.loading).toBe(false) | ||
| expect(store.error).toBeNull() | ||
| expect(store.total).toBe(0) | ||
| }) | ||
|
|
||
| it('opens GitHub link in new tab', () => { | ||
| const wrapper = mount(MeetingLogsPage) | ||
| const link = wrapper.find('a[href*="github.com"]') | ||
| expect(link.attributes('target')).toBe('_blank') | ||
| expect(link.attributes('rel')).toContain('noopener') | ||
| it('meeting store has expected API methods', () => { | ||
| const store = useMeetingStore() | ||
| expect(typeof store.fetchMeetings).toBe('function') | ||
| expect(typeof store.fetchMeeting).toBe('function') | ||
| expect(typeof store.triggerMeeting).toBe('function') | ||
| expect(typeof store.handleWsEvent).toBe('function') | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding behavioral tests for store methods.
The current tests verify initial state and method existence but don't test actual behavior. Consider adding tests that:
- Call
fetchMeetings()and verifymeetingsandtotalare updated from the mock - Call
triggerMeeting()and verify new records are appended - Call
handleWsEvent()with different event types and verify refresh behavior
This would provide more confidence in the store's correctness.
🧪 Example behavioral test
it('fetchMeetings updates state from API response', async () => {
const mockData = [{ meeting_id: 'mtg-1', status: 'completed' }]
vi.mocked(listMeetings).mockResolvedValueOnce({ data: mockData, total: 1, offset: 0, limit: 50 })
const store = useMeetingStore()
await store.fetchMeetings()
expect(store.meetings).toEqual(mockData)
expect(store.total).toBe(1)
expect(store.loading).toBe(false)
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/__tests__/views/MeetingLogsPage.test.ts` around lines 16 - 31, Tests
currently only assert initial state and method existence for useMeetingStore;
add behavioral tests that invoke store methods and assert state changes.
Specifically, mock the API helpers (e.g., listMeetings) and write an async test
that calls fetchMeetings() and asserts meetings, total, and loading are updated;
mock any create/append API and call triggerMeeting() to assert new records are
appended to meetings; simulate different websocket payloads and call
handleWsEvent() to assert the store refresh/modify behavior. Use the store
factory useMeetingStore() to get a fresh store instance per test and ensure
mocked functions are reset between tests.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #397 +/- ##
==========================================
- Coverage 93.84% 93.68% -0.17%
==========================================
Files 463 469 +6
Lines 21691 22189 +498
Branches 2086 2138 +52
==========================================
+ Hits 20357 20787 +430
- Misses 1032 1090 +58
- Partials 302 312 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ai_company/api/controllers/meetings.py`:
- Around line 41-44: The context Field in meetings.py is currently typed as
dict[str, str], which prevents API callers from passing multi-value entries that
RegistryParticipantResolver._resolve_entry expects (it handles list/tuple values
for multi-participant resolution); change the type to allow list values (e.g.,
dict[str, str | list[str]]) so callers can supply arrays like {"reviewers":
["r-1","r-2"]}, and update _validate_context_bounds (and any other validation)
to correctly handle list inputs when present so resolution logic in
RegistryParticipantResolver._resolve_entry continues to work.
In `@src/ai_company/communication/meeting/scheduler.py`:
- Around line 234-302: The _execute_meeting method is over the 50-line limit and
mixes participant resolution, validation, agenda building, orchestrator
invocation, and event publication; refactor by extracting two small helpers —
e.g., _resolve_and_validate_participants(meeting_type, context) which wraps
self._resolver.resolve(...) and the min-count checks (raising or returning None
on failure), and _invoke_orchestrator_and_publish(record_args) which calls
self._orchestrator.run_meeting(...) and then
self._publish_meeting_event(record); update _execute_meeting to call these
helpers and keep error handling where appropriate around _resolver.resolve,
_orchestrator.run_meeting, _build_default_agenda, and _publish_meeting_event so
behavior and logging remain identical.
- Around line 304-322: The _publish_meeting_event method currently constructs
the WS event name by formatting record.status.value; replace that with an
explicit status-to-event constant mapping using the domain event constants from
ai_company.observability.events (import the appropriate meeting/event constants
directly), e.g., build a dict that maps MeetingStatus enums or record.status
values to the specific event constants (like the meeting terminal/failure and
success event names) and call self._event_publisher with the mapped constant and
payload; keep the existing payload shape and the try/except behavior but
raise/log nothing extra—use the mapping inside _publish_meeting_event and import
the referenced constants so only supported event names are emitted.
- Around line 161-176: The manual trigger path in scheduler.py currently ignores
the MeetingsConfig.enabled flag and still runs matching meetings; update the
trigger logic in the method that builds matching = tuple(mt for mt in
self._config.types if mt.trigger == event_name) to first check
self._config.enabled (the same guard that start() uses) and return an empty
tuple immediately when enabled is False, so that _execute_meeting is not
scheduled for any matching meeting types; ensure you reference
self._config.enabled, the matching variable, and the _execute_meeting call when
making the change.
In `@tests/unit/api/controllers/test_meetings.py`:
- Around line 184-199: The test test_trigger_endpoint_calls_mock_scheduler
currently only checks the HTTP response but doesn't verify the controller
invoked the scheduler; update the test to assert the scheduler was called by
adding an await-aware assertion on mock_scheduler.trigger_event (e.g., use
mock_scheduler.trigger_event.assert_awaited_once_with(...) or
assert_called_once_with(...) depending on whether trigger_event is async) with
the expected arguments from meeting_client.post (event_name "deploy_complete"),
ensuring the controller wiring is covered; reference the test name
test_trigger_endpoint_calls_mock_scheduler and the mock method
mock_scheduler.trigger_event when adding this assertion.
- Around line 255-260: Add a positive boundary test for exact max key and value
lengths: inside tests/unit/api/controllers/test_meetings.py add a test
(alongside test_accepts_boundary_context) that constructs a key string of length
256 and a value string of length 1024, uses them in the context dict passed to
TriggerMeetingRequest(event_name="evt", context={key: value}), then asserts the
model accepted the input (e.g., len(req.context) == 1 and req.context[key] ==
value) to lock in the exact-boundary acceptance for TriggerMeetingRequest.
In `@tests/unit/communication/meeting/test_config_frequency.py`:
- Around line 13-20: Add a Google-style docstring to the public test function
test_accepts_enum_value describing what the test verifies (that
MeetingTypeConfig accepts an enum value for the frequency field and that the
frequency is set to MeetingFrequency.DAILY); update the docstring directly above
the def test_accepts_enum_value(...) declaration and keep it concise, mentioning
the behavior under test (usage of MeetingTypeConfig and MeetingFrequency) so it
satisfies ruff D rules.
In `@web/src/__tests__/views/MeetingLogsPage.test.ts`:
- Around line 11-22: The test file MeetingLogsPage.test.ts only verifies the
store API surface (checks presence of fetchMeetings, fetchMeeting,
triggerMeeting, handleWsEvent) and should be renamed to reflect that intent;
rename the file to MeetingLogsPage.store-integration.test.ts (or similar) so its
purpose is clear, and update any test-suite references or CI patterns if they
depend on the old filename.
🪄 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: f4551dd5-0778-442f-be99-8947d9147dab
📒 Files selected for processing (19)
CLAUDE.mddocs/design/communication.mdsrc/ai_company/api/app.pysrc/ai_company/api/channels.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/communication/meeting/scheduler.pysrc/ai_company/observability/events/meeting.pytests/unit/api/controllers/test_meetings.pytests/unit/api/test_app.pytests/unit/communication/meeting/test_config_frequency.pytests/unit/communication/meeting/test_participant.pytests/unit/communication/meeting/test_scheduler.pyweb/src/__tests__/stores/meetings.test.tsweb/src/__tests__/views/MeetingLogsPage.test.tsweb/src/api/types.tsweb/src/stores/meetings.tsweb/src/views/MeetingLogsPage.vue
📜 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). (5)
- GitHub Check: Build Web
- GitHub Check: Build Backend
- GitHub Check: Greptile Review
- GitHub Check: Test (Python 3.14)
- GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (7)
web/**
📄 CodeRabbit inference engine (CLAUDE.md)
web/**: Install frontend deps with: npm --prefix web install
Start web dashboard dev server with: npm --prefix web run dev (http://localhost:5173)
Build production web dashboard with: npm --prefix web run build
Lint web dashboard with: npm --prefix web run lint
Type-check web dashboard with: npm --prefix web run type-check
Test web dashboard with: npm --prefix web run test
Dashboard audit: npm audit (critical + high) runs per-PR via dashboard-audit job in ci.yml
Files:
web/src/__tests__/views/MeetingLogsPage.test.tsweb/src/stores/meetings.tsweb/src/views/MeetingLogsPage.vueweb/src/__tests__/stores/meetings.test.tsweb/src/api/types.ts
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Python 3.14+ required; use PEP 649 native lazy annotations (nofrom __future__ import annotations)
Use PEP 758 except syntax:except A, B:(no parentheses) — enforced by ruff on Python 3.14
Add type hints to all public functions; enforce mypy strict mode
Use Google style docstrings required on all public classes and functions — enforced by ruff D rules
Create new objects instead of mutating existing ones; for non-Pydantic internal collections (registries, BaseTool), use copy.deepcopy() at construction and MappingProxyType wrapping for read-only enforcement
For dict/list fields in frozen Pydantic models, rely on frozen=True for field reassignment prevention and copy.deepcopy() at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, persistence serialization)
Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (model_copy(update=...)) for runtime state that evolves — never mix static config fields with mutable runtime fields in one model
Use Pydantic v2 (BaseModel, model_validator, computed_field, ConfigDict); use@computed_fieldfor derived values instead of storing redundant fields; use NotBlankStr for all identifier/name fields (including optional and tuple variants)
Prefer asyncio.TaskGroup for fan-out/fan-in parallel operations in new code (multiple tool invocations, parallel agent calls) — prefer structured concurrency over bare create_task; existing code is being migrated incrementally
Enforce 88 character line length — ruff enforces this
Functions must be less than 50 lines; files must be less than 800 lines
Handle errors explicitly, never silently swallow exceptions
Validate at system boundaries (user input, external APIs, config files)
Files:
tests/unit/communication/meeting/test_config_frequency.pysrc/ai_company/api/controllers/meetings.pytests/unit/api/controllers/test_meetings.pytests/unit/communication/meeting/test_scheduler.pysrc/ai_company/api/channels.pytests/unit/communication/meeting/test_participant.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/observability/events/meeting.pysrc/ai_company/communication/meeting/scheduler.pytests/unit/api/test_app.pysrc/ai_company/api/app.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Mark tests with@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e, and@pytest.mark.slow
Use pytest-xdist via -n auto for parallel test execution — ALWAYS include -n auto when running pytest, never run tests sequentially
Prefer@pytest.mark.parametrizefor testing similar cases
Use test-provider, test-small-001, etc. in tests instead of real vendor names
Run unit tests with: uv run pytest tests/ -m unit -n auto
Run integration tests with: uv run pytest tests/ -m integration -n auto
Run e2e tests with: uv run pytest tests/ -m e2e -n auto
Run full test suite with coverage: uv run pytest tests/ -n auto --cov=ai_company --cov-fail-under=80
Files:
tests/unit/communication/meeting/test_config_frequency.pytests/unit/api/controllers/test_meetings.pytests/unit/communication/meeting/test_scheduler.pytests/unit/communication/meeting/test_participant.pytests/unit/api/test_app.py
docs/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Build docs with: uv run zensical build (output: _site/docs/) — use without --strict until zensical/backlog#72 is resolved
Files:
docs/design/communication.md
docs/**
📄 CodeRabbit inference engine (CLAUDE.md)
Docs source in docs/ (Markdown, built with Zensical); design spec in docs/design/ (7 pages: index, agents, organization, communication, engine, memory, operations)
Files:
docs/design/communication.md
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Every module with business logic must import get_logger from ai_company.observability and define logger = get_logger(name); never use import logging or logging.getLogger() or print() in application code
Always use logger variable name (not _logger or log)
Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Use structured kwargs in logging: logger.info(EVENT, key=value) — never 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 logging for object creation, internal flow, and entry/exit of key functions
Pure data models, enums, and re-exports do NOT need logging
Maintain 80% minimum code coverage — enforced in CI
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples — use generic names: example-provider, example-large-001, example-medium-001, example-small-001, or large/medium/small aliases
Lint Python code with: uv run ruff check src/ tests/
Auto-fix lint issues with: uv run ruff check src/ tests/ --fix
Format code with: uv run ruff format src/ tests/
Type-check with strict mode: uv run mypy src/ tests/
Files:
src/ai_company/api/controllers/meetings.pysrc/ai_company/api/channels.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/observability/events/meeting.pysrc/ai_company/communication/meeting/scheduler.pysrc/ai_company/api/app.py
src/ai_company/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/ai_company/**/*.py: Retryable errors (is_retryable=True) include: RateLimitError, ProviderTimeoutError, ProviderConnectionError, ProviderInternalError; non-retryable errors raise immediately without retry; RetryExhaustedError signals all retries failed
Rate limiter respects RateLimitError.retry_after from providers — automatically pauses future requests
Files:
src/ai_company/api/controllers/meetings.pysrc/ai_company/api/channels.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/observability/events/meeting.pysrc/ai_company/communication/meeting/scheduler.pysrc/ai_company/api/app.py
🧠 Learnings (12)
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to docs/** : Docs source in docs/ (Markdown, built with Zensical); design spec in docs/design/ (7 pages: index, agents, organization, communication, engine, memory, operations)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to web/package.json : Web dashboard Node.js 20+; dependencies in web/package.json (Vue 3, PrimeVue, Tailwind CSS, Pinia, VueFlow, ECharts, Axios, vue-draggable-plus, Vitest, ESLint, vue-tsc)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Every module with business logic must import get_logger from ai_company.observability and define logger = get_logger(__name__); never use import logging or logging.getLogger() or print() in application code
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly
Applied to files:
CLAUDE.mdsrc/ai_company/observability/events/meeting.pysrc/ai_company/api/app.py
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Use structured kwargs in logging: logger.info(EVENT, key=value) — never logger.info('msg %s', val)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : All error paths must log at WARNING or ERROR with context before raising
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : All state transitions must log at INFO level
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Always use logger variable name (not _logger or log)
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Use DEBUG logging for object creation, internal flow, and entry/exit of key functions
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to src/**/*.py : Pure data models, enums, and re-exports do NOT need logging
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to **/*.py : Use PEP 758 except syntax: `except A, B:` (no parentheses) — enforced by ruff on Python 3.14
Applied to files:
src/ai_company/communication/meeting/scheduler.py
📚 Learning: 2026-03-14T11:20:53.699Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T11:20:53.699Z
Learning: Applies to **/*.py : Handle errors explicitly, never silently swallow exceptions
Applied to files:
src/ai_company/communication/meeting/scheduler.py
🧬 Code graph analysis (11)
web/src/__tests__/views/MeetingLogsPage.test.ts (1)
web/src/stores/meetings.ts (1)
useMeetingStore(12-114)
web/src/stores/meetings.ts (5)
web/src/api/types.ts (4)
MeetingRecord(560-568)MeetingFilters(570-575)TriggerMeetingRequest(577-580)WsEvent(615-620)web/src/utils/errors.ts (1)
getErrorMessage(16-66)web/src/api/endpoints/meetings.ts (1)
triggerMeeting(24-29)src/ai_company/api/controllers/meetings.py (1)
TriggerMeetingRequest(28-59)src/ai_company/api/ws_models.py (1)
WsEvent(51-76)
tests/unit/communication/meeting/test_config_frequency.py (2)
src/ai_company/communication/config.py (1)
MeetingTypeConfig(83-137)src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)
web/src/__tests__/stores/meetings.test.ts (2)
web/src/api/types.ts (2)
MeetingRecord(560-568)WsEvent(615-620)web/src/stores/meetings.ts (1)
useMeetingStore(12-114)
src/ai_company/api/controllers/meetings.py (5)
src/ai_company/api/dto.py (2)
ApiResponse(39-57)PaginatedResponse(76-96)src/ai_company/api/guards.py (2)
require_read_access(83-107)require_write_access(56-80)src/ai_company/api/pagination.py (1)
paginate(26-58)src/ai_company/api/state.py (2)
meeting_orchestrator(149-154)meeting_scheduler(157-162)src/ai_company/communication/meeting/scheduler.py (1)
trigger_event(144-176)
tests/unit/api/controllers/test_meetings.py (2)
src/ai_company/api/app.py (1)
create_app(390-531)src/ai_company/api/controllers/meetings.py (1)
TriggerMeetingRequest(28-59)
tests/unit/communication/meeting/test_participant.py (2)
src/ai_company/communication/meeting/participant.py (3)
RegistryParticipantResolver(46-148)resolve(24-43)resolve(65-108)src/ai_company/communication/meeting/errors.py (1)
NoParticipantsResolvedError(34-35)
web/src/api/types.ts (1)
web/src/styles/theme.ts (1)
Priority(6-6)
src/ai_company/communication/meeting/scheduler.py (4)
src/ai_company/communication/meeting/frequency.py (1)
frequency_to_seconds(48-57)src/ai_company/communication/meeting/orchestrator.py (1)
run_meeting(103-181)src/ai_company/communication/meeting/participant.py (2)
resolve(24-43)resolve(65-108)src/ai_company/communication/config.py (2)
MeetingsConfig(140-166)MeetingTypeConfig(83-137)
tests/unit/api/test_app.py (1)
src/ai_company/api/app.py (2)
_safe_startup(256-342)_safe_shutdown(345-387)
src/ai_company/api/app.py (5)
src/ai_company/communication/meeting/orchestrator.py (1)
MeetingOrchestrator(68-457)src/ai_company/communication/meeting/scheduler.py (3)
MeetingScheduler(47-356)stop(130-142)start(90-128)src/ai_company/api/state.py (6)
persistence(101-103)message_bus(106-108)task_engine(121-123)meeting_scheduler(157-162)AppState(27-189)meeting_orchestrator(149-154)src/ai_company/persistence/protocol.py (1)
PersistenceBackend(29-181)src/ai_company/api/bus_bridge.py (2)
stop(99-110)start(59-97)
🪛 LanguageTool
CLAUDE.md
[style] ~155-~155: A comma is missing here.
Context: ...nder ai_company.observability.events (e.g. PROVIDER_CALL_START from `events.prov...
(EG_NO_COMMA)
🔇 Additional comments (28)
tests/unit/communication/meeting/test_config_frequency.py (1)
21-70: Strong coverage of frequency/trigger invariants and coercion paths.Nice work covering enum input, string coercion (including
per_sprint_day), and both validation failure branches plus trigger-only behavior.CLAUDE.md (2)
104-106: LGTM!Documentation correctly updated to reflect the new meeting domain structure including the scheduler, participant resolver, and orchestrator components.
155-155: LGTM!Event constants for the meeting domain are comprehensively documented, covering scheduler lifecycle events (
MEETING_SCHEDULER_STARTED,MEETING_SCHEDULER_STOPPED,MEETING_SCHEDULER_ERROR), periodic/event triggers, participant resolution, and meeting lifecycle events.src/ai_company/communication/meeting/errors.py (1)
30-39: LGTM!Exception hierarchy is well-structured with clear separation of concerns:
MeetingSchedulerErroras the base for scheduler-specific errorsNoParticipantsResolvedErrorfor empty participant resolutionSchedulerAlreadyRunningErrorfor lifecycle violationsThis follows the established pattern of extending domain-specific base errors. Based on learnings, pure data models and exception classes do not need logging.
src/ai_company/api/channels.py (1)
12-28: LGTM!Channel constants appropriately use
Final[str]type annotations for immutability enforcement. The newCHANNEL_MEETINGSconstant follows the established naming convention and is correctly added toALL_CHANNELS.web/src/views/MeetingLogsPage.vue (3)
43-58: LGTM!WebSocket lifecycle management is properly handled with try-catch blocks for both connection setup and initial data fetch. The component correctly:
- Connects only if authenticated and not already connected
- Subscribes to the "meetings" channel
- Registers the event handler for real-time updates
- Fetches initial data independently of WS setup
79-115: LGTM!The
handleTriggerfunction has comprehensive error handling with appropriate user feedback via toast notifications for success, no-match, and error scenarios. The input validation (if (!triggerEventName.value.trim()) return) provides early exit for empty input.
150-180: LGTM!DataTable integration is well-implemented with:
ErrorBoundarywrapper for graceful error handlingLoadingSkeletonfor initial loading state- Proper column definitions with sorting where appropriate
- Row click handler for detail navigation
StatusBadgecomponent for consistent status renderingweb/src/api/types.ts (3)
577-580: LGTM!The
TriggerMeetingRequestinterface correctly definescontextasRecord<string, string>, aligning with the backenddict[str, str]type inTriggerMeetingRequest(seesrc/ai_company/api/controllers/meetings.py). The previous type mismatch has been resolved.
490-568: LGTM!Meeting domain types are comprehensive and well-structured:
MeetingStatus,MeetingProtocolType,MeetingPhaseunions cover all backend statesMeetingMinutesincludes all contribution tracking, token counts, and timestampsMeetingRecordproperly handles nullableminutesanderror_messagefields- Types mirror backend Pydantic DTOs accurately
591-613: LGTM!WebSocket types correctly extended:
WsChannelincludes'meetings'matchingCHANNEL_MEETINGSconstantWsEventTypeincludes'meeting.started','meeting.completed','meeting.failed'for lifecycle eventsweb/src/stores/meetings.ts (3)
63-82: LGTM!The
_refreshMeetingfunction correctly addresses the previously flagged staleness issue by syncingselectedMeetingwhen the refreshed meeting ID matches (lines 76-78). The implementation uses immutable update patterns via.map()and spread operators.
84-101: LGTM!The
handleWsEventfunction now includes the channel guard at line 85 (if (event.channel !== 'meetings') return), addressing the previously flagged cross-channel refresh concern. The switch statement correctly handles all three meeting lifecycle events.
45-61: LGTM!The
triggerMeetingfunction implements proper deduplication logic before appending new records, using immutable array updates. Error handling correctly setserror.valueand returnsnullon failure.web/src/__tests__/stores/meetings.test.ts (5)
27-42: LGTM!Test setup is well-structured with proper isolation via
beforeEachthat creates a fresh Pinia instance and resets all mock functions between tests.
44-74: LGTM!Test properly verifies the WS event handling flow:
- Sets up initial state with
in_progressmeeting- Mocks
getMeetingto return completed record- Dispatches
meeting.completedevent- Verifies
getMeetingwas called and state updatedUses
flushPromises()correctly to wait for async_refreshMeetingcall.
94-116: LGTM!Edge case tests are well-designed:
- Malformed payload (empty object) correctly ignores the event
- Channel guard test verifies events from other channels (
'tasks') don't triggergetMeetingThese tests ensure defensive handling in
handleWsEvent.
137-162: LGTM!API integration tests cover both success and error paths for
fetchMeetings:
- Success path verifies
meetings,total,loading, anderrorstate updates- Error path verifies
erroris set andloadingis reset tofalse
164-207: LGTM!The
triggerMeetingtests comprehensively cover:
- Deduplication: existing meeting not duplicated, new meeting appended
- Return value: returns array on success,
nullon error- Error handling: sets
errorstate on failureThe graceful failure test confirms WS re-fetch errors don't crash the store.
docs/design/communication.md (1)
373-407: LGTM! Well-documented Meeting Scheduler section.The documentation clearly explains the two triggering modes (frequency-based and event-triggered), participant resolution workflow with the five-step cascade, and integration details. This aligns well with the implementation in
scheduler.pyandparticipant.py.tests/unit/api/test_app.py (1)
90-90: LGTM! Test calls correctly updated for newmeeting_schedulerparameter.The test invocations of
_safe_startupand_safe_shutdownproperly include the additionalNoneargument to match the updated signatures that now acceptmeeting_scheduler.Also applies to: 108-108, 138-138, 154-154
tests/unit/communication/meeting/test_participant.py (1)
1-147: LGTM! Comprehensive test coverage for RegistryParticipantResolver.The tests thoroughly cover all resolution paths (context-based, "all", department, agent name, pass-through), deduplication behavior, and the
NoParticipantsResolvedErroredge case. The fixture setup with mocked registry methods is clean and the assertions are precise.tests/unit/communication/meeting/test_scheduler.py (1)
1-407: LGTM! Thorough test coverage for MeetingScheduler.The tests comprehensively cover:
- Lifecycle management (start/stop, already running, disabled state)
- Event triggering (matching, unknown events, context propagation)
- Participant resolution (leader selection, too few participants)
- Error handling (orchestrator errors, NoParticipantsResolvedError)
- Event publishing (success, error resilience, MemoryError reraising)
- Agenda building helpers
The
ExceptionGroupassertion on line 385 correctly reflectsasyncio.TaskGroupbehavior when aMemoryErrorpropagates.src/ai_company/communication/meeting/participant.py (1)
1-148: LGTM! Clean implementation of the participant resolution protocol.The resolution cascade is correctly implemented with proper handling of:
- Context-based lookups (with validated list filtering on line 130)
- Special "all" value
- Department and agent name lookups
- Pass-through for literal IDs
- Deduplication while preserving insertion order
The previous review concern about coercing arbitrary context list values has been addressed—line 130 now properly filters to only valid non-empty strings.
src/ai_company/observability/events/meeting.py (1)
47-58: LGTM! Well-organized event constants for scheduler and API operations.The new constants follow the established naming conventions and are properly categorized:
- Scheduler lifecycle events (
meeting.scheduler.*)- API-level events (
meeting.api.not_found)The addition of
MEETING_NOT_FOUNDaddresses the previous review feedback about using semantically correct event constants for 404 responses.src/ai_company/api/app.py (1)
36-39: LGTM! Meeting scheduler properly integrated into application lifecycle.The lifecycle management is correctly implemented:
- Startup: meeting_scheduler started after task_engine
- Shutdown: meeting_scheduler stopped first (before task_engine), correctly reflecting dependency order
- Cleanup on failure: properly cleans up meeting_scheduler in reverse order
The previous review concern about lifecycle management has been fully addressed—the scheduler is now properly started during
_safe_startupand stopped during_safe_shutdown.Also applies to: 108-144, 168-211, 256-342, 345-387, 390-472
src/ai_company/api/controllers/meetings.py (2)
72-102: In-memory filtering acknowledged.The in-memory filtering approach for
list_meetingsis functional for the current use case. The previous nitpick about scalability is valid for high-volume scenarios but acceptable as a starting point. Consider pushing filters toorchestrator.get_records()if meeting volume grows significantly.
104-138: LGTM! Proper implementation of get and trigger endpoints.The
get_meetingendpoint correctly:
- Returns 404 with
MEETING_NOT_FOUNDlogging (addressing previous review feedback)- Uses explicit
Responsewrapper with proper status codesThe
trigger_meetingendpoint correctly:
- Applies
require_write_accessguard- Logs the trigger event with source attribution
- Delegates to scheduler with context
Also applies to: 140-173
| def _publish_meeting_event(self, record: MeetingRecord) -> None: | ||
| """Publish a WebSocket event for a meeting result. | ||
|
|
||
| Best-effort: publish errors are logged and swallowed. | ||
|
|
||
| Args: | ||
| record: The completed meeting record. | ||
| """ | ||
| if self._event_publisher is None: | ||
| return | ||
| try: | ||
| self._event_publisher( | ||
| f"meeting.{record.status.value}", | ||
| { | ||
| "meeting_id": record.meeting_id, | ||
| "meeting_type": record.meeting_type_name, | ||
| "status": record.status.value, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Map records to the supported WS event names instead of formatting them.
Line 316 derives the published event from record.status.value. That can emit names outside the advertised contract, such as meeting.budget_exhausted, so the frontend listener will miss those terminal failures. Use the dedicated meeting event constants and an explicit status-to-event mapping here. As per coding guidelines, "Use event name constants from domain-specific modules under ai_company.observability.events (e.g., PROVIDER_CALL_START from events.provider, BUDGET_RECORD_ADDED from events.budget, etc.) — import directly".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/ai_company/communication/meeting/scheduler.py` around lines 304 - 322,
The _publish_meeting_event method currently constructs the WS event name by
formatting record.status.value; replace that with an explicit status-to-event
constant mapping using the domain event constants from
ai_company.observability.events (import the appropriate meeting/event constants
directly), e.g., build a dict that maps MeetingStatus enums or record.status
values to the specific event constants (like the meeting terminal/failure and
success event names) and call self._event_publisher with the mapped constant and
payload; keep the existing payload shape and the try/except behavior but
raise/log nothing extra—use the mapping inside _publish_meeting_event and import
the referenced constants so only supported event names are emitted.
| describe('MeetingLogsPage store integration', () => { | ||
| beforeEach(() => { | ||
| setActivePinia(createPinia()) | ||
| }) | ||
|
|
||
| it('opens GitHub link in new tab', () => { | ||
| const wrapper = mount(MeetingLogsPage) | ||
| const link = wrapper.find('a[href*="github.com"]') | ||
| expect(link.attributes('target')).toBe('_blank') | ||
| expect(link.attributes('rel')).toContain('noopener') | ||
| it('meeting store has expected API methods', () => { | ||
| const store = useMeetingStore() | ||
| expect(typeof store.fetchMeetings).toBe('function') | ||
| expect(typeof store.fetchMeeting).toBe('function') | ||
| expect(typeof store.triggerMeeting).toBe('function') | ||
| expect(typeof store.handleWsEvent).toBe('function') | ||
| }) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Test coverage is minimal but complements comprehensive tests elsewhere.
This test file verifies the store API surface existence. The behavioral tests (fetchMeetings updates state, triggerMeeting deduplicates, handleWsEvent triggers refresh, etc.) are implemented in web/src/__tests__/stores/meetings.test.ts. The previous suggestion to add behavioral tests has been addressed in that file.
Consider renaming this file to better reflect its purpose (e.g., MeetingLogsPage.store-integration.test.ts) since it tests store API availability rather than the Vue component.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/__tests__/views/MeetingLogsPage.test.ts` around lines 11 - 22, The
test file MeetingLogsPage.test.ts only verifies the store API surface (checks
presence of fetchMeetings, fetchMeeting, triggerMeeting, handleWsEvent) and
should be renamed to reflect that intent; rename the file to
MeetingLogsPage.store-integration.test.ts (or similar) so its purpose is clear,
and update any test-suite references or CI patterns if they depend on the old
filename.
e4102d8 to
c84cdb9
Compare
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
This PR implements a meeting scheduler service with both frequency-based periodic scheduling and event-triggered meetings, along with REST API endpoints, WebSocket integration, and a frontend meeting logs page. It closes #264.
Changes:
- Added
MeetingSchedulerservice with periodic asyncio tasks and event triggering,MeetingFrequencyenum replacingNotBlankStr, andParticipantResolverprotocol with registry-based implementation - Added REST API endpoints (
GET /meetings,GET /meetings/{id},POST /meetings/trigger) with WebSocketmeetingschannel and meeting event types - Replaced the placeholder
MeetingLogsPagewith a full implementation featuring DataTable, detail sidebar, trigger dialog, and Pinia store with WebSocket re-fetch
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/ai_company/communication/meeting/scheduler.py |
New meeting scheduler with periodic + event-triggered execution |
src/ai_company/communication/meeting/participant.py |
New participant resolver protocol and registry implementation |
src/ai_company/communication/meeting/frequency.py |
New MeetingFrequency enum and interval conversion |
src/ai_company/communication/meeting/errors.py |
New scheduler error classes |
src/ai_company/communication/meeting/__init__.py |
Re-exports for new modules |
src/ai_company/communication/config.py |
Changed frequency field type to MeetingFrequency enum |
src/ai_company/api/controllers/meetings.py |
Full meeting controller replacing stub |
src/ai_company/api/app.py |
Lifecycle integration for meeting scheduler |
src/ai_company/api/state.py |
AppState with meeting orchestrator/scheduler |
src/ai_company/api/channels.py |
Added meetings channel |
src/ai_company/api/ws_models.py |
Added meeting WS event types |
src/ai_company/observability/events/meeting.py |
New scheduler/API event constants |
web/src/views/MeetingLogsPage.vue |
Full meeting logs UI |
web/src/stores/meetings.ts |
Pinia store for meetings |
web/src/api/endpoints/meetings.ts |
API client for meetings |
web/src/api/types.ts |
Meeting TypeScript types |
web/src/styles/theme.ts |
Status colors for meeting statuses |
tests/unit/communication/meeting/test_scheduler.py |
Scheduler tests |
tests/unit/communication/meeting/test_participant.py |
Participant resolver tests |
tests/unit/communication/meeting/test_frequency.py |
Frequency enum tests |
tests/unit/communication/meeting/test_config_frequency.py |
Config frequency coercion tests |
tests/unit/api/controllers/test_meetings.py |
Controller endpoint tests |
web/src/__tests__/stores/meetings.test.ts |
Store tests |
| Various test/config files | Updated for MeetingFrequency enum and new parameters |
docs/design/communication.md |
Documentation for meeting scheduler |
CLAUDE.md |
Updated project structure docs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/api/test_channels.py (1)
20-27: 🧹 Nitpick | 🔵 TrivialParameterize channel-membership assertions.
Line 27 extends an existing repeated-assert pattern. These are similar cases and should be a single parametrized test for maintainability.
♻️ Proposed refactor
`@pytest.mark.unit` class TestChannels: - def test_all_channels_contains_expected(self) -> None: - assert CHANNEL_TASKS in ALL_CHANNELS - assert CHANNEL_AGENTS in ALL_CHANNELS - assert CHANNEL_BUDGET in ALL_CHANNELS - assert CHANNEL_MESSAGES in ALL_CHANNELS - assert CHANNEL_SYSTEM in ALL_CHANNELS - assert CHANNEL_APPROVALS in ALL_CHANNELS - assert CHANNEL_MEETINGS in ALL_CHANNELS + `@pytest.mark.parametrize`( + "channel", + [ + CHANNEL_TASKS, + CHANNEL_AGENTS, + CHANNEL_BUDGET, + CHANNEL_MESSAGES, + CHANNEL_SYSTEM, + CHANNEL_APPROVALS, + CHANNEL_MEETINGS, + ], + ) + def test_all_channels_contains_expected(self, channel: str) -> None: + assert channel in ALL_CHANNELSAs per coding guidelines, "Use
@pytest.mark.parametrizefor testing similar cases."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/api/test_channels.py` around lines 20 - 27, The test_all_channels_contains_expected has repeated assert statements for each CHANNEL_* constant; replace it with a single parametrized test using pytest.mark.parametrize to iterate over the channel constants (CHANNEL_TASKS, CHANNEL_AGENTS, CHANNEL_BUDGET, CHANNEL_MESSAGES, CHANNEL_SYSTEM, CHANNEL_APPROVALS, CHANNEL_MEETINGS) and assert that each parameter is in ALL_CHANNELS; ensure pytest is imported and keep the test function name (test_all_channels_contains_expected) or rename consistently, and remove the individual asserts so the test body contains only the single parameterized assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Line 155: The sentence under "**Event names**" that begins "always use
constants ... (e.g. `PROVIDER_CALL_START` from `events.provider`, ...)" is
missing a comma after "e.g."; update that occurrence to "e.g.," (so the text
reads "(e.g., `PROVIDER_CALL_START` from `events.provider`, ...)") to satisfy
the style check and keep consistency across the example list.
In `@src/ai_company/communication/meeting/participant.py`:
- Around line 146-157: Add a DEBUG log before the final fallback return so we
record when an entry is passed through as a literal ID; in the method that
checks context/"all"/department/agent (the block that calls
self._registry.list_by_department and self._registry.get_by_name), insert a
debug call (e.g. self._logger.debug or the class's existing logger) immediately
before the line that returns [entry], logging the entry value and a short
message like "falling back to literal participant ID". This uses the existing
_registry, list_by_department, and get_by_name flow and ensures the log is
emitted only on that fallback path.
In `@tests/unit/api/controllers/test_meetings.py`:
- Around line 250-257: Add a unit test that asserts oversized list items in
context are rejected by the TriggerMeetingRequest validation: create a new test
method (e.g., test_rejects_oversized_context_list_item) that imports
TriggerMeetingRequest and calls it with event_name="evt" and context={"key":
["valid", "v" * 1025]}, expecting a ValueError with a message matching "list
item must be at most"; this ensures the _validate_context_bounds validator that
handles list[str] enforces the item length limit.
In `@tests/unit/api/test_app.py`:
- Line 90: Add at least one test case that passes a non-None scheduler to the
lifecycle helpers: instantiate a mock/fake meeting_scheduler (e.g.,
unittest.mock.Mock with start and stop callables), call
_safe_startup(persistence, bus, scheduler, None, None, app_state) and assert
meeting_scheduler.start was awaited/called, then call _safe_shutdown(...,
scheduler, ...) and assert meeting_scheduler.stop was awaited/called; update the
tests that currently call _safe_startup/_safe_shutdown with None (the cases
around the existing calls) to include this non-None scheduler case so the new
lifecycle path through meeting_scheduler.start/stop is exercised.
In `@tests/unit/communication/meeting/test_config_frequency.py`:
- Around line 45-58: The tests test_exactly_one_of_frequency_or_trigger and
test_neither_set_raises are catching ValueError but MeetingTypeConfig is a
Pydantic model that raises pydantic.ValidationError; update both
pytest.raises(...) calls to expect ValidationError instead and add/import
pydantic.ValidationError at the top of the test module so the assertions
correctly catch the Pydantic validation exceptions when constructing
MeetingTypeConfig.
In `@tests/unit/communication/meeting/test_participant.py`:
- Around line 97-108: Add a new async test that mirrors test_context_list_value
but passes a tuple instead of a list to resolver.resolve to cover the tuple
branch in participant.py (the logic around tuple/list handling at the code
handling context values around line with participant.py:129). Name it
test_context_tuple_value, use the same fixtures (RegistryParticipantResolver
resolver, MagicMock registry), call resolver.resolve(("reviewers",),
context={"reviewers": ("r-1","r-2")}) and assert the result equals ("r-1",
"r-2") so the tuple path is exercised.
In `@web/src/stores/meetings.ts`:
- Around line 50-55: The loop in triggerMeeting currently creates a new
meetings.value array per record; instead, build a single newRecords array by
filtering records for items whose meeting_id is not already in meetings.value,
then if newRecords.length > 0 set meetings.value = [...meetings.value,
...newRecords] and increment total.value by newRecords.length; refer to the
variables and identifiers meetings.value, total.value, records and the
triggerMeeting function to locate and replace the per-record spread update with
this batched update.
---
Outside diff comments:
In `@tests/unit/api/test_channels.py`:
- Around line 20-27: The test_all_channels_contains_expected has repeated assert
statements for each CHANNEL_* constant; replace it with a single parametrized
test using pytest.mark.parametrize to iterate over the channel constants
(CHANNEL_TASKS, CHANNEL_AGENTS, CHANNEL_BUDGET, CHANNEL_MESSAGES,
CHANNEL_SYSTEM, CHANNEL_APPROVALS, CHANNEL_MEETINGS) and assert that each
parameter is in ALL_CHANNELS; ensure pytest is imported and keep the test
function name (test_all_channels_contains_expected) or rename consistently, and
remove the individual asserts so the test body contains only the single
parameterized assertion.
🪄 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: c99cb273-c277-4459-a0e4-644bc4fa5ed7
📒 Files selected for processing (32)
CLAUDE.mddocs/design/communication.mdsrc/ai_company/api/app.pysrc/ai_company/api/channels.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/api/state.pysrc/ai_company/api/ws_models.pysrc/ai_company/communication/config.pysrc/ai_company/communication/meeting/__init__.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/communication/meeting/frequency.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/communication/meeting/scheduler.pysrc/ai_company/observability/events/meeting.pytests/integration/communication/test_meeting_integration.pytests/unit/api/controllers/test_meetings.pytests/unit/api/test_app.pytests/unit/api/test_channels.pytests/unit/communication/conftest.pytests/unit/communication/meeting/test_config_frequency.pytests/unit/communication/meeting/test_frequency.pytests/unit/communication/meeting/test_participant.pytests/unit/communication/meeting/test_scheduler.pytests/unit/communication/test_config.pyweb/src/__tests__/stores/meetings.test.tsweb/src/__tests__/views/MeetingLogsPage.store-integration.test.tsweb/src/__tests__/views/MeetingLogsPage.test.tsweb/src/api/endpoints/meetings.tsweb/src/api/types.tsweb/src/stores/meetings.tsweb/src/styles/theme.tsweb/src/views/MeetingLogsPage.vue
💤 Files with no reviewable changes (1)
- web/src/tests/views/MeetingLogsPage.test.ts
📜 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: Build Backend
- GitHub Check: Build Web
- GitHub Check: Greptile Review
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (6)
web/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript for Vue 3 web dashboard components. Enforce type checking via vue-tsc.
Files:
web/src/__tests__/stores/meetings.test.tsweb/src/views/MeetingLogsPage.vueweb/src/api/endpoints/meetings.tsweb/src/__tests__/views/MeetingLogsPage.store-integration.test.tsweb/src/stores/meetings.tsweb/src/api/types.tsweb/src/styles/theme.ts
web/src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
Use PrimeVue components in Vue 3 dashboard with Tailwind CSS styling.
Files:
web/src/views/MeetingLogsPage.vue
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do not usefrom __future__ import annotationsin Python code—Python 3.14 has PEP 649 native lazy annotations.
Use PEP 758 except syntax withexcept A, B:(no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Add type hints to all public functions. Enforce strict mypy mode.
Maintain line length of 88 characters—enforced by ruff.
Handle errors explicitly—never silently swallow exceptions.
Files:
tests/unit/communication/meeting/test_scheduler.pysrc/ai_company/api/ws_models.pysrc/ai_company/observability/events/meeting.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/api/state.pytests/unit/api/test_channels.pysrc/ai_company/communication/meeting/errors.pytests/unit/communication/meeting/test_config_frequency.pysrc/ai_company/api/app.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/api/channels.pytests/unit/communication/meeting/test_frequency.pytests/unit/communication/meeting/test_participant.pysrc/ai_company/communication/meeting/frequency.pysrc/ai_company/communication/config.pysrc/ai_company/communication/meeting/scheduler.pysrc/ai_company/communication/meeting/__init__.pytests/unit/api/controllers/test_meetings.pytests/unit/communication/conftest.pytests/unit/api/test_app.pytests/integration/communication/test_meeting_integration.pytests/unit/communication/test_config.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Apply pytest markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowto categorize tests.
Maintain 80% minimum code coverage—enforced in CI. Use@pytest.mark.parametrizefor testing similar cases.
Files:
tests/unit/communication/meeting/test_scheduler.pytests/unit/api/test_channels.pytests/unit/communication/meeting/test_config_frequency.pytests/unit/communication/meeting/test_frequency.pytests/unit/communication/meeting/test_participant.pytests/unit/api/controllers/test_meetings.pytests/unit/communication/conftest.pytests/unit/api/test_app.pytests/integration/communication/test_meeting_integration.pytests/unit/communication/test_config.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Use Google-style docstrings on all public classes and functions—required and enforced by ruff D rules.
Immutability: Create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Distinguish between config and runtime state: Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 conventions:BaseModel,model_validator,computed_field,ConfigDict. Use@computed_fieldfor derived values instead of storing redundant fields. UseNotBlankStr(fromcore.types) for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task. Existing code is being migrated incrementally.
Keep functions under 50 lines and files under 800 lines.
Validate at system boundaries: user input, external APIs, config files.
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. In tests, usetest-provider,test-small-001, etc. Vendor names may only appear in: (1) Operations design page provider list, (2).claude/skill/agent files, (3) thir...
Files:
src/ai_company/api/ws_models.pysrc/ai_company/observability/events/meeting.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/api/state.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/api/app.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/api/channels.pysrc/ai_company/communication/meeting/frequency.pysrc/ai_company/communication/config.pysrc/ai_company/communication/meeting/scheduler.pysrc/ai_company/communication/meeting/__init__.py
src/ai_company/!(observability)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/ai_company/!(observability)/**/*.py: Every module with business logic must includefrom ai_company.observability import get_loggerand setlogger = get_logger(__name__). Never useimport logging,logging.getLogger(), orprint()in application code. Use the variable namelogger(not_loggerorlog).
Use event name constants from domain-specific modules underai_company.observability.events(e.g.,PROVIDER_CALL_STARTfromevents.provider). Import directly:from ai_company.observability.events.<domain> import EVENT_CONSTANT.
Use structured logging kwargs:logger.info(EVENT, key=value)—never use format strings likelogger.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 logging for object creation, internal flow, and entry/exit of key functions.
Files:
src/ai_company/api/ws_models.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/api/state.pysrc/ai_company/communication/meeting/errors.pysrc/ai_company/api/app.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/api/channels.pysrc/ai_company/communication/meeting/frequency.pysrc/ai_company/communication/config.pysrc/ai_company/communication/meeting/scheduler.pysrc/ai_company/communication/meeting/__init__.py
🧠 Learnings (10)
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/ai_company/observability/events/meeting.pysrc/ai_company/api/state.pysrc/ai_company/api/app.pysrc/ai_company/api/controllers/meetings.pysrc/ai_company/communication/meeting/scheduler.pyCLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/ai_company/communication/meeting/participant.pyCLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/ai_company/communication/meeting/scheduler.py
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/ai_company/communication/meeting/scheduler.pyCLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/**/*.py : Keep functions under 50 lines and files under 800 lines.
Applied to files:
src/ai_company/communication/meeting/scheduler.py
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Organize web dashboard code by feature: `api/` (Axios client + types), `components/` (organized by feature), `composables/` (reusable functions), `router/`, `stores/` (Pinia), `styles/`, `utils/`, `views/` (pages).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Every module with business logic must include `from ai_company.observability import get_logger` and set `logger = get_logger(__name__)`. Never use `import logging`, `logging.getLogger()`, or `print()` in application code. Use the variable name `logger` (not `_logger` or `log`).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use structured logging kwargs: `logger.info(EVENT, key=value)`—never use format strings like `logger.info("msg %s", val)`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All state transitions must log at INFO level.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use DEBUG logging for object creation, internal flow, and entry/exit of key functions.
Applied to files:
CLAUDE.md
🧬 Code graph analysis (20)
web/src/__tests__/stores/meetings.test.ts (2)
web/src/api/types.ts (2)
MeetingRecord(560-568)WsEvent(615-620)web/src/stores/meetings.ts (1)
useMeetingStore(12-114)
web/src/api/endpoints/meetings.ts (2)
web/src/api/types.ts (5)
MeetingFilters(570-575)PaginatedResponse(105-107)MeetingRecord(560-568)ApiResponse(94-96)TriggerMeetingRequest(577-580)web/src/api/client.ts (3)
apiClient(12-16)unwrapPaginated(68-84)unwrap(56-62)
web/src/__tests__/views/MeetingLogsPage.store-integration.test.ts (1)
web/src/stores/meetings.ts (1)
useMeetingStore(12-114)
src/ai_company/communication/meeting/participant.py (2)
src/ai_company/communication/meeting/errors.py (1)
NoParticipantsResolvedError(34-35)src/ai_company/hr/registry.py (4)
AgentRegistryService(32-262)list_active(129-138)list_by_department(140-158)get_by_name(113-127)
src/ai_company/api/state.py (2)
src/ai_company/communication/meeting/orchestrator.py (1)
MeetingOrchestrator(68-457)src/ai_company/communication/meeting/scheduler.py (1)
MeetingScheduler(56-416)
web/src/stores/meetings.ts (3)
web/src/api/types.ts (4)
MeetingRecord(560-568)MeetingFilters(570-575)TriggerMeetingRequest(577-580)WsEvent(615-620)web/src/utils/errors.ts (1)
getErrorMessage(16-66)web/src/api/endpoints/meetings.ts (1)
triggerMeeting(24-29)
tests/unit/communication/meeting/test_config_frequency.py (2)
src/ai_company/communication/config.py (1)
MeetingTypeConfig(83-137)src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)
src/ai_company/api/app.py (4)
src/ai_company/communication/meeting/orchestrator.py (1)
MeetingOrchestrator(68-457)src/ai_company/api/state.py (4)
persistence(101-103)message_bus(106-108)task_engine(121-123)meeting_orchestrator(149-154)src/ai_company/persistence/protocol.py (1)
PersistenceBackend(29-181)src/ai_company/communication/bus_protocol.py (1)
MessageBus(20-209)
tests/unit/communication/meeting/test_frequency.py (1)
src/ai_company/communication/meeting/frequency.py (2)
MeetingFrequency(16-31)frequency_to_seconds(48-57)
web/src/api/types.ts (2)
web/src/styles/theme.ts (1)
Priority(6-6)src/ai_company/api/controllers/meetings.py (1)
TriggerMeetingRequest(28-67)
tests/unit/communication/meeting/test_participant.py (4)
src/ai_company/communication/meeting/participant.py (3)
RegistryParticipantResolver(46-157)resolve(24-43)resolve(65-108)src/ai_company/engine/parallel_models.py (1)
agent_id(79-81)src/ai_company/hr/registry.py (3)
list_active(129-138)list_by_department(140-158)get_by_name(113-127)src/ai_company/communication/meeting/errors.py (1)
NoParticipantsResolvedError(34-35)
src/ai_company/communication/config.py (1)
src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)
src/ai_company/communication/meeting/scheduler.py (5)
src/ai_company/communication/meeting/errors.py (2)
NoParticipantsResolvedError(34-35)SchedulerAlreadyRunningError(38-39)src/ai_company/communication/meeting/frequency.py (1)
frequency_to_seconds(48-57)src/ai_company/communication/meeting/orchestrator.py (1)
run_meeting(103-181)src/ai_company/communication/meeting/participant.py (3)
ParticipantResolver(21-43)resolve(24-43)resolve(65-108)src/ai_company/communication/config.py (2)
MeetingsConfig(140-166)MeetingTypeConfig(83-137)
src/ai_company/communication/meeting/__init__.py (4)
src/ai_company/communication/meeting/errors.py (3)
MeetingSchedulerError(30-31)NoParticipantsResolvedError(34-35)SchedulerAlreadyRunningError(38-39)src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)src/ai_company/communication/meeting/participant.py (2)
ParticipantResolver(21-43)RegistryParticipantResolver(46-157)src/ai_company/communication/meeting/scheduler.py (1)
MeetingScheduler(56-416)
web/src/styles/theme.ts (1)
web/src/api/types.ts (4)
TaskStatus(5-14)ApprovalStatus(28-28)AgentStatus(42-42)MeetingStatus(492-498)
tests/unit/api/controllers/test_meetings.py (1)
src/ai_company/api/controllers/meetings.py (1)
TriggerMeetingRequest(28-67)
tests/unit/communication/conftest.py (1)
src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)
tests/unit/api/test_app.py (3)
src/ai_company/api/state.py (1)
persistence(101-103)tests/unit/engine/conftest.py (1)
persistence(431-433)tests/unit/engine/test_task_engine_apply.py (1)
persistence(27-28)
tests/integration/communication/test_meeting_integration.py (1)
src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)
tests/unit/communication/test_config.py (2)
src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)src/ai_company/communication/config.py (2)
MeetingTypeConfig(83-137)MeetingsConfig(140-166)
🪛 LanguageTool
CLAUDE.md
[style] ~155-~155: A comma is missing here.
Context: ...nder ai_company.observability.events (e.g. PROVIDER_CALL_START from `events.prov...
(EG_NO_COMMA)
🔇 Additional comments (33)
src/ai_company/api/channels.py (1)
12-28: Consistent channel extension for meetings.
CHANNEL_MEETINGSis correctly added and included inALL_CHANNELS, keeping channel registration centralized and consistent.src/ai_company/api/ws_models.py (1)
46-49: LGTM!The new WebSocket event types for meeting lifecycle events are well-structured and follow the existing naming conventions. The values align with the observability event constants and the frontend WS event handling.
docs/design/communication.md (1)
373-408: LGTM!The Meeting Scheduler documentation clearly explains the two triggering modes, participant resolution cascade, and deduplication behavior. This aligns well with the implementation in
scheduler.pyandparticipant.py.src/ai_company/communication/meeting/errors.py (1)
28-39: LGTM!The new exception hierarchy is well-structured with
MeetingSchedulerErroras the base for scheduler-specific errors. The docstrings are concise and clearly describe each exception's purpose.src/ai_company/api/state.py (1)
13-16: LGTM!The
MeetingOrchestratorandMeetingSchedulerintegrations follow the established pattern for optional services with 503 fallback via_require_service. The slots, constructor parameters, and property accessors are consistent with existing service handling.Also applies to: 47-48, 68-69, 80-81, 148-162
src/ai_company/communication/meeting/__init__.py (1)
28-32: LGTM!The new public API surface is properly exported:
MeetingFrequency,MeetingScheduler, participant resolution types, and scheduler-related errors. The__all__list maintains alphabetical order consistent with the existing entries.Also applies to: 43-46, 57-57, 75-75, 85-89, 92-92, 95-95
src/ai_company/api/app.py (1)
36-39: LGTM!The meeting scheduler lifecycle management is properly integrated:
- Startup order: persistence → bus → bridge → task_engine → meeting_scheduler
- Shutdown order: meeting_scheduler → task_engine → bridge → bus → persistence
- Cleanup on failure mirrors the shutdown order
The implementation addresses the previously flagged issue about missing lifecycle management. Error handling follows the established patterns with
logger.exceptionand_try_stophelper.Also applies to: 108-146, 168-211, 256-342, 345-387, 390-473
src/ai_company/observability/events/meeting.py (1)
47-58: LGTM!The new observability event constants follow the established naming convention (
meeting.scheduler.*andmeeting.api.*). The scheduler lifecycle events provide comprehensive coverage for start/stop, periodic triggers, event triggers, participant resolution, and error conditions.web/src/views/MeetingLogsPage.vue (2)
1-123: LGTM!The script setup is well-structured with proper TypeScript types, lifecycle management for WebSocket subscriptions (subscribe in
onMounted, unsubscribe/cleanup inonUnmounted), and comprehensive error handling with toast notifications. ThehandleTriggerfunction correctly handles all cases: success with meetings, success with no matches, and errors.
125-284: LGTM!The template properly uses PrimeVue components (
DataTable,Sidebar,Dialog,Dropdown) with Tailwind CSS styling. The detail sidebar conditionally renders meeting minutes sections, and the trigger dialog includes proper validation with Enter key support. Accessibility is handled viaaria-labelon the filter dropdown.tests/unit/communication/meeting/test_frequency.py (1)
15-56: Good coverage of the frequency surface.This hits every
MeetingFrequencymember plus the seconds conversion path, so enum additions or mapping drift should fail loudly.src/ai_company/communication/config.py (1)
16-16: LGTM!The type change from
NotBlankStr | NonetoMeetingFrequency | Noneis clean. Pydantic will automatically coerce string values (e.g., from YAML config) to theMeetingFrequencyenum, and the existing_validate_frequency_or_triggervalidator continues to work correctly with the new type.Also applies to: 100-103
CLAUDE.md (1)
104-106: LGTM!Documentation updates for the meeting subdomain (
meeting/folder with scheduler, frequency, participant resolver, orchestrator) accurately reflect the new module structure.src/ai_company/communication/meeting/frequency.py (1)
1-57: LGTM!Clean implementation with:
- Proper
StrEnumfor automatic Pydantic string coercion- Defensive assertion ensuring all enum members have mappings
- Clear docstrings explaining the 30-day month approximation and
PER_SPRINT_DAYsemanticstests/unit/communication/meeting/test_participant.py (1)
26-41: LGTM!Well-structured test class with proper
@pytest.mark.unitmarker, clean fixtures, and good use ofAsyncMockfor the registry's async methods.web/src/__tests__/stores/meetings.test.ts (1)
1-208: LGTM!Comprehensive test coverage including:
- Initial state validation
- WS event handling for all three event types (
meeting.completed,meeting.started,meeting.failed)- Channel filtering to ignore non-meetings events
- Malformed payload handling
selectedMeetingsynchronization on refresh- Deduplication in
triggerMeeting- Graceful error handling for both API failures and WS refresh failures
src/ai_company/communication/meeting/participant.py (1)
124-139: LGTM — Previous issue addressed.The list validation now correctly filters to only valid string values (
isinstance(v, str) and v.strip()), addressing the previous review concern about arbitrary context values becoming synthetic IDs.tests/unit/communication/meeting/test_scheduler.py (2)
88-107: LGTM!Well-structured test class with proper fixtures for orchestrator and resolver mocks. The
_make_schedulerhelper keeps tests DRY while allowing customization.
370-386: Good test for critical error propagation.This correctly verifies that
MemoryError(and otherBaseExceptionsubclasses) aren't swallowed by the scheduler's error handling, which is important for resource exhaustion scenarios.web/src/stores/meetings.ts (2)
76-78: LGTM — Previous issue addressed.
selectedMeetingis now correctly synchronized when the refreshed meeting matches the currently selected one.
84-85: LGTM — Previous issue addressed.Channel guard now properly filters to only process events from the
'meetings'channel.web/src/api/endpoints/meetings.ts (1)
1-29: LGTM!Clean implementation of meeting API endpoints. Good practices observed:
- Proper URL encoding for
meetingIdparameter- Consistent use of
unwrap/unwrapPaginatedutilities- Types correctly aligned with backend contracts
src/ai_company/api/controllers/meetings.py (1)
1-181: LGTM!Well-structured controller implementation:
- Proper use of event constants (
MEETING_EVENT_TRIGGERED,MEETING_NOT_FOUND) from domain-specific observability moduleTriggerMeetingRequestvalidation handles bothstrandlist[str]context values correctly- Google-style docstrings present on all public methods
- Structured logging with keyword arguments throughout
All past review comments have been addressed.
web/src/api/types.ts (2)
490-580: LGTM!Comprehensive Meeting type definitions that correctly mirror backend models:
TriggerMeetingRequest.contextproperly typed asRecord<string, string | string[]>matching backend'sdict[str, str | list[str]]MeetingStatusincludes all terminal states (budget_exhausted,cancelled)MeetingMinutescaptures full meeting audit trail with token accounting
584-613: LGTM!WebSocket types correctly extended with meeting channel and event types (
meeting.started,meeting.completed,meeting.failed).tests/unit/api/controllers/test_meetings.py (1)
1-304: LGTM!Comprehensive test coverage for the meetings controller:
- All past review comments addressed (sync fixture, method naming, scheduler assertion, boundary tests)
- Good coverage of filter scenarios, 404 handling, and 503 fallback
TriggerMeetingRequestvalidation tests cover all boundary conditions including exact max lengthssrc/ai_company/communication/meeting/scheduler.py (7)
1-93: LGTM!Well-structured module with proper observability integration:
- Event constants imported from domain-specific module
_STATUS_TO_WS_EVENTmapping addresses previous review about explicit status-to-event mapping- Clear docstrings and
__slots__for memory efficiency
99-151: LGTM!Lifecycle methods properly implemented:
start()respectsconfig.enabledflagstop()gracefully cancels tasks and awaits completion- State transitions logged at INFO level
153-188: LGTM!The
trigger_eventmethod now correctly honorsconfig.enabled(line 170-171), addressing the previous review about the manual trigger path bypassing the config switch. Good use ofTaskGroupfor parallel execution.
246-277: LGTM!The
_execute_meetingmethod has been refactored into smaller helpers (_resolve_participants,_run_and_publish), bringing it well under the 50-line limit. Leader selection strategy is now documented (line 267).
361-392: LGTM!The
_publish_meeting_eventmethod correctly uses the_STATUS_TO_WS_EVENTmapping instead of formatting the event name from status value. Error handling is appropriate—re-raising fatal exceptions (MemoryError,RecursionError) while logging and swallowing transient publish failures.
394-416: LGTM!Clean agenda builder with proper handling of context items.
383-384: No action required. Theexcept MemoryError, RecursionError:syntax at lines 383-384 correctly follows PEP 758 and is properly aligned with the project's Python 3.14+ requirement and coding guidelines.
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ai_company/communication/meeting/participant.py`:
- Around line 125-139: The str branch currently accepts whitespace-only strings
and returns them as participant IDs; change the handling in the resolver (the
branch that checks isinstance(val, str)) to strip the string and only return
[val.strip()] when the stripped value is non-empty, otherwise log a warning
using MEETING_NO_PARTICIPANTS (similar to the non-str branch) and return an
empty list so blank context strings cannot become participant IDs passed into
MeetingScheduler._execute_meeting().
In `@src/ai_company/communication/meeting/scheduler.py`:
- Around line 143-147: The stop() method currently cancels self._tasks and calls
asyncio.gather(..., return_exceptions=True) which silently discards exceptions
from periodic tasks; change stop() to await asyncio.gather(*self._tasks) while
explicitly handling results: either call asyncio.gather without
return_exceptions inside a try/except and re-raise or log the first exception,
or if you keep return_exceptions=True then inspect the returned list and
re-raise or log any Exception instances so periodic task failures are not
silently swallowed; look for references to stop() and self._tasks in the
Scheduler/meeting scheduler class to implement this handling.
In `@tests/unit/api/controllers/test_meetings.py`:
- Around line 259-283: The context validator _validate_context_bounds currently
enforces max key count and string lengths but does not limit the length of list
values, so add a maximum list length (e.g., MAX_CONTEXT_LIST_ITEMS) and enforce
it inside _validate_context_bounds by raising ValueError when any context value
is a list longer than that limit; update TriggerMeetingRequest flow to call the
enhanced _validate_context_bounds and add a unit test mirroring
test_rejects_oversized_context_list_item that constructs TriggerMeetingRequest
with a list exceeding the new MAX_CONTEXT_LIST_ITEMS to assert a ValueError is
raised.
- Around line 85-89: The meeting_client pytest fixture currently annotates its
return type as Generator[TestClient[Any]]; change this to
Iterator[TestClient[Any]] to match yield-fixture conventions used elsewhere
(e.g., _reset_logging -> Iterator[None]). Update the fixture signature for
meeting_client and replace or add the import to use typing.Iterator instead of
typing.Generator so the annotation is correct.
In `@web/src/stores/meetings.ts`:
- Around line 45-61: The triggerMeeting function currently doesn't set loading,
causing inconsistent UI behavior; update triggerMeeting to set loading.value =
true before calling meetingsApi.triggerMeeting and ensure loading.value = false
in a finally block (after success or error) while preserving existing error
handling and record-append logic (references: triggerMeeting, loading,
meetings.value, total.value, meetingsApi.triggerMeeting, getErrorMessage).
🪄 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: abe473e2-d542-4c87-9ce2-9e72f97b4986
📒 Files selected for processing (9)
CLAUDE.mdsrc/ai_company/communication/meeting/participant.pysrc/ai_company/communication/meeting/scheduler.pytests/unit/api/controllers/test_meetings.pytests/unit/api/test_app.pytests/unit/api/test_channels.pytests/unit/communication/meeting/test_config_frequency.pytests/unit/communication/meeting/test_participant.pyweb/src/stores/meetings.ts
📜 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: Build Web
- GitHub Check: Build Backend
- GitHub Check: Greptile Review
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do not usefrom __future__ import annotationsin Python code—Python 3.14 has PEP 649 native lazy annotations.
Use PEP 758 except syntax withexcept A, B:(no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Add type hints to all public functions. Enforce strict mypy mode.
Maintain line length of 88 characters—enforced by ruff.
Handle errors explicitly—never silently swallow exceptions.
Files:
tests/unit/communication/meeting/test_participant.pytests/unit/communication/meeting/test_config_frequency.pytests/unit/api/test_app.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/communication/meeting/scheduler.pytests/unit/api/controllers/test_meetings.pytests/unit/api/test_channels.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Apply pytest markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowto categorize tests.
Maintain 80% minimum code coverage—enforced in CI. Use@pytest.mark.parametrizefor testing similar cases.
Files:
tests/unit/communication/meeting/test_participant.pytests/unit/communication/meeting/test_config_frequency.pytests/unit/api/test_app.pytests/unit/api/controllers/test_meetings.pytests/unit/api/test_channels.py
web/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript for Vue 3 web dashboard components. Enforce type checking via vue-tsc.
Files:
web/src/stores/meetings.ts
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Use Google-style docstrings on all public classes and functions—required and enforced by ruff D rules.
Immutability: Create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Distinguish between config and runtime state: Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 conventions:BaseModel,model_validator,computed_field,ConfigDict. Use@computed_fieldfor derived values instead of storing redundant fields. UseNotBlankStr(fromcore.types) for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task. Existing code is being migrated incrementally.
Keep functions under 50 lines and files under 800 lines.
Validate at system boundaries: user input, external APIs, config files.
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. In tests, usetest-provider,test-small-001, etc. Vendor names may only appear in: (1) Operations design page provider list, (2).claude/skill/agent files, (3) thir...
Files:
src/ai_company/communication/meeting/participant.pysrc/ai_company/communication/meeting/scheduler.py
src/ai_company/!(observability)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/ai_company/!(observability)/**/*.py: Every module with business logic must includefrom ai_company.observability import get_loggerand setlogger = get_logger(__name__). Never useimport logging,logging.getLogger(), orprint()in application code. Use the variable namelogger(not_loggerorlog).
Use event name constants from domain-specific modules underai_company.observability.events(e.g.,PROVIDER_CALL_STARTfromevents.provider). Import directly:from ai_company.observability.events.<domain> import EVENT_CONSTANT.
Use structured logging kwargs:logger.info(EVENT, key=value)—never use format strings likelogger.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 logging for object creation, internal flow, and entry/exit of key functions.
Files:
src/ai_company/communication/meeting/participant.pysrc/ai_company/communication/meeting/scheduler.py
🧠 Learnings (9)
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
CLAUDE.mdsrc/ai_company/communication/meeting/scheduler.py
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Organize web dashboard code by feature: `api/` (Axios client + types), `components/` (organized by feature), `composables/` (reusable functions), `router/`, `stores/` (Pinia), `styles/`, `utils/`, `views/` (pages).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Every module with business logic must include `from ai_company.observability import get_logger` and set `logger = get_logger(__name__)`. Never use `import logging`, `logging.getLogger()`, or `print()` in application code. Use the variable name `logger` (not `_logger` or `log`).
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use structured logging kwargs: `logger.info(EVENT, key=value)`—never use format strings like `logger.info("msg %s", val)`.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
CLAUDE.mdsrc/ai_company/communication/meeting/participant.py
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All state transitions must log at INFO level.
Applied to files:
CLAUDE.md
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use DEBUG logging for object creation, internal flow, and entry/exit of key functions.
Applied to files:
CLAUDE.mdsrc/ai_company/communication/meeting/participant.py
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
CLAUDE.mdsrc/ai_company/communication/meeting/scheduler.py
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/ai_company/communication/meeting/scheduler.py
🧬 Code graph analysis (7)
tests/unit/communication/meeting/test_participant.py (4)
src/ai_company/communication/meeting/participant.py (2)
resolve(24-43)resolve(65-108)src/ai_company/engine/parallel_models.py (1)
agent_id(79-81)src/ai_company/hr/registry.py (3)
list_active(129-138)list_by_department(140-158)get_by_name(113-127)src/ai_company/communication/meeting/errors.py (1)
NoParticipantsResolvedError(34-35)
tests/unit/communication/meeting/test_config_frequency.py (2)
src/ai_company/communication/config.py (1)
MeetingTypeConfig(83-137)src/ai_company/communication/meeting/frequency.py (1)
MeetingFrequency(16-31)
web/src/stores/meetings.ts (2)
web/src/api/types.ts (4)
MeetingRecord(560-568)MeetingFilters(570-575)TriggerMeetingRequest(577-580)WsEvent(615-620)web/src/api/endpoints/meetings.ts (1)
triggerMeeting(24-29)
tests/unit/api/test_app.py (2)
src/ai_company/api/app.py (2)
_safe_startup(256-342)_safe_shutdown(345-387)src/ai_company/api/state.py (2)
persistence(101-103)AppState(27-189)
src/ai_company/communication/meeting/participant.py (3)
src/ai_company/communication/meeting/errors.py (1)
NoParticipantsResolvedError(34-35)src/ai_company/hr/registry.py (3)
list_active(129-138)list_by_department(140-158)get_by_name(113-127)src/ai_company/observability/_logger.py (1)
get_logger(8-28)
src/ai_company/communication/meeting/scheduler.py (4)
src/ai_company/communication/meeting/errors.py (1)
NoParticipantsResolvedError(34-35)src/ai_company/communication/meeting/frequency.py (1)
frequency_to_seconds(48-57)src/ai_company/communication/meeting/participant.py (2)
resolve(24-43)resolve(65-108)src/ai_company/communication/config.py (2)
MeetingsConfig(140-166)MeetingTypeConfig(83-137)
tests/unit/api/controllers/test_meetings.py (3)
src/ai_company/api/app.py (1)
create_app(390-531)tests/unit/api/conftest.py (12)
FakePersistenceBackend(397-488)make_auth_headers(614-640)start(501-502)approval_store(676-677)auth_service(652-653)_seed_test_users(723-754)cost_tracker(671-672)get(60-61)get(223-224)get(294-295)get(322-323)get(383-384)src/ai_company/api/controllers/meetings.py (1)
TriggerMeetingRequest(28-67)
🔇 Additional comments (15)
tests/unit/api/test_channels.py (2)
10-10: LGTM!Import correctly added for the new
CHANNEL_MEETINGSconstant.
20-36: Well-structured parameterized test with complementary count assertion.The combination of the parameterized membership test and the count assertion effectively verifies that
ALL_CHANNELScontains exactly the expected 7 channels—no more, no less. This approach follows best practices by using@pytest.mark.parametrizefor similar cases. As per coding guidelines: "Use@pytest.mark.parametrizefor testing similar cases."tests/unit/communication/meeting/test_config_frequency.py (5)
1-7: LGTM!Imports are correctly structured. The
ValidationErrorimport frompydanticproperly addresses the exception type raised by Pydantic model validators.
10-21: LGTM!The test class is properly marked with
@pytest.mark.unit, includes a descriptive docstring, and thetest_accepts_enum_valuemethod has both a docstring and return type hint as required.
23-44: LGTM!The parametrized test correctly covers all five
MeetingFrequencyenum values (daily,weekly,bi_weekly,per_sprint_day,monthly). The# type: ignore[arg-type]is appropriate here since the test intentionally passes strings to verify Pydantic's coercion behavior.
46-62: LGTM!Both validation tests correctly catch
ValidationError(notValueError), which is the exception type Pydantic raises when model validators fail. The regex patterns match the error messages defined inMeetingTypeConfig._validate_frequency_or_trigger.
64-72: LGTM!Good coverage of the trigger-only configuration path, verifying that
frequencyremainsNonewhentriggeris set.web/src/stores/meetings.ts (7)
1-10: LGTM!Imports are well-organized with proper type imports and namespace import for the API module.
12-17: LGTM!Store state is well-typed with appropriate reactive refs for meeting data, selection, pagination total, and loading/error states.
19-31: LGTM!Standard async fetch pattern with proper loading/error state management and finally block.
33-43: LGTM!Single meeting fetch follows the same consistent pattern with proper state management.
63-82: LGTM!Background refresh logic is well-implemented with proper array update patterns and
selectedMeetingsync. Silent error handling viaconsole.warnis appropriate for best-effort refresh operations.
103-114: LGTM!Clean public API surface with appropriate exports. Internal
_refreshMeetingis correctly kept private.
84-101: Type safety confirmed — all event type literals ('meeting.completed','meeting.started','meeting.failed') are validWsEventTypevalues.tests/unit/api/test_app.py (1)
156-194: Good coverage for the new scheduler lifecycle path.This now exercises the non-
Nonemeeting_schedulerbranch through_safe_startup()and_safe_shutdown(), so the new app lifecycle wiring is covered instead of just the updated helper signatures.
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
This PR implements the meeting scheduler system (closes #264), adding both periodic frequency-based scheduling and event-triggered meetings. It introduces a MeetingFrequency enum replacing the raw string type, a ParticipantResolver protocol for resolving config references to agent IDs, a MeetingScheduler background service, REST API endpoints for listing/getting/triggering meetings, WebSocket event support, and a full frontend page with data table, detail sidebar, and trigger dialog.
Changes:
- Backend:
MeetingFrequencyenum,ParticipantResolverprotocol +RegistryParticipantResolver,MeetingSchedulerservice with periodic asyncio tasks and event triggering, integrated into app lifecycle - API:
GET /meetings(filtered/paginated),GET /meetings/{id},POST /meetings/triggerwith boundedTriggerMeetingRequest, plusmeetingsWS channel and event types - Frontend:
MeetingLogsPagewith DataTable, detail sidebar, trigger dialog, Pinia store with WS-driven re-fetch
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/ai_company/communication/meeting/frequency.py |
New MeetingFrequency StrEnum and interval conversion |
src/ai_company/communication/meeting/participant.py |
New ParticipantResolver protocol and registry-based implementation |
src/ai_company/communication/meeting/scheduler.py |
New MeetingScheduler background service |
src/ai_company/communication/meeting/errors.py |
New scheduler error types |
src/ai_company/communication/meeting/__init__.py |
Re-exports for new modules |
src/ai_company/communication/config.py |
frequency field type changed to MeetingFrequency |
src/ai_company/api/controllers/meetings.py |
Full controller replacing stubs |
src/ai_company/api/app.py |
Scheduler lifecycle integration |
src/ai_company/api/state.py |
meeting_orchestrator and meeting_scheduler state |
src/ai_company/api/channels.py |
meetings channel added |
src/ai_company/api/ws_models.py |
Meeting WS event types |
src/ai_company/observability/events/meeting.py |
Scheduler event constants |
web/src/views/MeetingLogsPage.vue |
Full meeting logs page |
web/src/stores/meetings.ts |
Pinia store with WS re-fetch |
web/src/api/endpoints/meetings.ts |
API client endpoints |
web/src/api/types.ts |
Meeting TypeScript types |
web/src/styles/theme.ts |
Status color mappings |
tests/unit/communication/meeting/test_scheduler.py |
Scheduler tests |
tests/unit/communication/meeting/test_participant.py |
Participant resolver tests |
tests/unit/communication/meeting/test_frequency.py |
Frequency enum tests |
tests/unit/communication/meeting/test_config_frequency.py |
Config type change tests |
tests/unit/api/controllers/test_meetings.py |
Controller tests |
tests/unit/api/test_app.py |
Lifecycle integration tests |
tests/unit/api/test_channels.py |
Channel count update |
web/src/__tests__/stores/meetings.test.ts |
Store tests |
web/src/__tests__/views/MeetingLogsPage.store-integration.test.ts |
Store integration test |
docs/design/communication.md |
Scheduler design docs |
CLAUDE.md |
Updated project structure docs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self, | ||
| meeting_type: MeetingTypeConfig, | ||
| ) -> None: | ||
| """Infinite loop: sleep for the interval, then execute the meeting. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ai_company/communication/meeting/scheduler.py`:
- Around line 426-434: The agenda item descriptions and the context string
currently use str(v), which renders lists as Python repr; update the builder
that creates MeetingAgendaItem and MeetingAgenda (the code that constructs items
= tuple(MeetingAgendaItem(...)) and the MeetingAgenda(...) call) to detect
list/tuple/set or other non-string iterables and format them as a human-readable
comma-joined string (e.g., ", ".join(map(str, v))) while leaving plain scalars
unchanged; ensure strings and bytes are not treated as iterables when applying
the join so existing string values remain intact.
In `@web/src/views/MeetingLogsPage.vue`:
- Line 150: The retry handler is being invoked immediately because the template
uses an invocation instead of a callback; change the ErrorBoundary retry binding
to pass a function reference or wrapper so it is called only when retry fires
(use meetingStore.fetchMeetings as the handler or an arrow wrapper like () =>
meetingStore.fetchMeetings() if ErrorBoundary passes arguments), updating the
<ErrorBoundary> `@retry` binding to reference meetingStore.fetchMeetings rather
than calling it.
- Around line 218-236: The template reads array properties like
selected.minutes.decisions, selected.minutes.action_items, and
selected.minutes.contributions and accesses .length directly which can throw if
those fields are undefined at runtime; update the conditional checks in
MeetingLogsPage.vue to use optional chaining (e.g.,
selected.minutes.decisions?.length > 0) or a fallback (e.g.,
(selected.minutes.decisions || []).length > 0) for decisions, action_items, and
contributions so the template is defensive against missing/null arrays.
🪄 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: a51c980d-f393-4c1c-b6fd-7f1991efa82d
📒 Files selected for processing (6)
src/ai_company/api/controllers/meetings.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/communication/meeting/scheduler.pytests/unit/api/controllers/test_meetings.pyweb/src/stores/meetings.tsweb/src/views/MeetingLogsPage.vue
📜 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: Build Web
- GitHub Check: Build Backend
- GitHub Check: Greptile Review
- GitHub Check: Test (Python 3.14)
🧰 Additional context used
📓 Path-based instructions (6)
web/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use TypeScript for Vue 3 web dashboard components. Enforce type checking via vue-tsc.
Files:
web/src/stores/meetings.tsweb/src/views/MeetingLogsPage.vue
web/src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
Use PrimeVue components in Vue 3 dashboard with Tailwind CSS styling.
Files:
web/src/views/MeetingLogsPage.vue
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Do not usefrom __future__ import annotationsin Python code—Python 3.14 has PEP 649 native lazy annotations.
Use PEP 758 except syntax withexcept A, B:(no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Add type hints to all public functions. Enforce strict mypy mode.
Maintain line length of 88 characters—enforced by ruff.
Handle errors explicitly—never silently swallow exceptions.
Files:
src/ai_company/api/controllers/meetings.pysrc/ai_company/communication/meeting/participant.pytests/unit/api/controllers/test_meetings.pysrc/ai_company/communication/meeting/scheduler.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Use Google-style docstrings on all public classes and functions—required and enforced by ruff D rules.
Immutability: Create new objects, never mutate existing ones. For non-Pydantic internal collections (registries,BaseTool), usecopy.deepcopy()at construction andMappingProxyTypewrapping for read-only enforcement.
Fordict/listfields in frozen Pydantic models, rely onfrozen=Truefor field reassignment prevention and usecopy.deepcopy()at system boundaries (tool execution, LLM provider serialization, inter-agent delegation, serializing for persistence).
Distinguish between config and runtime state: Use frozen Pydantic models for config/identity; use separate mutable-via-copy models (usingmodel_copy(update=...)) for runtime state that evolves. Never mix static config fields with mutable runtime fields in one model.
Use Pydantic v2 conventions:BaseModel,model_validator,computed_field,ConfigDict. Use@computed_fieldfor derived values instead of storing redundant fields. UseNotBlankStr(fromcore.types) for all identifier/name fields (including optional and tuple variants) instead of manual whitespace validators.
Preferasyncio.TaskGroupfor fan-out/fan-in parallel operations in new code (e.g. multiple tool invocations, parallel agent calls). Prefer structured concurrency over barecreate_task. Existing code is being migrated incrementally.
Keep functions under 50 lines and files under 800 lines.
Validate at system boundaries: user input, external APIs, config files.
Never use real vendor names (Anthropic, OpenAI, Claude, GPT, etc.) in project-owned code, docstrings, comments, tests, or config examples. Use generic names:example-provider,example-large-001,example-medium-001,example-small-001,large/medium/smallas aliases. In tests, usetest-provider,test-small-001, etc. Vendor names may only appear in: (1) Operations design page provider list, (2).claude/skill/agent files, (3) thir...
Files:
src/ai_company/api/controllers/meetings.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/communication/meeting/scheduler.py
src/ai_company/!(observability)/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/ai_company/!(observability)/**/*.py: Every module with business logic must includefrom ai_company.observability import get_loggerand setlogger = get_logger(__name__). Never useimport logging,logging.getLogger(), orprint()in application code. Use the variable namelogger(not_loggerorlog).
Use event name constants from domain-specific modules underai_company.observability.events(e.g.,PROVIDER_CALL_STARTfromevents.provider). Import directly:from ai_company.observability.events.<domain> import EVENT_CONSTANT.
Use structured logging kwargs:logger.info(EVENT, key=value)—never use format strings likelogger.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 logging for object creation, internal flow, and entry/exit of key functions.
Files:
src/ai_company/api/controllers/meetings.pysrc/ai_company/communication/meeting/participant.pysrc/ai_company/communication/meeting/scheduler.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Apply pytest markers:@pytest.mark.unit,@pytest.mark.integration,@pytest.mark.e2e,@pytest.mark.slowto categorize tests.
Maintain 80% minimum code coverage—enforced in CI. Use@pytest.mark.parametrizefor testing similar cases.
Files:
tests/unit/api/controllers/test_meetings.py
🧠 Learnings (5)
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use event name constants from domain-specific modules under `ai_company.observability.events` (e.g., `PROVIDER_CALL_START` from `events.provider`). Import directly: `from ai_company.observability.events.<domain> import EVENT_CONSTANT`.
Applied to files:
src/ai_company/api/controllers/meetings.pysrc/ai_company/communication/meeting/scheduler.py
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : All error paths must log at WARNING or ERROR with context before raising.
Applied to files:
src/ai_company/communication/meeting/participant.py
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to src/ai_company/!(observability)/**/*.py : Use DEBUG logging for object creation, internal flow, and entry/exit of key functions.
Applied to files:
src/ai_company/communication/meeting/participant.py
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to **/*.py : Use PEP 758 except syntax with `except A, B:` (no parentheses) for multiple exceptions—ruff enforces this on Python 3.14.
Applied to files:
src/ai_company/communication/meeting/scheduler.py
📚 Learning: 2026-03-14T15:34:22.487Z
Learnt from: CR
Repo: Aureliolo/synthorg PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-14T15:34:22.487Z
Learning: Applies to **/*.py : Handle errors explicitly—never silently swallow exceptions.
Applied to files:
src/ai_company/communication/meeting/scheduler.py
🧬 Code graph analysis (3)
src/ai_company/api/controllers/meetings.py (6)
src/ai_company/api/dto.py (2)
ApiResponse(39-57)PaginatedResponse(76-96)src/ai_company/api/guards.py (1)
require_write_access(56-80)src/ai_company/api/pagination.py (1)
paginate(26-58)src/ai_company/api/state.py (2)
meeting_orchestrator(149-154)meeting_scheduler(157-162)tests/unit/communication/meeting/conftest.py (1)
meeting_id(114-116)src/ai_company/communication/meeting/scheduler.py (1)
trigger_event(162-197)
src/ai_company/communication/meeting/participant.py (2)
src/ai_company/communication/meeting/errors.py (1)
NoParticipantsResolvedError(34-35)src/ai_company/hr/registry.py (3)
list_active(129-138)list_by_department(140-158)get_by_name(113-127)
src/ai_company/communication/meeting/scheduler.py (5)
src/ai_company/communication/meeting/errors.py (2)
NoParticipantsResolvedError(34-35)SchedulerAlreadyRunningError(38-39)src/ai_company/communication/meeting/frequency.py (1)
frequency_to_seconds(48-57)src/ai_company/communication/meeting/orchestrator.py (2)
MeetingOrchestrator(68-457)run_meeting(103-181)src/ai_company/communication/meeting/participant.py (3)
ParticipantResolver(21-43)resolve(24-43)resolve(65-108)src/ai_company/communication/config.py (2)
MeetingsConfig(140-166)MeetingTypeConfig(83-137)
🔇 Additional comments (8)
src/ai_company/communication/meeting/participant.py (1)
1-171: LGTM!The implementation is well-structured with proper:
- Protocol definition with clear docstrings
- Resolution cascade (context → "all" → department → agent name → literal ID)
- Validation of context values (blank strings, non-string types)
- Deduplication while preserving order
- Appropriate DEBUG/WARNING logging at all key paths
Previous review findings have been properly addressed.
src/ai_company/api/controllers/meetings.py (1)
1-188: LGTM!The controller implementation is solid:
TriggerMeetingRequestproperly validates context bounds (keys, key length, value length, list items, list length)- Context type correctly widened to
dict[str, str | list[str]]for multi-value participant resolution- Endpoints use proper guards (
require_read_access,require_write_access)- Event constants are properly imported and used for logging
- 404 handling uses the semantically correct
MEETING_NOT_FOUNDeventPrevious review findings have been properly addressed.
tests/unit/api/controllers/test_meetings.py (1)
1-321: LGTM!Comprehensive test coverage including:
- Controller endpoint tests (list, filter, get, trigger, 503)
- Validation boundary tests for
TriggerMeetingRequest- Proper use of
@pytest.mark.unitmarkers- Correct
Iterator[TestClient[Any]]type annotation for the yield fixture- Scheduler interaction assertion with
assert_awaited_once_withPrevious review findings have been properly addressed.
src/ai_company/communication/meeting/scheduler.py (2)
1-434: LGTM overall!The scheduler implementation is well-designed:
- Proper lifecycle management with
start()/stop()enabledflag respected in bothstart()andtrigger_event()- Error handling in
stop()logs exceptions from gather results- Status-to-WS-event mapping avoids dynamic event name construction
- Helper methods keep
_execute_meetingfocused and under line limits- Leader selection strategy is documented with inline comment
Previous review findings have been properly addressed.
401-403: The code is correct. The project explicitly targets Python 3.14+ (requires-python = ">=3.14"in pyproject.toml), so the PEP 758 syntaxexcept MemoryError, RecursionError:(without parentheses) is appropriate and valid. No changes needed.web/src/stores/meetings.ts (1)
1-117: LGTM! Well-structured Pinia store with proper real-time handling.The implementation correctly addresses all previously raised concerns:
- Batched array updates in
triggerMeetingusingSetfor efficient dedupselectedMeetingsync in_refreshMeeting- Channel guard in
handleWsEvent- Consistent
loadingstate management across all async actionsError handling and fire-and-forget pattern with
void _refreshMeeting()are appropriate for the WebSocket refresh use case.web/src/views/MeetingLogsPage.vue (2)
43-58: Good resilience pattern with separate error handling for WS and data fetch.The dual try/catch approach ensures initial data loads even if WebSocket setup fails, providing graceful degradation.
79-115: LGTM! Comprehensive trigger workflow with proper state management.The
handleTriggerfunction properly:
- Validates input before submission
- Manages local loading state
- Handles all three outcomes (success, no-match, error) with appropriate toast feedback
- Resets dialog state on success/no-match
Implement the meeting scheduler service that bridges meeting configuration
and meeting execution. Adds frequency-based periodic scheduling (via
asyncio tasks) and event-triggered meetings (on-demand API).
New modules:
- frequency.py: MeetingFrequency enum (daily/weekly/bi_weekly/per_sprint_day/monthly)
- participant.py: ParticipantResolver protocol + RegistryParticipantResolver
- scheduler.py: MeetingScheduler background service (start/stop/trigger_event)
API changes:
- GET /meetings: list meeting records with status/type filters + pagination
- GET /meetings/{id}: get meeting record by ID (or 404)
- POST /meetings/trigger: trigger event-based meetings
- Added "meetings" WebSocket channel + meeting event types
- AppState wired with meeting_orchestrator + meeting_scheduler
Config change:
- MeetingTypeConfig.frequency: NotBlankStr -> MeetingFrequency enum
(Pydantic coerces strings transparently, YAML configs work unchanged)
Frontend:
- Meeting TypeScript types + API client + Pinia store
- MeetingLogsPage: DataTable with filters, detail sidebar, trigger dialog
Pre-reviewed by 14 agents, 19 findings addressed: - Wrap event publisher in try/except (data-loss prevention) - Add error recovery in _run_periodic loop body - Extract _publish_meeting_event helper (function length) - Split _execute_meeting try/except for clarity - Replace assert with proper TypeError guard - Merge double context.items() iteration - Add exhaustiveness check for frequency mapping - Log before raising SchedulerAlreadyRunningError - Add logger usage in meetings controller (404 + trigger) - Bound TriggerMeetingRequest.context (max keys/lengths) - Remove ambiguous pass-through debug log - Add MeetingPhase type union in frontend types.ts - Re-fetch full record on WS meeting events (not just status) - Keep trigger dialog open on error - Update CLAUDE.md (package structure, stores, events) - Fix assert_awaited_with -> assert_awaited_once_with - Add NoParticipantsResolvedError test - Remove unnecessary async on sync static method tests
Critical fixes: - Wire scheduler start()/stop() into app lifecycle (_safe_startup/_safe_shutdown) - Add 5 missing meeting event constants to CLAUDE.md logging section - Add console.warn in meetings store empty catch block Major fixes: - Replace MEETING_SCHEDULER_ERROR with MEETING_NOT_FOUND for 404 path - Use TaskGroup for parallel meeting execution in trigger_event - Fix TriggerMeetingRequest.context type: Record<string, string> - Replace _tasks list mutation with immutable patterns - Sync selectedMeeting on WS refresh - Validate context list values in participant resolver - Re-raise CancelledError instead of swallowing in _run_periodic - Add Final annotations to all channel constants - Add Meeting Scheduler section to communication design doc - Add meeting/ subpackage entry in CLAUDE.md package structure - Add channel guard to handleWsEvent - Document leader selection convention - Make resolver raise NoParticipantsResolvedError on empty results - Rewrite MeetingLogsPage.test.ts to test store, not duplicate - Add fetchMeetings/triggerMeeting store tests - Add TriggerMeetingRequest validation boundary tests Medium fixes: - Add _publish_meeting_event error + MemoryError re-raise tests - Fix test name typo: callsmock → calls_mock - Fix comment typo: meetingmock_scheduler → meeting_scheduler - Remove unnecessary async from mock_orchestrator fixture - Use comprehensions in _build_default_agenda - Move NoParticipantsResolvedError import to top-level - Fix errors.py module docstring overclaim - Add per_sprint_day to coercion test parametrize - Pass data.context directly instead of `or None` - Extract _create_app_without_meetings helper - Replace inline styles with Tailwind classes in MeetingLogsPage - Add stop_noop_when_not_running test
…solver When a context value is present but is neither str nor list/tuple, explicitly log a warning and return empty instead of silently falling through to registry lookups. Prevents misresolution via pass-through for internal callers using dict[str, Any] context.
- Widen TriggerMeetingRequest.context to dict[str, str | list[str]] so API callers can pass multi-value participant entries; update validator to handle list items; align frontend TS type - Extract _resolve_participants and _run_and_publish from _execute_meeting to stay under 50-line function limit - Replace f-string status formatting in _publish_meeting_event with explicit _STATUS_TO_WS_EVENT mapping (no API layer import) - Guard trigger_event with self._config.enabled check - Assert mock_scheduler.trigger_event called with expected args - Add exact-boundary test for max key/value lengths - Add docstring to test_accepts_enum_value - Rename MeetingLogsPage.test.ts to .store-integration.test.ts
dict[str, str] is not assignable to dict[str, str | list[str]] under mypy strict mode (dict is invariant in value type). Add explicit type annotations on test variables.
- Remove unreachable "in_progress" → "meeting.started" mapping from _STATUS_TO_WS_EVENT (status is always terminal post-run_meeting) - Wrap _build_default_agenda in exception guard so failures don't propagate to TaskGroup and crash the entire trigger_event call - Execute meeting immediately on first periodic loop entry, then sleep (run-now-then-repeat pattern avoids missing today's meeting)
- Fix "e.g." → "e.g.," in CLAUDE.md event names line - Add DEBUG log before literal ID fallback in participant resolver - Add test for oversized context list items in TriggerMeetingRequest - Add meeting scheduler lifecycle test (_safe_startup/_safe_shutdown) - Use pydantic.ValidationError in config frequency tests - Add test_context_tuple_value for tuple context branch - Batch triggerMeeting store updates (single array spread) - Parametrize test_all_channels_contains_expected
Replace the local selected ref with a computed over meetingStore.selectedMeeting so the sidebar reflects real-time WS refresh updates instead of holding a stale object reference.
…tore - Strip whitespace-only context strings in participant resolver and log warning instead of passing blank IDs downstream - Log periodic task errors during scheduler stop() instead of silently discarding them via return_exceptions=True - Add _MAX_CONTEXT_LIST_ITEMS=50 limit to TriggerMeetingRequest validator with test coverage - Fix meeting_client fixture return type: Generator → Iterator - Add loading state to triggerMeeting store action for consistent UI
- Move gofmt from linters to formatters section in .golangci.yml (v2 reclassified it, causing "gofmt is a formatter" config error) - Force bash shell for go test step to prevent PowerShell from splitting -coverprofile=coverage.out into separate arguments
- Format list context values as comma-joined strings in agenda descriptions instead of Python repr (e.g. "a, b" not "['a', 'b']") - Re-raise MemoryError/RecursionError in _run_periodic before the broad except Exception handler - Use arrow wrapper for ErrorBoundary @Retry to prevent immediate invocation on render - Add optional chaining on decisions/action_items/contributions .length in sidebar template - Guard "Trigger Meeting" button with v-if="canWrite" to match write-action pattern used in TaskBoardPage
Aureliolo
force-pushed
the
feat/meeting-scheduler
branch
from
ef9f0b5 to
7166a56
Compare
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Aureliolo
temporarily deployed
to
cloudflare-preview
GitHub Actions
Inactive
Summary
MeetingScheduler) that bridges meeting configuration and execution — periodic scheduling via asyncio tasks + event-triggered meetings on demandMeetingFrequencyenum replacesNotBlankStronMeetingTypeConfig.frequency— compile-time validation, Pydantic coerces strings from YAML transparentlyParticipantResolverprotocol +RegistryParticipantResolver— resolves config references (department names, agent names,"all", literal IDs) to agent ID tuples viaAgentRegistryServiceGET /meetings(paginated + filters),GET /meetings/{id},POST /meetings/triggerwith boundedTriggerMeetingRequestmeetingschannel +meeting.started/completed/failedevent typesMeetingLogsPage(DataTable + detail sidebar + trigger dialog), Pinia store with WS re-fetch, API client, full TypeScript typesCloses #264
Test plan
uv run pytest tests/unit/communication/meeting/test_frequency.py -n auto— frequency enum + conversionuv run pytest tests/unit/communication/meeting/test_config_frequency.py -n auto— config type change + coercionuv run pytest tests/unit/communication/meeting/test_participant.py -n auto— all 5 resolution strategies + dedupuv run pytest tests/unit/communication/meeting/test_scheduler.py -n auto— start/stop/trigger/periodic/error pathsuv run pytest tests/unit/api/controllers/test_meetings.py -n auto— list/get/trigger/404/503 endpointsnpm --prefix web run test— store + WS handler + re-fetch testsuv run pytest tests/ -n auto --cov=ai_company --cov-fail-under=80— full suite + coverage gateuv run mypy src/ tests/— type checkingnpm --prefix web run type-check— frontend typesReview coverage
Pre-reviewed by 14 agents: code-reviewer, python-reviewer, test-analyzer, silent-failure-hunter, type-design-analyzer, logging-audit, conventions-enforcer, security-reviewer, frontend-reviewer, api-contract-drift, async-concurrency-reviewer, issue-resolution-verifier, test-quality-reviewer, docs-consistency. 19 findings addressed (2 critical, 7 major, 10 medium).