diff --git a/backend/app/api/v1/generation_sessions.py b/backend/app/api/v1/generation_sessions.py index 3bcddff..a57802d 100644 --- a/backend/app/api/v1/generation_sessions.py +++ b/backend/app/api/v1/generation_sessions.py @@ -34,7 +34,7 @@ EstimationSummary, WorkspaceEstimation, ComparativeAnalysis, - ComponentComparison, + PhaseComparison, RiskAssessment, SkippedWorkspaceP10Y, ) @@ -992,17 +992,17 @@ def _reconstruct_generation_session_response( continue comp_analysis_data = stored_result.get("comparative_analysis") or {} - component_comparison = {} - if "component_comparison" in comp_analysis_data: - for comp_name, comp_comp_data in comp_analysis_data["component_comparison"].items(): + phase_comparison = {} + if "phase_comparison" in comp_analysis_data: + for phase_key, phase_comp_data in comp_analysis_data["phase_comparison"].items(): try: - component_comparison[comp_name] = ComponentComparison(**comp_comp_data) + phase_comparison[phase_key] = PhaseComparison(**phase_comp_data) except Exception: continue comparative_analysis = ComparativeAnalysis( - component_comparison=component_comparison, - high_variance_components=comp_analysis_data.get("high_variance_components", []), + phase_comparison=phase_comparison, + high_variance_phases=comp_analysis_data.get("high_variance_phases", []), insights=comp_analysis_data.get("insights", []), ) diff --git a/backend/app/core/notifications.py b/backend/app/core/notifications.py index 4ca40ff..80aa58a 100644 --- a/backend/app/core/notifications.py +++ b/backend/app/core/notifications.py @@ -387,7 +387,7 @@ def build_coding_complete_pre_deploy_response( workspace_path="", total_hours=0.0, total_effective_output=0.0, - component_breakdown={}, + phase_breakdown={}, estimation_metrics=zero_metrics, commits_count=0, p10y_scored_commits=0, @@ -414,10 +414,10 @@ def build_coding_complete_pre_deploy_response( ), ) comparative_analysis = ComparativeAnalysis( - component_comparison={}, - high_variance_components=[], + phase_comparison={}, + high_variance_phases=[], insights=[ - "P10Y hour estimates and component breakdown are not available yet; " + "P10Y hour estimates and phase breakdown are not available yet; " "they will be included in the final notification after deployment and the P10Y phase.", ], ) @@ -821,20 +821,20 @@ def _build_generation_session_slack_blocks( if cost_ln: lines.append(cost_ln) - # Component breakdown: show top components by hours - component_breakdown = getattr(ws_est, "component_breakdown", None) - if component_breakdown: - sorted_comps = sorted( - component_breakdown.items(), + # Phase breakdown: show phases by hours (highest first) + phase_breakdown = getattr(ws_est, "phase_breakdown", None) + if phase_breakdown: + sorted_phases = sorted( + phase_breakdown.items(), key=lambda kv: getattr(kv[1], "hours", 0.0), reverse=True, ) - comp_parts = [ - f"{name}: {getattr(comp, 'hours', 0.0):.1f}h" - for name, comp in sorted_comps + phase_parts = [ + f"{getattr(ph, 'phase_name', key)}: {getattr(ph, 'hours', 0.0):.1f}h" + for key, ph in sorted_phases ] - if comp_parts: - lines.append(" " + " | ".join(comp_parts)) + if phase_parts: + lines.append(" " + " | ".join(phase_parts)) blocks.append({ "type": "section", @@ -1195,40 +1195,44 @@ def render_generation_session_report_html( except Exception as e: logger.warning(f"Failed to retrieve workspace {workspace_id}: {e}") - # Extract component breakdown - component_breakdown = {} + # Extract per-phase breakdown (phase key "NN"/"unphased" -> number, name, per-workspace hours) + phase_breakdown = {} if workspace_estimations: - # Collect all unique components across all workspaces - all_components = set() + # Collect all unique phase keys across all workspaces + all_keys = set() for ws_est in workspace_estimations: try: - if hasattr(ws_est, 'component_breakdown') and ws_est.component_breakdown: - all_components.update(ws_est.component_breakdown.keys()) + if hasattr(ws_est, 'phase_breakdown') and ws_est.phase_breakdown: + all_keys.update(ws_est.phase_breakdown.keys()) except (AttributeError, TypeError): - continue # Skip this workspace if component_breakdown is missing or invalid - - # Build component breakdown with hours per workspace - for component_name in sorted(all_components): - component_data = { - "name": component_name, + continue # Skip this workspace if phase_breakdown is missing or invalid + + # Build phase breakdown with hours per workspace (plan order; unphased sorts last) + for phase_key in sorted(all_keys): + phase_data = { + "key": phase_key, + "number": None, + "name": phase_key, "workspaces": {} } for ws_est in workspace_estimations: try: - if (hasattr(ws_est, 'component_breakdown') and - component_name in ws_est.component_breakdown): - comp = ws_est.component_breakdown[component_name] + if (hasattr(ws_est, 'phase_breakdown') and + phase_key in ws_est.phase_breakdown): + ph = ws_est.phase_breakdown[phase_key] + phase_data["number"] = getattr(ph, 'phase_number', None) + phase_data["name"] = getattr(ph, 'phase_name', phase_key) workspace_name = getattr(ws_est, 'workspace_name', 'unknown') - component_data["workspaces"][workspace_name] = { - "hours": getattr(comp, 'hours', 0.0), - "new_work": getattr(comp, 'new_work', 0.0), - "refactor": getattr(comp, 'refactor', 0.0), - "rework": getattr(comp, 'rework', 0.0), - "quality_score": getattr(comp, 'quality_score', 0.0) + phase_data["workspaces"][workspace_name] = { + "hours": getattr(ph, 'hours', 0.0), + "new_work": getattr(ph, 'new_work', 0.0), + "refactor": getattr(ph, 'refactor', 0.0), + "rework": getattr(ph, 'rework', 0.0), + "quality_score": getattr(ph, 'quality_score', 0.0) } except (AttributeError, TypeError, KeyError): - continue # Skip this workspace/component if data is missing - component_breakdown[component_name] = component_data + continue # Skip this workspace/phase if data is missing + phase_breakdown[phase_key] = phase_data # Build HTML content html_parts = [] @@ -1355,13 +1359,14 @@ def render_generation_session_report_html( html_parts.append('') html_parts.append('') - # Component breakdown section - if component_breakdown: + # Phase breakdown section + if phase_breakdown: html_parts.append('
') - html_parts.append('

Component Complexity Metrics Breakdown

') + html_parts.append('

Phase Breakdown

') html_parts.append('') html_parts.append('') - html_parts.append('') + html_parts.append('') + html_parts.append('') # Add workspace columns if workspace_estimations: for ws_est in workspace_estimations: @@ -1373,16 +1378,18 @@ def render_generation_session_report_html( html_parts.append('') html_parts.append('') - for component_name, component_data in component_breakdown.items(): + for _phase_key, phase_data in phase_breakdown.items(): + num = f'{phase_data["number"]:02d}' if phase_data["number"] is not None else '—' html_parts.append('') - html_parts.append(f'') + html_parts.append(f'') + html_parts.append(f'') # Add hours for each workspace if workspace_estimations: for ws_est in workspace_estimations: try: ws_name = getattr(ws_est, 'workspace_name', 'unknown') - if ws_name in component_data["workspaces"]: - hours = component_data["workspaces"][ws_name]["hours"] + if ws_name in phase_data["workspaces"]: + hours = phase_data["workspaces"][ws_name]["hours"] html_parts.append(f'') else: html_parts.append('') @@ -1457,11 +1464,11 @@ def render_generation_session_report_html( pass plain_parts.append("") - if component_breakdown: - plain_parts.append("COMPONENT BREAKDOWN:") + if phase_breakdown: + plain_parts.append("PHASE BREAKDOWN:") plain_parts.append("-" * 60) # Build header - header = "Component" + header = "Phase" if workspace_estimations: for ws_est in workspace_estimations: try: @@ -1472,14 +1479,15 @@ def render_generation_session_report_html( plain_parts.append(header) plain_parts.append("-" * len(header)) - for component_name, component_data in component_breakdown.items(): - row = component_name + for _phase_key, phase_data in phase_breakdown.items(): + num = f'{phase_data["number"]:02d}' if phase_data["number"] is not None else '—' + row = f'{num} {phase_data["name"]}' if workspace_estimations: for ws_est in workspace_estimations: try: ws_name = getattr(ws_est, 'workspace_name', 'unknown') - if ws_name in component_data["workspaces"]: - hours = component_data["workspaces"][ws_name]["hours"] + if ws_name in phase_data["workspaces"]: + hours = phase_data["workspaces"][ws_name]["hours"] row += f" | {hours:.1f}" else: row += " | -" diff --git a/backend/app/prompts/agents_claude_code.py b/backend/app/prompts/agents_claude_code.py index 4535abd..c91e54a 100644 --- a/backend/app/prompts/agents_claude_code.py +++ b/backend/app/prompts/agents_claude_code.py @@ -844,9 +844,10 @@ def phase_workflow_instructions(outputs_dir: str, phase_number: int): - Write the code following all engineering standards above - Write unit tests for the functionality - Verify code integrity (run linters/tests if the environment allows) - - **COMMIT** the changes with proper component attribution (see {COMMIT_STANDARDS_FILE_REL}) - * **Strict subject line**: `_` (underscore after component name, rest is free text) - * Example: `backend_implement JWT token generation and validation` + - **COMMIT** the changes with a clear, descriptive message (see {COMMIT_STANDARDS_FILE_REL}) + * **Subject line**: a plain ` ` — do NOT add any component or phase prefix + * The implementation-plan phase prefix (`pNN_`) is added automatically by a git hook + * Example: `implement JWT token generation and validation` * Do not mention contributors or coauthors * **IMPORTANT — only your phase commits**: - After each commit, optionally run `git log -1 --oneline` to confirm the message matches the schema @@ -948,13 +949,11 @@ def generate_production_agent_template( - **CRITICAL:** Whenever a logical component (e.g., a specific service, a feature, a module, or a set of related tests) is finished and stable, you must run `git add` and `git commit` with a descriptive message. - This commit history represents the "human" progress timeline. - 4. **Commit Strategy & Component Tracking (LOCKED GRANULARITY):** + 4. **Commit Strategy (LOCKED GRANULARITY):** - **REQUIRED**: Follow commit standards defined in {COMMIT_STANDARDS_FILE_REL} - - **MANDATORY**: Every commit subject encodes one primary component using this **strict first-line format**: - `_` - - The **first** underscore separates the component token from the rest (e.g. `backend_implement user service`). - - Use a component token from {COMMIT_STANDARDS_FILE_REL} (`backend`, `frontend`, `mobile`, `testing`, `infrastructure`, …). - - See {COMMIT_STANDARDS_FILE_REL} for component list, `SKIP_` rules for non-generation commits, and examples + - Write a plain, descriptive ` ` first line — do NOT add any component or phase prefix yourself. + - The implementation-plan phase prefix (`pNN_`) is added automatically by a git hook, so P10Y attribution never depends on your commit hygiene. + - See {COMMIT_STANDARDS_FILE_REL} for the `SKIP_` rule for non-generation commits, and examples - **CRITICAL: COMMIT GRANULARITY IS STANDARDIZED** - **Target**: 40-50 commits for typical applications (scales with complexity) - Commit when completing a logical unit within a single component or a phase @@ -1431,7 +1430,7 @@ def qa_agent_prompt(outputs_dir: str): def estimation_report_agent_template( summary: EstimationSummary, workspace_summaries: List[str], - component_comparison_text: str, + phase_comparison_text: str, comparative_analysis: ComparativeAnalysis, full_outputs_dir: str, model: str, @@ -1452,11 +1451,11 @@ def estimation_report_agent_template( ### Workspace Results: {chr(10).join(workspace_summaries)} - ### Component Comparison: - {chr(10).join(component_comparison_text)} + ### Phase Comparison: + {chr(10).join(phase_comparison_text)} - ### High Variance Components: - {', '.join(comparative_analysis.high_variance_components) if comparative_analysis.high_variance_components else 'None'} + ### High Variance Phases: + {', '.join(comparative_analysis.high_variance_phases) if comparative_analysis.high_variance_phases else 'None'} ### Key Insights: {chr(10).join(f"- {insight}" for insight in comparative_analysis.insights)} diff --git a/backend/app/schemas/estimate.py b/backend/app/schemas/estimate.py index d26a941..f9caa41 100644 --- a/backend/app/schemas/estimate.py +++ b/backend/app/schemas/estimate.py @@ -35,9 +35,10 @@ class EstimateGenerateResponse(BaseModel): # Multi-workspace P10Y estimation models -class ComponentEstimation(BaseModel): - """Estimation metrics for a single component.""" - component_name: str +class PhaseEstimation(BaseModel): + """Estimation metrics for a single implementation-plan phase.""" + phase_number: Optional[int] # None for the "unphased" bucket + phase_name: str hours: float new_work: float refactor: float @@ -64,10 +65,15 @@ class WorkspaceEstimation(BaseModel): workspace_path: str total_hours: float total_effective_output: float - component_breakdown: Dict[str, ComponentEstimation] + phase_breakdown: Dict[str, PhaseEstimation] estimation_metrics: EstimationMetrics commits_count: int p10y_scored_commits: Optional[int] = None + p10y_returned_commits: Optional[int] = Field( + default=None, + description="Eligible commits P10Y returned a row for, before the valid-technology " + "filter. Lets coverage be split into service-gap vs non-code (docs/config) causes.", + ) model_usage: Optional[ModelTokenUsage] = None total_usd_cost: Optional[float] = Field( default=None, @@ -88,9 +94,10 @@ def _serialize_model_usage(self, v: Optional[ModelTokenUsage]) -> Optional[Dict[ return None if v is None else v.to_dict() -class ComponentComparison(BaseModel): - """Comparison of a component across workspaces.""" - component_name: str +class PhaseComparison(BaseModel): + """Comparison of one implementation-plan phase across workspaces.""" + phase_number: Optional[int] # None for the "unphased" bucket + phase_name: str hours_by_workspace: Dict[str, float] # workspace_name -> hours average: float std_deviation: float @@ -99,8 +106,8 @@ class ComponentComparison(BaseModel): class ComparativeAnalysis(BaseModel): """Comparative analysis across all workspaces.""" - component_comparison: Dict[str, ComponentComparison] - high_variance_components: List[str] + phase_comparison: Dict[str, PhaseComparison] + high_variance_phases: List[str] insights: List[str] diff --git a/backend/app/services/claude_code.py b/backend/app/services/claude_code.py index 159a9a3..ed3fbbd 100644 --- a/backend/app/services/claude_code.py +++ b/backend/app/services/claude_code.py @@ -66,6 +66,8 @@ classify_error, get_fallback_model, ) +from app.services.git_utils import run_git +from app.utils.workspace_git_hooks import ensure_workspace_git_hooks from app.schemas.deploy_context import DeployGithubContext from app.schemas.planning import PlanningResult from app.schemas.specification import GenerateAppRequest, SpecReadiness @@ -1404,6 +1406,19 @@ async def execute_all_phases( f"(out of {planning_data.phase_count} total phases)" ) + # Deterministic phase attribution: install the prepare-commit-msg hook that stamps + # each commit subject with pNN_. Done here (not in a setup path) so it is guaranteed + # present before any commit regardless of how .git was created (init/clone/reset). + # The hook reads `specflow.phase`, set per codegen phase in the loop below; for the + # deploy loop we clear it so deploy commits stay unphased. + hook_root = Path(workspace.get_isolated_root()) + ensure_workspace_git_hooks(hook_root) + if is_deployment: + try: + await run_git(hook_root, ["config", "specflow.phase", ""]) + except Exception as exc: + logger.warning(f"[{workspace_name}] could not clear phase marker (non-fatal): {exc}") + step = WorkflowStepName.DEPLOY_AND_E2E if is_deployment else WorkflowStepName.GENERATION selector = McpSelector(manager.settings, enabled_mcps, logger) @@ -1446,6 +1461,14 @@ async def execute_all_phases( TelemetryContext.set_phase_name(phase_info.name or "") workspace_root = workspace.get_isolated_root() + if not is_deployment: + # Stamp commits made during this phase with pNN_ (consumed by the hook above). + try: + await run_git(Path(workspace_root), ["config", "specflow.phase", str(phase_num)]) + except Exception as exc: + logger.warning( + f"[{workspace_name}] could not set phase marker {phase_num} (non-fatal): {exc}" + ) if is_deployment: phase_prompt = generate_deploy_phase_agent_template( model=workspace.model, diff --git a/backend/app/services/p10y/estimation_report_generator.py b/backend/app/services/p10y/estimation_report_generator.py index 3bf067b..90bbd7f 100644 --- a/backend/app/services/p10y/estimation_report_generator.py +++ b/backend/app/services/p10y/estimation_report_generator.py @@ -9,77 +9,83 @@ from app.schemas.estimate import ( ComparativeAnalysis, - ComponentComparison, + PhaseComparison, EstimationSummary, SkippedWorkspaceP10Y, WorkspaceEstimation, ) +def _phase_number_label(phase_number: Optional[int]) -> str: + """Two-digit phase number for display, or an em dash for the unphased bucket.""" + return f"{phase_number:02d}" if phase_number is not None else "—" + + def create_comparison_table( workspace_estimations: List[WorkspaceEstimation], - component_comparison: Dict[str, ComponentComparison], + phase_comparison: Dict[str, PhaseComparison], ) -> str: """ - Create a markdown table comparing components across workspaces. - + Create a markdown table comparing implementation-plan phases across workspaces. + Args: workspace_estimations: List of workspace estimation results - component_comparison: Component comparison data - + phase_comparison: Per-phase comparison data (keyed in plan order) + Returns: Markdown formatted comparison table """ - if not component_comparison: - return "_No components to compare_\n" - + if not phase_comparison: + return "_No phases to compare_\n" + # Get all workspace names for column headers workspace_names = [ws.workspace_name for ws in workspace_estimations] - - # Build table header - header = "| Component | " + " | ".join(workspace_names) + " | Average | Std Dev | Variance % |\n" - separator = "|" + "---|" * (len(workspace_names) + 4) + "\n" - - # Build table rows + + # Build table header (Phase # + Phase name + one column per workspace + stats) + header = ( + "| Phase # | Description | " + + " | ".join(workspace_names) + + " | Average | Std Dev | Variance % |\n" + ) + separator = "|" + "---|" * (len(workspace_names) + 5) + "\n" + + # Build table rows in plan order (dict keys are zero-padded numbers; unphased sorts last) rows = [] - for comp_name, comp_comp in sorted(component_comparison.items()): - row_parts = [f"**{comp_name}**"] - - # Add hours for each workspace (or "-" if component not present) + for _key, comp in sorted(phase_comparison.items()): + row_parts = [_phase_number_label(comp.phase_number), f"**{comp.phase_name}**"] + + # Add hours for each workspace (or "-" if the phase produced no scored work there) for ws_name in workspace_names: - hours = comp_comp.hours_by_workspace.get(ws_name, 0.0) - if hours > 0: - row_parts.append(f"{hours:.1f}h") - else: - row_parts.append("-") - + hours = comp.hours_by_workspace.get(ws_name, 0.0) + row_parts.append(f"{hours:.1f}h" if hours > 0 else "-") + # Add average, std dev, and variance - row_parts.append(f"{comp_comp.average:.1f}h") - row_parts.append(f"{comp_comp.std_deviation:.1f}h") - row_parts.append(f"{comp_comp.variance_percentage:.1f}%") - + row_parts.append(f"{comp.average:.1f}h") + row_parts.append(f"{comp.std_deviation:.1f}h") + row_parts.append(f"{comp.variance_percentage:.1f}%") + rows.append("| " + " | ".join(row_parts) + " |") - + return header + separator + "\n".join(rows) + "\n" def visualize_variance(summary: EstimationSummary) -> str: """ Create ASCII/markdown visualization of variance. - + Args: summary: Estimation summary with statistics - + Returns: Markdown formatted variance visualization """ cv_percent = summary.coefficient_of_variation * 100 - + # Create a simple bar chart representation bar_length = 50 filled_length = min(int((cv_percent / 100) * bar_length), bar_length) bar = "█" * filled_length + "░" * (bar_length - filled_length) - + # Determine color indicator based on variance level if summary.variance_assessment == "low": indicator = "🟢" @@ -90,7 +96,7 @@ def visualize_variance(summary: EstimationSummary) -> str: else: # high indicator = "🔴" message = "High variance - spec clarity or implementation differences" - + visualization = f""" ### Variance Visualization @@ -107,7 +113,7 @@ def visualize_variance(summary: EstimationSummary) -> str: - **Range**: {summary.min_hours:.1f}h - {summary.max_hours:.1f}h - **Spread**: {summary.max_hours - summary.min_hours:.1f}h ({_spread_pct_of_average(summary):.1f}% of average) """ - + return visualization @@ -160,117 +166,129 @@ def _aggregate_p10y_executive_note( aggregate_p10y_commit_coverage_pct: Optional[float], ) -> str: """ - Executive-summary wording for commit coverage (ops v0.4.0 #4). + Executive-summary wording for commit coverage, split by cause. - - 100%: say nothing - - 0%: brief note only (no redundant “0%” spam) - - partial: total hours + % of commits not processed + "Unscored" commits are never SKIP_* (those are excluded from the eligible count upstream). + They fall into two very different buckets that this note keeps separate: + - service gap: commits P10Y did not return a score for (third-party availability), and + - non-code: commits P10Y returned but filtered out by technology (docs/config), which are + not expected to contribute code-complexity points. """ if not workspace_estimations: return "" - cov = aggregate_p10y_commit_coverage_pct - if cov is None: - return "" - if cov >= 99.95: + eligible = sum(ws.commits_count for ws in workspace_estimations) + if eligible <= 0: return "" - total_hours = sum(ws.total_hours for ws in workspace_estimations) - if cov <= 0.05: - return ( - f"\n**P10Y availability**: No eligible commits received P10Y scores in this run " - f"(aggregate hour total from workspaces: **{total_hours:.1f}h**).\n\n" + scored = sum((ws.p10y_scored_commits or 0) for ws in workspace_estimations) + # `returned` falls back to `scored` when the field is absent so older data shows no false gap. + returned = sum( + ( + ws.p10y_returned_commits + if ws.p10y_returned_commits is not None + else (ws.p10y_scored_commits or 0) ) - missing = 100.0 - cov + for ws in workspace_estimations + ) + service_gap = max(0, eligible - returned) + non_code = max(0, returned - scored) + total_hours = sum(ws.total_hours for ws in workspace_estimations) + + if service_gap == 0 and non_code == 0: + return "" + + gap_pct = service_gap / eligible * 100.0 + non_code_pct = non_code / eligible * 100.0 return ( - f"\n**P10Y availability**: Total effort across workspaces with estimates is " - f"**{total_hours:.1f}h**; approximately **{missing:.1f}%** of eligible commits did not " - f"receive P10Y scores (third-party service).\n\n" + f"\n**P10Y availability**: {scored}/{eligible} eligible commits scored " + f"(aggregate effort across workspaces **{total_hours:.1f}h**). Of the remainder, " + f"**{gap_pct:.1f}%** were not returned by P10Y (third-party service gap) and " + f"**{non_code_pct:.1f}%** were non-code commits filtered out by technology " + f"(docs/config — not expected to score).\n\n" ) def format_workspace_breakdown(workspace_estimations: List[WorkspaceEstimation]) -> str: """ Format per-workspace breakdown section. - + Args: workspace_estimations: List of workspace estimation results - + Returns: Markdown formatted workspace breakdown """ sections = [] - + for i, ws_est in enumerate(workspace_estimations, 1): p10y_line = _p10y_note_for_workspace(ws_est) section = f""" ### {i}. {ws_est.workspace_name} -**Total Estimated Hours**: {ws_est.total_hours:.1f}h -**Total Effective Output Points**: {ws_est.total_effective_output:.1f} +**Total Estimated Hours**: {ws_est.total_hours:.1f}h +**Total Effective Output Points**: {ws_est.total_effective_output:.1f} **Commits Analyzed**: {ws_est.commits_count} {p10y_line} #### Work Type Breakdown - **New Work**: {ws_est.estimation_metrics.new_work:.1f}{_pct_of_total_suffix(ws_est.estimation_metrics.new_work, ws_est.total_effective_output)} - **Refactor**: {ws_est.estimation_metrics.refactor:.1f}{_pct_of_total_suffix(ws_est.estimation_metrics.refactor, ws_est.total_effective_output)} - **Rework**: {ws_est.estimation_metrics.rework:.1f}{_pct_of_total_suffix(ws_est.estimation_metrics.rework, ws_est.total_effective_output)} -- **Removed Work**: {ws_est.estimation_metrics.removed_work:.1f} +- **Removed Work**: {ws_est.estimation_metrics.removed_work:.1f} - **Quality Score**: {ws_est.estimation_metrics.quality_score:.2f}/1.00 -- **Total Output**: {ws_est.estimation_metrics.total_output:.1f} -#### Component Complexity Metrics Breakdown ({len(ws_est.component_breakdown)} components) +- **Total Output**: {ws_est.estimation_metrics.total_output:.1f} +#### Phase Breakdown ({len(ws_est.phase_breakdown)} phases) """ - - # Sort components by hours (descending) - sorted_components = sorted( - ws_est.component_breakdown.items(), - key=lambda x: x[1].hours, - reverse=True - ) - - if sorted_components: - for comp_name, comp_est in sorted_components: - section += f"- **{comp_name}**: {comp_est.hours:.1f}h (Quality: {comp_est.quality_score:.2f})\n" + + # Sort phases in plan order (dict keys are zero-padded numbers; unphased sorts last) + sorted_phases = sorted(ws_est.phase_breakdown.items()) + + if sorted_phases: + for _key, phase_est in sorted_phases: + num = _phase_number_label(phase_est.phase_number) + section += f"- **{num} {phase_est.phase_name}**: {phase_est.hours:.1f}h (Quality: {phase_est.quality_score:.2f})\n" else: - section += "_No component breakdown available_\n" - + section += "_No phase breakdown available_\n" + sections.append(section) - + return "\n".join(sections) -def format_high_variance_components( +def format_high_variance_phases( comparative_analysis: ComparativeAnalysis, - component_comparison: Dict[str, ComponentComparison], + phase_comparison: Dict[str, PhaseComparison], ) -> str: """ - Format high variance components section. - + Format high variance phases section. + Args: comparative_analysis: Comparative analysis data - component_comparison: Component comparison data - + phase_comparison: Per-phase comparison data + Returns: Markdown formatted high variance section """ - if not comparative_analysis.high_variance_components: - return "✅ **No high variance components detected** - All components show consistent estimates across workspaces.\n" - - section = f"⚠️ **{len(comparative_analysis.high_variance_components)} High Variance Component(s) Detected**\n\n" - section += "These components show significant differences across workspaces (CV > 30%):\n\n" - - for comp_name in comparative_analysis.high_variance_components: - comp_comp = component_comparison[comp_name] - section += f"- **{comp_name}**\n" - section += f" - Average: {comp_comp.average:.1f}h ± {comp_comp.std_deviation:.1f}h\n" - section += f" - Variance: {comp_comp.variance_percentage:.1f}%\n" + if not comparative_analysis.high_variance_phases: + return "✅ **No high variance phases detected** - All phases show consistent estimates across workspaces.\n" + + section = f"⚠️ **{len(comparative_analysis.high_variance_phases)} High Variance Phase(s) Detected**\n\n" + section += "These phases show significant differences across workspaces (CV > 30%):\n\n" + + for key in comparative_analysis.high_variance_phases: + comp = phase_comparison[key] + num = _phase_number_label(comp.phase_number) + section += f"- **{num} {comp.phase_name}**\n" + section += f" - Average: {comp.average:.1f}h ± {comp.std_deviation:.1f}h\n" + section += f" - Variance: {comp.variance_percentage:.1f}%\n" section += " - Range: " - - hours_values = [h for h in comp_comp.hours_by_workspace.values() if h > 0] + + hours_values = [h for h in comp.hours_by_workspace.values() if h > 0] if hours_values: section += f"{min(hours_values):.1f}h - {max(hours_values):.1f}h\n" else: section += "N/A\n" section += "\n" - + return section @@ -283,16 +301,16 @@ def format_multi_workspace_report( ) -> str: """ Generate a comprehensive markdown report for multi-workspace estimation. - + This is the main function that assembles all report sections into a complete document. - + Args: workspace_estimations: List of workspace estimation results summary: Statistical summary comparative_analysis: Comparative analysis skipped_workspaces: Workspaces that produced no estimate (best-effort P10Y) aggregate_p10y_commit_coverage_pct: Share of commits with P10Y scores (%) - + Returns: Complete markdown formatted report """ @@ -301,22 +319,22 @@ def format_multi_workspace_report( skipped_workspaces = skipped_workspaces or [] report_sections = [] - + # Header report_sections.append(f"""# Multi-Workspace Estimation Report -**Generated**: {datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M:%S UTC")} +**Generated**: {datetime.now(timezone.utc).strftime("%Y-%m-%d %H:%M:%S UTC")} **Workspaces Analyzed**: {len(workspace_estimations)} **Workspaces Skipped (no P10Y estimate)**: {len(skipped_workspaces)} --- """) - + # Executive Summary report_sections.append("""## Executive Summary """) - + report_sections.append( _aggregate_p10y_executive_note( workspace_estimations, aggregate_p10y_commit_coverage_pct @@ -324,13 +342,13 @@ def format_multi_workspace_report( ) report_sections.append(f""" -**Average Estimated Hours**: {summary.average_hours:.1f}h ± {summary.std_deviation:.1f}h -**Range**: {summary.min_hours:.1f}h - {summary.max_hours:.1f}h -**Coefficient of Variation**: {summary.coefficient_of_variation*100:.1f}% +**Average Estimated Hours**: {summary.average_hours:.1f}h ± {summary.std_deviation:.1f}h +**Range**: {summary.min_hours:.1f}h - {summary.max_hours:.1f}h +**Coefficient of Variation**: {summary.coefficient_of_variation*100:.1f}% **Variance Assessment**: {summary.variance_assessment.upper()} """) - + # Risk Assessment Section if summary.risk_assessment: risk = summary.risk_assessment @@ -350,10 +368,10 @@ def format_multi_workspace_report( - **Total Buffer**: {risk.total_buffer_pct*100:.1f}% """) - + # Variance Visualization report_sections.append(visualize_variance(summary)) - + # Quick Stats Table report_sections.append(""" ### Quick Statistics @@ -366,11 +384,11 @@ def format_multi_workspace_report( report_sections.append(f"| Maximum Hours | {summary.max_hours:.1f}h |\n") report_sections.append(f"| Standard Deviation | {summary.std_deviation:.1f}h |\n") report_sections.append(f"| Variance Assessment | {summary.variance_assessment.upper()} |\n") - + if summary.risk_assessment: report_sections.append(f"| Total Buffer | {summary.risk_assessment.total_buffer_pct*100:.1f}% |\n") report_sections.append(f"| Final Estimate | {summary.risk_assessment.final_estimate:.1f}h |\n") - + total_commits = sum(ws.commits_count for ws in workspace_estimations) report_sections.append(f"| Total Commits Analyzed | {total_commits} |\n") @@ -384,7 +402,7 @@ def format_multi_workspace_report( f"| P10Y commit coverage (eligible → scored) | " f"{aggregate_p10y_commit_coverage_pct:.1f}% |\n" ) - + report_sections.append("\n---\n") report_sections.append(format_skipped_workspaces_section(skipped_workspaces)) @@ -393,23 +411,23 @@ def format_multi_workspace_report( report_sections.append("## Per-Workspace Breakdown\n") report_sections.append(format_workspace_breakdown(workspace_estimations)) report_sections.append("\n---\n") - - # Component Comparison - report_sections.append("## Component Comparison\n\n") + + # Phase Comparison + report_sections.append("## Phase Comparison\n\n") report_sections.append(create_comparison_table( workspace_estimations, - comparative_analysis.component_comparison + comparative_analysis.phase_comparison )) report_sections.append("\n") - - # High Variance Components - report_sections.append("## High Variance Components\n\n") - report_sections.append(format_high_variance_components( + + # High Variance Phases + report_sections.append("## High Variance Phases\n\n") + report_sections.append(format_high_variance_phases( comparative_analysis, - comparative_analysis.component_comparison + comparative_analysis.phase_comparison )) report_sections.append("\n---\n") - + # Insights & Recommendations report_sections.append("## Key Insights\n\n") if comparative_analysis.insights: @@ -417,39 +435,39 @@ def format_multi_workspace_report( report_sections.append(f"- {insight}\n") else: report_sections.append("_No specific insights generated_\n") - + report_sections.append("\n---\n") - + # Recommendations report_sections.append("""## Recommendations ### Budgeting Recommendation """) - + if summary.variance_assessment == "low": report_sections.append(f""" -✅ **Use Average Estimate**: {summary.average_hours:.1f}h -The low variance ({summary.coefficient_of_variation*100:.1f}%) indicates excellent consistency. +✅ **Use Average Estimate**: {summary.average_hours:.1f}h +The low variance ({summary.coefficient_of_variation*100:.1f}%) indicates excellent consistency. You can confidently use the average estimate with a small contingency buffer (10-15%). """) elif summary.variance_assessment == "medium": report_sections.append(f""" -⚠️ **Use Conservative Estimate**: {summary.max_hours:.1f}h +⚠️ **Use Conservative Estimate**: {summary.max_hours:.1f}h The medium variance ({summary.coefficient_of_variation*100:.1f}%) suggests some implementation differences. Budget for the higher estimate to account for potential variations. Consider adding 15-20% contingency. """) else: # high report_sections.append(f""" -🔴 **Review Specifications Before Budgeting**: Range {summary.min_hours:.1f}h - {summary.max_hours:.1f}h +🔴 **Review Specifications Before Budgeting**: Range {summary.min_hours:.1f}h - {summary.max_hours:.1f}h The high variance ({summary.coefficient_of_variation*100:.1f}%) indicates significant uncertainty. -Recommend spec review and clarification before finalizing budget. Consider budgeting for {summary.max_hours:.1f}h +Recommend spec review and clarification before finalizing budget. Consider budgeting for {summary.max_hours:.1f}h with 25-30% contingency, or investigate the root causes of variance first. """) - + report_sections.append(""" ### Next Steps -1. Review high-variance components (if any) to understand root causes +1. Review high-variance phases (if any) to understand root causes 2. Consider which workspace's approach best matches your requirements 3. Factor in team experience and tooling when applying these estimates 4. Add appropriate contingency based on variance assessment @@ -458,5 +476,5 @@ def format_multi_workspace_report( _This report is generated from actual code commits analyzed by multiple AI agents using P10Y metrics._ """) - + return "".join(report_sections) diff --git a/backend/app/services/p10y/multi_workspace_estimation.py b/backend/app/services/p10y/multi_workspace_estimation.py index 5199af2..ec99820 100644 --- a/backend/app/services/p10y/multi_workspace_estimation.py +++ b/backend/app/services/p10y/multi_workspace_estimation.py @@ -8,8 +8,8 @@ from app.schemas.model_token_usage import ModelTokenUsage from app.schemas.estimate import ( - ComponentComparison, - ComponentEstimation, + PhaseComparison, + PhaseEstimation, EstimationMetrics, EstimationSummary, WorkspaceEstimation, @@ -20,7 +20,7 @@ Estimation, apply_productivity_multiplier, calculate_estimation, - generate_component_breakdown, + generate_phase_breakdown, ) # Variance thresholds for coefficient of variation (CV) @@ -28,6 +28,18 @@ CV_MEDIUM = 0.30 # 15-30% variance - moderate # > 30% variance - high inconsistency +# Sentinel bucket for commits with no implementation-plan phase prefix. +UNPHASED_KEY = "unphased" + +# Shown when a phase carries no name in the plan. Deliberately not "Phase N": the number +# already has its own column, so repeating it would say nothing. +UNNAMED_PHASE = "—" + + +def _phase_key(phase_number: Optional[int]) -> str: + """Stable, sort-friendly dict key for a phase (zero-padded number, or the unphased sentinel).""" + return UNPHASED_KEY if phase_number is None else f"{phase_number:02d}" + def estimation_to_metrics(estimation: Estimation) -> EstimationMetrics: """Convert P10Y Estimation dataclass to EstimationMetrics Pydantic model.""" @@ -42,13 +54,17 @@ def estimation_to_metrics(estimation: Estimation) -> EstimationMetrics: ) -def estimation_to_component_estimation( - component_name: str, estimation: Estimation, multiplier: float = 2.0 -) -> ComponentEstimation: - """Convert P10Y Estimation dataclass to ComponentEstimation Pydantic model.""" +def estimation_to_phase_estimation( + phase_number: Optional[int], + phase_name: str, + estimation: Estimation, + multiplier: float = 2.0, +) -> PhaseEstimation: + """Convert a P10Y Estimation dataclass to a PhaseEstimation Pydantic model.""" hours = apply_productivity_multiplier(estimation, multiplier) - return ComponentEstimation( - component_name=component_name, + return PhaseEstimation( + phase_number=phase_number, + phase_name=phase_name, hours=hours, new_work=estimation.new_work, refactor=estimation.refactor, @@ -63,6 +79,8 @@ async def estimate_single_workspace( code_generation_metadata: CodeGenerationMetadata, logger: logging.Logger, multiplier: float = 2.0, + phase_names: Optional[Dict[int, str]] = None, + p10y_returned_commits: Optional[int] = None, ) -> Optional[WorkspaceEstimation]: """ Run estimation for a single workspace. @@ -72,13 +90,19 @@ async def estimate_single_workspace( filtered_commit_stats_data: P10Y commit stats already filtered for this workspace code_generation_metadata: Commit metadata already loaded by the caller (same snapshot used to derive the P10Y allowlist — avoids a second git-log subprocess and ensures - the commits used for component breakdown are identical to those used for filtering) + the commits used for the phase breakdown are identical to those used for filtering) logger: Logger instance multiplier: Productivity multiplier (default 2.0) + phase_names: Map of phase number -> name from the shared implementation plan. Used to + label the phase breakdown; the number is the join key across variants. + p10y_returned_commits: Count of eligible commits P10Y returned a row for (before the + valid-technology filter), used downstream to split coverage into service-gap vs + non-code causes. Returns: WorkspaceEstimation or None if estimation fails """ + phase_names = phase_names or {} try: if not code_generation_metadata or not code_generation_metadata.commits: logger.warning( @@ -86,36 +110,42 @@ async def estimate_single_workspace( f"(repo {workspace.full_workspace_path})" ) return None - + # Calculate overall estimation estimation: Estimation = await calculate_estimation(filtered_commit_stats_data) - - # Calculate component breakdown - component_breakdown_raw = generate_component_breakdown( + + # Calculate per-phase breakdown (keyed by phase number; None -> unphased) + phase_breakdown_raw = generate_phase_breakdown( code_generation_metadata, filtered_commit_stats_data ) - - # Convert to Pydantic models - component_breakdown = { - comp_name: estimation_to_component_estimation(comp_name, comp_est, multiplier) - for comp_name, comp_est in component_breakdown_raw.items() - } - + + # Convert to Pydantic models, resolving the human name from the shared plan. + phase_breakdown: Dict[str, PhaseEstimation] = {} + for phase_number, phase_est in phase_breakdown_raw.items(): + if phase_number is None: + name = UNPHASED_KEY + else: + name = phase_names.get(phase_number) or UNNAMED_PHASE + phase_breakdown[_phase_key(phase_number)] = estimation_to_phase_estimation( + phase_number, name, phase_est, multiplier + ) + # Calculate total hours total_hours = apply_productivity_multiplier(estimation, multiplier) - + return WorkspaceEstimation( workspace_name=workspace.name, workspace_path=str(workspace.workspace_path), total_hours=total_hours, total_effective_output=estimation.effective_output, - component_breakdown=component_breakdown, + phase_breakdown=phase_breakdown, estimation_metrics=estimation_to_metrics(estimation), commits_count=len(code_generation_metadata.commits), p10y_scored_commits=len(filtered_commit_stats_data), + p10y_returned_commits=p10y_returned_commits, model_usage=ModelTokenUsage(model_name=workspace.model or ""), ) - + except Exception as e: logger.error( f"Failed to estimate workspace {workspace.name}: {e}", exc_info=True @@ -128,32 +158,32 @@ def calculate_estimation_statistics( ) -> EstimationSummary: """ Calculate statistical summary across all workspace estimations. - + Args: workspace_estimations: List of workspace estimation results - + Returns: EstimationSummary with statistics """ if not workspace_estimations: raise ValueError("No workspace estimations provided") - + hours_list = [ws.total_hours for ws in workspace_estimations] - + average_hours = statistics.mean(hours_list) - + if len(hours_list) > 1: std_deviation = statistics.stdev(hours_list) coefficient_of_variation = std_deviation / average_hours if average_hours > 0 else 0 else: std_deviation = 0.0 coefficient_of_variation = 0.0 - + min_hours = min(hours_list) max_hours = max(hours_list) - + variance_assessment = assess_variance(coefficient_of_variation) - + return EstimationSummary( average_hours=average_hours, std_deviation=std_deviation, @@ -167,10 +197,10 @@ def calculate_estimation_statistics( def assess_variance(coefficient_of_variation: float) -> str: """ Classify variance level based on coefficient of variation. - + Args: coefficient_of_variation: CV value (std_dev / mean) - + Returns: "low", "medium", or "high" """ @@ -182,107 +212,113 @@ def assess_variance(coefficient_of_variation: float) -> str: return "high" -def generate_component_comparison( +def generate_phase_comparison( workspace_estimations: List[WorkspaceEstimation], -) -> Dict[str, ComponentComparison]: +) -> Dict[str, PhaseComparison]: """ - Generate component-level comparison across workspaces. - + Generate per-phase comparison across workspaces. + + The phase key (zero-padded number, or the unphased sentinel) is derived deterministically + from the shared implementation plan, so the same phase joins exactly across variants — no + fuzzy name matching needed. + Args: workspace_estimations: List of workspace estimation results - + Returns: - Dictionary mapping component names to ComponentComparison objects + Dictionary mapping phase key to PhaseComparison objects """ - # Collect all unique component names across workspaces - all_components: set[str] = set() + all_keys: set[str] = set() for ws_est in workspace_estimations: - all_components.update(ws_est.component_breakdown.keys()) - - component_comparisons = {} - - for component_name in all_components: - # Normalize component name for matching - normalized_name = component_name.lower().strip() - - # Collect hours for this component across workspaces - hours_by_workspace = {} + all_keys.update(ws_est.phase_breakdown.keys()) + + phase_comparisons: Dict[str, PhaseComparison] = {} + + for key in all_keys: + hours_by_workspace: Dict[str, float] = {} + phase_number: Optional[int] = None + phase_name: str = key for ws_est in workspace_estimations: - # Try to find component (case-insensitive) - for comp_key, comp_est in ws_est.component_breakdown.items(): - if comp_key.lower().strip() == normalized_name: - hours_by_workspace[ws_est.workspace_name] = comp_est.hours - break - - # Calculate statistics - if hours_by_workspace: - hours_values = list(hours_by_workspace.values()) - average = statistics.mean(hours_values) - - if len(hours_values) > 1: - std_deviation = statistics.stdev(hours_values) - variance_percentage = ( - (std_deviation / average * 100) if average > 0 else 0 - ) - else: - std_deviation = 0.0 - variance_percentage = 0.0 - - component_comparisons[component_name] = ComponentComparison( - component_name=component_name, - hours_by_workspace=hours_by_workspace, - average=average, - std_deviation=std_deviation, - variance_percentage=variance_percentage, - ) - - return component_comparisons + phase_est = ws_est.phase_breakdown.get(key) + if phase_est is None: + continue + hours_by_workspace[ws_est.workspace_name] = phase_est.hours + phase_number = phase_est.phase_number + phase_name = phase_est.phase_name + + if not hours_by_workspace: + continue + hours_values = list(hours_by_workspace.values()) + average = statistics.mean(hours_values) + if len(hours_values) > 1: + std_deviation = statistics.stdev(hours_values) + variance_percentage = (std_deviation / average * 100) if average > 0 else 0 + else: + std_deviation = 0.0 + variance_percentage = 0.0 -def identify_high_variance_components( - component_comparison: Dict[str, ComponentComparison], + phase_comparisons[key] = PhaseComparison( + phase_number=phase_number, + phase_name=phase_name, + hours_by_workspace=hours_by_workspace, + average=average, + std_deviation=std_deviation, + variance_percentage=variance_percentage, + ) + + return phase_comparisons + + +def identify_high_variance_phases( + phase_comparison: Dict[str, PhaseComparison], threshold: float = CV_MEDIUM, ) -> List[str]: """ - Identify components with high variance across workspaces. - + Identify phases with high variance across workspaces. + Args: - component_comparison: Dictionary of component comparisons + phase_comparison: Dictionary of phase comparisons threshold: CV threshold for high variance (default 0.30 = 30%) - + Returns: - List of component names with high variance + Sorted list of phase keys with high variance """ high_variance = [] - - for comp_name, comp_comp in component_comparison.items(): - # Calculate coefficient of variation - cv = comp_comp.std_deviation / comp_comp.average if comp_comp.average > 0 else 0 - + + for key, comp in phase_comparison.items(): + cv = comp.std_deviation / comp.average if comp.average > 0 else 0 if cv > threshold: - high_variance.append(comp_name) - + high_variance.append(key) + return sorted(high_variance) def generate_insights( workspace_estimations: List[WorkspaceEstimation], summary: EstimationSummary, - high_variance_components: List[str], + high_variance_phases: List[str], ) -> List[str]: """ Generate insights about variance and potential root causes. - + Args: workspace_estimations: List of workspace estimation results summary: Statistical summary - high_variance_components: List of components with high variance - + high_variance_phases: List of phase keys with high variance + Returns: List of insight strings """ insights = [] - + + # Human-friendly phase labels for messages (name resolved from the shared plan). + phase_label = { + key: pe.phase_name + for ws in workspace_estimations + for key, pe in ws.phase_breakdown.items() + } + # Overall variance assessment if summary.variance_assessment == "low": insights.append( @@ -296,7 +332,7 @@ def generate_insights( insights.append( "Overall variance is high (CV > 30%), indicating significant inconsistencies that warrant investigation." ) - + # Commit count variance commit_counts = [ws.commits_count for ws in workspace_estimations] if len(commit_counts) > 1: @@ -309,25 +345,26 @@ def generate_insights( insights.append( f"Large variance in commit counts ({min(commit_counts)}-{max(commit_counts)}) suggests different interpretation of specifications or commit strategies." ) - - # Component structure differences - component_sets = [set(ws.component_breakdown.keys()) for ws in workspace_estimations] - if len(component_sets) > 1: - common_components = set.intersection(*component_sets) - all_components = set.union(*component_sets) - coverage = len(common_components) / len(all_components) if all_components else 1.0 - + + # Phase coverage differences (with a shared plan this should trend high) + phase_sets = [set(ws.phase_breakdown.keys()) for ws in workspace_estimations] + if len(phase_sets) > 1: + common_phases = set.intersection(*phase_sets) + all_phases = set.union(*phase_sets) + coverage = len(common_phases) / len(all_phases) if all_phases else 1.0 + if coverage < 0.7: insights.append( - f"Only {coverage*100:.0f}% of components are common across workspaces, indicating architectural differences in implementation." + f"Only {coverage*100:.0f}% of plan phases produced measurable work across all workspaces, indicating some phases diverged in scope or commit hygiene." ) - - # High variance components - if high_variance_components: + + # High variance phases + if high_variance_phases: + labels = [phase_label.get(k, k) for k in high_variance_phases[:3]] insights.append( - f"Components with high variance ({', '.join(high_variance_components[:3])}) may require specification clarification or standardized implementation approach." + f"Phases with high variance ({', '.join(labels)}) may require specification clarification or a standardized implementation approach." ) - + # Quality score variance quality_scores = [ws.estimation_metrics.quality_score for ws in workspace_estimations] if len(quality_scores) > 1: @@ -340,7 +377,7 @@ def generate_insights( insights.append( f"Quality scores vary significantly (CV {quality_cv*100:.1f}%), suggesting different code quality standards or testing approaches." ) - + # Work type ratio analysis for ws in workspace_estimations: metrics = ws.estimation_metrics @@ -351,5 +388,5 @@ def generate_insights( insights.append( f"Workspace '{ws.workspace_name}' has high refactor ratio ({refactor_ratio*100:.0f}%), indicating significant code restructuring." ) - + return insights diff --git a/backend/app/services/p10y/p10y_lib.py b/backend/app/services/p10y/p10y_lib.py index 62473e2..8ec3e56 100644 --- a/backend/app/services/p10y/p10y_lib.py +++ b/backend/app/services/p10y/p10y_lib.py @@ -1,24 +1,17 @@ from dataclasses import dataclass import asyncio -import json import logging import os import subprocess -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, NamedTuple, Optional from app.services.p10y.p10y_api_client import P10YInternalAPIClient -# Commits whose first line starts with this prefix are excluded from P10Y / component breakdown +# Commits whose first line starts with this prefix are excluded from P10Y / phase breakdown # (e.g. user-provided initial seed: SKIP_initial_user_source, SKIP_generation_baseline). SKIP_COMMIT_PREFIX = "SKIP_" -# Valid component tokens — kept in sync with backend/app/standards/commit_standards.md. -KNOWN_COMPONENTS = frozenset({ - "backend", "frontend", "database", "api", "auth", - "infrastructure", "testing", "documentation", "pipeline", "ml", "mobile", "common", -}) - commit_stats_fields = set(["sha", "ep_total", "fp_delta_total", "fp_delta_positive_total", "fp_delta_negative_total", "ep_total_refactor", "commit_quality_score", "churn_rate", "technologies", "id_contributor", "refactor", "rework", "new_work", "removed_work", "quality_score", "effective_output", "total_output"]) @@ -70,7 +63,7 @@ def has_valid_technology(commit_stats: Dict[str, Any], logger: logging.Logger) - class CommitInfo: sha: str message: str - component: List[str] + phase: Optional[int] @dataclass @@ -138,19 +131,22 @@ def extract_estimation_from_commit_stats(estimation: Estimation, commit_stats: D estimation.total_output += float(commit_stats.get("total_output", 0)) return estimation -def generate_component_breakdown( - metadata: CodeGenerationMetadata, - commit_stats_data: List[Dict[str, Any]] -) -> Dict[str, Estimation]: +def generate_phase_breakdown( + metadata: CodeGenerationMetadata, + commit_stats_data: List[Dict[str, Any]], +) -> Dict[Optional[int], Estimation]: """ - Generate component-level breakdown of estimation metrics. - + Aggregate estimation metrics per implementation-plan phase. + + Each commit is attributed to its phase (parsed from the `pNN_` subject prefix). Commits + with no phase prefix are bucketed under None ("unphased"). + Args: - metadata: Code generation metadata with commit info including components - commit_stats_data: P10Y commit statistics data - + metadata: Code generation metadata with per-commit phase numbers. + commit_stats_data: P10Y commit statistics data. + Returns: - Dictionary mapping component names to their aggregated Estimation + Mapping of phase number (or None for unphased) to aggregated Estimation. """ commit_stats_map = { _normalize_git_sha(commit.get("sha") or ""): commit @@ -158,29 +154,23 @@ def generate_component_breakdown( if commit.get("sha") } - component_breakdown: Dict[str, List[Dict[str, Any]]] = {} + phase_commits: Dict[Optional[int], List[Dict[str, Any]]] = {} for commit_info in metadata.commits: commit_sha = _normalize_git_sha(commit_info.sha or "") if not commit_sha or commit_sha not in commit_stats_map: continue - commit_stats = commit_stats_map[commit_sha] - for component in commit_info.component: - if component not in component_breakdown: - component_breakdown[component] = [] - component_breakdown[component].append(commit_stats) - - component_estimations: Dict[str, Estimation] = {} - for component, stats_list in component_breakdown.items(): + phase_commits.setdefault(commit_info.phase, []).append(commit_stats_map[commit_sha]) + + phase_estimations: Dict[Optional[int], Estimation] = {} + for phase, stats_list in phase_commits.items(): estimation = Estimation.prepare_for_estimation() - for commit_stats in stats_list: estimation = extract_estimation_from_commit_stats(estimation, commit_stats) - - component_estimations[component] = estimation - - return component_estimations + phase_estimations[phase] = estimation + + return phase_estimations def apply_productivity_multiplier(estimation: Estimation, multiplier: float = 2.0) -> float: """ @@ -198,33 +188,6 @@ def apply_productivity_multiplier(estimation: Estimation, multiplier: float = 2. # The multiplier accounts for developer proficiency, tooling (Claude Code), etc. return estimation.function_points * multiplier -def format_component_breakdown(component_breakdown: Dict[str, Estimation], multiplier: float = 2.0) -> str: - """ - Format component breakdown for display in estimation summary. - - Args: - component_breakdown: Dictionary of component estimations - multiplier: Productivity multiplier for hours calculation - - Returns: - Formatted markdown string - """ - lines = [] - lines.append("## Component Breakdown\n") - - for component, estimation in sorted(component_breakdown.items()): - hours = apply_productivity_multiplier(estimation, multiplier) - lines.append(f"### {component.title()}") - lines.append(f"- Estimated Hours: {hours:.1f}") - lines.append(f"- New Work Units: {estimation.new_work:.1f}") - lines.append(f"- Refactor Units: {estimation.refactor:.1f}") - lines.append(f"- Rework Units: {estimation.rework:.1f}") - lines.append(f"- Removed Work Units: {estimation.removed_work:.1f}") - lines.append(f"- Quality Score: {estimation.quality_score:.2f}") - lines.append("") - - return "\n".join(lines) - def _subject_excluded_from_estimation(subject: str, logger: Optional[logging.Logger] = None) -> bool: s = (subject or "").strip() @@ -235,32 +198,26 @@ def _subject_excluded_from_estimation(subject: str, logger: Optional[logging.Log return s.upper().startswith(SKIP_COMMIT_PREFIX) -def _parse_component_from_subject(subject: str, logger: logging.Logger) -> List[str]: +def _parse_phase_from_subject(subject: str, logger: logging.Logger) -> Optional[int]: """ - Parse `{component}_{message}` commit subject (first line only). + Parse the implementation-plan phase from a `pNN_` commit subject (first line). - The substring before the first underscore is the component bucket; the rest is free text. - If there is no underscore, or the token is not in KNOWN_COMPONENTS, logs a warning - and falls back to "common". + The `pNN_` prefix is injected deterministically by the prepare-commit-msg git hook + (see app/utils/workspace_git_hooks.py), so a well-formed generation run always carries + it. Commits without a `pNN_` prefix (deploy, strays) return None and are bucketed as + "unphased" downstream. """ s = (subject or "").strip() if "_" not in s: - logger.warning( - "Commit subject has no underscore component prefix (expected component_message): %s — using [common]", - s[:80], - ) - return ["common"] - comp, _rest = s.split("_", 1) - comp = comp.lower().strip() - if not comp: - return ["common"] - if comp not in KNOWN_COMPONENTS: - logger.warning( - "Commit subject has unknown component token %r (not in KNOWN_COMPONENTS): %s — using as-is", - comp, - s[:80], + return None + token, _rest = s.split("_", 1) + token = token.strip().lower() + if len(token) < 2 or token[0] != "p" or not token[1:].isdigit(): + logger.debug( + "Commit subject has no pNN_ phase prefix: %s — using [unphased]", s[:80] ) - return [comp] + return None + return int(token[1:]) def _git_log_subject_lines(repo_root: str, logger: logging.Logger) -> List[tuple[str, str]]: @@ -333,9 +290,9 @@ def build_code_generation_metadata_from_git( subject[:60], ) continue - component = _parse_component_from_subject(subject, logger) + phase = _parse_phase_from_subject(subject, logger) commits.append( - CommitInfo(sha=sha, message=subject, component=component), + CommitInfo(sha=sha, message=subject, phase=phase), ) if not commits: @@ -343,16 +300,6 @@ def build_code_generation_metadata_from_git( return CodeGenerationMetadata(commits=commits) -def format_commits_metadata_for_prompt(metadata: CodeGenerationMetadata) -> str: - """Compact JSON for janitor / prompts (derived list, not an on-disk agent file).""" - data: List[Dict[str, Any]] = [ - {"sha": c.sha, "message": c.message, "component": c.component} - for c in metadata.commits - if c.sha - ] - return json.dumps(data, indent=2) - - async def load_code_generation_metadata(repo_root: str, logger: logging.Logger) -> Optional[CodeGenerationMetadata]: """ Load code-generation commit metadata from the workspace git history. @@ -438,6 +385,12 @@ def _normalize_git_sha(sha: str) -> str: return sha.strip().lower() +class FilteredCommitStats(NamedTuple): + """Result of intersecting P10Y rows with the local git allowlist.""" + rows: List[dict] # commits with a valid technology (used for scoring) + returned_count: int # commits P10Y returned for our allowlist, before the valid-tech filter + + async def fetch_and_filter_commit_stats( client: P10YInternalAPIClient, repository_id: int, @@ -445,7 +398,7 @@ async def fetch_and_filter_commit_stats( allowed_commit_shas: List[str], workspace_name: str, logger: logging.Logger, -) -> List[dict]: +) -> FilteredCommitStats: """ Fetch commit stats from P10Y and keep only rows whose ``sha`` is in the local git allowlist (full hash, case-insensitive). P10Y's API may return a longer history than @@ -460,7 +413,9 @@ async def fetch_and_filter_commit_stats( logger: Logger instance Returns: - Filtered list of commit stats (at most one row per allowed SHA, deduped) + FilteredCommitStats: the valid-technology rows used for scoring, plus the count of + commits P10Y returned for our allowlist (before the valid-technology filter) so + coverage can distinguish a P10Y service gap from non-code commits filtered by tech. """ allowed = {_normalize_git_sha(h) for h in allowed_commit_shas if h} count_of_commits = len(allowed) @@ -515,4 +470,7 @@ async def fetch_and_filter_commit_stats( f"Continuing with available data..." ) - return filtered_commit_stats_data \ No newline at end of file + return FilteredCommitStats( + rows=filtered_commit_stats_data, + returned_count=len(hash_filtered_commits), + ) \ No newline at end of file diff --git a/backend/app/services/skip_mode_mock.py b/backend/app/services/skip_mode_mock.py index 18a0d7f..ad3e425 100644 --- a/backend/app/services/skip_mode_mock.py +++ b/backend/app/services/skip_mode_mock.py @@ -29,7 +29,7 @@ from app.core.artifact_subdirs import ANALYSIS_SUBDIR, PLANNING_SUBDIR from app.core.telemetry_context import TelemetryContext from app.prompts.agents_claude_code import E2E_PHASES_FILE, PLANNING_PHASES_FILE -from app.schemas.estimate import ComponentEstimation, EstimationMetrics, WorkspaceEstimation +from app.schemas.estimate import PhaseEstimation, EstimationMetrics, WorkspaceEstimation from app.schemas.model_token_usage import ModelTokenUsage from app.schemas.planning import PhaseInfo, PlanningResult, PlanType from app.schemas.specification import SpecReadiness, SpecificationCompletenessResult @@ -906,10 +906,11 @@ def create_mock_workspace_estimation( effective_output = new_work + refactor total_output = new_work + refactor + rework + removed_work - # Single component that covers the mock code - component_breakdown: Dict[str, ComponentEstimation] = { - "specifications": ComponentEstimation( - component_name="specifications", + # Single phase that covers the mock code + phase_breakdown: Dict[str, PhaseEstimation] = { + "01": PhaseEstimation( + phase_number=1, + phase_name="Project Setup", hours=total_hours, new_work=new_work, refactor=refactor, @@ -938,10 +939,11 @@ def create_mock_workspace_estimation( workspace_path=str(workspace.workspace_path), total_hours=total_hours, total_effective_output=effective_output, - component_breakdown=component_breakdown, + phase_breakdown=phase_breakdown, estimation_metrics=estimation_metrics, commits_count=1, p10y_scored_commits=1, + p10y_returned_commits=1, model_usage=ModelTokenUsage(model_name=workspace.model or ""), ) diff --git a/backend/app/standards/commit_standards.md b/backend/app/standards/commit_standards.md index 7eee6d2..384a441 100644 --- a/backend/app/standards/commit_standards.md +++ b/backend/app/standards/commit_standards.md @@ -3,7 +3,7 @@ ## Purpose This document defines commit hygiene standards for code generation workflows to ensure: - Accurate P10Y metrics tracking -- Clear component attribution +- Clear phase attribution (which implementation-plan phase produced the work) - Proper granularity for generation breakdowns - Consistent commit history @@ -15,7 +15,7 @@ CRITICAL: add to .gitignore folders like .venv, node_modules, package-lock.json ### Ideal Commit Size - **50-300 lines of code changed** (optimal range) - Single logical unit of work -- One component or closely related components +- One feature or closely related change - Atomic: can be reverted without breaking unrelated features - Complete with tests (when applicable) @@ -30,13 +30,13 @@ CRITICAL: add to .gitignore folders like .venv, node_modules, package-lock.json - After setting up infrastructure or configuration that works end-to-end - After writing a batch of related tests for a feature -**Examples of good commits:** -- `backend_implement JWT token generation and validation` -- `frontend_add user profile form with validation` -- `database_create users and roles tables with migrations` -- `api_add REST endpoints for product catalog` -- `infrastructure_setup Docker compose with PostgreSQL and Redis` -- `testing_add integration tests for authentication flow` +**Examples of good commits** (you write the plain subject; the hook adds the phase prefix): +- `implement JWT token generation and validation` +- `add user profile form with validation` +- `create users and roles tables with migrations` +- `add REST endpoints for product catalog` +- `setup Docker compose with PostgreSQL and Redis` +- `add integration tests for authentication flow` ### When NOT to Commit ❌ @@ -47,7 +47,7 @@ CRITICAL: add to .gitignore folders like .venv, node_modules, package-lock.json - When code doesn't compile or has obvious errors **Examples of bad commits:** -- `Update everything` (too broad, no clear component) +- `Update everything` (too broad) - `Fix typo` (too small, should be bundled with feature work) - `WIP` (incomplete work, not atomic) - `Add all files` (too large, no clear scope) @@ -57,31 +57,23 @@ CRITICAL: add to .gitignore folders like .venv, node_modules, package-lock.json ### Standard Format (first line of commit message) -Use an **underscore** after the component token (metadata is parsed from `git log`, not from a JSON file): +Write a plain, descriptive first line: ``` -_ + ``` -Example: `backend_implement JWT token generation` - -Commits whose subject starts with **`SKIP_`** (case-insensitive) are **excluded** from P10Y / generation (e.g. `SKIP_initial_user_source` for user-provided seed code). - -### Components -Stick to already known component names, choose the closest matching name. -Valid component identifiers: -- `frontend` - UI/client-side code -- `backend` - Server-side application logic -- `api` - API endpoints and contracts -- `database` - Database schemas, migrations, models -- `auth` - Authentication and authorization -- `infrastructure` - Docker, deployment, CI/CD -- `testing` - Test code, test infrastructure -- `documentation` - README, docs, comments -- `pipeline` - data pipelines, orchestration of data projects -- `ml` - machine learning and data science, features, model training, A/B tests, evaluation, notebooks -- `mobile` - Mobile clients, native apps, cross-platform app manifests and build configuration -- `common` - Cross-cutting concerns, project setup +Example: `implement JWT token generation` + +**Do NOT add a component or phase prefix yourself.** During generation a `prepare-commit-msg` git +hook automatically prepends the current implementation-plan phase as `p_` (for example +`p07_implement JWT token generation`). P10Y groups commits by that phase prefix, so attribution is +deterministic and does not depend on your commit hygiene. Metadata is parsed from `git log`, not +from a JSON file. + +Commits whose subject starts with **`SKIP_`** (case-insensitive) are **excluded** from P10Y / +generation (e.g. `SKIP_initial_user_source` for user-provided seed code). The hook never adds a +phase prefix to `SKIP_` commits. ### Actions Common action verbs: @@ -98,16 +90,13 @@ Common action verbs: **Good commit messages:** ``` -backend_implement JWT token generation -frontend_add user profile form component -database_create initial schema with users table -api_add REST endpoints for order management -infrastructure_configure Docker Compose for local development -testing_add unit tests for payment service -common_setup project structure and dependencies -mobile_configure app manifest and build settings -frontend_refactor state management to use Redux Toolkit -backend_fix validation error handling in user endpoints +implement JWT token generation +add user profile form component +create initial schema with users table +add REST endpoints for order management +configure Docker Compose for local development +add unit tests for payment service +setup project structure and dependencies ``` **Bad commit messages:** @@ -116,37 +105,20 @@ update stuff fix changes wip -backend do things (missing underscore after component) implement everything (too broad) ``` -## Component Attribution Rules - -### Single Component Changes -When changes affect only one component: -- Single commit with single component identifier -- Example: `backend_implement password hashing` - -### Multi-Component Changes -When a feature requires changes across multiple components: -- **Sequential commits per component** (preferred) -- Each commit focuses on one component's changes -- Maintains clear attribution for generation +## Phase Attribution (automatic) -**Example sequence for a user registration feature:** -1. `database_create users table and migration` -2. `backend_implement user registration service` -3. `api_add user registration endpoint` -4. `frontend_add registration form component` -5. `testing_add integration tests for user registration` +Generation runs phase-by-phase from the implementation plan. Before each phase the harness records +the active phase number, and the `prepare-commit-msg` hook stamps every commit you make during that +phase with `p_`. Each included commit's subject is split on the **first** underscore for phase +grouping. -### Cross-Cutting Changes -For changes that truly affect multiple components simultaneously: -- Prefer several single-component commits; if one commit must cover everything, use `common_` - -## Metadata for P10Y (no JSON sidecar) - -Generation reads **`git log`** (oldest first, no merges). Each included commit’s subject is split on the **first** underscore for component grouping. +- One commit belongs to exactly one phase — the phase active when you committed. +- Prefer several small commits within a phase over one big commit. +- Commits made outside a codegen phase (initial seed, deployment) are left unphased and reported + separately; you never need to manage the prefix yourself. ## Commit Workflow @@ -160,12 +132,12 @@ Generation reads **`git log`** (oldest first, no merges). Each included commit 2. **Stage relevant files** ```bash - git add + git add ``` -3. **Create commit with proper message** +3. **Create commit with a plain descriptive message** (the hook adds the phase prefix) ```bash - git commit -m "component_action and subject" + git commit -m "implement user registration service" ``` 4. **Push commit** @@ -182,7 +154,7 @@ Generation reads **`git log`** (oldest first, no merges). Each included commit ### The "Big Bang" Commit ❌ **Problem**: One massive commit with entire application -- Impossible to attribute to specific components +- Impossible to attribute to specific phases - Can't track granular progress - Difficult to review or debug @@ -197,12 +169,11 @@ Generation reads **`git log`** (oldest first, no merges). Each included commit ✅ **Solution**: Bundle related small changes into logical commits ### The "Mixed Bag" Commit -❌ **Problem**: One commit touching frontend, backend, database, tests, docs -- Can't attribute to specific component +❌ **Problem**: One commit touching many unrelated areas at once - Breaks atomicity principle - Difficult to revert if needed -✅ **Solution**: Split into sequential commits per component +✅ **Solution**: Split into sequential, focused commits ### The "Vague Message" Commit ❌ **Problem**: Messages like "update", "fix", "changes" @@ -210,5 +181,4 @@ Generation reads **`git log`** (oldest first, no merges). Each included commit - Can't correlate with requirements - Poor documentation for future reference -✅ **Solution**: Use descriptive format: `component_action and subject` - +✅ **Solution**: Use a descriptive ` ` first line diff --git a/backend/app/utils/workspace_git_hooks.py b/backend/app/utils/workspace_git_hooks.py new file mode 100644 index 0000000..754e12a --- /dev/null +++ b/backend/app/utils/workspace_git_hooks.py @@ -0,0 +1,71 @@ +"""Install a ``prepare-commit-msg`` git hook that stamps the current implementation-plan +phase onto each commit subject (``pNN_``). + +The phase attribution used by P10Y must not depend on the agent's commit hygiene, so the +prefix is injected deterministically by git itself. The current phase number is read from +``git config specflow.phase``, which the codegen loop (``execute_all_phases``) sets before +each phase runs. Commits made outside that window (seed, janitor finalize, deploy) have no +(or a blank) marker and are left unprefixed — they fall into the ``unphased`` bucket or stay +excluded (``SKIP_*``). +""" + +import logging +from pathlib import Path + +logger = logging.getLogger(__name__) + +# Managed hook body. ``--no-verify`` bypasses pre-commit and commit-msg hooks but NOT +# prepare-commit-msg, so this fires on every commit the agent makes. +PREPARE_COMMIT_MSG_HOOK = """\ +#!/bin/sh +# SpecFlow: prefix the commit subject with the current implementation-plan phase (pNN_). +# Managed file — regenerated on every workspace setup. Do not edit by hand. +f="$1" +first=$(head -n 1 "$f") +case "$first" in + SKIP_*|skip_*) exit 0 ;; # never touch excluded seed/janitor commits + p[0-9][0-9]_*) exit 0 ;; # already prefixed (amend/rebase idempotency) +esac +phase=$(git config --get specflow.phase 2>/dev/null) || exit 0 +case "$phase" in ''|*[!0-9]*) exit 0 ;; esac # unset/non-numeric -> leave unphased +rest=$(tail -n +2 "$f") +printf 'p%02d_%s\\n%s' "$phase" "$first" "$rest" > "$f" +""" + + +def ensure_workspace_git_hooks(workspace_path: Path) -> bool: + """Install the ``prepare-commit-msg`` phase-stamping hook into the workspace repo. + + Idempotent (overwrites the managed file). Safe to call from init/prep paths: I/O + failures are logged and swallowed so they never propagate to abort git init or + workspace preparation. + + Returns: + True if the hook was written, False if unchanged/skipped or on error. + """ + try: + return _ensure_workspace_git_hooks(workspace_path) + except OSError as exc: + logger.warning( + "Could not install prepare-commit-msg hook in %s (non-fatal): %s", + workspace_path, + exc, + ) + return False + + +def _ensure_workspace_git_hooks(workspace_path: Path) -> bool: + if not (workspace_path / ".git").is_dir(): + logger.warning( + "No .git directory in %s — skipping prepare-commit-msg hook install", + workspace_path, + ) + return False + + hooks_dir = workspace_path / ".git" / "hooks" + hooks_dir.mkdir(parents=True, exist_ok=True) + hook_path = hooks_dir / "prepare-commit-msg" + hook_path.write_text(PREPARE_COMMIT_MSG_HOOK, encoding="utf-8") + hook_path.chmod(0o755) + logger.info("Installed prepare-commit-msg phase hook in %s", hooks_dir) + return True diff --git a/backend/app/workflows/multi_workspace_estimation_p10y.py b/backend/app/workflows/multi_workspace_estimation_p10y.py index f8377c0..e8690d2 100644 --- a/backend/app/workflows/multi_workspace_estimation_p10y.py +++ b/backend/app/workflows/multi_workspace_estimation_p10y.py @@ -52,9 +52,9 @@ from app.services.p10y.multi_workspace_estimation import ( calculate_estimation_statistics, estimate_single_workspace, - generate_component_comparison, + generate_phase_comparison, generate_insights, - identify_high_variance_components, + identify_high_variance_phases, ) from app.services.p10y.p10y_api_client import P10YInternalAPIClient from app.services.p10y.p10y_lib import ( @@ -83,6 +83,7 @@ async def _process_single_workspace( client: P10YInternalAPIClient, settings: Settings, logger: logging.Logger, + phase_names: Optional[Dict[int, str]] = None, ) -> WorkspaceEstimation | None: """ Process estimation for a single workspace. @@ -144,7 +145,7 @@ async def _process_single_workspace( ) # Fetch and filter commit stats - filtered_commit_stats_data = await fetch_and_filter_commit_stats( + filtered = await fetch_and_filter_commit_stats( client=client, repository_id=workspace.p10y_repository_id, organisation_id=settings.P10Y_ORGANISATION_ID, @@ -157,10 +158,12 @@ async def _process_single_workspace( # uses the same snapshot that was used to build the P10Y allowlist) workspace_estimation = await estimate_single_workspace( workspace=workspace, - filtered_commit_stats_data=filtered_commit_stats_data, + filtered_commit_stats_data=filtered.rows, code_generation_metadata=code_generation_metadata, logger=logger, multiplier=2.0, # Standard productivity multiplier + phase_names=phase_names, + p10y_returned_commits=filtered.returned_count, ) if workspace_estimation: @@ -204,6 +207,45 @@ def _aggregate_p10y_commit_coverage_pct( return (scored / eligible) * 100.0 +async def _load_phase_names( + generation_id: Optional[str], + db_adapter: Optional[StateMachineDBAdapter], + logger: logging.Logger, +) -> Dict[int, str]: + """Map phase number -> name from the shared implementation plan. + + The plan is persisted per workspace under ``workspace_phases[ws_id]["planning_data"]`` + (there is no top-level ``planning_data``); every entry carries the same plan, so the + first one with phases wins. The phase number is the cross-variant join key, so the + names resolved here label the same phase identically in every variant. + + Returns {} when unavailable — the breakdown then shows the phase number alone rather + than a label that just repeats it. + """ + if not generation_id or not db_adapter: + return {} + try: + doc = await db_adapter.get_generation_session(generation_id) + except Exception as e: + logger.warning("Could not load planning_data for phase names: %s", e) + return {} + for entry in ((doc or {}).get("workspace_phases") or {}).values(): + phases = ((entry or {}).get("planning_data") or {}).get("phases") or [] + names: Dict[int, str] = { + p["number"]: p["name"] + for p in phases + if isinstance(p.get("number"), int) and p.get("name") + } + if names: + return names + logger.warning( + "No implementation-plan phase names found for %s — the phase breakdown will show " + "numbers without descriptions.", + generation_id, + ) + return {} + + def _skipped_workspaces_from_parallel( parallel_results: List[ParallelGenerationResult], ) -> List[SkippedWorkspaceP10Y]: @@ -240,8 +282,8 @@ def _build_comparative_analysis( variance_assessment="high", ) comparative_analysis = ComparativeAnalysis( - component_comparison={}, - high_variance_components=[], + phase_comparison={}, + high_variance_phases=[], insights=[ "No workspace produced hour estimates from P10Y in this run; " "check skipped workspaces and P10Y service availability.", @@ -260,26 +302,26 @@ def _build_comparative_analysis( f"variance={summary.variance_assessment}" ) - # Generate component comparison - component_comparison = generate_component_comparison(workspace_estimations) + # Generate per-phase comparison + phase_comparison = generate_phase_comparison(workspace_estimations) - # Identify high variance components - high_variance_components = identify_high_variance_components(component_comparison) + # Identify high variance phases + high_variance_phases = identify_high_variance_phases(phase_comparison) - if high_variance_components: + if high_variance_phases: logger.info( - f"High variance components: {', '.join(high_variance_components)}" + f"High variance phases: {', '.join(high_variance_phases)}" ) # Generate insights insights = generate_insights( - workspace_estimations, summary, high_variance_components + workspace_estimations, summary, high_variance_phases ) # Build comparative analysis comparative_analysis = ComparativeAnalysis( - component_comparison=component_comparison, - high_variance_components=high_variance_components, + phase_comparison=phase_comparison, + high_variance_phases=high_variance_phases, insights=insights, ) @@ -343,19 +385,19 @@ async def _generate_ai_report( f"### {ws_est.workspace_name}\n" f"- Total Hours: {ws_est.total_hours:.1f}\n" f"- Commits: {ws_est.commits_count}\n" - f"- Components: {len(ws_est.component_breakdown)}\n" + f"- Phases: {len(ws_est.phase_breakdown)}\n" f"- New Work: {ws_est.estimation_metrics.new_work:.1f}\n" f"- Refactor: {ws_est.estimation_metrics.refactor:.1f}\n" f"- Quality Score: {ws_est.estimation_metrics.quality_score:.2f}\n" ) - # Format component comparison - component_comparison_text = [] - for comp_name, comp_comp in sorted(comparative_analysis.component_comparison.items()): - component_comparison_text.append( - f"**{comp_name}**: avg={comp_comp.average:.1f}h, " - f"std={comp_comp.std_deviation:.1f}h, " - f"variance={comp_comp.variance_percentage:.1f}%" + # Format per-phase comparison + phase_comparison_text = [] + for _key, comp in sorted(comparative_analysis.phase_comparison.items()): + phase_comparison_text.append( + f"**{comp.phase_name}**: avg={comp.average:.1f}h, " + f"std={comp.std_deviation:.1f}h, " + f"variance={comp.variance_percentage:.1f}%" ) # Construct full output path using primary workspace (first in list) @@ -401,7 +443,7 @@ async def _generate_ai_report( system_prompt=estimation_report_agent_template( summary=summary, workspace_summaries=workspace_summaries, - component_comparison_text=component_comparison_text, + phase_comparison_text=phase_comparison_text, comparative_analysis=comparative_analysis, full_outputs_dir=full_outputs_dir, model=final_model, @@ -517,6 +559,10 @@ async def multi_workspace_estimation_p10y_workflow( # interrupts this phase's sleeps on the owning pod. A cross-pod cancel during P10Y # simply lets this bounded, read-only phase finish and lands the session in CANCELLED # without a spurious failure notification (fail() rejects the terminal state). + # Phase names come from the shared implementation plan (Firestore planning_data); the phase + # number is the cross-variant join key, so names are identical across workspaces. + phase_names = await _load_phase_names(request.generation_id, db_adapter, logger) + logger.info("Executing estimations in parallel...") parallel_results = await execute_generation_parallel( workspaces=workspaces, @@ -527,6 +573,7 @@ async def multi_workspace_estimation_p10y_workflow( "client": client, "settings": settings, "logger": logger, + "phase_names": phase_names, }, logger=logger, ) diff --git a/backend/test/api/test_email_notifications.py b/backend/test/api/test_email_notifications.py index d6e2acb..a8502c7 100644 --- a/backend/test/api/test_email_notifications.py +++ b/backend/test/api/test_email_notifications.py @@ -28,9 +28,9 @@ EstimationSummary, WorkspaceEstimation, ComparativeAnalysis, - ComponentEstimation, + PhaseEstimation, EstimationMetrics, - ComponentComparison, + PhaseComparison, RiskAssessment, SimplifiedEstimationResponse, SkippedWorkspaceP10Y, @@ -184,17 +184,19 @@ def sample_estimation_result(): "anthropic/claude-sonnet-4.5", ] for i, ws_name in enumerate(["ws-01-1", "ws-01-2", "ws-01-3"], 1): - component_breakdown = { - "auth": ComponentEstimation( - component_name="auth", + phase_breakdown = { + "07": PhaseEstimation( + phase_number=7, + phase_name="Auth", hours=20.0 + i, new_work=15.0, refactor=3.0, rework=2.0 + i, quality_score=0.85, ), - "api": ComponentEstimation( - component_name="api", + "08": PhaseEstimation( + phase_number=8, + phase_name="API", hours=30.0 + i, new_work=25.0, refactor=4.0, @@ -218,7 +220,7 @@ def sample_estimation_result(): workspace_path=f"/workspaces/{ws_name}", total_hours=50.0 + i, total_effective_output=47.0, - component_breakdown=component_breakdown, + phase_breakdown=phase_breakdown, estimation_metrics=estimation_metrics, commits_count=10 + i, model_usage=ModelTokenUsage( @@ -233,9 +235,10 @@ def sample_estimation_result(): workspace_estimations.append(ws_est) # Create comparative analysis - component_comparison = { - "auth": ComponentComparison( - component_name="auth", + phase_comparison = { + "07": PhaseComparison( + phase_number=7, + phase_name="Auth", hours_by_workspace={ "ws-01-1": 21.0, "ws-01-2": 22.0, @@ -245,8 +248,9 @@ def sample_estimation_result(): std_deviation=0.82, variance_percentage=3.7, ), - "api": ComponentComparison( - component_name="api", + "08": PhaseComparison( + phase_number=8, + phase_name="API", hours_by_workspace={ "ws-01-1": 31.0, "ws-01-2": 32.0, @@ -257,10 +261,10 @@ def sample_estimation_result(): variance_percentage=2.6, ), } - + comparative_analysis = ComparativeAnalysis( - component_comparison=component_comparison, - high_variance_components=[], + phase_comparison=phase_comparison, + high_variance_phases=[], insights=["Low variance across workspaces", "Consistent estimates"], ) @@ -355,10 +359,10 @@ def test_notify_generation_session_complete_with_full_data( assert "input:" in html_content assert "cache write:" in html_content - # Check that component breakdown is included - assert "Component Complexity Metrics Breakdown" in html_content - assert "auth" in html_content - assert "api" in html_content + # Check that phase breakdown is included + assert "Phase Breakdown" in html_content + assert "Auth" in html_content + assert "API" in html_content # Check summary information (P10Y variance — not approval/rejection labels) assert "115.0" in html_content # Final estimate @@ -372,7 +376,7 @@ def test_notify_generation_session_complete_with_full_data( assert "est-test-123" in plain_content assert "VARIANTS:" in plain_content assert "SpecFlow ITERATION COMPLETE" in plain_content - assert "COMPONENT BREAKDOWN" in plain_content + assert "PHASE BREAKDOWN" in plain_content assert "model: anthropic/claude-sonnet-4.5" in plain_content @patch("app.core.notifications.smtplib.SMTP_SSL") @@ -453,17 +457,17 @@ def test_notify_generation_session_complete_without_component_breakdown( workspace_path="/workspaces/workspace-1", total_hours=50.0, total_effective_output=47.0, - component_breakdown={}, # Empty breakdown + phase_breakdown={}, # Empty breakdown estimation_metrics=estimation_metrics, commits_count=10, ) - + result = MultiWorkspaceEstimationResponse( summary=summary, workspace_estimations=[workspace_est], comparative_analysis=ComparativeAnalysis( - component_comparison={}, - high_variance_components=[], + phase_comparison={}, + high_variance_phases=[], insights=[], ), timestamp=datetime.now(timezone.utc).isoformat(), @@ -498,9 +502,9 @@ def test_notify_generation_session_complete_without_component_breakdown( assert html_content.strip() != "" break - # Component breakdown section should not be present + # Phase breakdown section should not be present # (it should be omitted when empty) - assert "Component Complexity Metrics Breakdown" not in html_content + assert "Phase Breakdown" not in html_content @patch('app.core.notifications.smtplib.SMTP_SSL') def test_notify_generation_session_complete_with_missing_workspace( @@ -568,8 +572,8 @@ def test_builds_html_and_plain_without_any_email_config( assert html_content.strip() != "" assert plain_content.strip() != "" assert " None: logger=logger, ) - assert len(out) == 1 - assert out[0]["sha"] == current + assert len(out.rows) == 1 + assert out.rows[0]["sha"] == current + assert out.returned_count == 1 @pytest.mark.asyncio @@ -80,4 +81,4 @@ async def test_fetch_and_filter_case_insensitive_and_dedupes() -> None: logger=logger, ) - assert len(out) == 1 + assert len(out.rows) == 1 diff --git a/backend/test/test_commit_metadata.py b/backend/test/test_commit_metadata.py index 9233f4f..82470fd 100644 --- a/backend/test/test_commit_metadata.py +++ b/backend/test/test_commit_metadata.py @@ -1,5 +1,5 @@ """ -Unit tests for commit metadata (git-derived) and component attribution. +Unit tests for commit metadata (git-derived) and phase attribution. """ import logging import subprocess @@ -8,11 +8,10 @@ import pytest from app.services.p10y.p10y_lib import ( - KNOWN_COMPONENTS, CommitInfo, CodeGenerationMetadata, build_code_generation_metadata_from_git, - _parse_component_from_subject, + _parse_phase_from_subject, _subject_excluded_from_estimation, ) @@ -21,8 +20,8 @@ def test_code_generation_metadata_str() -> None: """String representation lists commit messages.""" metadata = CodeGenerationMetadata( commits=[ - CommitInfo(sha="abc", message="First commit", component=["common"]), - CommitInfo(sha="def", message="Second commit", component=["backend"]), + CommitInfo(sha="abc", message="First commit", phase=None), + CommitInfo(sha="def", message="Second commit", phase=3), ] ) string_repr = str(metadata) @@ -62,7 +61,7 @@ def test_build_code_generation_metadata_from_git_skips_skip_prefix(tmp_path: Pat (tmp_path / "b.txt").write_text("b") subprocess.run(["git", "add", "b.txt"], cwd=tmp_path, check=True, capture_output=True) subprocess.run( - ["git", "commit", "-m", "backend_add API route"], + ["git", "commit", "-m", "p06_add API route"], cwd=tmp_path, check=True, capture_output=True, @@ -70,12 +69,12 @@ def test_build_code_generation_metadata_from_git_skips_skip_prefix(tmp_path: Pat meta = build_code_generation_metadata_from_git(str(tmp_path), logger) assert meta is not None assert len(meta.commits) == 1 - assert meta.commits[0].component == ["backend"] - assert meta.commits[0].message == "backend_add API route" + assert meta.commits[0].phase == 6 + assert meta.commits[0].message == "p06_add API route" -def test_build_code_generation_metadata_from_git_no_underscore_uses_common(tmp_path: Path) -> None: - """Subject without underscore falls back to component common.""" +def test_build_code_generation_metadata_from_git_no_prefix_is_unphased(tmp_path: Path) -> None: + """Subject without a pNN_ prefix parses to phase None (unphased).""" logger = logging.getLogger("test_commit_meta2") _git_init_with_user(tmp_path) (tmp_path / "x.txt").write_text("x") @@ -88,7 +87,7 @@ def test_build_code_generation_metadata_from_git_no_underscore_uses_common(tmp_p ) meta = build_code_generation_metadata_from_git(str(tmp_path), logger) assert meta is not None - assert meta.commits[0].component == ["common"] + assert meta.commits[0].phase is None @pytest.mark.asyncio @@ -100,7 +99,7 @@ async def test_load_code_generation_metadata_async_uses_git(tmp_path: Path) -> N (tmp_path / "f.txt").write_text("f") subprocess.run(["git", "add", "f.txt"], cwd=tmp_path, check=True, capture_output=True) subprocess.run( - ["git", "commit", "-m", "testing_add unit test"], + ["git", "commit", "-m", "p12_add unit test"], cwd=tmp_path, check=True, capture_output=True, @@ -108,7 +107,7 @@ async def test_load_code_generation_metadata_async_uses_git(tmp_path: Path) -> N meta = await load_code_generation_metadata(str(tmp_path), logger) assert meta is not None assert len(meta.commits) == 1 - assert meta.commits[0].component == ["testing"] + assert meta.commits[0].phase == 12 # --------------------------------------------------------------------------- @@ -119,8 +118,8 @@ async def test_load_code_generation_metadata_async_uses_git(tmp_path: Path) -> N ("SKIP_initial_user_source", True), ("skip_something", True), # case-insensitive ("SKIP_generation_baseline", True), - ("backend_add feature", False), - ("common_setup", False), + ("p03_add feature", False), + ("p01_setup", False), ("", True), # empty → excluded (" ", True), # whitespace-only → excluded ]) @@ -137,52 +136,23 @@ def test_subject_excluded_logs_debug_for_empty(caplog) -> None: # --------------------------------------------------------------------------- -# _parse_component_from_subject +# _parse_phase_from_subject # --------------------------------------------------------------------------- -@pytest.mark.parametrize("subject,expected_component", [ - ("backend_implement JWT", ["backend"]), - ("frontend_add login form", ["frontend"]), - ("mobile_finalize app manifest", ["mobile"]), - ("BACKEND_uppercase token", ["backend"]), # component lowercased - ("common_setup project", ["common"]), - ("_no_component_before_underscore", ["common"]), # empty token → common - ("no_underscore_at_all", ["no"]), # first underscore splits +@pytest.mark.parametrize("subject,expected_phase", [ + ("p03_implement JWT", 3), + ("p13_add login form", 13), + ("p1_single digit", 1), + ("P07_uppercase prefix", 7), # case-insensitive + ("backend_implement thing", None), # non-phase token → unphased + ("noUnderscoreHere", None), # no underscore → unphased + ("_leading underscore", None), # empty token → unphased + ("px_not a number", None), # p + non-digit → unphased ]) -def test_parse_component_from_subject_known(subject: str, expected_component: list) -> None: - logger = logging.getLogger("test_parse_comp") - result = _parse_component_from_subject(subject, logger) - assert result == expected_component - - -def test_parse_component_no_underscore_logs_warning(caplog) -> None: - logger = logging.getLogger("test_parse_no_underscore") - with caplog.at_level(logging.WARNING, logger="test_parse_no_underscore"): - result = _parse_component_from_subject("noUnderscoreHere", logger) - assert result == ["common"] - assert "no underscore component prefix" in caplog.text - - -def test_parse_component_unknown_token_logs_warning(caplog) -> None: - logger = logging.getLogger("test_parse_unknown") - with caplog.at_level(logging.WARNING, logger="test_parse_unknown"): - result = _parse_component_from_subject("typo_implement thing", logger) - assert result == ["typo"] - assert "unknown component token" in caplog.text - - -def test_parse_component_known_token_no_warning(caplog) -> None: - logger = logging.getLogger("test_parse_known") - with caplog.at_level(logging.WARNING, logger="test_parse_known"): - _parse_component_from_subject("mobile_finalize app manifest", logger) - assert "unknown component token" not in caplog.text - - -def test_known_components_matches_standards() -> None: - """KNOWN_COMPONENTS must include the tokens listed in commit_standards.md.""" - expected = {"backend", "frontend", "database", "api", "auth", - "infrastructure", "testing", "documentation", "pipeline", "ml", "mobile", "common"} - assert expected == set(KNOWN_COMPONENTS) +def test_parse_phase_from_subject(subject: str, expected_phase) -> None: + logger = logging.getLogger("test_parse_phase") + result = _parse_phase_from_subject(subject, logger) + assert result == expected_phase # --------------------------------------------------------------------------- @@ -245,23 +215,24 @@ def test_build_metadata_two_baseline_commits_excluded(tmp_path: Path) -> None: ["git", "commit", "--allow-empty", "-m", "SKIP_generation_baseline"], cwd=tmp_path, check=True, capture_output=True, ) - # agent commits + # agent commits (phase prefix as the hook would inject) (tmp_path / "api.py").write_text("api") subprocess.run(["git", "add", "api.py"], cwd=tmp_path, check=True, capture_output=True) subprocess.run( - ["git", "commit", "-m", "api_add health endpoint"], + ["git", "commit", "-m", "p06_add health endpoint"], cwd=tmp_path, check=True, capture_output=True, ) (tmp_path / "db.py").write_text("db") subprocess.run(["git", "add", "db.py"], cwd=tmp_path, check=True, capture_output=True) subprocess.run( - ["git", "commit", "-m", "database_create users table"], + ["git", "commit", "-m", "p05_create users table"], cwd=tmp_path, check=True, capture_output=True, ) meta = build_code_generation_metadata_from_git(str(tmp_path), logger) assert meta is not None assert len(meta.commits) == 2 subjects = [c.message for c in meta.commits] - assert "api_add health endpoint" in subjects - assert "database_create users table" in subjects + assert "p06_add health endpoint" in subjects + assert "p05_create users table" in subjects + assert {c.phase for c in meta.commits} == {5, 6} assert all(not s.upper().startswith("SKIP_") for s in subjects) diff --git a/backend/test/test_estimation_components.py b/backend/test/test_estimation_components.py index cd2cbc9..3fd8e7e 100644 --- a/backend/test/test_estimation_components.py +++ b/backend/test/test_estimation_components.py @@ -1,29 +1,28 @@ """ -Unit tests for component breakdown and estimation functions. +Unit tests for per-phase breakdown and estimation functions. """ from app.services.p10y.p10y_lib import ( Estimation, CodeGenerationMetadata, CommitInfo, - generate_component_breakdown, + generate_phase_breakdown, apply_productivity_multiplier, - format_component_breakdown, ) -class TestComponentBreakdown: - """Tests for generate_component_breakdown function.""" - - def test_single_component_per_commit(self): - """Test breakdown with one component per commit.""" +class TestPhaseBreakdown: + """Tests for generate_phase_breakdown function.""" + + def test_commits_grouped_by_phase(self): + """Commits are aggregated by their phase number.""" metadata = CodeGenerationMetadata(commits=[ - CommitInfo(sha="commit1", message="backend: add API", component=["backend"]), - CommitInfo(sha="commit2", message="frontend: add UI", component=["frontend"]), - CommitInfo(sha="commit3", message="backend: add tests", component=["backend"]), + CommitInfo(sha="commit1", message="p06_add API", phase=6), + CommitInfo(sha="commit2", message="p13_add UI", phase=13), + CommitInfo(sha="commit3", message="p06_add tests", phase=6), ]) - + commit_stats = [ - {"sha": "commit1", "fp_delta_total": 10.0, "commit_quality_score": 0.8, + {"sha": "commit1", "fp_delta_total": 10.0, "commit_quality_score": 0.8, "churn_rate": 0.1, "refactor": 0, "rework": 0, "new_work": 10.0, "removed_work": 0, "quality_score": 0.8, "effective_output": 9.0, "total_output": 10.0}, {"sha": "commit2", "fp_delta_total": 15.0, "commit_quality_score": 0.9, @@ -33,23 +32,23 @@ def test_single_component_per_commit(self): "churn_rate": 0.08, "refactor": 0, "rework": 0, "new_work": 5.0, "removed_work": 0, "quality_score": 0.85, "effective_output": 4.5, "total_output": 5.0}, ] - - breakdown = generate_component_breakdown(metadata, commit_stats) - - assert "backend" in breakdown - assert "frontend" in breakdown - assert breakdown["backend"].function_points == 15.0 # commit1 + commit3 - assert breakdown["frontend"].function_points == 15.0 # commit2 - assert breakdown["backend"].new_work == 15.0 - assert breakdown["frontend"].new_work == 15.0 - - def test_multiple_components_per_commit(self): - """Test breakdown when commits have multiple components.""" + + breakdown = generate_phase_breakdown(metadata, commit_stats) + + assert 6 in breakdown + assert 13 in breakdown + assert breakdown[6].function_points == 15.0 # commit1 + commit3 + assert breakdown[13].function_points == 15.0 # commit2 + assert breakdown[6].new_work == 15.0 + assert breakdown[13].new_work == 15.0 + + def test_unphased_commits_bucketed_under_none(self): + """Commits with no phase prefix aggregate under the None (unphased) key.""" metadata = CodeGenerationMetadata(commits=[ - CommitInfo(sha="commit1", message="backend/api: add endpoint", component=["backend", "api"]), - CommitInfo(sha="commit2", message="frontend/api: integrate", component=["frontend", "api"]), + CommitInfo(sha="commit1", message="p03_add endpoint", phase=3), + CommitInfo(sha="commit2", message="chore misc", phase=None), ]) - + commit_stats = [ {"sha": "commit1", "fp_delta_total": 20.0, "commit_quality_score": 0.8, "churn_rate": 0.1, "refactor": 2.0, "rework": 1.0, "new_work": 17.0, @@ -58,54 +57,47 @@ def test_multiple_components_per_commit(self): "churn_rate": 0.12, "refactor": 3.0, "rework": 2.0, "new_work": 20.0, "removed_work": 0, "quality_score": 0.85, "effective_output": 22.0, "total_output": 25.0}, ] - - breakdown = generate_component_breakdown(metadata, commit_stats) - - # Both commits should be counted for "api" component - assert "api" in breakdown - assert breakdown["api"].function_points == 45.0 # commit1 + commit2 - - # Backend only gets commit1 - assert "backend" in breakdown - assert breakdown["backend"].function_points == 20.0 - - # Frontend only gets commit2 - assert "frontend" in breakdown - assert breakdown["frontend"].function_points == 25.0 - + + breakdown = generate_phase_breakdown(metadata, commit_stats) + + assert 3 in breakdown + assert None in breakdown + assert breakdown[3].function_points == 20.0 + assert breakdown[None].function_points == 25.0 + def test_empty_commits(self): """Test breakdown with no commits.""" metadata = CodeGenerationMetadata(commits=[]) commit_stats = [] - - breakdown = generate_component_breakdown(metadata, commit_stats) - + + breakdown = generate_phase_breakdown(metadata, commit_stats) + assert len(breakdown) == 0 - + def test_commit_not_in_stats(self): """Test that commits without stats are skipped.""" metadata = CodeGenerationMetadata(commits=[ - CommitInfo(sha="commit1", message="backend: add API", component=["backend"]), - CommitInfo(sha="commit2", message="frontend: add UI", component=["frontend"]), + CommitInfo(sha="commit1", message="p06_add API", phase=6), + CommitInfo(sha="commit2", message="p13_add UI", phase=13), ]) - + # Only provide stats for commit1 commit_stats = [ {"sha": "commit1", "fp_delta_total": 10.0, "commit_quality_score": 0.8, "churn_rate": 0.1, "refactor": 0, "rework": 0, "new_work": 10.0, "removed_work": 0, "quality_score": 0.8, "effective_output": 9.0, "total_output": 10.0}, ] - - breakdown = generate_component_breakdown(metadata, commit_stats) - - assert "backend" in breakdown - assert "frontend" not in breakdown - assert breakdown["backend"].function_points == 10.0 + + breakdown = generate_phase_breakdown(metadata, commit_stats) + + assert 6 in breakdown + assert 13 not in breakdown + assert breakdown[6].function_points == 10.0 class TestProductivityMultiplier: """Tests for apply_productivity_multiplier function.""" - + def test_default_multiplier(self): """Test with default 2.0x multiplier.""" estimation = Estimation( @@ -122,11 +114,11 @@ def test_default_multiplier(self): effective_output=95.0, total_output=100.0, ) - + hours = apply_productivity_multiplier(estimation) - + assert hours == 200.0 # 100 FP * 2.0 - + def test_custom_multiplier(self): """Test with custom multiplier.""" estimation = Estimation( @@ -143,11 +135,11 @@ def test_custom_multiplier(self): effective_output=48.0, total_output=50.0, ) - + hours = apply_productivity_multiplier(estimation, multiplier=1.5) - + assert hours == 75.0 # 50 FP * 1.5 - + def test_zero_function_points(self): """Test with zero function points.""" estimation = Estimation( @@ -164,111 +156,7 @@ def test_zero_function_points(self): effective_output=0, total_output=0, ) - - hours = apply_productivity_multiplier(estimation) - - assert hours == 0.0 + hours = apply_productivity_multiplier(estimation) -class TestFormatComponentBreakdown: - """Tests for format_component_breakdown function.""" - - def test_format_single_component(self): - """Test formatting with single component.""" - component_breakdown = { - "backend": Estimation( - function_points=100.0, - commit_quality_score=0.8, - churn_rate=0.1, - technologies=[], - id_contributor=1, - refactor=10.0, - rework=5.0, - new_work=85.0, - removed_work=0, - quality_score=0.85, - effective_output=95.0, - total_output=100.0, - ) - } - - formatted = format_component_breakdown(component_breakdown) - - assert "## Component Breakdown" in formatted - assert "### Backend" in formatted - assert "Estimated Hours: 200.0" in formatted # 100 * 2.0 - assert "Quality Score: 0.85" in formatted - - def test_format_multiple_components(self): - """Test formatting with multiple components.""" - component_breakdown = { - "backend": Estimation( - function_points=50.0, - commit_quality_score=0.8, - churn_rate=0.1, - technologies=[], - id_contributor=1, - refactor=5.0, - rework=2.0, - new_work=43.0, - removed_work=0, - quality_score=0.8, - effective_output=48.0, - total_output=50.0, - ), - "frontend": Estimation( - function_points=60.0, - commit_quality_score=0.9, - churn_rate=0.05, - technologies=[], - id_contributor=1, - refactor=3.0, - rework=1.0, - new_work=56.0, - removed_work=0, - quality_score=0.9, - effective_output=59.0, - total_output=60.0, - ), - } - - formatted = format_component_breakdown(component_breakdown) - - assert "### Backend" in formatted - assert "### Frontend" in formatted - assert "Estimated Hours: 100.0" in formatted # 50 * 2.0 - assert "Estimated Hours: 120.0" in formatted # 60 * 2.0 - - def test_format_empty_breakdown(self): - """Test formatting with no components.""" - component_breakdown = {} - - formatted = format_component_breakdown(component_breakdown) - - assert "## Component Breakdown" in formatted - # Should only have the header, no component sections - assert "###" not in formatted - - def test_format_custom_multiplier(self): - """Test formatting with custom multiplier.""" - component_breakdown = { - "backend": Estimation( - function_points=100.0, - commit_quality_score=0.8, - churn_rate=0.1, - technologies=[], - id_contributor=1, - refactor=10.0, - rework=5.0, - new_work=85.0, - removed_work=0, - quality_score=0.8, - effective_output=95.0, - total_output=100.0, - ) - } - - formatted = format_component_breakdown(component_breakdown, multiplier=1.5) - - assert "Estimated Hours: 150.0" in formatted # 100 * 1.5 - + assert hours == 0.0 diff --git a/backend/test/test_execute_all_phases_connection_error.py b/backend/test/test_execute_all_phases_connection_error.py index f9aad91..0ae0b5c 100644 --- a/backend/test/test_execute_all_phases_connection_error.py +++ b/backend/test/test_execute_all_phases_connection_error.py @@ -54,10 +54,6 @@ async def test_connection_error_aborts_without_checkpoint(tmp_path: Path) -> Non svc.db_adapter = None # DB-less unit test: raise_if_cancelled no-ops without an adapter svc.update_workspace_phase = AsyncMock() svc.update_deployment_workspace_phase = AsyncMock() - # No live DB in this unit test: execute_all_phases derives db_adapter from the - # service for the per-phase cancellation check; None makes raise_if_cancelled a - # no-op (its documented DB-less path) so the connection-error abort is reached. - svc.db_adapter = None async def fake_phase_fn(**kwargs): return AgentResult( diff --git a/backend/test/test_model_selection.py b/backend/test/test_model_selection.py index a3fbb68..28a2300 100644 --- a/backend/test/test_model_selection.py +++ b/backend/test/test_model_selection.py @@ -369,14 +369,14 @@ async def mock_agent_query(**kwargs): variance_assessment="low", ) comparative = ComparativeAnalysis( - component_comparison={}, high_variance_components=[], insights=[], + phase_comparison={}, high_variance_phases=[], insights=[], ) ws_est = WorkspaceEstimation( workspace_name="ws-1", workspace_path=str(tmp_path), total_hours=100.0, total_effective_output=50.0, - component_breakdown={}, + phase_breakdown={}, estimation_metrics=EstimationMetrics( new_work=10.0, refactor=0.0, @@ -433,14 +433,14 @@ async def mock_agent_query(**kwargs): variance_assessment="low", ) comparative = ComparativeAnalysis( - component_comparison={}, high_variance_components=[], insights=[], + phase_comparison={}, high_variance_phases=[], insights=[], ) ws_est = WorkspaceEstimation( workspace_name="ws-1", workspace_path=str(tmp_path), total_hours=100.0, total_effective_output=50.0, - component_breakdown={}, + phase_breakdown={}, estimation_metrics=EstimationMetrics( new_work=10.0, refactor=0.0, diff --git a/backend/test/test_p10y_lib_components.py b/backend/test/test_p10y_lib_components.py index 7bc31d0..af30462 100644 --- a/backend/test/test_p10y_lib_components.py +++ b/backend/test/test_p10y_lib_components.py @@ -1,5 +1,5 @@ """ -Tests for p10y_lib component breakdown and productivity multiplier functions. +Tests for p10y_lib per-phase breakdown, productivity multiplier, and technology filtering. """ import logging from app.services.p10y.p10y_lib import ( @@ -8,22 +8,19 @@ Estimation, VALID_TECHNOLOGIES, apply_productivity_multiplier, - format_component_breakdown, - generate_component_breakdown, + generate_phase_breakdown, has_valid_technology, ) -def test_generate_component_breakdown(): - """Test component breakdown generation.""" - # Create test metadata +def test_generate_phase_breakdown(): + """Test per-phase breakdown generation.""" metadata = CodeGenerationMetadata(commits=[ - CommitInfo(sha="abc123", message="backend: add service", component=["backend"]), - CommitInfo(sha="def456", message="frontend: add UI", component=["frontend"]), - CommitInfo(sha="ghi789", message="backend/api: add endpoint", component=["backend", "api"]), + CommitInfo(sha="abc123", message="p06_add service", phase=6), + CommitInfo(sha="def456", message="p13_add UI", phase=13), + CommitInfo(sha="ghi789", message="p06_add endpoint", phase=6), ]) - - # Create test commit stats + commit_stats_data = [ { "sha": "abc123", @@ -65,26 +62,19 @@ def test_generate_component_breakdown(): "total_output": 12.0, }, ] - - # Generate breakdown - breakdown = generate_component_breakdown(metadata, commit_stats_data) - - # Verify breakdown - assert "backend" in breakdown - assert "frontend" in breakdown - assert "api" in breakdown - - # Backend should include commits abc123 and ghi789 - assert breakdown["backend"].function_points == 22.0 # 10 + 12 - assert breakdown["backend"].new_work == 14.0 # 7 + 7 - - # Frontend should include commit def456 - assert breakdown["frontend"].function_points == 8.0 - assert breakdown["frontend"].new_work == 5.0 - - # API should include commit ghi789 - assert breakdown["api"].function_points == 12.0 - assert breakdown["api"].new_work == 7.0 + + breakdown = generate_phase_breakdown(metadata, commit_stats_data) + + assert 6 in breakdown + assert 13 in breakdown + + # Phase 6 should include commits abc123 and ghi789 + assert breakdown[6].function_points == 22.0 # 10 + 12 + assert breakdown[6].new_work == 14.0 # 7 + 7 + + # Phase 13 should include commit def456 + assert breakdown[13].function_points == 8.0 + assert breakdown[13].new_work == 5.0 def test_apply_productivity_multiplier(): @@ -103,66 +93,23 @@ def test_apply_productivity_multiplier(): effective_output=45.0, total_output=50.0, ) - + # Test with default multiplier (2.0) hours = apply_productivity_multiplier(generation) assert hours == 100.0 - + # Test with custom multiplier hours = apply_productivity_multiplier(generation, multiplier=3.0) assert hours == 150.0 -def test_format_component_breakdown(): - """Test component breakdown formatting.""" - component_breakdown = { - "backend": Estimation( - function_points=30.0, - commit_quality_score=0.8, - churn_rate=0.1, - technologies=[], - id_contributor=1, - refactor=8.0, - rework=3.0, - new_work=19.0, - removed_work=1.0, - quality_score=0.85, - effective_output=27.0, - total_output=30.0, - ), - "frontend": Estimation( - function_points=20.0, - commit_quality_score=0.75, - churn_rate=0.15, - technologies=[], - id_contributor=1, - refactor=5.0, - rework=2.0, - new_work=13.0, - removed_work=0.5, - quality_score=0.80, - effective_output=18.0, - total_output=20.0, - ), - } - - result = format_component_breakdown(component_breakdown, multiplier=2.0) - - # Verify output contains expected sections - assert "## Component Breakdown" in result - assert "### Backend" in result - assert "### Frontend" in result - assert "Estimated Hours: 60.0" in result # 30 * 2.0 - assert "Estimated Hours: 40.0" in result # 20 * 2.0 - - -def test_generate_component_breakdown_with_missing_commits(): - """Test component breakdown when some commits are not in stats.""" +def test_generate_phase_breakdown_with_missing_commits(): + """Test per-phase breakdown when some commits are not in stats.""" metadata = CodeGenerationMetadata(commits=[ - CommitInfo(sha="abc123", message="backend: add service", component=["backend"]), - CommitInfo(sha="missing", message="frontend: add UI", component=["frontend"]), + CommitInfo(sha="abc123", message="p06_add service", phase=6), + CommitInfo(sha="missing", message="p13_add UI", phase=13), ]) - + commit_stats_data = [ { "sha": "abc123", @@ -178,22 +125,22 @@ def test_generate_component_breakdown_with_missing_commits(): "total_output": 10.0, }, ] - - breakdown = generate_component_breakdown(metadata, commit_stats_data) - - # Only backend should be in breakdown - assert "backend" in breakdown - assert "frontend" not in breakdown - assert breakdown["backend"].effective_output == 9.0 + breakdown = generate_phase_breakdown(metadata, commit_stats_data) + + # Only phase 6 should be in breakdown + assert 6 in breakdown + assert 13 not in breakdown + assert breakdown[6].effective_output == 9.0 -def test_generate_component_breakdown_empty(): - """Test component breakdown with no commits.""" + +def test_generate_phase_breakdown_empty(): + """Test per-phase breakdown with no commits.""" metadata = CodeGenerationMetadata(commits=[]) commit_stats_data = [] - - breakdown = generate_component_breakdown(metadata, commit_stats_data) - + + breakdown = generate_phase_breakdown(metadata, commit_stats_data) + assert breakdown == {} @@ -210,12 +157,12 @@ def test_valid_technologies_predefined_list(): def test_has_valid_technology_with_valid_tech(caplog): """Test has_valid_technology returns True for commits with valid technologies.""" logger = logging.getLogger("test") - + commit_stats = { "sha": "abc123", "technologies": {"supported": ["Python", "JavaScript", "CSS"]} } - + assert has_valid_technology(commit_stats, logger) is True # Should not log warning assert "Skipping commit" not in caplog.text @@ -224,25 +171,25 @@ def test_has_valid_technology_with_valid_tech(caplog): def test_has_valid_technology_case_insensitive(caplog): """Test has_valid_technology handles case-insensitive comparison.""" logger = logging.getLogger("test") - + # Mix of uppercase and lowercase commit_stats = { "sha": "abc123", "technologies": {"supported": ["PYTHON", "javascript", "TypeScript"]} } - + assert has_valid_technology(commit_stats, logger) is True def test_has_valid_technology_with_invalid_tech(caplog): """Test has_valid_technology returns False for commits with no valid technologies.""" logger = logging.getLogger("test") - + commit_stats = { "sha": "abc123def456", "technologies": {"supported": ["Markdown", "JSON", "YAML"]} } - + with caplog.at_level(logging.WARNING): assert has_valid_technology(commit_stats, logger) is False # Should log warning @@ -254,35 +201,34 @@ def test_has_valid_technology_with_invalid_tech(caplog): def test_has_valid_technology_with_empty_list(caplog): """Test has_valid_technology returns False for commits with empty technologies list.""" logger = logging.getLogger("test") - + commit_stats = { "sha": "abc123", "technologies": {} } - + assert has_valid_technology(commit_stats, logger) is False def test_has_valid_technology_with_none(caplog): """Test has_valid_technology returns False for commits with None technologies.""" logger = logging.getLogger("test") - + commit_stats = { "sha": "abc123", "technologies": None } - + assert has_valid_technology(commit_stats, logger) is False def test_has_valid_technology_with_mixed_valid_invalid(caplog): """Test has_valid_technology returns True if at least one tech is valid.""" logger = logging.getLogger("test") - + commit_stats = { "sha": "abc123", "technologies": {"supported": ["Markdown", "Python", "JSON"]} } - - assert has_valid_technology(commit_stats, logger) is True + assert has_valid_technology(commit_stats, logger) is True diff --git a/backend/test/test_parallel_execution.py b/backend/test/test_parallel_execution.py index 6ac05e2..3f5abe3 100644 --- a/backend/test/test_parallel_execution.py +++ b/backend/test/test_parallel_execution.py @@ -10,7 +10,7 @@ from app.core.config import Settings from app.schemas.agent import AgentResult -from app.schemas.estimate import ComponentEstimation, EstimationMetrics, WorkspaceEstimation +from app.schemas.estimate import PhaseEstimation, EstimationMetrics, WorkspaceEstimation from app.schemas.workspace import WorkspaceSettings from app.services.parallel_executor import ParallelAgentExecutor, execute_generation_parallel from app.services.workspace_config_loader import WorkspaceConfigLoader @@ -368,17 +368,19 @@ async def mock_generation_fn(workspace, **kwargs): total_hours=100.0 + workspace.p10y_repository_id, # Unique value per workspace total_effective_output=50.0, commits_count=10, - component_breakdown={ - "backend": ComponentEstimation( - component_name="backend", + phase_breakdown={ + "06": PhaseEstimation( + phase_number=6, + phase_name="Backend Domain", hours=50.0, new_work=40.0, refactor=8.0, rework=2.0, quality_score=0.85 ), - "frontend": ComponentEstimation( - component_name="frontend", + "13": PhaseEstimation( + phase_number=13, + phase_name="Frontend Shell", hours=50.0, new_work=40.0, refactor=8.0, @@ -435,7 +437,7 @@ async def mock_generation_fn(workspace, **kwargs): total_hours=100.0, total_effective_output=50.0, commits_count=10, - component_breakdown={}, + phase_breakdown={}, estimation_metrics=EstimationMetrics( new_work=80.0, refactor=16.0, @@ -476,7 +478,7 @@ async def mock_generation_fn(workspace, **kwargs): total_hours=100.0, total_effective_output=50.0, commits_count=10, - component_breakdown={}, + phase_breakdown={}, estimation_metrics=EstimationMetrics( new_work=80.0, refactor=16.0, @@ -519,7 +521,7 @@ async def mock_generation_fn(workspace, **kwargs): total_hours=150.0, total_effective_output=75.0, commits_count=15, - component_breakdown={}, + phase_breakdown={}, estimation_metrics=EstimationMetrics( new_work=120.0, refactor=24.0, @@ -583,7 +585,7 @@ async def mock_generation_fn(workspace, **kwargs): total_hours=100.0 * workspace_num, total_effective_output=50.0 * workspace_num, commits_count=workspace_num * 5, - component_breakdown={}, + phase_breakdown={}, estimation_metrics=EstimationMetrics( new_work=80.0, refactor=16.0, diff --git a/backend/test/test_phase_comparison.py b/backend/test/test_phase_comparison.py new file mode 100644 index 0000000..8ee858e --- /dev/null +++ b/backend/test/test_phase_comparison.py @@ -0,0 +1,97 @@ +""" +Tests for cross-workspace per-phase comparison and high-variance detection. +""" +from app.schemas.estimate import EstimationMetrics, PhaseEstimation, WorkspaceEstimation +from app.services.p10y.multi_workspace_estimation import ( + generate_phase_comparison, + identify_high_variance_phases, +) + + +def _phase(number: int, name: str, hours: float) -> PhaseEstimation: + return PhaseEstimation( + phase_number=number, + phase_name=name, + hours=hours, + new_work=hours, + refactor=0.0, + rework=0.0, + quality_score=0.8, + ) + + +def _ws(name: str, phases: dict) -> WorkspaceEstimation: + return WorkspaceEstimation( + workspace_name=name, + workspace_path=f"/tmp/{name}", + total_hours=sum(p.hours for p in phases.values()), + total_effective_output=0.0, + phase_breakdown=phases, + estimation_metrics=EstimationMetrics( + new_work=0.0, refactor=0.0, rework=0.0, removed_work=0.0, + quality_score=0.8, effective_output=0.0, total_output=0.0, + ), + commits_count=1, + ) + + +def _sample_workspaces(): + return [ + _ws("ws-1", {"06": _phase(6, "Backend", 40.0), "13": _phase(13, "Frontend", 70.0)}), + _ws("ws-2", {"06": _phase(6, "Backend", 35.0), "13": _phase(13, "Frontend", 28.0)}), + _ws("ws-3", {"06": _phase(6, "Backend", 30.0), "13": _phase(13, "Frontend", 33.0)}), + ] + + +def test_phase_comparison_joins_on_phase_key_with_names(): + comparison = generate_phase_comparison(_sample_workspaces()) + + assert set(comparison.keys()) == {"06", "13"} + backend = comparison["06"] + assert backend.phase_number == 6 + assert backend.phase_name == "Backend" + assert backend.hours_by_workspace == {"ws-1": 40.0, "ws-2": 35.0, "ws-3": 30.0} + assert backend.average == 35.0 # mean(40,35,30) + # sample stdev of (40,35,30) is 5.0 -> 14.3% + assert round(backend.variance_percentage, 1) == 14.3 + + +def test_high_variance_phase_detected(): + comparison = generate_phase_comparison(_sample_workspaces()) + # Frontend hours (70,28,33) => CV > 30%; Backend (40,35,30) => < 30%. + high = identify_high_variance_phases(comparison) + assert high == ["13"] + + +def test_phase_present_in_one_workspace_has_zero_variance(): + workspaces = [ + _ws("ws-1", {"06": _phase(6, "Backend", 40.0)}), + _ws("ws-2", {"07": _phase(7, "Auth", 10.0)}), + ] + comparison = generate_phase_comparison(workspaces) + assert comparison["06"].variance_percentage == 0.0 + assert comparison["07"].variance_percentage == 0.0 + assert identify_high_variance_phases(comparison) == [] + + +def test_unphased_bucket_compared_like_any_phase(): + workspaces = [ + _ws("ws-1", {"unphased": _phase_unphased(5.0)}), + _ws("ws-2", {"unphased": _phase_unphased(15.0)}), + ] + comparison = generate_phase_comparison(workspaces) + assert "unphased" in comparison + assert comparison["unphased"].phase_number is None + assert comparison["unphased"].average == 10.0 + + +def _phase_unphased(hours: float) -> PhaseEstimation: + return PhaseEstimation( + phase_number=None, + phase_name="unphased", + hours=hours, + new_work=hours, + refactor=0.0, + rework=0.0, + quality_score=0.8, + ) diff --git a/backend/test/test_phase_name_resolution.py b/backend/test/test_phase_name_resolution.py new file mode 100644 index 0000000..39dd7c0 --- /dev/null +++ b/backend/test/test_phase_name_resolution.py @@ -0,0 +1,166 @@ +""" +Phase labels in the P10Y breakdown come from the shared implementation plan. + +The plan is persisted per workspace under ``workspace_phases[ws_id]["planning_data"]`` — +reading a top-level ``planning_data`` finds nothing and silently degrades every row to a +label that only repeats the phase number. +""" +from __future__ import annotations + +import logging +from unittest.mock import AsyncMock, Mock + +import pytest + +from app.schemas.workspace import WorkspaceSettings +from app.services.p10y.multi_workspace_estimation import ( + UNNAMED_PHASE, + estimate_single_workspace, +) +from app.services.p10y.p10y_lib import CodeGenerationMetadata, CommitInfo +from app.workflows.multi_workspace_estimation_p10y import _load_phase_names + +LOGGER = logging.getLogger("test_phase_names") + + +def _adapter(doc: dict | None) -> Mock: + adapter = Mock() + adapter.get_generation_session = AsyncMock(return_value=doc) + return adapter + + +def _plan(*phases: tuple[int, str]) -> dict: + return {"phases": [{"number": n, "name": name} for n, name in phases]} + + +@pytest.mark.asyncio +async def test_reads_names_from_workspace_phases() -> None: + """The plan lives under workspace_phases, not at the document root.""" + doc = { + "workspace_phases": { + "ws-05-1": { + "last_completed_phase": 2, + "total_phases": 2, + "planning_data": _plan((1, "Repo Scaffold & Tooling"), (2, "Hold lifecycle")), + } + } + } + assert await _load_phase_names("gen-1", _adapter(doc), LOGGER) == { + 1: "Repo Scaffold & Tooling", + 2: "Hold lifecycle", + } + + +@pytest.mark.asyncio +async def test_top_level_planning_data_is_not_where_the_plan_lives() -> None: + """Guards the regression: a root-level planning_data must not be the only place we look.""" + doc = {"planning_data": _plan((1, "Root Level"))} + assert await _load_phase_names("gen-1", _adapter(doc), LOGGER) == {} + + +@pytest.mark.asyncio +async def test_first_workspace_entry_with_phases_wins() -> None: + """Every workspace carries the same plan; entries without one are skipped.""" + doc = { + "workspace_phases": { + "ws-05-1": {"last_completed_phase": 0}, + "ws-05-2": {"planning_data": _plan((3, "Backend Domain"))}, + } + } + assert await _load_phase_names("gen-1", _adapter(doc), LOGGER) == {3: "Backend Domain"} + + +@pytest.mark.asyncio +async def test_unusable_phase_entries_are_ignored() -> None: + doc = { + "workspace_phases": { + "ws-1": { + "planning_data": { + "phases": [ + {"number": 1, "name": "Kept"}, + {"number": "2", "name": "Non-int number"}, + {"number": 3, "name": ""}, + {"name": "No number"}, + ] + } + } + } + } + assert await _load_phase_names("gen-1", _adapter(doc), LOGGER) == {1: "Kept"} + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "doc", + [None, {}, {"workspace_phases": {}}, {"workspace_phases": {"ws-1": {"planning_data": {}}}}], +) +async def test_missing_plan_yields_no_names(doc) -> None: + assert await _load_phase_names("gen-1", _adapter(doc), LOGGER) == {} + + +@pytest.mark.asyncio +async def test_no_generation_id_or_adapter_skips_the_read() -> None: + adapter = _adapter({"workspace_phases": {}}) + assert await _load_phase_names(None, adapter, LOGGER) == {} + assert await _load_phase_names("gen-1", None, LOGGER) == {} + adapter.get_generation_session.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_adapter_failure_is_non_fatal() -> None: + adapter = Mock() + adapter.get_generation_session = AsyncMock(side_effect=RuntimeError("firestore down")) + assert await _load_phase_names("gen-1", adapter, LOGGER) == {} + + +# --------------------------------------------------------------------------- +# How the resolved names reach the breakdown labels +# --------------------------------------------------------------------------- + +def _commit_stats(sha: str, fp: float) -> dict: + return { + "sha": sha, + "fp_delta_total": fp, + "commit_quality_score": 0.8, + "churn_rate": 0.1, + "refactor": 0.0, + "rework": 0.0, + "new_work": fp, + "removed_work": 0.0, + "quality_score": 0.8, + "effective_output": fp, + "total_output": fp, + } + + +async def _estimate_with(phase_names: dict[int, str] | None): + workspace = WorkspaceSettings(workspace_path="/tmp/ws-1", provider="openrouter", model="m", name="ws-1") + metadata = CodeGenerationMetadata( + commits=[CommitInfo(sha="abc123", message="p03_add endpoint", phase=3)] + ) + return await estimate_single_workspace( + workspace=workspace, + filtered_commit_stats_data=[_commit_stats("abc123", 5.0)], + code_generation_metadata=metadata, + logger=LOGGER, + phase_names=phase_names, + ) + + +@pytest.mark.asyncio +async def test_plan_name_becomes_the_phase_label() -> None: + est = await _estimate_with({3: "Backend — Projection & Modes"}) + assert est is not None + assert est.phase_breakdown["03"].phase_name == "Backend — Projection & Modes" + assert est.phase_breakdown["03"].phase_number == 3 + + +@pytest.mark.asyncio +async def test_unnamed_phase_does_not_repeat_the_number() -> None: + """Without a name the label must not become "Phase 3" — the number has its own column.""" + est = await _estimate_with(None) + assert est is not None + label = est.phase_breakdown["03"] + assert label.phase_number == 3 + assert label.phase_name == UNNAMED_PHASE + assert "Phase 3" not in label.phase_name diff --git a/backend/test/test_report_generation.py b/backend/test/test_report_generation.py index 117c060..81f866e 100644 --- a/backend/test/test_report_generation.py +++ b/backend/test/test_report_generation.py @@ -6,8 +6,8 @@ from app.schemas.estimate import ( ComparativeAnalysis, - ComponentComparison, - ComponentEstimation, + PhaseComparison, + PhaseEstimation, EstimationMetrics, EstimationSummary, SkippedWorkspaceP10Y, @@ -15,7 +15,7 @@ ) from app.services.p10y.estimation_report_generator import ( create_comparison_table, - format_high_variance_components, + format_high_variance_phases, format_multi_workspace_report, format_workspace_breakdown, visualize_variance, @@ -32,17 +32,19 @@ def sample_workspace_estimations(): total_hours=150.0, total_effective_output=75.0, commits_count=15, - component_breakdown={ - "backend": ComponentEstimation( - component_name="backend", + phase_breakdown={ + "06": PhaseEstimation( + phase_number=6, + phase_name="Backend Domain", hours=80.0, new_work=60.0, refactor=15.0, rework=5.0, quality_score=0.85 ), - "frontend": ComponentEstimation( - component_name="frontend", + "13": PhaseEstimation( + phase_number=13, + phase_name="Frontend Shell", hours=70.0, new_work=55.0, refactor=12.0, @@ -66,17 +68,19 @@ def sample_workspace_estimations(): total_hours=180.0, total_effective_output=90.0, commits_count=18, - component_breakdown={ - "backend": ComponentEstimation( - component_name="backend", + phase_breakdown={ + "06": PhaseEstimation( + phase_number=6, + phase_name="Backend Domain", hours=100.0, new_work=75.0, refactor=20.0, rework=5.0, quality_score=0.80 ), - "frontend": ComponentEstimation( - component_name="frontend", + "13": PhaseEstimation( + phase_number=13, + phase_name="Frontend Shell", hours=80.0, new_work=60.0, refactor=15.0, @@ -100,17 +104,19 @@ def sample_workspace_estimations(): total_hours=165.0, total_effective_output=82.5, commits_count=16, - component_breakdown={ - "backend": ComponentEstimation( - component_name="backend", + phase_breakdown={ + "06": PhaseEstimation( + phase_number=6, + phase_name="Backend Domain", hours=90.0, new_work=68.0, refactor=18.0, rework=4.0, quality_score=0.83 ), - "frontend": ComponentEstimation( - component_name="frontend", + "13": PhaseEstimation( + phase_number=13, + phase_name="Frontend Shell", hours=75.0, new_work=58.0, refactor=14.0, @@ -145,11 +151,12 @@ def sample_summary(): @pytest.fixture -def sample_component_comparison(): - """Create sample component comparison.""" +def sample_phase_comparison(): + """Create sample phase comparison.""" return { - "backend": ComponentComparison( - component_name="backend", + "06": PhaseComparison( + phase_number=6, + phase_name="Backend Domain", hours_by_workspace={ "workspace-1": 80.0, "workspace-2": 100.0, @@ -159,8 +166,9 @@ def sample_component_comparison(): std_deviation=10.0, variance_percentage=11.1 ), - "frontend": ComponentComparison( - component_name="frontend", + "13": PhaseComparison( + phase_number=13, + phase_name="Frontend Shell", hours_by_workspace={ "workspace-1": 70.0, "workspace-2": 80.0, @@ -174,11 +182,11 @@ def sample_component_comparison(): @pytest.fixture -def sample_comparative_analysis(sample_component_comparison): +def sample_comparative_analysis(sample_phase_comparison): """Create sample comparative analysis.""" return ComparativeAnalysis( - component_comparison=sample_component_comparison, - high_variance_components=[], + phase_comparison=sample_phase_comparison, + high_variance_phases=[], insights=[ "All workspaces show consistent estimates (CV < 15%)", "Quality scores are uniformly high across workspaces (0.82-0.87)", @@ -191,42 +199,43 @@ class TestCreateComparisonTable: """Tests for create_comparison_table function.""" def test_creates_valid_markdown_table( - self, sample_workspace_estimations, sample_component_comparison + self, sample_workspace_estimations, sample_phase_comparison ): """Test that a valid markdown table is created.""" table = create_comparison_table( sample_workspace_estimations, - sample_component_comparison + sample_phase_comparison ) - + # Check table structure - assert "| Component |" in table + assert "| Phase # | Description |" in table assert "workspace-1" in table assert "workspace-2" in table assert "workspace-3" in table assert "Average" in table assert "Std Dev" in table assert "Variance %" in table - - # Check component data - assert "backend" in table - assert "frontend" in table - assert "80.0h" in table # workspace-1 backend - assert "100.0h" in table # workspace-2 backend - + + # Check phase data + assert "Backend Domain" in table + assert "Frontend Shell" in table + assert "80.0h" in table # workspace-1 backend phase + assert "100.0h" in table # workspace-2 backend phase + def test_handles_empty_comparison(self, sample_workspace_estimations): - """Test handling of empty component comparison.""" + """Test handling of empty phase comparison.""" table = create_comparison_table(sample_workspace_estimations, {}) - assert "No components to compare" in table - + assert "No phases to compare" in table + def test_handles_missing_components(self, sample_workspace_estimations): - """Test handling when some workspaces don't have a component.""" - component_comparison = { - "backend": ComponentComparison( - component_name="backend", + """Test handling when some workspaces don't have a phase.""" + phase_comparison = { + "06": PhaseComparison( + phase_number=6, + phase_name="Backend Domain", hours_by_workspace={ "workspace-1": 80.0, - # workspace-2 missing backend + # workspace-2 missing backend phase "workspace-3": 90.0, }, average=85.0, @@ -234,13 +243,13 @@ def test_handles_missing_components(self, sample_workspace_estimations): variance_percentage=8.3 ), } - + table = create_comparison_table( sample_workspace_estimations, - component_comparison + phase_comparison ) - - # Should show "-" for missing component + + # Should show "-" for missing phase assert "-" in table @@ -323,16 +332,16 @@ def test_includes_key_metrics(self, sample_workspace_estimations): assert "Total Effective Output Points" in breakdown assert "Commits Analyzed" in breakdown assert "Work Type Breakdown" in breakdown - assert "Component Complexity Metrics Breakdown" in breakdown + assert "Phase Breakdown" in breakdown assert "Quality Score" in breakdown - + def test_component_sorting(self, sample_workspace_estimations): - """Test that components are sorted by hours (descending).""" + """Test that phases are sorted in plan order (by phase number).""" breakdown = format_workspace_breakdown(sample_workspace_estimations) - - # For workspace-1, backend (80h) should appear before frontend (70h) - backend_pos = breakdown.find("**backend**") - frontend_pos = breakdown.find("**frontend**") + + # Phase 06 (Backend Domain) should appear before phase 13 (Frontend Shell) + backend_pos = breakdown.find("**06 Backend Domain**") + frontend_pos = breakdown.find("**13 Frontend Shell**") assert backend_pos < frontend_pos def test_zero_total_effective_output_omits_share_percentages(self): @@ -343,7 +352,7 @@ def test_zero_total_effective_output_omits_share_percentages(self): total_hours=0.0, total_effective_output=0.0, commits_count=0, - component_breakdown={}, + phase_breakdown={}, estimation_metrics=EstimationMetrics( new_work=0.0, refactor=0.0, @@ -362,34 +371,34 @@ def test_zero_total_effective_output_omits_share_percentages(self): class TestFormatHighVarianceComponents: - """Tests for format_high_variance_components function.""" - - def test_no_high_variance_components(self, sample_comparative_analysis, sample_component_comparison): - """Test when there are no high variance components.""" - result = format_high_variance_components( + """Tests for format_high_variance_phases function.""" + + def test_no_high_variance_components(self, sample_comparative_analysis, sample_phase_comparison): + """Test when there are no high variance phases.""" + result = format_high_variance_phases( sample_comparative_analysis, - sample_component_comparison + sample_phase_comparison ) - - assert "No high variance components detected" in result + + assert "No high variance phases detected" in result assert "✅" in result - - def test_with_high_variance_components(self, sample_component_comparison): - """Test when there are high variance components.""" + + def test_with_high_variance_components(self, sample_phase_comparison): + """Test when there are high variance phases.""" analysis = ComparativeAnalysis( - component_comparison=sample_component_comparison, - high_variance_components=["backend"], + phase_comparison=sample_phase_comparison, + high_variance_phases=["06"], insights=[] ) - - result = format_high_variance_components( + + result = format_high_variance_phases( analysis, - sample_component_comparison + sample_phase_comparison ) - - assert "High Variance Component(s) Detected" in result + + assert "High Variance Phase(s) Detected" in result assert "⚠️" in result - assert "backend" in result + assert "Backend Domain" in result assert "Average:" in result assert "Variance:" in result @@ -414,8 +423,8 @@ def test_generates_complete_report( assert "# Multi-Workspace Estimation Report" in report assert "## Executive Summary" in report assert "## Per-Workspace Breakdown" in report - assert "## Component Comparison" in report - assert "## High Variance Components" in report + assert "## Phase Comparison" in report + assert "## High Variance Phases" in report assert "## Key Insights" in report assert "## Recommendations" in report @@ -645,9 +654,10 @@ async def test_html_report_written_next_to_markdown_report(self, tmp_path): workspace_path=str(tmp_path / "ws-01-1"), total_hours=100.0, total_effective_output=90.0, - component_breakdown={ - "auth": ComponentEstimation( - component_name="auth", + phase_breakdown={ + "07": PhaseEstimation( + phase_number=7, + phase_name="Auth", hours=40.0, new_work=30.0, refactor=8.0, @@ -713,7 +723,7 @@ async def test_html_report_written_next_to_markdown_report(self, tmp_path): assert html_report.exists() html_content = html_report.read_text() assert " str: + return subprocess.run( + ["git", *args], cwd=repo, check=True, capture_output=True, text=True + ).stdout.strip() + + +def _init_repo(repo: Path) -> None: + _git(repo, "init") + _git(repo, "config", "user.email", "t@test") + _git(repo, "config", "user.name", "Test") + ensure_workspace_git_hooks(repo) + + +def _commit(repo: Path, message: str, filename: str) -> None: + (repo / filename).write_text(filename) + _git(repo, "add", filename) + _git(repo, "commit", "-m", message) + + +def _head_subject(repo: Path) -> str: + return _git(repo, "log", "-1", "--format=%s") + + +def test_hook_installed_executable(tmp_path: Path) -> None: + _init_repo(tmp_path) + hook = tmp_path / ".git" / "hooks" / "prepare-commit-msg" + assert hook.is_file() + assert hook.stat().st_mode & 0o111 # executable bit set + + +def test_hook_stamps_phase_prefix(tmp_path: Path) -> None: + _init_repo(tmp_path) + _git(tmp_path, "config", "specflow.phase", "7") + _commit(tmp_path, "add neighbors endpoint", "a.txt") + assert _head_subject(tmp_path) == "p07_add neighbors endpoint" + + +def test_hook_zero_pads_and_reads_current_phase(tmp_path: Path) -> None: + _init_repo(tmp_path) + _git(tmp_path, "config", "specflow.phase", "13") + _commit(tmp_path, "build galaxy view", "b.txt") + assert _head_subject(tmp_path) == "p13_build galaxy view" + + +def test_hook_leaves_skip_commits_untouched(tmp_path: Path) -> None: + """SKIP_ commits must stay excluded — the hook must not turn them into pNN_SKIP_.""" + _init_repo(tmp_path) + _git(tmp_path, "config", "specflow.phase", "24") + _commit(tmp_path, "SKIP_janitor_finalize", "c.txt") + assert _head_subject(tmp_path) == "SKIP_janitor_finalize" + + +def test_hook_is_idempotent_for_already_prefixed(tmp_path: Path) -> None: + _init_repo(tmp_path) + _git(tmp_path, "config", "specflow.phase", "7") + _commit(tmp_path, "p03_already prefixed", "d.txt") + # Stays p03_, not double-stamped to p07_p03_ + assert _head_subject(tmp_path) == "p03_already prefixed" + + +def test_hook_no_prefix_when_marker_empty(tmp_path: Path) -> None: + """An empty marker (as cleared for the deploy loop) leaves commits unphased.""" + _init_repo(tmp_path) + _git(tmp_path, "config", "specflow.phase", "") + _commit(tmp_path, "deploy config change", "e.txt") + assert _head_subject(tmp_path) == "deploy config change" + + +def test_hook_no_prefix_when_marker_unset(tmp_path: Path) -> None: + _init_repo(tmp_path) # never sets specflow.phase + _commit(tmp_path, "pre-generation seed work", "f.txt") + assert _head_subject(tmp_path) == "pre-generation seed work" + + +def test_ensure_hooks_no_git_dir_returns_false(tmp_path: Path) -> None: + # Plain directory without a .git — safe no-op. + assert ensure_workspace_git_hooks(tmp_path) is False diff --git a/mcp_server/tests/test_tui_app.py b/mcp_server/tests/test_tui_app.py index 592f8f9..a11df0f 100644 --- a/mcp_server/tests/test_tui_app.py +++ b/mcp_server/tests/test_tui_app.py @@ -80,7 +80,7 @@ def test_none_payload_renders_waiting(self): out = self._render(tui_app.build_dashboard(None, Path("/tmp/x"), "gen_x")) assert "Waiting for status" in out - def test_completed_renders_component_breakdown(self): + def test_completed_renders_phase_breakdown(self): payload = { "generation_id": "gen_x", "status": "completed", @@ -90,9 +90,10 @@ def test_completed_renders_component_breakdown(self): "summary": {"average_hours": 318}, "workspace_estimations": [], "comparative_analysis": { - "component_comparison": { - "auth": { - "component_name": "auth", + "phase_comparison": { + "07": { + "phase_number": 7, + "phase_name": "Auth API", "average": 40.0, "variance_percentage": 12.0, } @@ -101,9 +102,33 @@ def test_completed_renders_component_breakdown(self): }, } out = self._render(tui_app.build_dashboard(payload, Path("/tmp/acme"), "gen_x")) - assert "auth" in out + assert "Auth API" in out assert "40" in out assert "12%" in out + assert "07" in out + + def test_buffer_percentage_is_scaled_to_percent(self): + # total_buffer_pct is a fraction (0.83); the panel must render +83%, not +1%. + payload = { + "generation_id": "gen_x", + "status": "completed", + "checkpoint": "estimation_done", + "progress": {"workspace_phases": {}}, + "result": { + "summary": { + "average_hours": 151, + "risk_assessment": { + "status": "Approved", + "total_buffer_pct": 0.83, + "final_estimate": 277, + }, + }, + "workspace_estimations": [], + }, + } + out = self._render(tui_app.build_dashboard(payload, Path("/tmp/acme"), "gen_x")) + assert "+83%" in out + assert "+1%" not in out def test_completed_renders_report_hint(self): # The report file lives inside the backend container, not on the host diff --git a/mcp_server/tests/test_tui_render.py b/mcp_server/tests/test_tui_render.py index d62e21d..4b0340e 100644 --- a/mcp_server/tests/test_tui_render.py +++ b/mcp_server/tests/test_tui_render.py @@ -213,7 +213,7 @@ def test_full_result(self): assert panel.risk_status == "Approved" assert panel.per_workspace == [("ws-01-1", 305.0), ("ws-01-2", 331.0)] assert panel.total_usd_cost == 94.1 - assert panel.component_comparison == [] + assert panel.phase_comparison == [] def test_partial_result_is_tolerant(self): panel = render.estimate_panel({"result": {"summary": {"average_hours": 100}}}) @@ -221,20 +221,47 @@ def test_partial_result_is_tolerant(self): assert panel.risk_status is None assert panel.per_workspace == [] - def test_component_comparison_sorted_by_variance_descending(self): + def test_phase_comparison_sorted_by_phase_number(self): result = { "summary": {}, "comparative_analysis": { - "component_comparison": { - "auth": {"component_name": "auth", "average": 40.0, "variance_percentage": 5.0}, - "billing": {"component_name": "billing", "average": 20.0, "variance_percentage": 25.0}, + "phase_comparison": { + "13": {"phase_number": 13, "phase_name": "Frontend", "average": 20.0, "variance_percentage": 25.0}, + "06": {"phase_number": 6, "phase_name": "Backend", "average": 40.0, "variance_percentage": 5.0}, } }, } panel = render.estimate_panel({"result": result}) - assert [row.component_name for row in panel.component_comparison] == ["billing", "auth"] - assert panel.component_comparison[0].average_hours == 20.0 - assert panel.component_comparison[0].variance_percentage == 25.0 + # Plan/timeline order (ascending by phase number), not variance order. + assert [row.phase_number for row in panel.phase_comparison] == [6, 13] + assert [row.phase_name for row in panel.phase_comparison] == ["Backend", "Frontend"] + assert panel.phase_comparison[0].average_hours == 40.0 + + def test_unphased_row_sorts_last(self): + result = { + "summary": {}, + "comparative_analysis": { + "phase_comparison": { + "unphased": {"phase_number": None, "phase_name": "unphased", "average": 5.0, "variance_percentage": 0.0}, + "06": {"phase_number": 6, "phase_name": "Backend", "average": 40.0, "variance_percentage": 5.0}, + } + }, + } + panel = render.estimate_panel({"result": result}) + assert [row.phase_number for row in panel.phase_comparison] == [6, None] + + +class TestTruncate: + def test_short_string_unchanged(self): + assert render.truncate("Backend", 32) == "Backend" + + def test_long_string_gets_ellipsis(self): + result = render.truncate("A very long phase description name", 10) + assert result == "A very lo…" + assert len(result) == 10 + + def test_none_is_safe(self): + assert render.truncate(None, 5) == "" class _Event: diff --git a/mcp_server/tui/app.py b/mcp_server/tui/app.py index e35e5cd..275fd70 100644 --- a/mcp_server/tui/app.py +++ b/mcp_server/tui/app.py @@ -170,7 +170,7 @@ def _workspaces_panel(payload: dict[str, Any], selected_ws_id: str | None = None Text(marker, style="yellow"), Text(bar.workspace_id, style=id_style), Text(bar.phase_label, style="cyan"), - Text(bar.phase_name[:32], style="dim"), + Text(render.truncate(bar.phase_name, 32), style="dim"), Text(render.progress_bar(bar.fraction), style="green"), Text(f"{bar.percent}%"), _workspace_badge(bar, flagged), @@ -199,7 +199,7 @@ def fmt_h(v: float | None) -> str: if panel.min_hours is not None and panel.max_hours is not None: grid.add_row("Range", f"{panel.min_hours:.0f}–{panel.max_hours:.0f} h") if panel.total_buffer_pct is not None or panel.final_estimate is not None: - buf = f"+{panel.total_buffer_pct:.0f}%" if panel.total_buffer_pct is not None else "" + buf = f"+{panel.total_buffer_pct*100:.0f}%" if panel.total_buffer_pct is not None else "" final = f" → {panel.final_estimate:.0f} h" if panel.final_estimate is not None else "" grid.add_row("Buffer", f"{buf}{final}".strip()) if panel.per_workspace: @@ -209,14 +209,17 @@ def fmt_h(v: float | None) -> str: grid.add_row("Total spend", f"${panel.total_usd_cost:.2f}") renderables: list[RenderableType] = [grid] - if panel.component_comparison: + if panel.phase_comparison: breakdown = Table(box=None, padding=(0, 2)) - breakdown.add_column("Component", justify="left") + breakdown.add_column("Phase #", justify="right") + breakdown.add_column("Description", justify="left") breakdown.add_column("Avg hours", justify="right") breakdown.add_column("Variance", justify="right") - for row in panel.component_comparison: + for row in panel.phase_comparison: + num = f"{row.phase_number:02d}" if row.phase_number is not None else "—" breakdown.add_row( - row.component_name, + num, + render.truncate(row.phase_name, 32), f"{row.average_hours:.0f} h", f"{row.variance_percentage:.0f}%", ) diff --git a/mcp_server/tui/render.py b/mcp_server/tui/render.py index 34d9772..c9a9b37 100644 --- a/mcp_server/tui/render.py +++ b/mcp_server/tui/render.py @@ -28,6 +28,12 @@ _IN_PROGRESS_STATUSES = frozenset({"running", "initializing", "pending"}) +def truncate(text: str, limit: int) -> str: + """Truncate to `limit` characters with a trailing ellipsis when it overflows.""" + text = text or "" + return text if len(text) <= limit else text[: max(0, limit - 1)] + "…" + + @dataclass(frozen=True) class PipelineStep: """One step in the checkpoint stepper.""" @@ -70,10 +76,11 @@ def phase_label(self) -> str: @dataclass(frozen=True) -class ComponentBreakdownRow: - """One row of the cross-workspace component comparison table.""" +class PhaseBreakdownRow: + """One row of the cross-workspace per-phase comparison table.""" - component_name: str + phase_number: int | None + phase_name: str average_hours: float variance_percentage: float @@ -92,7 +99,7 @@ class EstimatePanel: final_estimate: float | None per_workspace: list[tuple[str, float]] = field(default_factory=list) total_usd_cost: float | None = None - component_comparison: list[ComponentBreakdownRow] = field(default_factory=list) + phase_comparison: list[PhaseBreakdownRow] = field(default_factory=list) def status_pill(status: str | None) -> tuple[str, str]: @@ -238,18 +245,20 @@ def clear_ws_ineligible_message(payload: dict[str, Any] | None) -> str: return "Nothing to clear — these workspaces are not awaiting cleanup." -def _component_comparison_rows(result: dict[str, Any]) -> list[ComponentBreakdownRow]: - """Cross-workspace per-component breakdown, highest-variance first.""" - comparison = (result.get("comparative_analysis") or {}).get("component_comparison") or {} +def _phase_comparison_rows(result: dict[str, Any]) -> list[PhaseBreakdownRow]: + """Cross-workspace per-phase breakdown, in plan order (unphased last).""" + comparison = (result.get("comparative_analysis") or {}).get("phase_comparison") or {} rows = [ - ComponentBreakdownRow( - component_name=data.get("component_name") or name, + PhaseBreakdownRow( + phase_number=data.get("phase_number"), + phase_name=data.get("phase_name") or name, average_hours=float(data.get("average") or 0.0), variance_percentage=float(data.get("variance_percentage") or 0.0), ) for name, data in comparison.items() ] - rows.sort(key=lambda row: row.variance_percentage, reverse=True) + # Plan order (timeline); unphased (no number) sorts last. + rows.sort(key=lambda row: (row.phase_number is None, row.phase_number or 0)) return rows @@ -285,7 +294,7 @@ def estimate_panel(payload: dict[str, Any] | None) -> EstimatePanel | None: final_estimate=risk.get("final_estimate"), per_workspace=per_workspace, total_usd_cost=result.get("total_usd_cost"), - component_comparison=_component_comparison_rows(result), + phase_comparison=_phase_comparison_rows(result), )
ComponentPhase #Description
{component_name}{num}{phase_data["name"]}{hours:.1f}-