Merged
Conversation
|
Note Reviews pausedUse the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds an "Answer Now" manual wrap-up flow: UI widget and styles, TUI click-to-request plumbing, orchestrator methods to prime/consume per-agent hook payloads, RoundTimeout state and hook changes, backend logging tidy-ups, tests, docs updates, and a package version bump to 0.1.77. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as TextualUI
participant Orch as Orchestrator
participant Hook as RoundTimeoutPostHook
participant Backend as AgentBackend
participant Agent
User->>UI: Click "Answer Now"
UI->>Orch: request_answer_now()
Orch->>Hook: request_wrap_up(agent)
alt manual injection already queued
Hook-->>Orch: consume_pending_wrap_up_injection() -> content
else build/produce injection
Hook->>Hook: _build_wrap_up_injection(manual=True)
Hook-->>Orch: content
end
Orch->>Backend: write_post_tool_use_hook(agent, content)
Orch->>UI: update_timeout_status(agent, wrap_up_requested=True)
Orch->>Orch: _maybe_interrupt_background_wait_for_agent(agent)
Backend->>Agent: deliver hook_payload.json
Agent-->>Orch: response/completion
Orch->>UI: display notification (requested/already/skipped)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@massgen/__init__.py`:
- Line 89: The package version in massgen/__init__.py was bumped to "__version__
= \"0.1.77\"" but documentation wasn't updated; update CHANGELOG.md by adding a
new top entry for v0.1.77 (matching the release notes format used for v0.1.76),
and in README.md and README_PYPI.md replace all occurrences of "v0.1.76" with
"v0.1.77" (including section headers and installation command references noted
in the review) so docs remain consistent with the __version__ change.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f110a319-72cf-4a09-acd5-4495d91fa90b
📒 Files selected for processing (2)
ROADMAP_v0.1.78.mdmassgen/__init__.py
feat: Add answer now button for faster answer
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
massgen/frontend/displays/textual_terminal_display.py (1)
3244-3304:⚠️ Potential issue | 🟠 MajorClear
_timeout_stateson turn resets, not only on full status-bar reset.
_timeout_statesis only wiped inStatusBar.reset(), but multi-turn flows reuse the same app viaTextualApp.reset_turn_state()/prepare_for_new_turn()without hitting that path. That lets the next turn start with stale “Wrapping up” / “Blocked” state until fresh timeout updates arrive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/frontend/displays/textual_terminal_display.py` around lines 3244 - 3304, The timeout-state map (_timeout_states) is only cleared in StatusBar.reset(), causing stale "Wrapping up"/"Blocked" UI across reused app turns; update the turn-reset path to clear it: in the turn-reset methods (e.g., TextualApp.reset_turn_state() or prepare_for_new_turn()) call StatusBar.stop_timer() and clear StatusBar._timeout_states (or add and call a new StatusBar.clear_timeout_states() helper) so set_timeout_state/_update_answer_now_display start from an empty dict for each new turn.
🧹 Nitpick comments (3)
massgen/mcp_tools/hooks.py (1)
1483-1500: Please convert the new timeout-helper docstrings to Google style.These changed Python functions add/modify public behavior, but their docstrings don't follow the repo's required
Args/Returnsformat.As per coding guidelines: "
**/*.py: For new or changed functions, include Google-style docstrings".Also applies to: 1594-1637, 1694-1701
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/mcp_tools/hooks.py` around lines 1483 - 1500, Update the docstrings for mark_soft_fired and get_effective_grace_seconds to Google style: for mark_soft_fired (method mark_soft_fired) add an Args section documenting reason: str (default "timeout") and a Returns section (None) and describe idempotency and side-effects on attributes soft_timeout_fired_at and soft_timeout_reason; for get_effective_grace_seconds (function get_effective_grace_seconds) add Args documenting default_grace_seconds: int and a Returns section documenting the returned int and explain the special-case when soft_timeout_reason == "manual" uses MANUAL_WRAP_UP_GRACE_SECONDS to cap the grace window; follow the same Google-style changes for the other affected docstrings at the referenced ranges.massgen/frontend/displays/textual_terminal_display.py (1)
1003-1006: Use Google-style docstrings for the new parameterized helpers.These changed functions add parameters but still use one-line docstrings, which makes them inconsistent with the repo’s Python docstring rule. As per coding guidelines,
**/*.py: For new or changed functions, include Google-style docstrings.Also applies to: 3236-3248, 5456-5458, 11031-11037
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/frontend/displays/textual_terminal_display.py` around lines 1003 - 1006, The one-line docstring for set_answer_now_callback does not follow the project's Google-style docstring requirement for changed functions; update the docstring for set_answer_now_callback (and similarly for the other changed functions at the referenced ranges) to a Google-style docstring that documents parameters and return value (i.e., Args: callback: callable; Returns: None) and a short description, keeping the function signature and behavior unchanged; locate set_answer_now_callback in textual_terminal_display.py and replace the single-line docstring with the multi-line Google-style form.massgen/orchestrator.py (1)
8868-8872: Use Google-style docstrings for the new helpers.These added methods only have one-line summaries. The repo rule for changed Python functions asks for Google-style docstrings with
Args/Returnssections.As per coding guidelines, `**/*.py`: For new or changed functions, include Google-style docstrings.Example
async def _collect_round_timeout_runtime_sections( self, agent_id: str, ) -> list[str]: - """Collect timeout/wrap-up injection content for hybrid or hookless delivery paths.""" + """Collect timeout/wrap-up injection content for hybrid or hookless delivery paths. + + Args: + agent_id: Agent whose round-timeout state should be flushed. + + Returns: + Runtime sections to append to the pending injection payload. + """Also applies to: 10448-10550
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/orchestrator.py` around lines 8868 - 8872, The new helper _collect_round_timeout_runtime_sections currently has only a one-line docstring; update it (and the other added methods around lines 10448-10550) to follow the repo's Google-style docstring convention by adding Args and Returns sections describing parameters and return type; specifically, for _collect_round_timeout_runtime_sections document the agent_id parameter as str under Args and document that the function returns list[str] under Returns, keeping the one-line summary at the top and using the same formatting used elsewhere in the repo (Google-style) for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@massgen/frontend/displays/textual_terminal_display.py`:
- Around line 11039-11079: The _trigger_answer_now method currently calls the
external _answer_now_callback and assumes its return is a dict; wrap the
callback invocation in a try/except to catch and log/notify on any exception,
and then defensively coerce/validate the result before parsing (e.g., if not
isinstance(result, dict) treat as {}), before reading
requested_agents/already_requested_agents/skipped_agents and calling
add_orchestrator_event/notify; ensure any exception path returns early so the UI
control remains usable.
In `@massgen/frontend/displays/textual_widgets/agent_status_ribbon.py`:
- Around line 132-147: AnswerNowLabel is focusable but only implements
on_click(), so keyboard users can't activate it; update the control to support
Enter/Space activation by either converting AnswerNowLabel to subclass
textual.widgets.Button (preserving _agent_id, set_agent_id and the click
behavior) or keep it as Label and add explicit key handling (e.g., add BINDINGS
or implement on_key/event handler) that when Enter or Space is pressed calls the
same logic as on_click (post_message(AnswerNowClicked(self._agent_id))). Ensure
the activation path uses the existing _agent_id and set_agent_id API so behavior
remains identical.
In `@massgen/mcp_tools/hooks.py`:
- Around line 1594-1613: request_wrap_up currently latches
_manual_wrap_up_requested even when this round has no timeout, leaving the flag
stuck; change request_wrap_up to call _get_timeout_for_current_round() and
return False (without setting _manual_wrap_up_requested) when it returns None,
preserving the existing early-return behavior for _soft_timeout_fired; ensure
consume_pending_wrap_up_injection behavior remains unchanged and also apply the
same guard to the other identical request_wrap_up occurrence in the file so both
implementations check _get_timeout_for_current_round() before setting
_manual_wrap_up_requested.
In `@massgen/orchestrator.py`:
- Around line 10557-10568: The code currently calls the destructive
_consume_pending_answer_now_injection(agent_id) before attempting the backend
handoff via write_hook/write_post_tool_use_hook, which can drop the payload on
failure; change the flow to first peek/read the pending payload
non-destructively (add or use a new method like
peek_pending_answer_now_injection(agent_id) or add an ack flag to
_consume_pending_answer_now_injection), call write_hook/write_post_tool_use_hook
with that payload, and only after write_hook returns successfully call the
destructive acknowledge/consume method (e.g.,
_consume_pending_answer_now_injection or
ack_pending_answer_now_injection(agent_id)); ensure exceptions from write_hook
do not clear the queued request and that the destructive path is executed only
on success.
- Around line 8935-8938: RoundTimeoutPostHook's state (_manual_wrap_up_requested
/ _soft_timeout_fired) is mutated concurrently by request_wrap_up() and
consume_pending_wrap_up_injection(), causing lost or double transitions; fix by
serializing access per agent: add a per-agent lock (e.g., an asyncio.Lock
stored/managed by RoundTimeoutPostHook or a per-agent lock map) and wrap both
request_wrap_up() and consume_pending_wrap_up_injection() (and any other paths
mentioned around lines ~9132, ~10496, ~10525) in the same lock, or alternatively
expose a single atomic API method on RoundTimeoutPostHook (e.g.,
request_and_consume_atomic()) that acquires the lock and performs the state
checks and mutations together so no overlapping TUI callbacks or runtime
flushers can race on _manual_wrap_up_requested/_soft_timeout_fired.
In `@massgen/tests/integration/test_orchestrator_restart_and_external_tools.py`:
- Around line 724-798: Add backend-parity tests mirroring the Codex cases for a
base_with_custom_tool_and_mcp backend and for claude_code: duplicate the
test_codex_answer_now_flushes_manual_wrap_up_payload and
test_setup_hook_manager_for_codex_hybrid_registers_round_timeout_hooks logic but
set agent.backend._provider_name to "base_with_custom_tool_and_mcp" and
"claude_code" respectively, ensure
supports_native_hooks/supports_mcp_server_hooks/get_native_hook_adapter/set_native_hooks_config/set_background_wait_interrupt_provider
are stubbed the same way, attach a write_post_tool_use_hook that appends to
written_payloads, call orchestrator._setup_hook_manager_for_agent(agent_id,
agent, {}) and orchestrator.request_answer_now(), and assert the
written_payloads contains the "ANSWER NOW REQUESTED" payload and that
orchestrator.agent_states[agent_id].round_timeout_hooks and round_timeout_state
are initialized (use the same symbols:
orchestrator._setup_hook_manager_for_agent, request_answer_now,
agent.backend._provider_name, agent.backend.write_post_tool_use_hook,
orchestrator.agent_states[agent_id].round_start_time).
---
Outside diff comments:
In `@massgen/frontend/displays/textual_terminal_display.py`:
- Around line 3244-3304: The timeout-state map (_timeout_states) is only cleared
in StatusBar.reset(), causing stale "Wrapping up"/"Blocked" UI across reused app
turns; update the turn-reset path to clear it: in the turn-reset methods (e.g.,
TextualApp.reset_turn_state() or prepare_for_new_turn()) call
StatusBar.stop_timer() and clear StatusBar._timeout_states (or add and call a
new StatusBar.clear_timeout_states() helper) so
set_timeout_state/_update_answer_now_display start from an empty dict for each
new turn.
---
Nitpick comments:
In `@massgen/frontend/displays/textual_terminal_display.py`:
- Around line 1003-1006: The one-line docstring for set_answer_now_callback does
not follow the project's Google-style docstring requirement for changed
functions; update the docstring for set_answer_now_callback (and similarly for
the other changed functions at the referenced ranges) to a Google-style
docstring that documents parameters and return value (i.e., Args: callback:
callable; Returns: None) and a short description, keeping the function signature
and behavior unchanged; locate set_answer_now_callback in
textual_terminal_display.py and replace the single-line docstring with the
multi-line Google-style form.
In `@massgen/mcp_tools/hooks.py`:
- Around line 1483-1500: Update the docstrings for mark_soft_fired and
get_effective_grace_seconds to Google style: for mark_soft_fired (method
mark_soft_fired) add an Args section documenting reason: str (default "timeout")
and a Returns section (None) and describe idempotency and side-effects on
attributes soft_timeout_fired_at and soft_timeout_reason; for
get_effective_grace_seconds (function get_effective_grace_seconds) add Args
documenting default_grace_seconds: int and a Returns section documenting the
returned int and explain the special-case when soft_timeout_reason == "manual"
uses MANUAL_WRAP_UP_GRACE_SECONDS to cap the grace window; follow the same
Google-style changes for the other affected docstrings at the referenced ranges.
In `@massgen/orchestrator.py`:
- Around line 8868-8872: The new helper _collect_round_timeout_runtime_sections
currently has only a one-line docstring; update it (and the other added methods
around lines 10448-10550) to follow the repo's Google-style docstring convention
by adding Args and Returns sections describing parameters and return type;
specifically, for _collect_round_timeout_runtime_sections document the agent_id
parameter as str under Args and document that the function returns list[str]
under Returns, keeping the one-line summary at the top and using the same
formatting used elsewhere in the repo (Google-style) for consistency.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0243599-86a3-4af6-ad35-c250c29056e1
📒 Files selected for processing (13)
docs/modules/coordination_workflow.mdmassgen/backend/codex.pymassgen/backend/gemini_cli.pymassgen/frontend/displays/textual_terminal_display.pymassgen/frontend/displays/textual_themes/base.tcssmassgen/frontend/displays/textual_themes/dark.tcssmassgen/frontend/displays/textual_widgets/__init__.pymassgen/frontend/displays/textual_widgets/agent_status_ribbon.pymassgen/mcp_tools/hooks.pymassgen/mcp_tools/standalone/checkpoint_instructions.mdmassgen/orchestrator.pymassgen/tests/integration/test_orchestrator_restart_and_external_tools.pymassgen/tests/test_hook_framework.py
✅ Files skipped from review due to trivial changes (7)
- docs/modules/coordination_workflow.md
- massgen/backend/gemini_cli.py
- massgen/mcp_tools/standalone/checkpoint_instructions.md
- massgen/backend/codex.py
- massgen/frontend/displays/textual_themes/base.tcss
- massgen/tests/test_hook_framework.py
- massgen/frontend/displays/textual_themes/dark.tcss
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
massgen/mcp_tools/hooks.py (1)
1585-1604:⚠️ Potential issue | 🟠 MajorDon't accept a manual wrap-up request when this round has no timeout.
request_wrap_up()still returnsTrueand latches_manual_wrap_up_requestedeven when_get_timeout_for_current_round()isNone. Bothconsume_pending_wrap_up_injection()andexecute()bail out on that condition, leaving the flag stuck—subsequent clicks become no-ops despite never delivering the injection.🔧 Proposed fix
def request_wrap_up(self) -> bool: """Request a manual soft-timeout injection on the next delivery opportunity. Returns: True if a new wrap-up request was queued, False if this round is already wrapping up or the soft timeout has already fired. """ if self._soft_timeout_fired or self._manual_wrap_up_requested: return False + if self._get_timeout_for_current_round() is None: + return False self._manual_wrap_up_requested = True return True def consume_pending_wrap_up_injection(self) -> str | None: """Consume any queued manual wrap-up request and return its injection text.""" if self._soft_timeout_fired or not self._manual_wrap_up_requested: return None timeout = self._get_timeout_for_current_round() if timeout is None: + self._manual_wrap_up_requested = False return None,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/mcp_tools/hooks.py` around lines 1585 - 1604, request_wrap_up currently sets _manual_wrap_up_requested and returns True even when the current round has no timeout, leaving the flag stuck; change request_wrap_up (and/or call _get_timeout_for_current_round from it) to check that _get_timeout_for_current_round() is not None before latching _manual_wrap_up_requested and returning True, and return False (without setting the flag) when timeout is None; ensure the unique symbols referenced are request_wrap_up, _manual_wrap_up_requested, and _get_timeout_for_current_round so the guard is added in that method and the flag never becomes stuck when no timeout exists.
🧹 Nitpick comments (3)
massgen/mcp_tools/hooks.py (1)
1597-1619: Consider adding Google-style docstrings to new methods.Per coding guidelines, new or changed functions should include Google-style docstrings. Methods like
consume_pending_wrap_up_injection()and_build_wrap_up_injection()would benefit from explicitArgsandReturnssections for maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/mcp_tools/hooks.py` around lines 1597 - 1619, Add a Google-style docstring to the method consume_pending_wrap_up_injection (and likewise update _build_wrap_up_injection if missing) that briefly describes purpose, lists Args (none or any parameters like self) and Returns (str | None) and any side effects (e.g., sets _manual_wrap_up_requested and _soft_timeout_fired, calls _shared_state.mark_soft_fired). Place the docstring immediately under the def line, include a one-line summary, an Args section (if applicable), a Returns section describing the return value, and a Notes/Side effects line describing state changes for maintainability.massgen/orchestrator.py (2)
9069-9069: Simplify f-string concatenation.The adjacent f-strings can be merged into a single f-string for clarity.
♻️ Suggested simplification
logger.info( - f"[Orchestrator] Wrote {len(combined)} chars to hook file for {agent_id} " f"({len(injection_parts)} parts)", + f"[Orchestrator] Wrote {len(combined)} chars to hook file for {agent_id} ({len(injection_parts)} parts)", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/orchestrator.py` at line 9069, The log message uses two adjacent f-strings; merge them into a single f-string to simplify the expression—replace the concatenated f"[Orchestrator] Wrote {len(combined)} chars to hook file for {agent_id} " f"({len(injection_parts)} parts)" with one f-string that includes len(combined), agent_id, and len(injection_parts) in a single interpolation (refer to the variables combined, agent_id, injection_parts and the surrounding log/write call).
8888-8894: Consider narrowing the exception scope.The broad
Exceptioncatch here is defensive, which is reasonable for hook execution, but it can mask programming errors (e.g.,TypeError,AttributeError). Consider catching a more specific base like(RuntimeError, ValueError, TimeoutError)or at least logging the exception type for easier debugging.♻️ Suggested narrower exception handling
- except Exception as e: + except (RuntimeError, ValueError, TimeoutError, OSError) as e: logger.warning( "[Orchestrator] Failed to evaluate round timeout hook for %s: %s", agent_id, e, + exc_info=True, ) return []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/orchestrator.py` around lines 8888 - 8894, The except block in the round timeout hook evaluator currently catches a broad Exception in the orchestrator (the try/except around the hook execution for agent_id), which can hide programming errors; change it to catch a narrower set such as (RuntimeError, ValueError, TimeoutError) instead of Exception, and update the logging to include the exception type and stack (use logger.exception or include type(e).__name__ and traceback info) so failures remain defensive but are easier to debug; modify the except clause and the logger.warning call in the same handler (the block referencing agent_id and e) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@massgen/orchestrator.py`:
- Around line 10502-10512: The fire-and-forget task created with
loop.create_task(...) can silently drop exceptions from
_maybe_interrupt_background_wait_for_agent; capture the Task object (e.g., task
= loop.create_task(...)), set a name if supported, and attach a done callback
(task.add_done_callback) that checks task.exception() and logs exceptions (using
the orchestrator's logger or process logger) along with context such as agent_id
and trigger="answer_now_requested"; alternatively use asyncio.ensure_future but
still attach a done-callback to surface and log any errors from
_maybe_interrupt_background_wait_for_agent.
- Around line 10448-10523: Add comprehensive unit tests exercising
Orchestrator.request_answer_now covering multiple agent states and backend
paths: create scenarios where _active_streams and _active_tasks contain agents
and where both are empty to trigger the fallback into agent_states; include
agents that are active, is_killed, and already-requested (where
post_hook.request_wrap_up() returns False) as well as agents with non-callable
request_wrap_up and missing round_timeout_hooks to hit skipped branches. Mock or
stub coordination_ui.display and ensure display.update_timeout_status(agent_id,
timeout_state) is called when get_agent_timeout_state(agent_id) returns a truthy
timeout_state; also assert that _prime_answer_now_hook_payload(agent_id) is
called for requested agents and that _maybe_interrupt_background_wait_for_agent
is scheduled on the running event loop (mock
asyncio.get_running_loop/create_task as needed). Finally, add tests for
different backend behaviors (e.g., Codex vs Claude/Gemini) by configuring
agents/backends appropriately so all return branches from request_answer_now
(requested_agents, already_requested_agents, skipped_agents) are asserted.
---
Duplicate comments:
In `@massgen/mcp_tools/hooks.py`:
- Around line 1585-1604: request_wrap_up currently sets
_manual_wrap_up_requested and returns True even when the current round has no
timeout, leaving the flag stuck; change request_wrap_up (and/or call
_get_timeout_for_current_round from it) to check that
_get_timeout_for_current_round() is not None before latching
_manual_wrap_up_requested and returning True, and return False (without setting
the flag) when timeout is None; ensure the unique symbols referenced are
request_wrap_up, _manual_wrap_up_requested, and _get_timeout_for_current_round
so the guard is added in that method and the flag never becomes stuck when no
timeout exists.
---
Nitpick comments:
In `@massgen/mcp_tools/hooks.py`:
- Around line 1597-1619: Add a Google-style docstring to the method
consume_pending_wrap_up_injection (and likewise update _build_wrap_up_injection
if missing) that briefly describes purpose, lists Args (none or any parameters
like self) and Returns (str | None) and any side effects (e.g., sets
_manual_wrap_up_requested and _soft_timeout_fired, calls
_shared_state.mark_soft_fired). Place the docstring immediately under the def
line, include a one-line summary, an Args section (if applicable), a Returns
section describing the return value, and a Notes/Side effects line describing
state changes for maintainability.
In `@massgen/orchestrator.py`:
- Line 9069: The log message uses two adjacent f-strings; merge them into a
single f-string to simplify the expression—replace the concatenated
f"[Orchestrator] Wrote {len(combined)} chars to hook file for {agent_id} "
f"({len(injection_parts)} parts)" with one f-string that includes len(combined),
agent_id, and len(injection_parts) in a single interpolation (refer to the
variables combined, agent_id, injection_parts and the surrounding log/write
call).
- Around line 8888-8894: The except block in the round timeout hook evaluator
currently catches a broad Exception in the orchestrator (the try/except around
the hook execution for agent_id), which can hide programming errors; change it
to catch a narrower set such as (RuntimeError, ValueError, TimeoutError) instead
of Exception, and update the logging to include the exception type and stack
(use logger.exception or include type(e).__name__ and traceback info) so
failures remain defensive but are easier to debug; modify the except clause and
the logger.warning call in the same handler (the block referencing agent_id and
e) accordingly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7b783a99-ccf3-47bf-88a1-c19acaff6cea
📒 Files selected for processing (3)
massgen/mcp_tools/hooks.pymassgen/orchestrator.pymassgen/tests/test_hook_framework.py
✅ Files skipped from review due to trivial changes (1)
- massgen/tests/test_hook_framework.py
docs: docs for v0.1.77
a5507203
approved these changes
Apr 15, 2026
ncrispino
approved these changes
Apr 15, 2026
Collaborator
|
@coderabbitai pause |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 23-35: Update the [0.1.77] changelog block to include the required
categories "Fixed" and "Documentation, Configurations and Resources", convert
each change bullet to include concrete file paths (e.g., the files changed by
the Answer Now button and checkpoint instructions such as
frontend/src/components/AnswerNowButton.tsx,
backend/src/agents/checkpointMcp.ts, docs/coordination.md, etc.), and correct
the PR reference from `#1062` to the release PR `#1060` (or add both if both are
relevant) in the "PRs Merged" line; edit the section header "[0.1.77]" entry to
list those added file-pathed bullets under the appropriate categories and ensure
"Contributors" and "Technical Details" remain consistent.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5cda7458-fafa-4bc1-a206-67efe46b2636
📒 Files selected for processing (12)
CHANGELOG.mdCONTRIBUTING.mdREADME.mdREADME_PYPI.mdROADMAP.mdROADMAP_v0.1.78.mddocs/announcements/archive/v0.1.76.mddocs/announcements/current-release.mddocs/announcements/github-release-v0.1.76.mddocs/announcements/github-release-v0.1.77.mddocs/source/index.rstmassgen/configs/README.md
💤 Files with no reviewable changes (1)
- docs/announcements/github-release-v0.1.76.md
✅ Files skipped from review due to trivial changes (7)
- docs/announcements/current-release.md
- docs/announcements/archive/v0.1.76.md
- CONTRIBUTING.md
- docs/announcements/github-release-v0.1.77.md
- docs/source/index.rst
- ROADMAP_v0.1.78.md
- massgen/configs/README.md
✅ Actions performedReviews paused. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Title Format
Your PR title must follow the format:
<type>: <brief description>Valid types:
fix:- Bug fixesfeat:- New featuresbreaking:- Breaking changesdocs:- Documentation updatesrefactor:- Code refactoringtest:- Test additions/modificationschore:- Maintenance tasksperf:- Performance improvementsstyle:- Code style changesci:- CI/CD configuration changesExamples:
fix: resolve memory leak in data processingfeat: add export to CSV functionalitybreaking: change API response formatdocs: update installation guideDescription
Brief description of the changes in this PR
Type of change
fix:) - Non-breaking change which fixes an issuefeat:) - Non-breaking change which adds functionalitybreaking:) - Fix or feature that would cause existing functionality to not work as expecteddocs:) - Documentation updatesrefactor:) - Code changes that neither fix a bug nor add a featuretest:) - Adding missing tests or correcting existing testschore:) - Maintenance tasks, dependency updates, etc.perf:) - Code changes that improve performancestyle:) - Changes that do not affect the meaning of the code (formatting, missing semi-colons, etc.)ci:) - Changes to CI/CD configuration files and scriptsChecklist
Pre-commit status
How to Test
Add test method for this PR.
Test CLI Command
Write down the test bash command. If there is pre-requests, please emphasize.
Expected Results
Description/screenshots of expected results.
Additional context
Add any other context about the PR here.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Documentation
Tests
Chores