Merged
Conversation
added 7 commits
March 2, 2026 15:31
…ed if generate media video has some problems
📝 WalkthroughWalkthroughThis PR introduces coordination workflow enhancements including new configuration fields for learning capture modes and quality improvement gates, provider prioritization in quickstart flows, workspace path tokenization for privacy, Docker setup improvements for Playwright, and marketing updates promoting "4x speed" messaging across documentation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 7
🧹 Nitpick comments (5)
massgen/backend/gemini.py (1)
338-370: Add defensive checks and complete the Google-style docstring.The method accesses
self._mcp_functionsandself._custom_tool_nameswithout None guards, unlike similar code elsewhere (e.g., line 1324 usesself._custom_tool_names or set()). Additionally, the docstring is missing Args and Returns sections per Google-style conventions.♻️ Suggested improvements
def _normalize_and_resolve_tool_name(self, tool_name: str) -> str: """Normalize Gemini tool names and resolve MCP aliases. Gemini can emit tool calls in different forms: - API-prefixed names (for example, ``default_api:tool``) - Canonical MCP names (``mcp__server__tool``) - Bare server/tool MCP names (``server__tool``) We normalize provider-specific prefixes first, then map bare MCP names to their canonical ``mcp__...`` form when the canonical name is registered. + + Args: + tool_name: The raw tool name from Gemini API response. + + Returns: + Normalized tool name, potentially mapped to canonical MCP form. """ normalized_name = _normalize_gemini_tool_name(tool_name or "") if not normalized_name: return normalized_name + mcp_functions = self._mcp_functions or {} + custom_tool_names = self._custom_tool_names or set() + # Keep already-known names unchanged. - if normalized_name in self._mcp_functions: + if normalized_name in mcp_functions: return normalized_name - if normalized_name in self._custom_tool_names: + if normalized_name in custom_tool_names: return normalized_name # Preserve explicit namespace prefixes when present. if normalized_name.startswith("mcp__"): return normalized_name if normalized_name.startswith("custom_tool__"): return normalized_name # Gemini may emit MCP names without the mcp__ prefix. mcp_alias = f"mcp__{normalized_name}" - if mcp_alias in self._mcp_functions: + if mcp_alias in mcp_functions: return mcp_alias return normalized_nameAs per coding guidelines: "For new or changed functions, include Google-style docstrings".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/backend/gemini.py` around lines 338 - 370, The _normalize_and_resolve_tool_name method lacks None-guards for self._mcp_functions and self._custom_tool_names and its docstring is missing Google-style Args/Returns; update the docstring to include short description plus Args (tool_name: str) and Returns (str) sections, and add defensive checks using e.g. local variables like mcp_funcs = self._mcp_functions or {} and custom_names = self._custom_tool_names or set() and then replace direct accesses to self._mcp_functions/_custom_tool_names with those locals so the function safely handles None values while preserving existing resolution logic (including checks for "mcp__" and "custom_tool__" prefixes and the mcp_alias lookup).massgen/evaluation_criteria_generator.py (1)
849-864: Update the stale “pure LLM reasoning” comment.Line 849 says this path uses “no tools,” but Line 863 explicitly keeps file-operation MCPs enabled. Please align the comment with actual behavior to avoid future misconfiguration.
✏️ Suggested comment-only cleanup
- # Simplified agent configs (no tools, pure LLM reasoning) + # Simplified agent configs (no command-line/code-based tools). + # Keep file-operation MCPs available so subagents can still write criteria.json.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/evaluation_criteria_generator.py` around lines 849 - 864, The comment "Simplified agent configs (no tools, pure LLM reasoning)" above the loop that builds simplified from agent_configs is stale because the backend dict sets "exclude_file_operation_mcps": False, so file-operation MCPs remain enabled; update that comment to accurately describe the behavior (e.g., "Simplified agent configs (no external tools/command-line MCPs; file-operation MCPs retained)") and ensure it references the code that sets enable_mcp_command_line, enable_code_based_tools, and exclude_file_operation_mcps in the simplified backend.massgen/frontend/web/server.py (1)
4871-4874: LGTM - consider extracting shared CoordinationConfig construction.The parameter is correctly added here as well. However, the
CoordinationConfigconstruction is now duplicated acrossrun_coordination(lines 4838-4884) andrun_coordination_with_history(lines 4417-4463). This makes it easy for future changes to update one location but miss the other.Consider extracting a helper function like
_build_coordination_config(coord_cfg: dict) -> CoordinationConfigto centralize this logic.
,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/frontend/web/server.py` around lines 4871 - 4874, The CoordinationConfig construction is duplicated in run_coordination and run_coordination_with_history; extract a helper function named _build_coordination_config(coord_cfg: dict) -> CoordinationConfig that encapsulates the current construction logic (including disable_final_only_round_capture_fallback and all other fields used in both locations) and replace the inline builds in run_coordination and run_coordination_with_history with calls to _build_coordination_config(coord_cfg) so both functions use the single shared factory.massgen/agent_config.py (1)
317-330: Reject unknownimprovementskeys to prevent silent quality-gate misconfiguration.Current validation checks known keys when present, but typos/unknown keys are silently accepted.
♻️ Proposed fix
@@ if not isinstance(self.improvements, dict): raise ValueError( "improvements must be a dictionary", ) - for key in ("min_transformative", "min_structural", "min_non_incremental"): + allowed_keys = {"min_transformative", "min_structural", "min_non_incremental"} + unknown_keys = set(self.improvements) - allowed_keys + if unknown_keys: + raise ValueError( + "improvements contains unknown keys: " + f"{sorted(unknown_keys)}. Allowed keys: {sorted(allowed_keys)}", + ) + + for key in allowed_keys: if key not in self.improvements: continue value = self.improvements[key]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/agent_config.py` around lines 317 - 330, The validation for self.improvements currently only checks known keys but silently accepts unknown ones; update the validation in the same block that references self.improvements and the keys ("min_transformative", "min_structural", "min_non_incremental") to detect any keys outside this allowed set and raise a ValueError listing the invalid keys (or a clear message) so typos/unknown config entries are rejected rather than ignored.massgen/frontend/displays/textual_widgets/quickstart_wizard.py (1)
313-317: Validatedefault_effortagainst available choices before persisting it.At Line [315] and Line [595],
selected_effortis set directly fromdefault_effort. If profile defaults drift from available choices, state can retain an invalid value.♻️ Proposed hardening
@@ choices = profile.get("choices", []) default_effort = profile.get("default_effort", "medium") - selected_effort = default_effort + choice_values = { + choice[1] if isinstance(choice, tuple) and len(choice) > 1 else choice + for choice in choices + } + selected_effort = ( + default_effort + if default_effort in choice_values + else next(iter(choice_values), None) + ) + if selected_effort is None: + self._current_reasoning_effort = None + self._set_reasoning_visibility(False) + returnAlso applies to: 593-597
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/frontend/displays/textual_widgets/quickstart_wizard.py` around lines 313 - 317, Validate that profile["default_effort"] exists in the profile["choices"] before assigning it to state: obtain choices = profile.get("choices", []), get default_effort = profile.get("default_effort", "medium"), then set selected_effort = default_effort if default_effort in choices else (choices[0] if choices else "medium"); finally assign self._current_reasoning_effort = selected_effort. Apply the same validation logic wherever selected_effort/default_effort is used (e.g., the other occurrence assigning self._current_reasoning_effort) to avoid persisting an invalid value.
🤖 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/agent_config.py`:
- Around line 87-93: The docstring describing the learning_capture_mode options
is stale: it claims "final_only" is the default but the actual default value for
learning_capture_mode is "verification_and_final_only"; update the docstring
text near the learning_capture_mode description to state that
"verification_and_final_only" is the default (and adjust any explanatory bullets
if needed to match that default behavior), referencing the learning_capture_mode
setting so you change the documented default to align with the runtime default.
- Around line 311-313: Add a Google-style docstring to the newly introduced
_validate_improvements method to describe its purpose and contract; include a
short one-line summary ("Validate improvements quality-gate configuration."), an
Args section documenting self, a Returns section stating None, and a Raises
section noting the exception type raised on invalid configuration (e.g.,
ValueError) so callers know what to expect; place this docstring immediately
under the def _validate_improvements(self): signature in
massgen/agent_config.py.
In `@massgen/cli.py`:
- Around line 462-473: Update the docstrings for _format_chunk_target_line and
should_include_quick_edit_hint to use Google-style format: add an Args section
describing the parameter (target_chunks: int | None for
_format_chunk_target_line, planning_turn_mode: str | None for
should_include_quick_edit_hint) and a Returns section describing the return type
and meaning (str for both; describe the exact string semantics). Keep the
existing one-line summary, then append the Args and Returns blocks following
Google-style conventions.
In `@massgen/coordination_tracker.py`:
- Around line 267-275: get_path_token currently returns a freshly generated
token for unknown agent_ids without persisting it, so repeated calls can return
different tokens; modify get_path_token to generate a token with
secrets.token_hex(4) only when agent_id is not present in self._path_tokens,
store that token into self._path_tokens[agent_id], and then return it (keep the
existing behavior for registered agent_ids); reference: get_path_token and the
_path_tokens mapping.
In `@massgen/filesystem_manager/_filesystem_manager.py`:
- Around line 429-440: The code uses workspace_token directly in filesystem
paths (used in _setup_workspace and when building
snapshot_storage/agent_temporary_workspace), which allows absolute paths or
path-traversal; add a validation/sanitization step (e.g., a new helper
validate_or_sanitize_workspace_token(token)) and call it before assigning
self.workspace_token or passing it into _setup_workspace or Path concatenation;
the validator should reject/normalize absolute paths, strip path separators,
forbid '..' segments, and permit only a safe whitelist (e.g., alphanumeric,
dash, underscore) or replace invalid tokens with a safe fallback (UUID/hex);
update usage sites (workspace_token assignment and the _setup_workspace call
that constructs self.agent_temporary_workspace from
self.agent_temporary_workspace_parent / self.workspace_token) to use the
validated token and raise/log an error if validation fails.
In `@massgen/frontend/displays/textual_widgets/quickstart_wizard.py`:
- Around line 459-464: Replace the empty bare-except blocks that swallow errors
when restoring reasoning state (the try/except around assigning
self._reasoning_select.value and setting self._current_reasoning_effort) with
explicit exception handling that logs the exception; e.g., catch Exception as e
and call an appropriate logger (self._logger.exception(...) if available,
otherwise use the logging module via logging.exception(...)) with a clear
message like "failed to restore reasoning effort" and include the saved value,
and keep this change in both places where the silent except appears so failures
are visible for debugging.
- Line 1815: The new step-branching logic (the skip_condition lambda using
state.get("agent_count", 3) and state.get("setup_mode"), plus the dynamic
insertion behavior around lines 1890-1923) needs explicit regression tests: add
unit/integration tests that exercise the quickstart wizard flow for (a)
agent_count == 1, (b) agent_count > 1 with setup_mode == "same", and (c)
agent_count > 1 with setup_mode == "different", asserting the expected steps are
skipped or inserted accordingly; in tests call the same entry points that drive
the wizard (the quickstart wizard handler/orchestrator entry used in this diff)
and manipulate state["agent_count"] and state["setup_mode"] to verify
skip_condition and the dynamic insertion paths, failing the PR if any of the
three scenarios diverge.
---
Nitpick comments:
In `@massgen/agent_config.py`:
- Around line 317-330: The validation for self.improvements currently only
checks known keys but silently accepts unknown ones; update the validation in
the same block that references self.improvements and the keys
("min_transformative", "min_structural", "min_non_incremental") to detect any
keys outside this allowed set and raise a ValueError listing the invalid keys
(or a clear message) so typos/unknown config entries are rejected rather than
ignored.
In `@massgen/backend/gemini.py`:
- Around line 338-370: The _normalize_and_resolve_tool_name method lacks
None-guards for self._mcp_functions and self._custom_tool_names and its
docstring is missing Google-style Args/Returns; update the docstring to include
short description plus Args (tool_name: str) and Returns (str) sections, and add
defensive checks using e.g. local variables like mcp_funcs = self._mcp_functions
or {} and custom_names = self._custom_tool_names or set() and then replace
direct accesses to self._mcp_functions/_custom_tool_names with those locals so
the function safely handles None values while preserving existing resolution
logic (including checks for "mcp__" and "custom_tool__" prefixes and the
mcp_alias lookup).
In `@massgen/evaluation_criteria_generator.py`:
- Around line 849-864: The comment "Simplified agent configs (no tools, pure LLM
reasoning)" above the loop that builds simplified from agent_configs is stale
because the backend dict sets "exclude_file_operation_mcps": False, so
file-operation MCPs remain enabled; update that comment to accurately describe
the behavior (e.g., "Simplified agent configs (no external tools/command-line
MCPs; file-operation MCPs retained)") and ensure it references the code that
sets enable_mcp_command_line, enable_code_based_tools, and
exclude_file_operation_mcps in the simplified backend.
In `@massgen/frontend/displays/textual_widgets/quickstart_wizard.py`:
- Around line 313-317: Validate that profile["default_effort"] exists in the
profile["choices"] before assigning it to state: obtain choices =
profile.get("choices", []), get default_effort = profile.get("default_effort",
"medium"), then set selected_effort = default_effort if default_effort in
choices else (choices[0] if choices else "medium"); finally assign
self._current_reasoning_effort = selected_effort. Apply the same validation
logic wherever selected_effort/default_effort is used (e.g., the other
occurrence assigning self._current_reasoning_effort) to avoid persisting an
invalid value.
In `@massgen/frontend/web/server.py`:
- Around line 4871-4874: The CoordinationConfig construction is duplicated in
run_coordination and run_coordination_with_history; extract a helper function
named _build_coordination_config(coord_cfg: dict) -> CoordinationConfig that
encapsulates the current construction logic (including
disable_final_only_round_capture_fallback and all other fields used in both
locations) and replace the inline builds in run_coordination and
run_coordination_with_history with calls to
_build_coordination_config(coord_cfg) so both functions use the single shared
factory.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9ed033a5-affe-40a2-b986-890112cabe2b
⛔ Files ignored due to path filters (106)
docs/source/_static/images/readme.gifis excluded by!**/*.gif,!**/*.gifmassgen/frontend/web/static/assets/_baseUniq-AVQuLNgG.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/_baseUniq-AVQuLNgG.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/arc-DPg5OnLA.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/arc-DPg5OnLA.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/architectureDiagram-VXUJARFQ-D3lxBS0w.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/architectureDiagram-VXUJARFQ-D3lxBS0w.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/base-80a1f760-BjKqezyN.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/base-80a1f760-BjKqezyN.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/blockDiagram-VD42YOAC-Df0BbuW_.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/blockDiagram-VD42YOAC-Df0BbuW_.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/c4Diagram-YG6GDRKO-CAQ4nRHx.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/c4Diagram-YG6GDRKO-CAQ4nRHx.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/channel-BhzQJK09.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/channel-BhzQJK09.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/channel-D0pBKfCz.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-4BX2VUAB-B20LnYuB.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-4BX2VUAB-B20LnYuB.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-55IACEB6-BWGMW8Vm.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-55IACEB6-BWGMW8Vm.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-B4BG7PRW-ClZ5TgR0.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-B4BG7PRW-ClZ5TgR0.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-DI55MBZ5-Dc2ha-30.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-DI55MBZ5-Dc2ha-30.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-FMBD7UC4-yiugaJUa.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-FMBD7UC4-yiugaJUa.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-QN33PNHL-CIcC1sfw.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-QN33PNHL-CIcC1sfw.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-QZHKN3VN-CuDZpdaD.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-QZHKN3VN-CuDZpdaD.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-TZMSLE5B-D655xVqH.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-TZMSLE5B-D655xVqH.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/classDiagram-2ON5EDUG-B8iqM3hs.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/classDiagram-2ON5EDUG-XgpY-eTv.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/classDiagram-2ON5EDUG-XgpY-eTv.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/classDiagram-v2-WZHVMYZB-B8iqM3hs.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/classDiagram-v2-WZHVMYZB-XgpY-eTv.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/classDiagram-v2-WZHVMYZB-XgpY-eTv.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/clone-DSpvyh0c.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/clone-DSpvyh0c.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/clone-dVHY6V2C.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/consoleHook-59e792cb-KmOPYMUg.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/consoleHook-59e792cb-KmOPYMUg.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/cose-bilkent-S5V4N54A-Dq3BKwwh.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/cose-bilkent-S5V4N54A-Dq3BKwwh.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/dagre-6UL2VRFP-CmrIRMzL.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/dagre-6UL2VRFP-CmrIRMzL.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/diagram-PSM6KHXK-BQgucv5d.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/diagram-PSM6KHXK-BQgucv5d.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/diagram-QEK2KX5R-Catj1rLJ.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/diagram-QEK2KX5R-Catj1rLJ.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/diagram-S2PKOQOG-VXJN1JaO.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/diagram-S2PKOQOG-VXJN1JaO.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/erDiagram-Q2GNP2WA-CPGS8tEN.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/erDiagram-Q2GNP2WA-CPGS8tEN.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/flowDiagram-NV44I4VS-hAegpCjE.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/flowDiagram-NV44I4VS-hAegpCjE.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/ganttDiagram-JELNMOA3-C-7qBc2P.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/ganttDiagram-JELNMOA3-C-7qBc2P.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/gitGraphDiagram-NY62KEGX-D9qnlshv.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/gitGraphDiagram-NY62KEGX-D9qnlshv.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/graph-B6okqyVZ.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/graph-B6okqyVZ.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/index-599aeaf7-JXrsLrix.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/index-599aeaf7-JXrsLrix.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/index-BOesCYHe.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/index-BOesCYHe.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/index-C5CsgCb4.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/index-C5CsgCb4.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/index-D2vUXYM2.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/index-D2vUXYM2.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/infoDiagram-WHAUD3N6-B_3Q_RkO.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/infoDiagram-WHAUD3N6-B_3Q_RkO.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/journeyDiagram-XKPGCS4Q-CeNyXUtm.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/journeyDiagram-XKPGCS4Q-CeNyXUtm.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/kanban-definition-3W4ZIXB7-103x-s8b.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/kanban-definition-3W4ZIXB7-103x-s8b.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/layout-GkxW5DBM.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/layout-GkxW5DBM.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/linear-BSHJYnL5.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/linear-BSHJYnL5.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/min-CRnWvUj0.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/min-CRnWvUj0.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/mindmap-definition-VGOIOE7T-0_XShANc.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/mindmap-definition-VGOIOE7T-0_XShANc.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/pieDiagram-ADFJNKIX-_2PFyUu6.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/pieDiagram-ADFJNKIX-_2PFyUu6.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/quadrantDiagram-AYHSOK5B-vw--f_Dw.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/quadrantDiagram-AYHSOK5B-vw--f_Dw.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/requirementDiagram-UZGBJVZJ-9aYMn9Fu.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/requirementDiagram-UZGBJVZJ-9aYMn9Fu.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/sankeyDiagram-TZEHDZUN-3I1RkOCW.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/sankeyDiagram-TZEHDZUN-3I1RkOCW.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/sequenceDiagram-WL72ISMW-BEwD3Anh.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/sequenceDiagram-WL72ISMW-BEwD3Anh.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/stateDiagram-FKZM4ZOC-Dmc3fwx3.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/stateDiagram-FKZM4ZOC-Dmc3fwx3.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/stateDiagram-v2-4FDKWEC3-Cwy05MBx.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/stateDiagram-v2-4FDKWEC3-Cwy05MBx.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/stateDiagram-v2-4FDKWEC3-DTF_pzMQ.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/timeline-definition-IT6M3QCI-DlU2EEc7.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/timeline-definition-IT6M3QCI-DlU2EEc7.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/treemap-KMMF4GRG-1GASY6Y-.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/treemap-KMMF4GRG-1GASY6Y-.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/xychartDiagram-PRI3JC2R-CaJQ0NZo.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/xychartDiagram-PRI3JC2R-CaJQ0NZo.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.map
📒 Files selected for processing (67)
README.mdREADME_PYPI.mddocs/modules/coordination_workflow.mddocs/source/index.rstmassgen/agent_config.pymassgen/api_params_handler/_api_params_handler_base.pymassgen/backend/base.pymassgen/backend/capabilities.pymassgen/backend/gemini.pymassgen/cli.pymassgen/config_builder.pymassgen/config_validator.pymassgen/coordination_tracker.pymassgen/docker/Dockerfile.sudomassgen/evaluation_criteria_generator.pymassgen/filesystem_manager/_docker_manager.pymassgen/filesystem_manager/_filesystem_manager.pymassgen/frontend/displays/textual_widgets/quickstart_wizard.pymassgen/frontend/web/server.pymassgen/frontend/web/static/index.htmlmassgen/mcp_tools/checklist_tools_server.pymassgen/mcp_tools/custom_tools_server.pymassgen/mcp_tools/planning/_planning_mcp_server.pymassgen/mcp_tools/planning/planning_dataclasses.pymassgen/orchestrator.pymassgen/skills/video-generation/SKILL.mdmassgen/skills/video-generation/references/production.mdmassgen/subagent/manager.pymassgen/subagent_types/builder/SUBAGENT.mdmassgen/subagent_types/evaluator/SUBAGENT.mdmassgen/subagent_types/novelty/SUBAGENT.mdmassgen/subagent_types/quality_rethinking/SUBAGENT.mdmassgen/system_message_builder.pymassgen/system_prompt_sections.pymassgen/tests/frontend/test_quickstart_wizard_config_filename.pymassgen/tests/test_answer_count_increment.pymassgen/tests/test_api_params_exclusion.pymassgen/tests/test_backend_capabilities.pymassgen/tests/test_changedoc_system_prompt.pymassgen/tests/test_checklist_tools_server.pymassgen/tests/test_concurrent_logging_sessions.pymassgen/tests/test_config_builder.pymassgen/tests/test_config_validator.pymassgen/tests/test_convergence_novelty.pymassgen/tests/test_coordination_improvements_config.pymassgen/tests/test_custom_tools_server_background.pymassgen/tests/test_docker_skills_write_access.pymassgen/tests/test_evaluation_criteria_generator.pymassgen/tests/test_filesystem_manager_workspace_token.pymassgen/tests/test_gemini_tool_name_normalization.pymassgen/tests/test_novelty_injection.pymassgen/tests/test_plan_mode_chunk_defaults.pymassgen/tests/test_plan_review_prompt_refinement.pymassgen/tests/test_planning_injection.pymassgen/tests/test_planning_tools.pymassgen/tests/test_read_media_analysis.pymassgen/tests/test_refinement_quality.pymassgen/tests/test_specialized_subagents.pymassgen/tests/test_subagent_docker_logs.pymassgen/tests/test_subagent_manager.pymassgen/tests/test_system_message_builder_phases.pymassgen/tests/test_system_prompt_sections.pymassgen/tests/unit/test_coordination_tracker.pymassgen/tool/_multimodal_tools/backend_selector.pymassgen/tool/_multimodal_tools/read_media.pymassgen/tool/_multimodal_tools/understand_video.pywebui/src/stores/wizardStore.ts
Comment on lines
+87
to
93
| - "round": full round-time capture (evolving skills + memory reminders/tasks) | ||
| - "verification_and_final_only": round-time verification replay memo only; | ||
| full consolidation remains presenter/final-time | ||
| - "final_only": defer all learning capture to presenter/final-time | ||
| - "round": Existing behavior. Capture can be produced in coordination rounds. | ||
| - "final_only" (default): Keep changedoc behavior, but defer evolving-skill + memory production | ||
| to the final presenter stage. Coordination rounds remain read-focused. |
There was a problem hiding this comment.
Fix the learning_capture_mode default documented in the docstring.
Lines [91-93] still describe "final_only" as default, but Line [181] sets "verification_and_final_only" as default.
📝 Proposed doc fix
@@
- learning_capture_mode: Controls when evolving-skill + memory capture is produced.
- - "round": full round-time capture (evolving skills + memory reminders/tasks)
- - "verification_and_final_only": round-time verification replay memo only;
- full consolidation remains presenter/final-time
- - "final_only": defer all learning capture to presenter/final-time
- - "round": Existing behavior. Capture can be produced in coordination rounds.
- - "final_only" (default): Keep changedoc behavior, but defer evolving-skill + memory production
- to the final presenter stage. Coordination rounds remain read-focused.
+ learning_capture_mode: Controls when evolving-skill + memory capture is produced.
+ - "round": Existing behavior. Capture can be produced in coordination rounds.
+ - "verification_and_final_only" (default): round-time verification replay memo only;
+ full consolidation remains presenter/final-time.
+ - "final_only": defer all learning capture to presenter/final-time.Also applies to: 181-181
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/agent_config.py` around lines 87 - 93, The docstring describing the
learning_capture_mode options is stale: it claims "final_only" is the default
but the actual default value for learning_capture_mode is
"verification_and_final_only"; update the docstring text near the
learning_capture_mode description to state that "verification_and_final_only" is
the default (and adjust any explanatory bullets if needed to match that default
behavior), referencing the learning_capture_mode setting so you change the
documented default to align with the runtime default.
Comment on lines
+311
to
+313
| def _validate_improvements(self): | ||
| """Validate improvements quality-gate configuration.""" | ||
| if self.improvements is None: |
There was a problem hiding this comment.
Add a Google-style docstring for the new validator.
_validate_improvements is newly introduced but does not use a Google-style docstring.
📝 Proposed docstring shape
@@
def _validate_improvements(self):
- """Validate improvements quality-gate configuration."""
+ """Validate improvements quality-gate configuration.
+
+ Raises:
+ ValueError: If `improvements` is not a dict, contains unknown keys,
+ or has invalid threshold values.
+ """As per coding guidelines "**/*.py: For new or changed functions, include Google-style docstrings"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _validate_improvements(self): | |
| """Validate improvements quality-gate configuration.""" | |
| if self.improvements is None: | |
| def _validate_improvements(self): | |
| """Validate improvements quality-gate configuration. | |
| Raises: | |
| ValueError: If `improvements` is not a dict, contains unknown keys, | |
| or has invalid threshold values. | |
| """ | |
| if self.improvements is None: |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/agent_config.py` around lines 311 - 313, Add a Google-style docstring
to the newly introduced _validate_improvements method to describe its purpose
and contract; include a short one-line summary ("Validate improvements
quality-gate configuration."), an Args section documenting self, a Returns
section stating None, and a Raises section noting the exception type raised on
invalid configuration (e.g., ValueError) so callers know what to expect; place
this docstring immediately under the def _validate_improvements(self): signature
in massgen/agent_config.py.
Comment on lines
+462
to
+473
| def _format_chunk_target_line(target_chunks: int | None) -> str: | ||
| """Return chunk guidance text for planning/spec prompts.""" | ||
| if target_chunks == 1 or target_chunks is None: | ||
| return "- Target chunks: exactly 1" | ||
| if target_chunks > 1: | ||
| return f"- Target chunks: around {target_chunks}" | ||
| return "- Target chunks: exactly 1" | ||
|
|
||
|
|
||
| def should_include_quick_edit_hint(planning_turn_mode: str | None) -> bool: | ||
| """Show quick-edit hint only for explicit single-turn refinement mode.""" | ||
| return planning_turn_mode == "single" |
There was a problem hiding this comment.
Use Google-style docstrings for the new helper functions.
Line 462 and Line 471 add new functions, but their docstrings omit Google-style Args/Returns sections required for changed Python functions in this repo.
✍️ Proposed docstring update
def _format_chunk_target_line(target_chunks: int | None) -> str:
- """Return chunk guidance text for planning/spec prompts."""
+ """Return chunk guidance text for planning/spec prompts.
+
+ Args:
+ target_chunks: Desired number of chunks, or None for default behavior.
+
+ Returns:
+ Chunk guidance line formatted for inclusion in prompts.
+ """
@@
def should_include_quick_edit_hint(planning_turn_mode: str | None) -> bool:
- """Show quick-edit hint only for explicit single-turn refinement mode."""
+ """Determine whether to include the quick-edit hint.
+
+ Args:
+ planning_turn_mode: Planning turn mode string, if set.
+
+ Returns:
+ True when quick-edit hint should be included; otherwise False.
+ """
return planning_turn_mode == "single"As per coding guidelines: **/*.py: For new or changed functions, include Google-style docstrings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/cli.py` around lines 462 - 473, Update the docstrings for
_format_chunk_target_line and should_include_quick_edit_hint to use Google-style
format: add an Args section describing the parameter (target_chunks: int | None
for _format_chunk_target_line, planning_turn_mode: str | None for
should_include_quick_edit_hint) and a Returns section describing the return type
and meaning (str for both; describe the exact string semantics). Keep the
existing one-line summary, then append the Args and Returns blocks following
Google-style conventions.
Comment on lines
+267
to
+275
| def get_path_token(self, agent_id: str) -> str: | ||
| """Get stable anonymous path token for this agent's workspace directories. | ||
|
|
||
| Returns an 8-character hex string that is stable within a session but | ||
| does not reveal the real agent_id. Falls back to generating a new token | ||
| if the agent_id was not registered at session initialization. | ||
| """ | ||
| return self._path_tokens.get(agent_id, secrets.token_hex(4)) | ||
|
|
There was a problem hiding this comment.
Persist fallback tokens to keep path mapping stable.
Line 274 generates a new token for unknown agents but does not store it, so repeated calls can return different values for the same agent_id.
🔧 Suggested fix
def get_path_token(self, agent_id: str) -> str:
@@
- return self._path_tokens.get(agent_id, secrets.token_hex(4))
+ token = self._path_tokens.get(agent_id)
+ if token is None:
+ token = secrets.token_hex(4)
+ self._path_tokens[agent_id] = token
+ return token📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def get_path_token(self, agent_id: str) -> str: | |
| """Get stable anonymous path token for this agent's workspace directories. | |
| Returns an 8-character hex string that is stable within a session but | |
| does not reveal the real agent_id. Falls back to generating a new token | |
| if the agent_id was not registered at session initialization. | |
| """ | |
| return self._path_tokens.get(agent_id, secrets.token_hex(4)) | |
| def get_path_token(self, agent_id: str) -> str: | |
| """Get stable anonymous path token for this agent's workspace directories. | |
| Returns an 8-character hex string that is stable within a session but | |
| does not reveal the real agent_id. Falls back to generating a new token | |
| if the agent_id was not registered at session initialization. | |
| """ | |
| token = self._path_tokens.get(agent_id) | |
| if token is None: | |
| token = secrets.token_hex(4) | |
| self._path_tokens[agent_id] = token | |
| return token |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/coordination_tracker.py` around lines 267 - 275, get_path_token
currently returns a freshly generated token for unknown agent_ids without
persisting it, so repeated calls can return different tokens; modify
get_path_token to generate a token with secrets.token_hex(4) only when agent_id
is not present in self._path_tokens, store that token into
self._path_tokens[agent_id], and then return it (keep the existing behavior for
registered agent_ids); reference: get_path_token and the _path_tokens mapping.
Comment on lines
+429
to
440
| # Use token for temp workspace path to avoid leaking real agent_id (MAS-338) | ||
| self.workspace_token = workspace_token or agent_id | ||
|
|
||
| # Setup snapshot storage if provided | ||
| if snapshot_storage and self.agent_id: | ||
| self.snapshot_storage = Path(snapshot_storage) / self.agent_id | ||
| self.snapshot_storage.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| # Setup temporary workspace for context sharing | ||
| # Setup temporary workspace for context sharing (uses token to hide real agent_id) | ||
| if agent_temporary_workspace and self.agent_id: | ||
| self.agent_temporary_workspace = self._setup_workspace(self.agent_temporary_workspace_parent / self.agent_id) | ||
| self.agent_temporary_workspace = self._setup_workspace(self.agent_temporary_workspace_parent / self.workspace_token) | ||
|
|
There was a problem hiding this comment.
Validate workspace_token before using it in filesystem paths.
Line 430 accepts arbitrary token text, and Line 439 feeds it into _setup_workspace(), which clears directory contents. A malformed token (e.g., absolute path or traversal segments) can target unintended directories.
🛡️ Proposed hardening
self.agent_id = agent_id
- # Use token for temp workspace path to avoid leaking real agent_id (MAS-338)
- self.workspace_token = workspace_token or agent_id
+ # Use token for temp workspace path to avoid leaking real agent_id (MAS-338)
+ token = (workspace_token or agent_id).strip()
+ token_path = Path(token)
+ if not token or token_path.is_absolute() or len(token_path.parts) != 1 or token in {".", ".."}:
+ raise ValueError(f"Invalid workspace_token: {workspace_token!r}")
+ self.workspace_token = token
@@
# Setup temporary workspace for context sharing (uses token to hide real agent_id)
if agent_temporary_workspace and self.agent_id:
- self.agent_temporary_workspace = self._setup_workspace(self.agent_temporary_workspace_parent / self.workspace_token)
+ if not self.agent_temporary_workspace_parent:
+ raise ValueError("agent_temporary_workspace_parent must be configured when temp workspace is enabled")
+ temp_parent = Path(self.agent_temporary_workspace_parent).resolve()
+ temp_workspace_path = (temp_parent / self.workspace_token).resolve()
+ try:
+ temp_workspace_path.relative_to(temp_parent)
+ except ValueError as exc:
+ raise ValueError(
+ f"workspace_token escapes temp workspace parent: {self.workspace_token!r}",
+ ) from exc
+ self.agent_temporary_workspace = self._setup_workspace(str(temp_workspace_path))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/filesystem_manager/_filesystem_manager.py` around lines 429 - 440,
The code uses workspace_token directly in filesystem paths (used in
_setup_workspace and when building snapshot_storage/agent_temporary_workspace),
which allows absolute paths or path-traversal; add a validation/sanitization
step (e.g., a new helper validate_or_sanitize_workspace_token(token)) and call
it before assigning self.workspace_token or passing it into _setup_workspace or
Path concatenation; the validator should reject/normalize absolute paths, strip
path separators, forbid '..' segments, and permit only a safe whitelist (e.g.,
alphanumeric, dash, underscore) or replace invalid tokens with a safe fallback
(UUID/hex); update usage sites (workspace_token assignment and the
_setup_workspace call that constructs self.agent_temporary_workspace from
self.agent_temporary_workspace_parent / self.workspace_token) to use the
validated token and raise/log an error if validation fails.
Comment on lines
+459
to
+464
| if saved_reasoning_effort and self._reasoning_select: | ||
| try: | ||
| self._reasoning_select.value = saved_reasoning_effort | ||
| self._current_reasoning_effort = saved_reasoning_effort | ||
| except Exception: | ||
| pass |
There was a problem hiding this comment.
Stop swallowing reasoning-restore errors silently.
At Line [463] and Line [770], exceptions are ignored, which makes state-restore failures invisible and hard to debug.
🛠️ Proposed fix
@@
if saved_reasoning_effort and self._reasoning_select:
try:
self._reasoning_select.value = saved_reasoning_effort
self._current_reasoning_effort = saved_reasoning_effort
- except Exception:
- pass
+ except Exception as e:
+ _quickstart_log(
+ "ProviderModelStep.set_value: failed to restore "
+ f"reasoning_effort={saved_reasoning_effort!r}: {e}"
+ )
@@
if reasoning_select and reasoning_effort:
try:
reasoning_select.value = reasoning_effort
self._tab_selections[agent_key]["reasoning_effort"] = reasoning_effort
- except Exception:
- pass
+ except Exception as e:
+ _quickstart_log(
+ "TabbedProviderModelStep.set_value: failed to restore "
+ f"agent={agent_key} reasoning_effort={reasoning_effort!r}: {e}"
+ )Also applies to: 765-771
🧰 Tools
🪛 Ruff (0.15.2)
[error] 463-464: try-except-pass detected, consider logging the exception
(S110)
[warning] 463-463: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/frontend/displays/textual_widgets/quickstart_wizard.py` around lines
459 - 464, Replace the empty bare-except blocks that swallow errors when
restoring reasoning state (the try/except around assigning
self._reasoning_select.value and setting self._current_reasoning_effort) with
explicit exception handling that logs the exception; e.g., catch Exception as e
and call an appropriate logger (self._logger.exception(...) if available,
otherwise use the logging module via logging.exception(...)) with a clear
message like "failed to restore reasoning effort" and include the saved value,
and keep this change in both places where the silent except appears so failures
are visible for debugging.
| description="Choose your AI provider and model", | ||
| component_class=ProviderModelStep, | ||
| skip_condition=lambda state: state.get("setup_mode") == "different", | ||
| skip_condition=lambda state: state.get("agent_count", 3) > 1 and state.get("setup_mode") == "different", |
There was a problem hiding this comment.
Add regression tests for the new step-branching behavior before merge.
Lines [1815] and [1890-1923] materially change navigation flow (agent_count × setup_mode × dynamic insertion). Please add explicit coverage for:
- 1 agent
- multi-agent +
same - multi-agent +
different
Based on learnings: "Any change touching orchestrator.py, chat_agent.py, backend/*.py, mcp_tools/, or coordination_tracker.py is always non-trivial and requires TDD. Any change to TUI widgets, content handlers, or event processing is always non-trivial. Any change to system prompts, config validation, or YAML parsing is always non-trivial"
Also applies to: 1890-1923
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/frontend/displays/textual_widgets/quickstart_wizard.py` at line 1815,
The new step-branching logic (the skip_condition lambda using
state.get("agent_count", 3) and state.get("setup_mode"), plus the dynamic
insertion behavior around lines 1890-1923) needs explicit regression tests: add
unit/integration tests that exercise the quickstart wizard flow for (a)
agent_count == 1, (b) agent_count > 1 with setup_mode == "same", and (c)
agent_count > 1 with setup_mode == "different", asserting the expected steps are
skipped or inserted accordingly; in tests call the same entry points that drive
the wizard (the quickstart wizard handler/orchestrator entry used in this diff)
and manipulate state["agent_count"] and state["setup_mode"] to verify
skip_condition and the dynamic insertion paths, failing the PR if any of the
three scenarios diverge.
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
2 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
Release Notes
New Features
Improvements
Documentation