Merged
Conversation
… refinement guard; try personas; prompt improvmeent there
📝 WalkthroughWalkthroughThe pull request adds prompt improvement and evaluator persona coordination features, introduces step mode documentation, implements workspace path rewriting and agent ID scrubbing utilities, and enhances the web and terminal UIs with parallel pre-collaboration phase orchestration, improved prompt display, historical session replay, and mode override propagation. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
massgen/filesystem_manager/_filesystem_manager.py (1)
2342-2375:⚠️ Potential issue | 🟠 MajorAdd
.claudeto_WORKSPACE_METADATA_DIRStoo.This block now classifies
.claudeas framework metadata when copying temp snapshots, buthas_meaningful_content()still treats.claude/as deliverable content. A workspace containing only.claude/can therefore overwrite/favor an effectively empty snapshot instead of falling back tosnapshot_storage.💡 Minimal fix
-_WORKSPACE_METADATA_DIRS = frozenset({".git", ".codex", ".gemini", ".massgen", "memory"}) +_WORKSPACE_METADATA_DIRS = frozenset({".git", ".codex", ".gemini", ".claude", ".massgen", "memory"})As per coding guidelines,
Backends that create config directories in the workspace (e.g., .codex/ for Codex) must add those directory names to the _metadata_dirs set in FilesystemManager.save_snapshot().🤖 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 2342 - 2375, The snapshot-copy logic already excludes ".claude" via _snapshot_exclude_dirs but has_meaningful_content() still treats ".claude/" as deliverable content; update the workspace metadata set used when deciding meaningful snapshots by adding ".claude" to _WORKSPACE_METADATA_DIRS (the set referenced in FilesystemManager.save_snapshot() / has_meaningful_content()) so that snapshots containing only ".claude/" are considered metadata-only and will fall back to snapshot_storage; locate the _WORKSPACE_METADATA_DIRS (or equivalent _metadata_dirs) definition and append "'.claude'" to it and ensure any tests expecting metadata behavior are updated accordingly.
🧹 Nitpick comments (3)
massgen/frontend/displays/web_display.py (1)
124-151: Consider adding debug-level logging for swallowed exceptions.While the defensive
try-except-passpattern is intentional here (EventEmitter integration is optional and failures shouldn't crash the display), completely silent exception handling can make debugging difficult when things go wrong.♻️ Proposed improvement: Add debug logging
def _register_event_listener(self) -> None: """Register as a listener on the EventEmitter to forward structured events.""" try: from massgen.logger_config import get_event_emitter emitter = get_event_emitter() if emitter: self._event_listener = self._on_structured_event emitter.add_listener(self._event_listener) - except Exception: - pass + except Exception as e: + import logging + logging.getLogger(__name__).debug( + "WebDisplay: EventEmitter registration skipped: %s", e + ) def _on_structured_event(self, event: Any) -> None: """Forward a structured MassGenEvent over WebSocket.""" if self._closed: return try: self._emit( "structured_event", { "event_type": event.event_type, "agent_id": event.agent_id, "round_number": event.round_number, "data": event.data, }, ) - except Exception: - pass + except Exception as e: + import logging + logging.getLogger(__name__).debug( + "WebDisplay: Failed to forward structured event: %s", e + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@massgen/frontend/displays/web_display.py` around lines 124 - 151, Replace the silent "except Exception: pass" blocks in _register_event_listener and _on_structured_event with debug-level logging so swallowed exceptions are recorded for troubleshooting: catch the exception as e (e.g., "except Exception as e"), import or use the module logger (or massgen.logger_config's logger) and call logger.debug with a short message that includes the function name and exception details (for example reference _register_event_listener, _event_listener, get_event_emitter, and in _on_structured_event reference _emit and _closed) so failures remain non-fatal but are visible in debug logs.massgen/frontend/displays/textual_terminal_display.py (1)
6454-6565: Add Google-style docstrings to the new pre-collab helpers.This block introduces the core orchestration helpers for the unified pre-collab flow, but their docstrings still omit
Args:/ side-effect details. Please align them with the repository docstring rule while these APIs are still fresh.As 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/frontend/displays/textual_terminal_display.py` around lines 6454 - 6565, Update the four new helper methods (_try_open_unified_precollab_screen, _open_unified_precollab_screen, _wait_and_open_unified_screen, set_precollab_batch) to include Google-style docstrings that document Args (including types and defaults for attempt and subagents/pre_collab_ids), the return value (None), and important side effects (mutating self._parallel_precollab_expected, self._parallel_precollab_screen_opened, timers via set_timer, pushing screens via push_screen, and callbacks used like _subagent_message_callback/_subagent_continue_callback); also note any timeouts/retry behavior and conditions under which the method exits early (e.g., when screen already opened or no subagents) so callers and maintainers have clear behavioral contracts.massgen/frontend/web/server.py (1)
120-219: Use Google-style docstrings for the new replay helpers.These new/changed functions still use summary-only docstrings, so they miss the required
Args:/Returns:sections for repo Python code.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/frontend/web/server.py` around lines 120 - 219, The three helper functions (_find_latest_attempt, _scan_log_dirs, _scan_log_dirs_cached) currently have summary-only docstrings; update each to Google-style docstrings including an "Args:" section describing parameters (e.g., log_dir: Path or logs_root: Path) and a "Returns:" section describing the return type and meaning (e.g., Path | None for _find_latest_attempt, list[dict[str, Any]] for the scanners), and where helpful a short "Raises:" note if exceptions are expected; keep the existing summary and examples of keys (session_id, question, config, etc.) in the _scan_log_dirs Returns description to match repo docstring style.
🤖 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 220-222: AgentConfig currently drops prompt_improver and
enable_evaluator_personas when serializing and rehydrates prompt_improver as a
plain dict; update AgentConfig.to_dict to include the prompt_improver field
(serialize it properly, e.g., call its to_dict or similar) and include
enable_evaluator_personas in the returned dict, and update AgentConfig.from_dict
to detect if prompt_improver is a dict and instantiate a PromptImproverConfig
from it (or accept an existing PromptImproverConfig), ensuring
enable_evaluator_personas is read back into the AgentConfig instance so
saved/resumed configs preserve these values; reference the prompt_improver
attribute and the AgentConfig.to_dict / AgentConfig.from_dict methods while
making the changes.
In `@massgen/cli.py`:
- Around line 10078-10086: The ACTION and STATUS machine-readable lines are
printed after calling orchestrator.finalize_step_mode, which can delay or fail
the handoff; move the _automation_print calls (using action_data['action'] and
the STATUS path built from args.session_dir and real_agent_id) to occur before
orchestrator.finalize_step_mode(log_session_dir) so the step result is emitted
immediately (ensure you still call finalize_step_mode afterwards to save extra
artifacts).
- Around line 11743-11750: The code appends "/?v=2" to browser_url which breaks
existing query params (variable browser_url in massgen/cli.py); instead parse
browser_url with urllib.parse (urlparse/parse_qs), update the query dict to set
"v"="2" and add "setup"="open" or "wizard"="open" when args.setup or
args.quickstart are set, then rebuild the URL with urlencode and urlunparse so
existing params (e.g., prompt, config) are preserved; modify the block that
checks getattr(args, "setup"/"quickstart") to perform this parse-modify-rebuild
flow rather than string concatenation.
- Around line 3507-3514: coord_cfg may contain non-mapping values for
"prompt_improver" which makes pi_cfg not support .get and crashes; update the
parsing logic that builds prompt_improver_config to validate that
coord_cfg["prompt_improver"] is a mapping (e.g., isinstance(pi_cfg, dict) or
collections.abc.Mapping) before calling pi_cfg.get(...), and if it is not a
mapping raise a clear configuration error (ValueError or custom) that surfaces
the bad config; keep using PromptImproverConfig(enabled=...,
persist_across_turns=...) when the value is valid and fall back to the default
PromptImproverConfig() otherwise.
In `@massgen/evaluation_criteria_generator.py`:
- Around line 1052-1062: The current blanket except in the block that reads
criteria_file and calls _parse_criteria_response hides unexpected errors; change
criteria_file.read_text() to explicitly read as UTF-8
(read_text(encoding="utf-8")) and replace the broad "except Exception" with
catching only expected errors (e.g., UnicodeDecodeError, json.JSONDecodeError
from the parser, and file-related errors like FileNotFoundError/PermissionError)
so those are suppressed/logged as before, but re-raise any other exceptions so
they surface to the caller; keep references to criteria_file,
_parse_criteria_response, and logger.debug to locate and update the code.
In `@massgen/frontend/displays/textual_terminal_display.py`:
- Around line 4969-4972: The handler for event.event_type ==
"pre_collab_batch_announced" is clobbering the pending/open guard by always
resetting _parallel_precollab_screen_opened = False; change it to preserve an
already-pending/opened state by only setting _parallel_precollab_screen_opened
to False when the attribute doesn't already indicate a pending/open screen
(e.g., if not getattr(self, "_parallel_precollab_screen_opened", False):
self._parallel_precollab_screen_opened = False) or better: avoid assigning at
all when self._parallel_precollab_screen_opened is True; apply the same
conditional-preservation fix to the other two handlers referenced (the blocks
updating _parallel_precollab_expected/_parallel_precollab_screen_opened at the
other occurrences).
- Around line 1226-1230: notify_prompt_improved currently only updates the live
UI via _call_app_method("update_improved_prompt", ...) so replayed sessions
never get the improved prompt; modify notify_prompt_improved to also persist the
change to the replayable event/state stream (e.g., emit an
"improved_prompt_updated" event with the improved_prompt payload to whatever
event feeder or state appender is used elsewhere), and update the replay/restore
logic that rebuilds viewer state (the handler that consumes events during
replay) to handle that event and populate the session-info modal's
improved_prompt field during rebuild; ensure the same persisted event is
produced in the other call sites mentioned (around the blocks at/near 4173-4179
and 7924-7927) so both live UI and replay restore receive the improved prompt.
In `@massgen/frontend/displays/textual_widgets/subagent_screen.py`:
- Around line 121-125: The title builder incorrectly uses self._multi (tab
count) to decide the "Preparation" label; change _build_title to only return
"Preparation" when an explicit preparation/pre-collab signal is present (e.g.,
check a dedicated flag like self._subagent.is_preparation or
self._is_preparation_mode) and otherwise compute label from
getattr(self._subagent, "subagent_type", None) or self._subagent.id; also update
where _multi is set/used so tab count does not drive the preparation title.
- Around line 1647-1663: The current patch rebuilds a SubagentDisplayData
instance but only copies a subset of fields, dropping fields like context_paths
and context_paths_labeled; fix by constructing the new SubagentDisplayData from
the existing new_data while only overriding subagent_type (e.g., use a dataclass
replace/constructor that copies all existing fields from new_data and sets
subagent_type=sa.subagent_type) so new_data, sa and SubagentDisplayData retain
all present fields.
- Around line 2619-2634: The code is currently prepending the entire system
prompt from _inner_agent_system_prompts (via persona variable) into the content,
which leaks unrelated hidden instructions; instead, parse persona to extract
only the evaluator persona text (the substring after "Evaluator Persona:" up to
the next newline or end) and prepend that extracted evaluator persona (not the
full system_prompt) to content; keep the existing title logic that reads
"Evaluator Persona:" to derive persona_label, and use the same parsing logic in
the content-building block that currently adds persona so only the evaluator
persona snippet is inserted when present.
In `@massgen/frontend/web/server.py`:
- Around line 693-704: The code registers the transient WebSocket session_id
into SessionRegistry causing duplicate entries because _save_session_metadata()
later uses session_dir.name as the canonical ID; update the register flow so it
records the same canonical ID used by _save_session_metadata (use
session_dir.name or a canonical_session_id variable) instead of the transient
session_id when calling SessionRegistry.register_session, keep the same
config_path, description (question[:100] if present) and
status="completed"/source="webui", and ensure any subsequent references use that
canonical ID consistently.
- Around line 2364-2418: The delete_session handler currently only removes
completed_sessions, displays, tasks, orchestrators and some session maps; also
remove any live connection and workspace indexes and the historical log-dir
reference so deleted sessions don't reappear: in delete_session, check and
remove session_id from manager.active_connections (and close/terminate any
connection object if applicable), remove session_id entries from
workspace_manager.workspace_connections, and ensure
manager.session_log_dirs[session_id] is deleted (it may already be present in
code but ensure it's removed before returning); perform these removals before
calling SessionRegistry().delete_session and keep the same warning/JSONResponse
behavior.
- Around line 907-913: The endpoint currently reads any filesystem path via
config_path = Path(path) and safe_loads it, widening file disclosure; restrict
reads by reapplying the same config-path allowlist logic used in
/api/config/content: resolve config_path (config_path.resolve()), check it is
inside the approved config directories or matches allowed filenames (compare
against the allowlist set or ensure any(allowed.resolve() in
config_path.resolve().parents or config_path.resolve()==allowed.resolve() for
allowed in ALLOWED_CONFIG_PATHS)), and return JSONResponse 404 if the check
fails; keep the existing YAML loading (raw = _yaml.safe_load(...)) only after
the allowlist/containment check.
- Around line 1769-1789: The current code dumps the post-processed config into
original_yaml which leaks env-expanded values; instead call load_config_file to
capture the raw representation and the parsed config (e.g. raw, config =
load_config_file(...)), use yaml.dump on the raw value for original_yaml (or
re-read resolved_path as raw text), then call _apply_mode_overrides(config,
mode_overrides) and yaml.dump that processed config into resolved_yaml while
keeping the parsed config object returned for further use.
- Around line 5791-5799: The plan_mode branch currently only sets flags and can
leave stale booleans on coord (created via orch.setdefault("coordination", {}));
update the logic to first reset all planning-related flags
(enable_agent_task_planning, task_planning_filesystem_mode, spec_mode,
analysis_mode) to False on coord before applying the new overrides["plan_mode"]
values so selecting "normal" clears prior modes and switching modes won't leave
stale flags; modify the block that reads overrides.get("plan_mode") to
explicitly assign False to each of those keys on coord then set the appropriate
ones to True based on overrides["plan_mode"].
- Around line 917-930: The agent listing only reads model from
backend.get("model") so configs that store the model under backend_params.model
are returned blank; update the loop that builds agents_result (the
agents_list/agent_cfg/backend block) to compute model by checking
backend.get("model") first and falling back to backend.get("backend_params", {})
.get("model") (or vice‑versa if preferred), then use that computed model value
for the "model" field while keeping provider from backend.get("type"). Ensure
backend is still normalized to a dict as currently done.
- Around line 2351-2356: The merged session sort fails when comparing start_time
(string) and completed_at (float); update the _sort_key used by sessions.sort to
normalize timestamps to a numeric value before comparison: in the _sort_key
function (used when sorting sessions), coerce s.get("start_time") and
s.get("completed_at") into a float (e.g., parse start_time if it's a string,
fallback to completed_at, and return 0.0 on missing/invalid values) so the sort
always compares floats and cannot raise TypeError.
---
Outside diff comments:
In `@massgen/filesystem_manager/_filesystem_manager.py`:
- Around line 2342-2375: The snapshot-copy logic already excludes ".claude" via
_snapshot_exclude_dirs but has_meaningful_content() still treats ".claude/" as
deliverable content; update the workspace metadata set used when deciding
meaningful snapshots by adding ".claude" to _WORKSPACE_METADATA_DIRS (the set
referenced in FilesystemManager.save_snapshot() / has_meaningful_content()) so
that snapshots containing only ".claude/" are considered metadata-only and will
fall back to snapshot_storage; locate the _WORKSPACE_METADATA_DIRS (or
equivalent _metadata_dirs) definition and append "'.claude'" to it and ensure
any tests expecting metadata behavior are updated accordingly.
---
Nitpick comments:
In `@massgen/frontend/displays/textual_terminal_display.py`:
- Around line 6454-6565: Update the four new helper methods
(_try_open_unified_precollab_screen, _open_unified_precollab_screen,
_wait_and_open_unified_screen, set_precollab_batch) to include Google-style
docstrings that document Args (including types and defaults for attempt and
subagents/pre_collab_ids), the return value (None), and important side effects
(mutating self._parallel_precollab_expected,
self._parallel_precollab_screen_opened, timers via set_timer, pushing screens
via push_screen, and callbacks used like
_subagent_message_callback/_subagent_continue_callback); also note any
timeouts/retry behavior and conditions under which the method exits early (e.g.,
when screen already opened or no subagents) so callers and maintainers have
clear behavioral contracts.
In `@massgen/frontend/displays/web_display.py`:
- Around line 124-151: Replace the silent "except Exception: pass" blocks in
_register_event_listener and _on_structured_event with debug-level logging so
swallowed exceptions are recorded for troubleshooting: catch the exception as e
(e.g., "except Exception as e"), import or use the module logger (or
massgen.logger_config's logger) and call logger.debug with a short message that
includes the function name and exception details (for example reference
_register_event_listener, _event_listener, get_event_emitter, and in
_on_structured_event reference _emit and _closed) so failures remain non-fatal
but are visible in debug logs.
In `@massgen/frontend/web/server.py`:
- Around line 120-219: The three helper functions (_find_latest_attempt,
_scan_log_dirs, _scan_log_dirs_cached) currently have summary-only docstrings;
update each to Google-style docstrings including an "Args:" section describing
parameters (e.g., log_dir: Path or logs_root: Path) and a "Returns:" section
describing the return type and meaning (e.g., Path | None for
_find_latest_attempt, list[dict[str, Any]] for the scanners), and where helpful
a short "Raises:" note if exceptions are expected; keep the existing summary and
examples of keys (session_id, question, config, etc.) in the _scan_log_dirs
Returns description to match repo docstring style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b0e30fd-25df-420f-9c9c-7e84bf9b5891
⛔ Files ignored due to path filters (109)
massgen/frontend/web/static/assets/_baseUniq-DHPGDDgx.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/_baseUniq-DHPGDDgx.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/arc-CokCsZM6.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/arc-CokCsZM6.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/architectureDiagram-VXUJARFQ-CFWMw_rK.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/architectureDiagram-VXUJARFQ-CFWMw_rK.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/base-80a1f760-0bJi2RFl.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/base-80a1f760-0bJi2RFl.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/blockDiagram-VD42YOAC-Co5-MpGL.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/blockDiagram-VD42YOAC-Co5-MpGL.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/c4Diagram-YG6GDRKO-DYPuNmEF.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/c4Diagram-YG6GDRKO-DYPuNmEF.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/channel-D1dpymym.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/channel-D1dpymym.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/channel-DTOQwt0x.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-4BX2VUAB-CebFxQ-R.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-4BX2VUAB-CebFxQ-R.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-55IACEB6-DiNHxzzY.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-55IACEB6-DiNHxzzY.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-B4BG7PRW-CoPQlgSP.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-B4BG7PRW-CoPQlgSP.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-DI55MBZ5-BtSW5YBQ.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-DI55MBZ5-BtSW5YBQ.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-FMBD7UC4-wkPVgmHa.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-FMBD7UC4-wkPVgmHa.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-QN33PNHL-CZNl_8Ek.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-QN33PNHL-CZNl_8Ek.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-QZHKN3VN-BwbdsYhL.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-QZHKN3VN-BwbdsYhL.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/chunk-TZMSLE5B-CxyG-zuc.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/chunk-TZMSLE5B-CxyG-zuc.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/classDiagram-2ON5EDUG--rHFO9II.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/classDiagram-2ON5EDUG-CqUIDWJ9.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/classDiagram-2ON5EDUG-CqUIDWJ9.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/classDiagram-v2-WZHVMYZB--rHFO9II.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/classDiagram-v2-WZHVMYZB-CqUIDWJ9.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/classDiagram-v2-WZHVMYZB-CqUIDWJ9.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/clone-Cs1GzyJB.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/clone-r7v9nELZ.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/clone-r7v9nELZ.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/consoleHook-59e792cb-CJy5coaP.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/consoleHook-59e792cb-CJy5coaP.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/cose-bilkent-S5V4N54A-TTlzxN-U.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/cose-bilkent-S5V4N54A-TTlzxN-U.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/dagre-6UL2VRFP-BxPbgi2q.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/dagre-6UL2VRFP-BxPbgi2q.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/diagram-PSM6KHXK-d1E9v6HU.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/diagram-PSM6KHXK-d1E9v6HU.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/diagram-QEK2KX5R-BrdW4vHb.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/diagram-QEK2KX5R-BrdW4vHb.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/diagram-S2PKOQOG-Dl5n2kz6.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/diagram-S2PKOQOG-Dl5n2kz6.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/erDiagram-Q2GNP2WA-DoTyzorr.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/erDiagram-Q2GNP2WA-DoTyzorr.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/flowDiagram-NV44I4VS-BaelXQjr.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/flowDiagram-NV44I4VS-BaelXQjr.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/ganttDiagram-JELNMOA3-BfHDONdS.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/ganttDiagram-JELNMOA3-BfHDONdS.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/gitGraphDiagram-NY62KEGX--ygv52PS.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/gitGraphDiagram-NY62KEGX--ygv52PS.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/graph-BB7zubDf.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/graph-BB7zubDf.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/index-599aeaf7-Beoqm9RB.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/index-599aeaf7-Beoqm9RB.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/index-BXbvd5or.cssis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/index-C2qniOIu.cssis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/index-CGMkk2HX.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/index-CGMkk2HX.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/index-CoEp30O1.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/index-CoEp30O1.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/index-DBXMMTf0.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/index-DBXMMTf0.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/index-JoaRpYIu.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/infoDiagram-WHAUD3N6-C29jQaQb.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/infoDiagram-WHAUD3N6-C29jQaQb.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/journeyDiagram-XKPGCS4Q-Bm72xyLG.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/journeyDiagram-XKPGCS4Q-Bm72xyLG.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/kanban-definition-3W4ZIXB7-CTKr6jqq.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/kanban-definition-3W4ZIXB7-CTKr6jqq.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/layout-bA2TPHVZ.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/layout-bA2TPHVZ.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/linear-97fYS-uu.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/linear-97fYS-uu.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/min-De6IWHqk.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/min-De6IWHqk.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/mindmap-definition-VGOIOE7T-DJMLKmyE.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/mindmap-definition-VGOIOE7T-DJMLKmyE.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/pieDiagram-ADFJNKIX-C6vPU95a.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/pieDiagram-ADFJNKIX-C6vPU95a.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/quadrantDiagram-AYHSOK5B-BPAyAJBT.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/quadrantDiagram-AYHSOK5B-BPAyAJBT.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/requirementDiagram-UZGBJVZJ-CQUDmrBe.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/requirementDiagram-UZGBJVZJ-CQUDmrBe.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/sankeyDiagram-TZEHDZUN-Bypvbwc1.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/sankeyDiagram-TZEHDZUN-Bypvbwc1.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/sequenceDiagram-WL72ISMW-CLtEGs2b.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/sequenceDiagram-WL72ISMW-CLtEGs2b.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/stateDiagram-FKZM4ZOC-Cwn7jp1i.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/stateDiagram-FKZM4ZOC-Cwn7jp1i.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/stateDiagram-v2-4FDKWEC3-BTZICaqL.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/stateDiagram-v2-4FDKWEC3-BhsNILxt.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/stateDiagram-v2-4FDKWEC3-BhsNILxt.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/timeline-definition-IT6M3QCI-Cuuo442X.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/timeline-definition-IT6M3QCI-Cuuo442X.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/treemap-KMMF4GRG-CvlYEM91.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/treemap-KMMF4GRG-CvlYEM91.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapmassgen/frontend/web/static/assets/xychartDiagram-PRI3JC2R-g0uoLKrB.jsis excluded by!massgen/frontend/web/static/assets/**massgen/frontend/web/static/assets/xychartDiagram-PRI3JC2R-g0uoLKrB.js.mapis excluded by!**/*.map,!massgen/frontend/web/static/assets/**,!**/*.mapwebui/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (145)
AGENTS.mdCLAUDE.mddocs/modules/step_mode.mdmassgen/agent_config.pymassgen/backend/codex.pymassgen/cli.pymassgen/configs/features/round_evaluator_example.yamlmassgen/coordination_tracker.pymassgen/evaluation_criteria_generator.pymassgen/events.pymassgen/filesystem_manager/__init__.pymassgen/filesystem_manager/_filesystem_manager.pymassgen/filesystem_manager/_path_rewriter.pymassgen/frontend/displays/textual_terminal_display.pymassgen/frontend/displays/textual_widgets/subagent_screen.pymassgen/frontend/displays/textual_widgets/tab_bar.pymassgen/frontend/displays/web_display.pymassgen/frontend/web/server.pymassgen/frontend/web/static/index.htmlmassgen/mcp_tools/checklist_tools_server.pymassgen/message_templates.pymassgen/orchestrator.pymassgen/persona_generator.pymassgen/precollab_utils.pymassgen/prompt_improver.pymassgen/step_mode.pymassgen/subagent/manager.pymassgen/subagent/models.pymassgen/subagent_types/regression_guard/SUBAGENT.mdmassgen/subagent_types/round_evaluator/SUBAGENT.mdmassgen/system_message_builder.pymassgen/system_prompt_sections.pymassgen/task_decomposer.pymassgen/tests/frontend/test_subagent_screen_input.pymassgen/tests/frontend/test_subagent_screen_wiring.pymassgen/tests/integration/test_orchestrator_hooks_broadcast_subagents.pymassgen/tests/test_answer_count_increment.pymassgen/tests/test_claude_code_context_sharing.pymassgen/tests/test_codex_mcp_tool_visibility.pymassgen/tests/test_decomposition_mode.pymassgen/tests/test_evaluator_personas.pymassgen/tests/test_path_rewriter.pymassgen/tests/test_precollab_utils.pymassgen/tests/test_prompt_improver.pymassgen/tests/test_round_evaluator_loop.pymassgen/tests/test_specialized_subagents.pymassgen/tests/test_step_mode.pymassgen/tests/test_step_mode_live.pymassgen/tests/test_subagent_docker_logs.pymassgen/tests/test_unified_precollab.pymassgen/tests/test_web_mode_overrides.pymassgen/tests/test_web_quickstart_cli.pymassgen/tests/test_web_setup_status_api.pymassgen/tests/test_web_workspace_connection.pymassgen/tests/test_write_mode_scratch.pymassgen/tests/unit/test_coordination_tracker.pymassgen/tests/unit/test_orchestrator_unit.pywebui/index.htmlwebui/package.jsonwebui/src/App.tsxwebui/src/components/InlineArtifactPreview.tsxwebui/src/components/setup/ApiKeysSection.tsxwebui/src/components/setup/DockerSection.tsxwebui/src/components/setup/SkillsSection.tsxwebui/src/components/v2/channel/AgentChannel.tsxwebui/src/components/v2/channel/FinalAnswerSection.tsxwebui/src/components/v2/channel/ModeBar.test.tsxwebui/src/components/v2/channel/ModeBar.tsxwebui/src/components/v2/channel/StreamingIndicator.tsxwebui/src/components/v2/channel/TaskPlanPanel.test.tsxwebui/src/components/v2/channel/TaskPlanPanel.tsxwebui/src/components/v2/channel/messages/AnswerMessageView.test.tsxwebui/src/components/v2/channel/messages/AnswerMessageView.tsxwebui/src/components/v2/channel/messages/BroadcastMessageView.tsxwebui/src/components/v2/channel/messages/CompletionMessageView.tsxwebui/src/components/v2/channel/messages/ContentMessageView.tsxwebui/src/components/v2/channel/messages/MessageRenderer.tsxwebui/src/components/v2/channel/messages/RoundDividerView.tsxwebui/src/components/v2/channel/messages/StatusMessageView.tsxwebui/src/components/v2/channel/messages/SubagentMessageView.tsxwebui/src/components/v2/channel/messages/ToolBatchView.test.tsxwebui/src/components/v2/channel/messages/ToolBatchView.tsxwebui/src/components/v2/channel/messages/ToolCallMessageView.tsxwebui/src/components/v2/channel/messages/VoteMessageView.test.tsxwebui/src/components/v2/channel/messages/VoteMessageView.tsxwebui/src/components/v2/layout/AgentMentionAutocomplete.tsxwebui/src/components/v2/layout/AppShell.test.tsxwebui/src/components/v2/layout/AppShell.tsxwebui/src/components/v2/layout/ConfigViewerModal.tsxwebui/src/components/v2/layout/FinalAnswerOverlay.tsxwebui/src/components/v2/layout/GlobalInputBar.test.tsxwebui/src/components/v2/layout/GlobalInputBar.tsxwebui/src/components/v2/layout/LaunchIndicator.test.tsxwebui/src/components/v2/layout/LaunchIndicator.tsxwebui/src/components/v2/layout/ModeConfigBar.test.tsxwebui/src/components/v2/layout/ModeConfigBar.tsxwebui/src/components/v2/layout/V2QuickstartWizard.test.tsxwebui/src/components/v2/layout/V2QuickstartWizard.tsxwebui/src/components/v2/layout/V2SetupOverlay.test.tsxwebui/src/components/v2/layout/V2SetupOverlay.tsxwebui/src/components/v2/sidebar/ChannelSection.tsxwebui/src/components/v2/sidebar/QuickAccessSection.tsxwebui/src/components/v2/sidebar/SessionSection.tsxwebui/src/components/v2/sidebar/Sidebar.tsxwebui/src/components/v2/sidebar/SidebarFooter.tsxwebui/src/components/v2/sidebar/SidebarHeader.tsxwebui/src/components/v2/sidebar/ThreadSection.tsxwebui/src/components/v2/tiles/EmptyState.tsxwebui/src/components/v2/tiles/FileViewerTile.tsxwebui/src/components/v2/tiles/OrientationToggle.tsxwebui/src/components/v2/tiles/PromptBanner.tsxwebui/src/components/v2/tiles/SubagentTile.tsxwebui/src/components/v2/tiles/TileContainer.test.tsxwebui/src/components/v2/tiles/TileContainer.tsxwebui/src/components/v2/tiles/TileDragContext.tsxwebui/src/components/v2/tiles/TileDragHandle.tsxwebui/src/components/v2/tiles/TileWrapper.tsxwebui/src/components/v2/tiles/TimelineTile.tsxwebui/src/components/v2/tiles/V2TimelineView.tsxwebui/src/components/v2/tiles/VoteResultsTile.tsxwebui/src/components/v2/tiles/WorkspaceBrowserTile.test.tsxwebui/src/components/v2/tiles/WorkspaceBrowserTile.tsxwebui/src/hooks/useV2KeyboardShortcuts.tswebui/src/hooks/useWebSocket.test.tsxwebui/src/hooks/useWebSocket.tswebui/src/hooks/useWorkspaceConnection.test.tsxwebui/src/hooks/useWorkspaceConnection.tswebui/src/main.tsxwebui/src/pages/SetupPage.tsxwebui/src/stores/agentStore.tswebui/src/stores/setupStore.tswebui/src/stores/v2/messageStore.test.tswebui/src/stores/v2/messageStore.tswebui/src/stores/v2/modeStore.test.tswebui/src/stores/v2/modeStore.tswebui/src/stores/v2/tileStore.test.tswebui/src/stores/v2/tileStore.tswebui/src/stores/wizardStore.tswebui/src/styles/globals.csswebui/src/test/setup.tswebui/src/types/index.tswebui/src/utils/broadcastTargets.test.tswebui/src/utils/broadcastTargets.tswebui/tailwind.config.jswebui/vite.config.ts
Comment on lines
+220
to
+222
| prompt_improver: PromptImproverConfig = field( | ||
| default_factory=PromptImproverConfig, | ||
| ) |
There was a problem hiding this comment.
Round-tripping these new coordination fields is currently broken.
AgentConfig.to_dict() still drops both prompt_improver and enable_evaluator_personas, and AgentConfig.from_dict() would rehydrate prompt_improver as a plain dict if you start serializing it. Any saved/resumed config will silently lose these settings or later fail on attribute access like prompt_improver.enabled.
💡 Minimal fix
result["coordination_config"] = {
"enable_planning_mode": self.coordination_config.enable_planning_mode,
"planning_mode_instruction": self.coordination_config.planning_mode_instruction,
"max_orchestration_restarts": self.coordination_config.max_orchestration_restarts,
"drift_conflict_policy": self.coordination_config.drift_conflict_policy,
"round_evaluator_transformation_pressure": self.coordination_config.round_evaluator_transformation_pressure,
+ "prompt_improver": {
+ "enabled": self.coordination_config.prompt_improver.enabled,
+ "persist_across_turns": self.coordination_config.prompt_improver.persist_across_turns,
+ },
+ "enable_evaluator_personas": self.coordination_config.enable_evaluator_personas,
}
...
coordination_config = CoordinationConfig()
coordination_data = data.get("coordination_config", {})
if coordination_data:
+ coordination_data = coordination_data.copy()
+ prompt_improver_data = coordination_data.get("prompt_improver")
+ if isinstance(prompt_improver_data, dict):
+ coordination_data["prompt_improver"] = PromptImproverConfig(**prompt_improver_data)
coordination_config = CoordinationConfig(**coordination_data)Also applies to: 250-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/agent_config.py` around lines 220 - 222, AgentConfig currently drops
prompt_improver and enable_evaluator_personas when serializing and rehydrates
prompt_improver as a plain dict; update AgentConfig.to_dict to include the
prompt_improver field (serialize it properly, e.g., call its to_dict or similar)
and include enable_evaluator_personas in the returned dict, and update
AgentConfig.from_dict to detect if prompt_improver is a dict and instantiate a
PromptImproverConfig from it (or accept an existing PromptImproverConfig),
ensuring enable_evaluator_personas is read back into the AgentConfig instance so
saved/resumed configs preserve these values; reference the prompt_improver
attribute and the AgentConfig.to_dict / AgentConfig.from_dict methods while
making the changes.
Comment on lines
+3507
to
+3514
| # Parse prompt_improver config if present | ||
| prompt_improver_config = PromptImproverConfig() | ||
| if "prompt_improver" in coord_cfg: | ||
| pi_cfg = coord_cfg["prompt_improver"] | ||
| prompt_improver_config = PromptImproverConfig( | ||
| enabled=pi_cfg.get("enabled", False), | ||
| persist_across_turns=pi_cfg.get("persist_across_turns", False), | ||
| ) |
There was a problem hiding this comment.
Validate prompt_improver as a mapping.
prompt_improver: with no nested keys loads as None in YAML, and prompt_improver: true loads as a bool. Both cases hit pi_cfg.get(...) and crash startup with AttributeError instead of surfacing a config error.
Suggested fix
# Parse prompt_improver config if present
prompt_improver_config = PromptImproverConfig()
- if "prompt_improver" in coord_cfg:
- pi_cfg = coord_cfg["prompt_improver"]
+ pi_cfg = coord_cfg.get("prompt_improver")
+ if pi_cfg is not None:
+ if not isinstance(pi_cfg, dict):
+ raise ConfigurationError(
+ "orchestrator.coordination.prompt_improver must be a mapping"
+ )
prompt_improver_config = PromptImproverConfig(
enabled=pi_cfg.get("enabled", False),
persist_across_turns=pi_cfg.get("persist_across_turns", False),
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/cli.py` around lines 3507 - 3514, coord_cfg may contain non-mapping
values for "prompt_improver" which makes pi_cfg not support .get and crashes;
update the parsing logic that builds prompt_improver_config to validate that
coord_cfg["prompt_improver"] is a mapping (e.g., isinstance(pi_cfg, dict) or
collections.abc.Mapping) before calling pi_cfg.get(...), and if it is not a
mapping raise a clear configuration error (ValueError or custom) that surfaces
the bad config; keep using PromptImproverConfig(enabled=...,
persist_across_turns=...) when the value is valid and fall back to the default
PromptImproverConfig() otherwise.
Comment on lines
+10078
to
+10086
| # Save post-coordination artifacts (final/, coordination_events.json, etc.) | ||
| from massgen.logger_config import get_log_session_dir | ||
|
|
||
| log_session_dir = get_log_session_dir() | ||
| if log_session_dir: | ||
| orchestrator.finalize_step_mode(log_session_dir) | ||
|
|
||
| _automation_print(f"ACTION: {action_data['action']}") | ||
| _automation_print(f"STATUS: {Path(args.session_dir).resolve() / 'last_action.json'}") | ||
| _automation_print(f"STATUS: {Path(args.session_dir).resolve() / 'agents' / real_agent_id / 'last_action.json'}") |
There was a problem hiding this comment.
Print the step result before finalizing extra artifacts.
save_step_mode_output() has already written last_action.json, but finalize_step_mode() now runs before the ACTION / STATUS lines are emitted. Any slow copy or exception there delays or suppresses the machine-readable handoff the parent process is waiting for.
Suggested fix
- # Save post-coordination artifacts (final/, coordination_events.json, etc.)
- from massgen.logger_config import get_log_session_dir
-
- log_session_dir = get_log_session_dir()
- if log_session_dir:
- orchestrator.finalize_step_mode(log_session_dir)
-
_automation_print(f"ACTION: {action_data['action']}")
_automation_print(f"STATUS: {Path(args.session_dir).resolve() / 'agents' / real_agent_id / 'last_action.json'}")
+
+ try:
+ from massgen.logger_config import get_log_session_dir
+
+ log_session_dir = get_log_session_dir()
+ if log_session_dir:
+ orchestrator.finalize_step_mode(log_session_dir)
+ except Exception as exc:
+ logger.warning(f"Failed to finalize step mode artifacts: {exc}")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Save post-coordination artifacts (final/, coordination_events.json, etc.) | |
| from massgen.logger_config import get_log_session_dir | |
| log_session_dir = get_log_session_dir() | |
| if log_session_dir: | |
| orchestrator.finalize_step_mode(log_session_dir) | |
| _automation_print(f"ACTION: {action_data['action']}") | |
| _automation_print(f"STATUS: {Path(args.session_dir).resolve() / 'last_action.json'}") | |
| _automation_print(f"STATUS: {Path(args.session_dir).resolve() / 'agents' / real_agent_id / 'last_action.json'}") | |
| _automation_print(f"ACTION: {action_data['action']}") | |
| _automation_print(f"STATUS: {Path(args.session_dir).resolve() / 'agents' / real_agent_id / 'last_action.json'}") | |
| try: | |
| from massgen.logger_config import get_log_session_dir | |
| log_session_dir = get_log_session_dir() | |
| if log_session_dir: | |
| orchestrator.finalize_step_mode(log_session_dir) | |
| except Exception as exc: | |
| logger.warning(f"Failed to finalize step mode artifacts: {exc}") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/cli.py` around lines 10078 - 10086, The ACTION and STATUS
machine-readable lines are printed after calling
orchestrator.finalize_step_mode, which can delay or fail the handoff; move the
_automation_print calls (using action_data['action'] and the STATUS path built
from args.session_dir and real_agent_id) to occur before
orchestrator.finalize_step_mode(log_session_dir) so the step result is emitted
immediately (ensure you still call finalize_step_mode afterwards to save extra
artifacts).
Comment on lines
+11743
to
+11750
| # Default to v2 UI; --quickstart opens the wizard overlay, | ||
| # --setup opens the setup overlay in v2. | ||
| if getattr(args, "setup", False): | ||
| browser_url += "/setup" | ||
| browser_url += "/?v=2&setup=open" | ||
| elif getattr(args, "quickstart", False): | ||
| browser_url += "/?wizard=open" | ||
| browser_url += "/?v=2&wizard=open" | ||
| else: | ||
| browser_url += "/?v=2" |
There was a problem hiding this comment.
Preserve existing query params when forcing v=2.
When auto_url is already something like http://host:port/?prompt=...&config=..., appending /?v=2 turns v=2 into part of the last query value. The browser opens a malformed URL and loses the prefilled prompt/config state.
Suggested fix
- browser_url = auto_url if auto_url else f"http://{args.web_host}:{args.web_port}"
- # Remove trailing slash to avoid double slashes
- browser_url = browser_url.rstrip("/")
-
- # Default to v2 UI; --quickstart opens the wizard overlay,
- # --setup opens the setup overlay in v2.
- if getattr(args, "setup", False):
- browser_url += "/?v=2&setup=open"
- elif getattr(args, "quickstart", False):
- browser_url += "/?v=2&wizard=open"
- else:
- browser_url += "/?v=2"
+ split_url = urllib.parse.urlsplit(auto_url or base_url)
+ params = dict(
+ urllib.parse.parse_qsl(
+ split_url.query,
+ keep_blank_values=True,
+ )
+ )
+ params["v"] = "2"
+ if getattr(args, "setup", False):
+ params["setup"] = "open"
+ elif getattr(args, "quickstart", False):
+ params["wizard"] = "open"
+
+ browser_url = urllib.parse.urlunsplit(
+ (
+ split_url.scheme,
+ split_url.netloc,
+ split_url.path or "/",
+ urllib.parse.urlencode(params),
+ split_url.fragment,
+ )
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@massgen/cli.py` around lines 11743 - 11750, The code appends "/?v=2" to
browser_url which breaks existing query params (variable browser_url in
massgen/cli.py); instead parse browser_url with urllib.parse
(urlparse/parse_qs), update the query dict to set "v"="2" and add "setup"="open"
or "wizard"="open" when args.setup or args.quickstart are set, then rebuild the
URL with urlencode and urlunparse so existing params (e.g., prompt, config) are
preserved; modify the block that checks getattr(args, "setup"/"quickstart") to
perform this parse-modify-rebuild flow rather than string concatenation.
Comment on lines
+1052
to
+1062
| try: | ||
| content = criteria_file.read_text() | ||
| criteria = _parse_criteria_response( | ||
| content, | ||
| min_criteria, | ||
| max_criteria, | ||
| ) | ||
| if criteria: | ||
| return criteria | ||
| except Exception as e: | ||
| logger.debug(f"Failed to parse {criteria_file}: {e}") |
There was a problem hiding this comment.
Don't silently downgrade to defaults on unexpected criteria-file failures.
This path now does a single artifact read, and the blanket except Exception turns decode/parser regressions into a quiet fallback with only a debug log. That makes a broken criteria.json path look the same as “no usable artifact found.” Read the file as UTF-8 and only suppress expected file/JSON errors here; let unexpected exceptions surface so the caller logs them loudly.
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 1061-1061: 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/evaluation_criteria_generator.py` around lines 1052 - 1062, The
current blanket except in the block that reads criteria_file and calls
_parse_criteria_response hides unexpected errors; change
criteria_file.read_text() to explicitly read as UTF-8
(read_text(encoding="utf-8")) and replace the broad "except Exception" with
catching only expected errors (e.g., UnicodeDecodeError, json.JSONDecodeError
from the parser, and file-related errors like FileNotFoundError/PermissionError)
so those are suppressed/logged as before, but re-raise any other exceptions so
they surface to the caller; keep references to criteria_file,
_parse_criteria_response, and logger.debug to locate and update the code.
Comment on lines
+917
to
+930
| agents_list = raw.get("agents", []) | ||
| agents_result = [] | ||
| for agent_cfg in agents_list: | ||
| if not isinstance(agent_cfg, dict): | ||
| continue | ||
| backend = agent_cfg.get("backend", {}) | ||
| if not isinstance(backend, dict): | ||
| backend = {} | ||
| agents_result.append( | ||
| { | ||
| "id": agent_cfg.get("id", ""), | ||
| "provider": backend.get("type", None), | ||
| "model": backend.get("model", None), | ||
| }, |
There was a problem hiding this comment.
Read model IDs from both supported config shapes.
This only looks at backend.model, but the rest of this file already handles configs that store the model in backend_params.model. Those valid configs will come back with blank model labels in the WebUI.
Suggested fix
agents_list = raw.get("agents", [])
agents_result = []
for agent_cfg in agents_list:
if not isinstance(agent_cfg, dict):
continue
backend = agent_cfg.get("backend", {})
if not isinstance(backend, dict):
backend = {}
+ backend_params = agent_cfg.get("backend_params", {})
+ if not isinstance(backend_params, dict):
+ backend_params = {}
agents_result.append(
{
"id": agent_cfg.get("id", ""),
"provider": backend.get("type", None),
- "model": backend.get("model", None),
+ "model": backend.get("model") or backend_params.get("model"),
},
)🤖 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 917 - 930, The agent listing
only reads model from backend.get("model") so configs that store the model under
backend_params.model are returned blank; update the loop that builds
agents_result (the agents_list/agent_cfg/backend block) to compute model by
checking backend.get("model") first and falling back to
backend.get("backend_params", {}) .get("model") (or vice‑versa if preferred),
then use that computed model value for the "model" field while keeping provider
from backend.get("type"). Ensure backend is still normalized to a dict as
currently done.
Comment on lines
+1769
to
+1789
| config, _ = load_config_file(str(resolved_path)) | ||
| original_yaml = yaml.dump( | ||
| copy.deepcopy(config), | ||
| default_flow_style=False, | ||
| sort_keys=False, | ||
| allow_unicode=True, | ||
| ) | ||
|
|
||
| _apply_mode_overrides(config, mode_overrides) | ||
|
|
||
| resolved_yaml = yaml.dump( | ||
| config, | ||
| default_flow_style=False, | ||
| sort_keys=False, | ||
| allow_unicode=True, | ||
| ) | ||
|
|
||
| return { | ||
| "original_yaml": original_yaml, | ||
| "resolved_yaml": resolved_yaml, | ||
| "config": config, |
There was a problem hiding this comment.
Don't serialize the resolved config as original_yaml.
load_config_file() returns a raw/safe representation elsewhere in this file for metadata logging. Dumping config here makes the “original” preview inaccurate and can leak env-expanded values to the browser. Use the raw return value (or reread the file) for original_yaml, and keep config only for the post-override preview.
🤖 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 1769 - 1789, The current code
dumps the post-processed config into original_yaml which leaks env-expanded
values; instead call load_config_file to capture the raw representation and the
parsed config (e.g. raw, config = load_config_file(...)), use yaml.dump on the
raw value for original_yaml (or re-read resolved_path as raw text), then call
_apply_mode_overrides(config, mode_overrides) and yaml.dump that processed
config into resolved_yaml while keeping the parsed config object returned for
further use.
Comment on lines
+2351
to
+2356
| # Sort by start_time/completed_at descending, limit to 50 | ||
| def _sort_key(s: dict) -> str: | ||
| return s.get("start_time") or s.get("completed_at") or "" | ||
|
|
||
| sessions.sort(key=_sort_key, reverse=True) | ||
| return {"sessions": sessions[:50]} |
There was a problem hiding this comment.
Normalize timestamps before sorting the merged session list.
completed_at is stored as a float (time.time()), while historical start_time values are strings. Python 3 cannot compare float and str, so once both sources are present /api/sessions can fail with TypeError instead of returning results.
Suggested fix
- def _sort_key(s: dict) -> str:
- return s.get("start_time") or s.get("completed_at") or ""
+ from datetime import datetime
+
+ def _sort_key(s: dict) -> float:
+ for value in (s.get("completed_at"), s.get("start_time")):
+ if isinstance(value, (int, float)):
+ return float(value)
+ if isinstance(value, str) and value:
+ try:
+ return datetime.fromisoformat(
+ value.replace("Z", "+00:00"),
+ ).timestamp()
+ except ValueError:
+ try:
+ return float(value)
+ except ValueError:
+ continue
+ return 0.0🤖 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 2351 - 2356, The merged session
sort fails when comparing start_time (string) and completed_at (float); update
the _sort_key used by sessions.sort to normalize timestamps to a numeric value
before comparison: in the _sort_key function (used when sorting sessions),
coerce s.get("start_time") and s.get("completed_at") into a float (e.g., parse
start_time if it's a string, fallback to completed_at, and return 0.0 on
missing/invalid values) so the sort always compares floats and cannot raise
TypeError.
Comment on lines
+2364
to
+2418
| @app.delete("/api/sessions/{session_id}") | ||
| async def delete_session(session_id: str): | ||
| """Delete a session and clean up its resources.""" | ||
| removed = False | ||
|
|
||
| # Remove from completed sessions | ||
| if session_id in manager.completed_sessions: | ||
| del manager.completed_sessions[session_id] | ||
| removed = True | ||
|
|
||
| # Remove display | ||
| if session_id in manager.displays: | ||
| del manager.displays[session_id] | ||
| removed = True | ||
|
|
||
| # Cancel and remove task | ||
| if session_id in manager.tasks: | ||
| task = manager.tasks[session_id] | ||
| if not task.done(): | ||
| task.cancel() | ||
| del manager.tasks[session_id] | ||
| removed = True | ||
|
|
||
| # Remove orchestrator | ||
| if session_id in manager.orchestrators: | ||
| del manager.orchestrators[session_id] | ||
| removed = True | ||
|
|
||
| # Remove log dir reference | ||
| if session_id in manager.session_log_dirs: | ||
| del manager.session_log_dirs[session_id] | ||
|
|
||
| # Remove config reference | ||
| if session_id in manager.session_configs: | ||
| del manager.session_configs[session_id] | ||
|
|
||
| # Remove from persistent registry | ||
| try: | ||
| from massgen.session import SessionRegistry | ||
|
|
||
| SessionRegistry().delete_session(session_id) | ||
| except Exception: | ||
| logger.warning( | ||
| "Failed to delete session %s from registry", | ||
| session_id, | ||
| exc_info=True, | ||
| ) | ||
|
|
||
| if not removed: | ||
| return JSONResponse( | ||
| {"error": "Session not found"}, | ||
| status_code=404, | ||
| ) | ||
|
|
||
| return JSONResponse({"status": "deleted", "session_id": session_id}) |
There was a problem hiding this comment.
DELETE doesn't clear all session indexes.
This only removes task/display/orchestrator metadata. It leaves manager.active_connections, workspace_manager.workspace_connections, and the historical log-directory source intact, so a “deleted” session can still appear as active or reappear from log scanning.
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 2405-2405: 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/web/server.py` around lines 2364 - 2418, The delete_session
handler currently only removes completed_sessions, displays, tasks,
orchestrators and some session maps; also remove any live connection and
workspace indexes and the historical log-dir reference so deleted sessions don't
reappear: in delete_session, check and remove session_id from
manager.active_connections (and close/terminate any connection object if
applicable), remove session_id entries from
workspace_manager.workspace_connections, and ensure
manager.session_log_dirs[session_id] is deleted (it may already be present in
code but ensure it's removed before returning); perform these removals before
calling SessionRegistry().delete_session and keep the same warning/JSONResponse
behavior.
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
New Features
Bug Fixes
Documentation