Skip to main content

fallow_output/
health_findings.rs

1//! Health-finding wrappers, action context, and typed action builders.
2//!
3//! This module keeps the wire envelopes typed while preserving the existing
4//! flattened JSON shape.
5
6use fallow_types::output_health::{
7    HealthFindingAction, HealthFindingActionType, HotspotAction, HotspotActionHeuristic,
8    HotspotActionType, RefactoringTargetAction, RefactoringTargetActionType,
9};
10use std::ops::Deref;
11use std::path::Path;
12
13use crate::{
14    ComplexityViolation, CoverageTier, ExceededThreshold, HotspotEntry, OwnershipMetrics,
15    OwnershipState, RecommendationCategory, RefactoringTarget,
16};
17
18/// Options controlling how the action builder populates `actions`.
19#[derive(Debug, Clone, Copy, Default)]
20pub struct HealthActionOptions {
21    /// Skip `suppress-line` action entries.
22    pub omit_suppress_line: bool,
23    /// Reason surfaced in `actions_meta` when `omit_suppress_line` is true.
24    pub omit_reason: Option<&'static str>,
25}
26
27/// Construction-time context for [`HealthFinding::with_actions`].
28#[derive(Debug, Clone, Copy)]
29pub struct HealthActionContext {
30    /// Action-emission options.
31    pub opts: HealthActionOptions,
32    /// Cyclomatic-complexity ceiling.
33    pub max_cyclomatic_threshold: u16,
34    /// Cognitive-complexity ceiling.
35    pub max_cognitive_threshold: u16,
36    /// CRAP ceiling.
37    pub max_crap_threshold: f64,
38    /// Band below `max_cyclomatic_threshold` where a CRAP-only finding also
39    /// gets a secondary `refactor-function` action.
40    pub crap_refactor_band: u16,
41}
42
43/// Wire envelope for a single complexity finding.
44#[derive(Debug, Clone, serde::Serialize)]
45#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
46pub struct HealthFinding {
47    /// Inner complexity-violation payload.
48    #[serde(flatten)]
49    pub violation: ComplexityViolation,
50    /// Machine-actionable fix and suppress hints.
51    pub actions: Vec<HealthFindingAction>,
52    /// Audit-mode flag indicating whether the finding is new versus the base
53    /// snapshot.
54    #[serde(default, skip_serializing_if = "Option::is_none")]
55    pub introduced: Option<bool>,
56}
57
58impl Deref for HealthFinding {
59    type Target = ComplexityViolation;
60
61    fn deref(&self) -> &Self::Target {
62        &self.violation
63    }
64}
65
66impl From<ComplexityViolation> for HealthFinding {
67    /// Wrap a violation with empty actions and no `introduced` flag.
68    fn from(violation: ComplexityViolation) -> Self {
69        Self {
70            violation,
71            actions: Vec::new(),
72            introduced: None,
73        }
74    }
75}
76
77impl HealthFinding {
78    /// Construct a wrapper around a pre-computed action list.
79    #[must_use]
80    #[allow(
81        dead_code,
82        reason = "intentional public constructor for audit / test paths that supply their own actions; with_actions is the production constructor"
83    )]
84    pub fn new(
85        violation: ComplexityViolation,
86        actions: Vec<HealthFindingAction>,
87        introduced: Option<bool>,
88    ) -> Self {
89        Self {
90            violation,
91            actions,
92            introduced,
93        }
94    }
95
96    /// Construct a wrapper with `actions` computed from the finding and
97    /// report-wide context.
98    #[must_use]
99    pub fn with_actions(violation: ComplexityViolation, ctx: &HealthActionContext) -> Self {
100        let actions = build_health_finding_actions(&violation, ctx);
101        Self {
102            violation,
103            actions,
104            introduced: None,
105        }
106    }
107}
108
109/// Compute the typed `actions` list for a complexity finding.
110#[must_use]
111pub fn build_health_finding_actions(
112    violation: &ComplexityViolation,
113    ctx: &HealthActionContext,
114) -> Vec<HealthFindingAction> {
115    let name = violation.name.as_str();
116    let exceeded = violation.exceeded;
117    let includes_crap = exceeded.includes_crap();
118    let crap_only = matches!(exceeded, ExceededThreshold::Crap);
119    let cyclomatic = violation.cyclomatic;
120    let cognitive = violation.cognitive;
121    let max_cyclomatic_threshold = violation
122        .effective_thresholds
123        .map_or(ctx.max_cyclomatic_threshold, |thresholds| {
124            thresholds.max_cyclomatic
125        });
126    let max_cognitive_threshold = violation
127        .effective_thresholds
128        .map_or(ctx.max_cognitive_threshold, |thresholds| {
129            thresholds.max_cognitive
130        });
131    let max_crap_threshold = violation
132        .effective_thresholds
133        .map_or(ctx.max_crap_threshold, |thresholds| thresholds.max_crap);
134    let full_coverage_can_clear_crap = !includes_crap || f64::from(cyclomatic) < max_crap_threshold;
135
136    let mut actions: Vec<HealthFindingAction> = Vec::new();
137
138    let inherited_from = violation.inherited_from.as_deref();
139    if includes_crap
140        && let Some(action) = build_crap_coverage_action(
141            name,
142            violation.coverage_tier,
143            full_coverage_can_clear_crap,
144            inherited_from,
145        )
146    {
147        actions.push(action);
148    }
149
150    let is_template = name == "<template>";
151    let is_component = name == "<component>";
152    if should_add_refactor_action(RefactorActionDecision {
153        crap_only,
154        full_coverage_can_clear_crap,
155        cyclomatic,
156        cognitive,
157        max_cyclomatic_threshold,
158        max_cognitive_threshold,
159        ctx,
160    }) {
161        actions.push(build_refactor_action(
162            violation,
163            name,
164            is_template,
165            is_component,
166        ));
167    }
168
169    if !ctx.opts.omit_suppress_line {
170        actions.push(build_suppress_action(violation, is_template, is_component));
171    }
172
173    actions
174}
175
176#[derive(Clone, Copy)]
177struct RefactorActionDecision<'a> {
178    crap_only: bool,
179    full_coverage_can_clear_crap: bool,
180    cyclomatic: u16,
181    cognitive: u16,
182    max_cyclomatic_threshold: u16,
183    max_cognitive_threshold: u16,
184    ctx: &'a HealthActionContext,
185}
186
187fn should_add_refactor_action(input: RefactorActionDecision<'_>) -> bool {
188    let crap_only_needs_complexity_reduction =
189        input.crap_only && !input.full_coverage_can_clear_crap;
190    let cognitive_floor = input.max_cognitive_threshold / 2;
191    let near_cyclomatic_threshold = input.crap_only
192        && input.cyclomatic > 0
193        && input.cyclomatic
194            >= input
195                .max_cyclomatic_threshold
196                .saturating_sub(input.ctx.crap_refactor_band)
197        && input.cognitive >= cognitive_floor;
198    !input.crap_only || crap_only_needs_complexity_reduction || near_cyclomatic_threshold
199}
200
201fn build_refactor_action(
202    violation: &ComplexityViolation,
203    name: &str,
204    is_template: bool,
205    is_component: bool,
206) -> HealthFindingAction {
207    let (description, note): (String, &str) = if is_component {
208        component_refactor_copy(violation)
209    } else if is_template {
210        (
211            format!(
212                "Refactor `{name}` to reduce template complexity (simplify control flow and bindings)"
213            ),
214            "Consider splitting complex template branches into smaller components or simpler bindings",
215        )
216    } else {
217        (
218            format!(
219                "Refactor `{name}` to reduce complexity (extract helper functions, simplify branching)"
220            ),
221            "Consider splitting into smaller functions with single responsibilities",
222        )
223    };
224    HealthFindingAction {
225        kind: HealthFindingActionType::RefactorFunction,
226        auto_fixable: false,
227        description,
228        note: Some(note.to_string()),
229        comment: None,
230        placement: None,
231        target_path: None,
232    }
233}
234
235fn component_refactor_copy(violation: &ComplexityViolation) -> (String, &'static str) {
236    let rollup = violation.component_rollup.as_ref();
237    let class_name = rollup.map_or("the component", |r| r.component.as_str());
238    let worst_method = rollup.map_or("the worst class method", |r| {
239        r.class_worst_function.as_str()
240    });
241    let class_cyc = rollup.map_or(0_u16, |r| r.class_cyclomatic);
242    let template_cyc = rollup.map_or(0_u16, |r| r.template_cyclomatic);
243    (
244        format!(
245            "Refactor `{class_name}` to reduce component complexity (rolled-up cyclomatic {} = {class_cyc} on `{worst_method}` + {template_cyc} on the template)",
246            violation.cyclomatic
247        ),
248        "Consider splitting the template into smaller components OR extracting helpers from the worst class method; the rollup reflects the component as one complexity unit",
249    )
250}
251
252fn build_suppress_action(
253    violation: &ComplexityViolation,
254    is_template: bool,
255    is_component: bool,
256) -> HealthFindingAction {
257    if is_template
258        && violation
259            .path
260            .extension()
261            .is_some_and(|ext| ext.eq_ignore_ascii_case("html"))
262    {
263        return suppress_file_action(
264            "Suppress with an HTML comment at the top of the template",
265            "<!-- fallow-ignore-file complexity -->",
266            "top-of-template",
267        );
268    }
269    if is_template {
270        return suppress_line_action(
271            "Suppress with an inline comment above the Angular decorator",
272            "above-angular-decorator",
273        );
274    }
275    if is_component {
276        return suppress_line_action(
277            "Suppress with an inline comment above the worst class method (the rollup is anchored at that method's line, so a comment above it hides both the function finding and the rollup)",
278            "above-component-worst-method",
279        );
280    }
281    suppress_line_action(
282        "Suppress with an inline comment above the function declaration",
283        "above-function-declaration",
284    )
285}
286
287fn suppress_file_action(description: &str, comment: &str, placement: &str) -> HealthFindingAction {
288    HealthFindingAction {
289        kind: HealthFindingActionType::SuppressFile,
290        auto_fixable: false,
291        description: description.to_string(),
292        note: None,
293        comment: Some(comment.to_string()),
294        placement: Some(placement.to_string()),
295        target_path: None,
296    }
297}
298
299fn suppress_line_action(description: &str, placement: &str) -> HealthFindingAction {
300    HealthFindingAction {
301        kind: HealthFindingActionType::SuppressLine,
302        auto_fixable: false,
303        description: description.to_string(),
304        note: None,
305        comment: Some("// fallow-ignore-next-line complexity".to_string()),
306        placement: Some(placement.to_string()),
307        target_path: None,
308    }
309}
310
311/// Build the coverage-leaning action for a CRAP-contributing finding.
312fn build_crap_coverage_action(
313    name: &str,
314    tier: Option<CoverageTier>,
315    full_coverage_can_clear_crap: bool,
316    inherited_from: Option<&Path>,
317) -> Option<HealthFindingAction> {
318    if !full_coverage_can_clear_crap {
319        return None;
320    }
321
322    if let Some(owner) = inherited_from {
323        let owner_str = owner.to_string_lossy().into_owned();
324        return Some(HealthFindingAction {
325            kind: HealthFindingActionType::IncreaseCoverage,
326            auto_fixable: false,
327            description: format!(
328                "Increase test coverage on `{owner_str}` (the CRAP score on `{name}` is inherited from this Angular component; add component tests there rather than against the template)"
329            ),
330            note: Some(
331                "CRAP = CC^2 * (1 - cov/100)^3 + CC; .html templates are exercised through their @Component class, so the test target is the .ts file referenced by `inherited_from`".to_string(),
332            ),
333            comment: None,
334            placement: None,
335            target_path: Some(owner_str),
336        });
337    }
338
339    match tier {
340        Some(CoverageTier::Partial | CoverageTier::High) => Some(HealthFindingAction {
341            kind: HealthFindingActionType::IncreaseCoverage,
342            auto_fixable: false,
343            description: format!(
344                "Increase test coverage for `{name}` (file is reachable from existing tests; add targeted assertions for uncovered branches)"
345            ),
346            note: Some(
347                "CRAP = CC^2 * (1 - cov/100)^3 + CC; targeted branch coverage is more efficient than scaffolding new test files when the file already has coverage".to_string(),
348            ),
349            comment: None,
350            placement: None,
351            target_path: None,
352        }),
353        _ => Some(HealthFindingAction {
354            kind: HealthFindingActionType::AddTests,
355            auto_fixable: false,
356            description: format!(
357                "Add test coverage for `{name}` to lower its CRAP score (coverage reduces risk even without refactoring)"
358            ),
359            note: Some(
360                "CRAP = CC^2 * (1 - cov/100)^3 + CC; higher coverage is the fastest way to bring CRAP under threshold".to_string(),
361            ),
362            comment: None,
363            placement: None,
364            target_path: None,
365        }),
366    }
367}
368
369/// Wire envelope for a single hotspot entry.
370///
371/// Flattens [`HotspotEntry`] for wire continuity and adds the typed
372/// `actions` list. The `#[serde(flatten)]` keeps each `hotspots[]` item
373/// byte-identical to the pre-wrapper shape: inner fields (`path`,
374/// `score`, `commits`, `weighted_commits`, ...) sit at the top level
375/// alongside `actions`. Optional inner fields (`ownership`,
376/// `is_test_path`) keep their original `skip_serializing_if` behaviour
377/// because serde applies the flatten before the parent serializer runs.
378///
379/// Construct via [`HotspotFinding::with_actions`] in the typical health
380/// pipeline (the typed action builder operates on the inner
381/// [`HotspotEntry`]) or via [`HotspotFinding::from`] for fixture and
382/// test code.
383#[derive(Debug, Clone, serde::Serialize)]
384#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
385pub struct HotspotFinding {
386    /// Inner hotspot payload. Flattened on the wire.
387    #[serde(flatten)]
388    pub entry: HotspotEntry,
389    /// Machine-actionable refactor and review hints. Always populated;
390    /// the list never empties because the action selector unconditionally
391    /// emits `refactor-file` plus `add-tests`. Ownership-derived variants
392    /// (`low-bus-factor`, `unowned-hotspot`, `ownership-drift`) are
393    /// appended when `--ownership` is active and the corresponding signal
394    /// fires.
395    pub actions: Vec<HotspotAction>,
396}
397
398impl Deref for HotspotFinding {
399    type Target = HotspotEntry;
400
401    fn deref(&self) -> &Self::Target {
402        &self.entry
403    }
404}
405
406impl From<HotspotEntry> for HotspotFinding {
407    /// Convenience conversion: wrap a hotspot entry with an empty
408    /// `actions` list. Used by tests and fixture builders. Production
409    /// code should call [`HotspotFinding::with_actions`] so the wire
410    /// shape carries the typed actions.
411    fn from(entry: HotspotEntry) -> Self {
412        Self {
413            entry,
414            actions: Vec::new(),
415        }
416    }
417}
418
419impl HotspotFinding {
420    /// Construct a wrapper with the `actions` list computed from the
421    /// hotspot's measured signals plus its ownership block (when
422    /// present).
423    ///
424    /// `root` is the project root used to strip the absolute
425    /// [`HotspotEntry::path`] when composing action descriptions like
426    /// `"Refactor `{path}`, ..."`.
427    /// The JSON post-pass that this wrapper retires ran AFTER
428    /// `strip_root_prefix`, so the typed builder must apply the same
429    /// stripping here for byte-identical wire output.
430    #[must_use]
431    pub fn with_actions(entry: HotspotEntry, root: &Path) -> Self {
432        let actions = build_hotspot_actions(&entry, root);
433        Self { entry, actions }
434    }
435}
436
437/// Compute the typed `actions` list for a hotspot entry.
438///
439/// The list always begins with `refactor-file` plus `add-tests`. The
440/// ownership-derived variants (`low-bus-factor`, `unowned-hotspot`,
441/// `ownership-drift`) are appended when [`HotspotEntry::ownership`] is
442/// present and the corresponding signal fires.
443fn build_hotspot_actions(entry: &HotspotEntry, root: &Path) -> Vec<HotspotAction> {
444    let relative = entry.path.strip_prefix(root).unwrap_or(&entry.path);
445    let path = relative.to_string_lossy().replace('\\', "/");
446    let mut actions = base_hotspot_actions(&path);
447    if let Some(ownership) = entry.ownership.as_ref() {
448        append_ownership_hotspot_actions(&mut actions, ownership, &path);
449    }
450    actions
451}
452
453fn base_hotspot_actions(path: &str) -> Vec<HotspotAction> {
454    vec![
455        HotspotAction {
456            kind: HotspotActionType::RefactorFile,
457            auto_fixable: false,
458            description: format!(
459                "Refactor `{path}`, high complexity combined with frequent changes makes this a maintenance risk"
460            ),
461            note: Some(
462                "Prioritize extracting complex functions, adding tests, or splitting the module"
463                    .to_string(),
464            ),
465            suggested_pattern: None,
466            heuristic: None,
467        },
468        HotspotAction {
469            kind: HotspotActionType::AddTests,
470            auto_fixable: false,
471            description: format!("Add test coverage for `{path}` to reduce change risk"),
472            note: Some(
473                "Frequently changed complex files benefit most from comprehensive test coverage"
474                    .to_string(),
475            ),
476            suggested_pattern: None,
477            heuristic: None,
478        },
479    ]
480}
481
482fn append_ownership_hotspot_actions(
483    actions: &mut Vec<HotspotAction>,
484    ownership: &OwnershipMetrics,
485    path: &str,
486) {
487    if ownership.bus_factor == 1 {
488        actions.push(low_bus_factor_action(ownership, path));
489    }
490
491    if ownership.unowned == Some(true) {
492        actions.push(unowned_hotspot_action(path));
493    }
494
495    if ownership.ownership_state == OwnershipState::Drifting && ownership.drift {
496        actions.push(ownership_drift_action(ownership, path));
497    }
498}
499
500fn low_bus_factor_action(ownership: &OwnershipMetrics, path: &str) -> HotspotAction {
501    let top = &ownership.top_contributor;
502    let owner = top.identifier.as_str();
503    HotspotAction {
504        kind: HotspotActionType::LowBusFactor,
505        auto_fixable: false,
506        description: format!(
507            "{owner} is the sole recent contributor to `{path}`; adding a second reviewer reduces knowledge-loss risk"
508        ),
509        note: low_bus_factor_note(ownership),
510        suggested_pattern: None,
511        heuristic: None,
512    }
513}
514
515fn low_bus_factor_note(ownership: &OwnershipMetrics) -> Option<String> {
516    let suggested: Vec<&str> = ownership
517        .suggested_reviewers
518        .iter()
519        .map(|r| r.identifier.as_str())
520        .collect();
521    if suggested.is_empty() {
522        return (ownership.top_contributor.commits < 5).then(|| {
523            "Single recent contributor on a low-commit file. Consider a pair review for major changes."
524                .to_string()
525        });
526    }
527
528    let list = suggested
529        .iter()
530        .map(|s| format!("@{s}"))
531        .collect::<Vec<_>>()
532        .join(", ");
533    Some(format!("Candidate reviewers: {list}"))
534}
535
536fn unowned_hotspot_action(path: &str) -> HotspotAction {
537    HotspotAction {
538        kind: HotspotActionType::UnownedHotspot,
539        auto_fixable: false,
540        description: format!("Add a CODEOWNERS entry for `{path}`"),
541        note: Some(
542            "Frequently-changed files without declared owners create review bottlenecks"
543                .to_string(),
544        ),
545        suggested_pattern: Some(suggest_codeowners_pattern(path)),
546        heuristic: Some(HotspotActionHeuristic::DirectoryDeepest),
547    }
548}
549
550fn ownership_drift_action(ownership: &OwnershipMetrics, path: &str) -> HotspotAction {
551    let reason = ownership
552        .drift_reason
553        .as_deref()
554        .unwrap_or("ownership has shifted from the original author");
555    HotspotAction {
556        kind: HotspotActionType::OwnershipDrift,
557        auto_fixable: false,
558        description: format!("Update CODEOWNERS for `{path}`: {reason}"),
559        note: Some(
560            "Drift suggests the declared or original owner is no longer the right reviewer"
561                .to_string(),
562        ),
563        suggested_pattern: None,
564        heuristic: None,
565    }
566}
567
568/// Suggest a CODEOWNERS pattern for an unowned hotspot.
569///
570/// Picks the deepest directory containing the file
571/// (e.g. `src/api/users/handlers.ts` -> `/src/api/users/`) so agents can
572/// paste a tightly-scoped default. Earlier versions used the first two
573/// directory levels but that catches too many siblings in monorepos
574/// (`/src/api/` could span 200 files across 8 sub-domains). The deepest
575/// directory keeps the suggestion reviewable while still being a directory
576/// pattern rather than a per-file rule.
577///
578/// The action emits this alongside
579/// [`HotspotActionHeuristic::DirectoryDeepest`] so consumers can branch
580/// on the strategy if it evolves.
581fn suggest_codeowners_pattern(path: &str) -> String {
582    let normalized = path.replace('\\', "/");
583    let trimmed = normalized.trim_start_matches('/');
584    let mut components: Vec<&str> = trimmed.split('/').collect();
585    components.pop(); // drop the file itself
586    if components.is_empty() {
587        return format!("/{trimmed}");
588    }
589    format!("/{}/", components.join("/"))
590}
591
592/// Wire envelope for a single refactoring target.
593///
594/// Flattens [`RefactoringTarget`] for wire continuity and adds the typed
595/// `actions` list. The `#[serde(flatten)]` keeps each `targets[]` item
596/// byte-identical to the pre-wrapper shape: inner fields (`path`,
597/// `priority`, `efficiency`, `recommendation`, `category`, ...) sit at
598/// the top level alongside `actions`. Optional inner fields (`factors`,
599/// `evidence`) keep their original `skip_serializing_if` behaviour.
600///
601/// Construct via [`RefactoringTargetFinding::with_actions`] in the
602/// typical health pipeline or via [`RefactoringTargetFinding::from`] for
603/// fixture and test code.
604#[derive(Debug, Clone, serde::Serialize)]
605#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
606pub struct RefactoringTargetFinding {
607    /// Inner refactoring target payload. Flattened on the wire.
608    #[serde(flatten)]
609    pub target: RefactoringTarget,
610    /// Machine-actionable refactoring and suppression hints. Always
611    /// populated; the list never empties because the action selector
612    /// unconditionally emits `apply-refactoring`. A trailing
613    /// `suppress-line` is appended only when the target carries
614    /// [`RefactoringTarget::evidence`] linking to specific functions.
615    pub actions: Vec<RefactoringTargetAction>,
616}
617
618impl Deref for RefactoringTargetFinding {
619    type Target = RefactoringTarget;
620
621    fn deref(&self) -> &Self::Target {
622        &self.target
623    }
624}
625
626impl From<RefactoringTarget> for RefactoringTargetFinding {
627    /// Convenience conversion: wrap a refactoring target with an empty
628    /// `actions` list. Used by tests and fixture builders. Production
629    /// code should call [`RefactoringTargetFinding::with_actions`] so
630    /// the wire shape carries the typed actions.
631    fn from(target: RefactoringTarget) -> Self {
632        Self {
633            target,
634            actions: Vec::new(),
635        }
636    }
637}
638
639impl RefactoringTargetFinding {
640    /// Construct a wrapper with the `actions` list computed from the
641    /// target's `recommendation`, `category`, and optional `evidence`.
642    ///
643    /// Asymmetry with [`HotspotFinding::with_actions`]: this constructor
644    /// does NOT take a `root: &Path` because refactoring-target action
645    /// descriptions never interpolate the file path; they pass
646    /// [`RefactoringTarget::recommendation`] verbatim into the
647    /// `apply-refactoring` action. The [`RefactoringTarget::category`]
648    /// field flows into the action's `category` field as the serde
649    /// snake-case form.
650    #[must_use]
651    pub fn with_actions(target: RefactoringTarget) -> Self {
652        let actions = build_refactoring_target_actions(&target);
653        Self { target, actions }
654    }
655}
656
657/// Compute the typed `actions` list for a refactoring target.
658///
659/// The list always begins with `apply-refactoring`. A trailing
660/// `suppress-line` is appended only when the target carries
661/// [`RefactoringTarget::evidence`] linking to specific functions.
662fn build_refactoring_target_actions(target: &RefactoringTarget) -> Vec<RefactoringTargetAction> {
663    let mut actions = vec![RefactoringTargetAction {
664        kind: RefactoringTargetActionType::ApplyRefactoring,
665        auto_fixable: false,
666        description: target.recommendation.clone(),
667        category: Some(category_snake_case(&target.category).to_string()),
668        comment: None,
669    }];
670
671    if target.evidence.is_some() {
672        actions.push(RefactoringTargetAction {
673            kind: RefactoringTargetActionType::SuppressLine,
674            auto_fixable: false,
675            description: "Suppress the underlying complexity finding".to_string(),
676            category: None,
677            comment: Some("// fallow-ignore-next-line complexity".to_string()),
678        });
679    }
680
681    actions
682}
683
684/// Serde-rename_all-snake_case form of a [`RecommendationCategory`]
685/// variant.
686///
687/// `RefactoringTargetAction.category` is `Option<String>` carrying the
688/// serde-encoded form of [`RecommendationCategory`]. The JSON post-pass
689/// retired by issue #408 read this string from the serialized JSON
690/// value; the typed action builder needs the same form without paying
691/// for a serde round-trip per target. The
692/// `recommendation_category_snake_case_round_trips` test in this module
693/// asserts every variant matches `serde_json::to_value` byte-for-byte,
694/// so silent drift between this function and the
695/// `#[serde(rename_all = "snake_case")]` attribute is caught at test
696/// time.
697const fn category_snake_case(cat: &RecommendationCategory) -> &'static str {
698    match cat {
699        RecommendationCategory::UrgentChurnComplexity => "urgent_churn_complexity",
700        RecommendationCategory::BreakCircularDependency => "break_circular_dependency",
701        RecommendationCategory::SplitHighImpact => "split_high_impact",
702        RecommendationCategory::RemoveDeadCode => "remove_dead_code",
703        RecommendationCategory::ExtractComplexFunctions => "extract_complex_functions",
704        RecommendationCategory::ExtractDependencies => "extract_dependencies",
705        RecommendationCategory::AddTestCoverage => "add_test_coverage",
706    }
707}
708
709#[cfg(test)]
710mod hotspot_target_tests {
711    use super::*;
712    use crate::{
713        Confidence, ContributorEntry, ContributorIdentifierFormat, EffortEstimate,
714        EvidenceFunction, OwnershipMetrics, OwnershipState, TargetEvidence,
715    };
716    use fallow_types::churn::ChurnTrend;
717    use std::path::PathBuf;
718
719    fn sample_entry(path: &str) -> HotspotEntry {
720        HotspotEntry {
721            path: PathBuf::from(path),
722            score: 80.0,
723            commits: 12,
724            weighted_commits: 8.0,
725            lines_added: 100,
726            lines_deleted: 40,
727            complexity_density: 1.5,
728            fan_in: 3,
729            trend: ChurnTrend::Stable,
730            ownership: None,
731            is_test_path: false,
732        }
733    }
734
735    fn contributor(identifier: &str, commits: u32) -> ContributorEntry {
736        ContributorEntry {
737            identifier: identifier.to_string(),
738            format: ContributorIdentifierFormat::Handle,
739            share: 1.0,
740            stale_days: 1,
741            commits,
742        }
743    }
744
745    fn sample_target() -> RefactoringTarget {
746        RefactoringTarget {
747            path: PathBuf::from("/root/src/foo.ts"),
748            priority: 75.0,
749            efficiency: 75.0,
750            recommendation: "Extract `handleRequest` into helpers".to_string(),
751            category: RecommendationCategory::ExtractComplexFunctions,
752            effort: EffortEstimate::Low,
753            confidence: Confidence::High,
754            factors: Vec::new(),
755            evidence: None,
756        }
757    }
758
759    #[test]
760    fn hotspot_finding_flattens_inner_fields_at_top_level() {
761        let entry = sample_entry("/root/src/api.ts");
762        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
763        let json = serde_json::to_value(&finding).expect("hotspot finding should serialize");
764        let obj = json
765            .as_object()
766            .expect("hotspot finding should serialize as object");
767        assert!(obj.contains_key("score"));
768        assert!(obj.contains_key("commits"));
769        assert!(obj.contains_key("weighted_commits"));
770        assert!(obj.contains_key("actions"));
771        assert!(!obj.contains_key("ownership"));
772        assert!(!obj.contains_key("is_test_path"));
773    }
774
775    #[test]
776    fn hotspot_actions_default_pair_when_ownership_absent() {
777        let entry = sample_entry("/root/src/api.ts");
778        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
779        assert_eq!(finding.actions.len(), 2);
780        assert_eq!(finding.actions[0].kind, HotspotActionType::RefactorFile);
781        assert_eq!(finding.actions[1].kind, HotspotActionType::AddTests);
782        assert!(finding.actions[0].description.contains("src/api.ts"));
783    }
784
785    #[test]
786    fn hotspot_low_bus_factor_with_suggested_reviewers_lists_them() {
787        let mut entry = sample_entry("/root/src/api.ts");
788        entry.ownership = Some(OwnershipMetrics {
789            bus_factor: 1,
790            contributor_count: 1,
791            top_contributor: contributor("alice", 30),
792            recent_contributors: Vec::new(),
793            suggested_reviewers: vec![contributor("bob", 4), contributor("carol", 2)],
794            declared_owner: None,
795            unowned: None,
796            ownership_state: OwnershipState::Active,
797            drift: false,
798            drift_reason: None,
799        });
800        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
801        let low_bus = finding
802            .actions
803            .iter()
804            .find(|a| a.kind == HotspotActionType::LowBusFactor)
805            .expect("low-bus-factor action present");
806        assert_eq!(
807            low_bus.note.as_deref(),
808            Some("Candidate reviewers: @bob, @carol"),
809        );
810    }
811
812    #[test]
813    fn hotspot_low_bus_factor_softens_for_low_commit_files() {
814        let mut entry = sample_entry("/root/src/api.ts");
815        entry.ownership = Some(OwnershipMetrics {
816            bus_factor: 1,
817            contributor_count: 1,
818            top_contributor: contributor("alice", 3),
819            recent_contributors: Vec::new(),
820            suggested_reviewers: Vec::new(),
821            declared_owner: None,
822            unowned: None,
823            ownership_state: OwnershipState::Active,
824            drift: false,
825            drift_reason: None,
826        });
827        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
828        let low_bus = finding
829            .actions
830            .iter()
831            .find(|a| a.kind == HotspotActionType::LowBusFactor)
832            .expect("low-bus-factor action present");
833        assert_eq!(
834            low_bus.note.as_deref(),
835            Some(
836                "Single recent contributor on a low-commit file. Consider a pair review for major changes.",
837            ),
838        );
839    }
840
841    #[test]
842    fn hotspot_low_bus_factor_omits_note_for_high_commit_no_reviewers() {
843        let mut entry = sample_entry("/root/src/api.ts");
844        entry.ownership = Some(OwnershipMetrics {
845            bus_factor: 1,
846            contributor_count: 1,
847            top_contributor: contributor("alice", 50),
848            recent_contributors: Vec::new(),
849            suggested_reviewers: Vec::new(),
850            declared_owner: None,
851            unowned: None,
852            ownership_state: OwnershipState::Active,
853            drift: false,
854            drift_reason: None,
855        });
856        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
857        let low_bus = finding
858            .actions
859            .iter()
860            .find(|a| a.kind == HotspotActionType::LowBusFactor)
861            .expect("low-bus-factor action present");
862        assert!(low_bus.note.is_none());
863    }
864
865    #[test]
866    fn hotspot_unowned_action_carries_deepest_directory_pattern() {
867        let mut entry = sample_entry("/root/src/api/users/handlers.ts");
868        entry.ownership = Some(OwnershipMetrics {
869            bus_factor: 2,
870            contributor_count: 3,
871            top_contributor: contributor("alice", 10),
872            recent_contributors: Vec::new(),
873            suggested_reviewers: Vec::new(),
874            declared_owner: None,
875            unowned: Some(true),
876            ownership_state: OwnershipState::Unowned,
877            drift: false,
878            drift_reason: None,
879        });
880        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
881        let unowned = finding
882            .actions
883            .iter()
884            .find(|a| a.kind == HotspotActionType::UnownedHotspot)
885            .expect("unowned-hotspot action present");
886        assert_eq!(
887            unowned.suggested_pattern.as_deref(),
888            Some("/src/api/users/")
889        );
890        assert_eq!(
891            unowned.heuristic,
892            Some(HotspotActionHeuristic::DirectoryDeepest)
893        );
894    }
895
896    #[test]
897    fn hotspot_action_descriptions_normalise_windows_separators() {
898        let mut entry = sample_entry("src\\api\\users.ts");
899        entry.ownership = Some(OwnershipMetrics {
900            bus_factor: 2,
901            contributor_count: 3,
902            top_contributor: contributor("alice", 10),
903            recent_contributors: Vec::new(),
904            suggested_reviewers: Vec::new(),
905            declared_owner: None,
906            unowned: Some(true),
907            ownership_state: OwnershipState::Unowned,
908            drift: false,
909            drift_reason: None,
910        });
911        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
912        let refactor = finding
913            .actions
914            .iter()
915            .find(|a| a.kind == HotspotActionType::RefactorFile)
916            .expect("refactor-file action present");
917        assert!(refactor.description.contains("src/api/users.ts"));
918        assert!(!refactor.description.contains('\\'));
919        let unowned = finding
920            .actions
921            .iter()
922            .find(|a| a.kind == HotspotActionType::UnownedHotspot)
923            .expect("unowned-hotspot action present");
924        assert_eq!(unowned.suggested_pattern.as_deref(), Some("/src/api/"));
925    }
926
927    #[test]
928    fn hotspot_drift_action_uses_provided_reason() {
929        let mut entry = sample_entry("/root/src/api.ts");
930        entry.ownership = Some(OwnershipMetrics {
931            bus_factor: 2,
932            contributor_count: 4,
933            top_contributor: contributor("alice", 10),
934            recent_contributors: Vec::new(),
935            suggested_reviewers: Vec::new(),
936            declared_owner: None,
937            unowned: Some(false),
938            ownership_state: OwnershipState::Drifting,
939            drift: true,
940            drift_reason: Some("top contributor changed in last 6 months".to_string()),
941        });
942        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
943        let drift = finding
944            .actions
945            .iter()
946            .find(|a| a.kind == HotspotActionType::OwnershipDrift)
947            .expect("ownership-drift action present");
948        assert!(
949            drift
950                .description
951                .contains("top contributor changed in last 6 months"),
952        );
953    }
954
955    #[test]
956    fn refactoring_target_finding_flattens_inner_fields_at_top_level() {
957        let target = sample_target();
958        let finding = RefactoringTargetFinding::with_actions(target);
959        let json =
960            serde_json::to_value(&finding).expect("refactoring target finding should serialize");
961        let obj = json
962            .as_object()
963            .expect("refactoring target finding should serialize as object");
964        assert!(obj.contains_key("priority"));
965        assert!(obj.contains_key("efficiency"));
966        assert!(obj.contains_key("recommendation"));
967        assert!(obj.contains_key("category"));
968        assert!(obj.contains_key("actions"));
969        assert!(!obj.contains_key("factors"));
970        assert!(!obj.contains_key("evidence"));
971    }
972
973    #[test]
974    fn refactoring_target_actions_default_to_apply_only_without_evidence() {
975        let target = sample_target();
976        let finding = RefactoringTargetFinding::with_actions(target);
977        assert_eq!(finding.actions.len(), 1);
978        assert_eq!(
979            finding.actions[0].kind,
980            RefactoringTargetActionType::ApplyRefactoring,
981        );
982        assert_eq!(
983            finding.actions[0].category.as_deref(),
984            Some("extract_complex_functions"),
985        );
986        assert_eq!(
987            finding.actions[0].description,
988            "Extract `handleRequest` into helpers",
989        );
990    }
991
992    #[test]
993    fn refactoring_target_actions_append_suppress_when_evidence_present() {
994        let mut target = sample_target();
995        target.evidence = Some(TargetEvidence {
996            unused_exports: Vec::new(),
997            complex_functions: vec![EvidenceFunction {
998                name: "handleRequest".to_string(),
999                line: 12,
1000                cognitive: 30,
1001            }],
1002            cycle_path: Vec::new(),
1003            ..Default::default()
1004        });
1005        let finding = RefactoringTargetFinding::with_actions(target);
1006        assert_eq!(finding.actions.len(), 2);
1007        assert_eq!(
1008            finding.actions[1].kind,
1009            RefactoringTargetActionType::SuppressLine,
1010        );
1011        assert_eq!(
1012            finding.actions[1].comment.as_deref(),
1013            Some("// fallow-ignore-next-line complexity"),
1014        );
1015    }
1016
1017    #[test]
1018    fn codeowners_pattern_uses_deepest_directory() {
1019        assert_eq!(
1020            suggest_codeowners_pattern("src/api/users/handlers.ts"),
1021            "/src/api/users/",
1022        );
1023    }
1024
1025    #[test]
1026    fn codeowners_pattern_for_root_file() {
1027        assert_eq!(suggest_codeowners_pattern("README.md"), "/README.md");
1028    }
1029
1030    #[test]
1031    fn codeowners_pattern_normalizes_backslashes() {
1032        assert_eq!(
1033            suggest_codeowners_pattern("src\\api\\users.ts"),
1034            "/src/api/",
1035        );
1036    }
1037
1038    #[test]
1039    fn codeowners_pattern_two_level_path() {
1040        assert_eq!(suggest_codeowners_pattern("src/foo.ts"), "/src/");
1041    }
1042
1043    #[test]
1044    fn recommendation_category_snake_case_round_trips_through_serde() {
1045        let variants = [
1046            RecommendationCategory::UrgentChurnComplexity,
1047            RecommendationCategory::BreakCircularDependency,
1048            RecommendationCategory::SplitHighImpact,
1049            RecommendationCategory::RemoveDeadCode,
1050            RecommendationCategory::ExtractComplexFunctions,
1051            RecommendationCategory::ExtractDependencies,
1052            RecommendationCategory::AddTestCoverage,
1053        ];
1054        for cat in &variants {
1055            let via_serde = serde_json::to_value(cat).expect("category should serialize");
1056            let serde_str = via_serde
1057                .as_str()
1058                .expect("category should serialize as string");
1059            assert_eq!(
1060                serde_str,
1061                category_snake_case(cat),
1062                "category_snake_case for {cat:?} drifted from serde rename_all",
1063            );
1064        }
1065    }
1066}