Skip to main content

fallow_cli/health_types/
finding.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::health_types::scores::{
14    ComplexityViolation, CoverageTier, HotspotEntry, OwnershipState,
15};
16use crate::health_types::targets::{RecommendationCategory, RefactoringTarget};
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, crate::health_types::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: &crate::health_types::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(
501    ownership: &crate::health_types::OwnershipMetrics,
502    path: &str,
503) -> HotspotAction {
504    let top = &ownership.top_contributor;
505    let owner = top.identifier.as_str();
506    HotspotAction {
507        kind: HotspotActionType::LowBusFactor,
508        auto_fixable: false,
509        description: format!(
510            "{owner} is the sole recent contributor to `{path}`; adding a second reviewer reduces knowledge-loss risk"
511        ),
512        note: low_bus_factor_note(ownership),
513        suggested_pattern: None,
514        heuristic: None,
515    }
516}
517
518fn low_bus_factor_note(ownership: &crate::health_types::OwnershipMetrics) -> Option<String> {
519    let suggested: Vec<&str> = ownership
520        .suggested_reviewers
521        .iter()
522        .map(|r| r.identifier.as_str())
523        .collect();
524    if suggested.is_empty() {
525        return (ownership.top_contributor.commits < 5).then(|| {
526            "Single recent contributor on a low-commit file. Consider a pair review for major changes."
527                .to_string()
528        });
529    }
530
531    let list = suggested
532        .iter()
533        .map(|s| format!("@{s}"))
534        .collect::<Vec<_>>()
535        .join(", ");
536    Some(format!("Candidate reviewers: {list}"))
537}
538
539fn unowned_hotspot_action(path: &str) -> HotspotAction {
540    HotspotAction {
541        kind: HotspotActionType::UnownedHotspot,
542        auto_fixable: false,
543        description: format!("Add a CODEOWNERS entry for `{path}`"),
544        note: Some(
545            "Frequently-changed files without declared owners create review bottlenecks"
546                .to_string(),
547        ),
548        suggested_pattern: Some(suggest_codeowners_pattern(path)),
549        heuristic: Some(HotspotActionHeuristic::DirectoryDeepest),
550    }
551}
552
553fn ownership_drift_action(
554    ownership: &crate::health_types::OwnershipMetrics,
555    path: &str,
556) -> HotspotAction {
557    let reason = ownership
558        .drift_reason
559        .as_deref()
560        .unwrap_or("ownership has shifted from the original author");
561    HotspotAction {
562        kind: HotspotActionType::OwnershipDrift,
563        auto_fixable: false,
564        description: format!("Update CODEOWNERS for `{path}`: {reason}"),
565        note: Some(
566            "Drift suggests the declared or original owner is no longer the right reviewer"
567                .to_string(),
568        ),
569        suggested_pattern: None,
570        heuristic: None,
571    }
572}
573
574/// Suggest a CODEOWNERS pattern for an unowned hotspot.
575///
576/// Picks the deepest directory containing the file
577/// (e.g. `src/api/users/handlers.ts` -> `/src/api/users/`) so agents can
578/// paste a tightly-scoped default. Earlier versions used the first two
579/// directory levels but that catches too many siblings in monorepos
580/// (`/src/api/` could span 200 files across 8 sub-domains). The deepest
581/// directory keeps the suggestion reviewable while still being a directory
582/// pattern rather than a per-file rule.
583///
584/// The action emits this alongside
585/// [`HotspotActionHeuristic::DirectoryDeepest`] so consumers can branch
586/// on the strategy if it evolves.
587fn suggest_codeowners_pattern(path: &str) -> String {
588    let normalized = path.replace('\\', "/");
589    let trimmed = normalized.trim_start_matches('/');
590    let mut components: Vec<&str> = trimmed.split('/').collect();
591    components.pop(); // drop the file itself
592    if components.is_empty() {
593        return format!("/{trimmed}");
594    }
595    format!("/{}/", components.join("/"))
596}
597
598/// Wire envelope for a single refactoring target.
599///
600/// Flattens [`RefactoringTarget`] for wire continuity and adds the typed
601/// `actions` list. The `#[serde(flatten)]` keeps each `targets[]` item
602/// byte-identical to the pre-wrapper shape: inner fields (`path`,
603/// `priority`, `efficiency`, `recommendation`, `category`, ...) sit at
604/// the top level alongside `actions`. Optional inner fields (`factors`,
605/// `evidence`) keep their original `skip_serializing_if` behaviour.
606///
607/// Construct via [`RefactoringTargetFinding::with_actions`] in the
608/// typical health pipeline or via [`RefactoringTargetFinding::from`] for
609/// fixture and test code.
610#[derive(Debug, Clone, serde::Serialize)]
611#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
612pub struct RefactoringTargetFinding {
613    /// Inner refactoring target payload. Flattened on the wire.
614    #[serde(flatten)]
615    pub target: RefactoringTarget,
616    /// Machine-actionable refactoring and suppression hints. Always
617    /// populated; the list never empties because the action selector
618    /// unconditionally emits `apply-refactoring`. A trailing
619    /// `suppress-line` is appended only when the target carries
620    /// [`RefactoringTarget::evidence`] linking to specific functions.
621    pub actions: Vec<RefactoringTargetAction>,
622}
623
624impl Deref for RefactoringTargetFinding {
625    type Target = RefactoringTarget;
626
627    fn deref(&self) -> &Self::Target {
628        &self.target
629    }
630}
631
632impl From<RefactoringTarget> for RefactoringTargetFinding {
633    /// Convenience conversion: wrap a refactoring target with an empty
634    /// `actions` list. Used by tests and fixture builders. Production
635    /// code should call [`RefactoringTargetFinding::with_actions`] so
636    /// the wire shape carries the typed actions.
637    fn from(target: RefactoringTarget) -> Self {
638        Self {
639            target,
640            actions: Vec::new(),
641        }
642    }
643}
644
645impl RefactoringTargetFinding {
646    /// Construct a wrapper with the `actions` list computed from the
647    /// target's `recommendation`, `category`, and optional `evidence`.
648    ///
649    /// Asymmetry with [`HotspotFinding::with_actions`]: this constructor
650    /// does NOT take a `root: &Path` because refactoring-target action
651    /// descriptions never interpolate the file path; they pass
652    /// [`RefactoringTarget::recommendation`] verbatim into the
653    /// `apply-refactoring` action. The [`RefactoringTarget::category`]
654    /// field flows into the action's `category` field as the serde
655    /// snake-case form.
656    #[must_use]
657    pub fn with_actions(target: RefactoringTarget) -> Self {
658        let actions = build_refactoring_target_actions(&target);
659        Self { target, actions }
660    }
661}
662
663/// Compute the typed `actions` list for a refactoring target.
664///
665/// The list always begins with `apply-refactoring`. A trailing
666/// `suppress-line` is appended only when the target carries
667/// [`RefactoringTarget::evidence`] linking to specific functions.
668fn build_refactoring_target_actions(target: &RefactoringTarget) -> Vec<RefactoringTargetAction> {
669    let mut actions = vec![RefactoringTargetAction {
670        kind: RefactoringTargetActionType::ApplyRefactoring,
671        auto_fixable: false,
672        description: target.recommendation.clone(),
673        category: Some(category_snake_case(&target.category).to_string()),
674        comment: None,
675    }];
676
677    if target.evidence.is_some() {
678        actions.push(RefactoringTargetAction {
679            kind: RefactoringTargetActionType::SuppressLine,
680            auto_fixable: false,
681            description: "Suppress the underlying complexity finding".to_string(),
682            category: None,
683            comment: Some("// fallow-ignore-next-line complexity".to_string()),
684        });
685    }
686
687    actions
688}
689
690/// Serde-rename_all-snake_case form of a [`RecommendationCategory`]
691/// variant.
692///
693/// `RefactoringTargetAction.category` is `Option<String>` carrying the
694/// serde-encoded form of [`RecommendationCategory`]. The JSON post-pass
695/// retired by issue #408 read this string from the serialized JSON
696/// value; the typed action builder needs the same form without paying
697/// for a serde round-trip per target. The
698/// `recommendation_category_snake_case_round_trips` test in this module
699/// asserts every variant matches `serde_json::to_value` byte-for-byte,
700/// so silent drift between this function and the
701/// `#[serde(rename_all = "snake_case")]` attribute is caught at test
702/// time.
703const fn category_snake_case(cat: &RecommendationCategory) -> &'static str {
704    match cat {
705        RecommendationCategory::UrgentChurnComplexity => "urgent_churn_complexity",
706        RecommendationCategory::BreakCircularDependency => "break_circular_dependency",
707        RecommendationCategory::SplitHighImpact => "split_high_impact",
708        RecommendationCategory::RemoveDeadCode => "remove_dead_code",
709        RecommendationCategory::ExtractComplexFunctions => "extract_complex_functions",
710        RecommendationCategory::ExtractDependencies => "extract_dependencies",
711        RecommendationCategory::AddTestCoverage => "add_test_coverage",
712    }
713}
714
715#[cfg(test)]
716mod hotspot_target_tests {
717    use super::*;
718    use crate::health_types::scores::{
719        ContributorEntry, ContributorIdentifierFormat, OwnershipMetrics, OwnershipState,
720    };
721    use fallow_core::churn::ChurnTrend;
722    use std::path::PathBuf;
723
724    fn sample_entry(path: &str) -> HotspotEntry {
725        HotspotEntry {
726            path: PathBuf::from(path),
727            score: 80.0,
728            commits: 12,
729            weighted_commits: 8.0,
730            lines_added: 100,
731            lines_deleted: 40,
732            complexity_density: 1.5,
733            fan_in: 3,
734            trend: ChurnTrend::Stable,
735            ownership: None,
736            is_test_path: false,
737        }
738    }
739
740    fn contributor(identifier: &str, commits: u32) -> ContributorEntry {
741        ContributorEntry {
742            identifier: identifier.to_string(),
743            format: ContributorIdentifierFormat::Handle,
744            share: 1.0,
745            stale_days: 1,
746            commits,
747        }
748    }
749
750    fn sample_target() -> RefactoringTarget {
751        RefactoringTarget {
752            path: PathBuf::from("/root/src/foo.ts"),
753            priority: 75.0,
754            efficiency: 75.0,
755            recommendation: "Extract `handleRequest` into helpers".to_string(),
756            category: RecommendationCategory::ExtractComplexFunctions,
757            effort: crate::health_types::EffortEstimate::Low,
758            confidence: crate::health_types::Confidence::High,
759            factors: Vec::new(),
760            evidence: None,
761        }
762    }
763
764    #[test]
765    fn hotspot_finding_flattens_inner_fields_at_top_level() {
766        let entry = sample_entry("/root/src/api.ts");
767        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
768        let json = serde_json::to_value(&finding).unwrap();
769        let obj = json.as_object().unwrap();
770        assert!(obj.contains_key("score"));
771        assert!(obj.contains_key("commits"));
772        assert!(obj.contains_key("weighted_commits"));
773        assert!(obj.contains_key("actions"));
774        assert!(!obj.contains_key("ownership"));
775        assert!(!obj.contains_key("is_test_path"));
776    }
777
778    #[test]
779    fn hotspot_actions_default_pair_when_ownership_absent() {
780        let entry = sample_entry("/root/src/api.ts");
781        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
782        assert_eq!(finding.actions.len(), 2);
783        assert_eq!(finding.actions[0].kind, HotspotActionType::RefactorFile);
784        assert_eq!(finding.actions[1].kind, HotspotActionType::AddTests);
785        assert!(finding.actions[0].description.contains("src/api.ts"));
786    }
787
788    #[test]
789    fn hotspot_low_bus_factor_with_suggested_reviewers_lists_them() {
790        let mut entry = sample_entry("/root/src/api.ts");
791        entry.ownership = Some(OwnershipMetrics {
792            bus_factor: 1,
793            contributor_count: 1,
794            top_contributor: contributor("alice", 30),
795            recent_contributors: Vec::new(),
796            suggested_reviewers: vec![contributor("bob", 4), contributor("carol", 2)],
797            declared_owner: None,
798            unowned: None,
799            ownership_state: OwnershipState::Active,
800            drift: false,
801            drift_reason: None,
802        });
803        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
804        let low_bus = finding
805            .actions
806            .iter()
807            .find(|a| a.kind == HotspotActionType::LowBusFactor)
808            .expect("low-bus-factor action present");
809        assert_eq!(
810            low_bus.note.as_deref(),
811            Some("Candidate reviewers: @bob, @carol"),
812        );
813    }
814
815    #[test]
816    fn hotspot_low_bus_factor_softens_for_low_commit_files() {
817        let mut entry = sample_entry("/root/src/api.ts");
818        entry.ownership = Some(OwnershipMetrics {
819            bus_factor: 1,
820            contributor_count: 1,
821            top_contributor: contributor("alice", 3),
822            recent_contributors: Vec::new(),
823            suggested_reviewers: Vec::new(),
824            declared_owner: None,
825            unowned: None,
826            ownership_state: OwnershipState::Active,
827            drift: false,
828            drift_reason: None,
829        });
830        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
831        let low_bus = finding
832            .actions
833            .iter()
834            .find(|a| a.kind == HotspotActionType::LowBusFactor)
835            .expect("low-bus-factor action present");
836        assert_eq!(
837            low_bus.note.as_deref(),
838            Some(
839                "Single recent contributor on a low-commit file. Consider a pair review for major changes.",
840            ),
841        );
842    }
843
844    #[test]
845    fn hotspot_low_bus_factor_omits_note_for_high_commit_no_reviewers() {
846        let mut entry = sample_entry("/root/src/api.ts");
847        entry.ownership = Some(OwnershipMetrics {
848            bus_factor: 1,
849            contributor_count: 1,
850            top_contributor: contributor("alice", 50),
851            recent_contributors: Vec::new(),
852            suggested_reviewers: Vec::new(),
853            declared_owner: None,
854            unowned: None,
855            ownership_state: OwnershipState::Active,
856            drift: false,
857            drift_reason: None,
858        });
859        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
860        let low_bus = finding
861            .actions
862            .iter()
863            .find(|a| a.kind == HotspotActionType::LowBusFactor)
864            .expect("low-bus-factor action present");
865        assert!(low_bus.note.is_none());
866    }
867
868    #[test]
869    fn hotspot_unowned_action_carries_deepest_directory_pattern() {
870        let mut entry = sample_entry("/root/src/api/users/handlers.ts");
871        entry.ownership = Some(OwnershipMetrics {
872            bus_factor: 2,
873            contributor_count: 3,
874            top_contributor: contributor("alice", 10),
875            recent_contributors: Vec::new(),
876            suggested_reviewers: Vec::new(),
877            declared_owner: None,
878            unowned: Some(true),
879            ownership_state: OwnershipState::Unowned,
880            drift: false,
881            drift_reason: None,
882        });
883        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
884        let unowned = finding
885            .actions
886            .iter()
887            .find(|a| a.kind == HotspotActionType::UnownedHotspot)
888            .expect("unowned-hotspot action present");
889        assert_eq!(
890            unowned.suggested_pattern.as_deref(),
891            Some("/src/api/users/")
892        );
893        assert_eq!(
894            unowned.heuristic,
895            Some(HotspotActionHeuristic::DirectoryDeepest)
896        );
897    }
898
899    #[test]
900    fn hotspot_action_descriptions_normalise_windows_separators() {
901        let mut entry = sample_entry("src\\api\\users.ts");
902        entry.ownership = Some(OwnershipMetrics {
903            bus_factor: 2,
904            contributor_count: 3,
905            top_contributor: contributor("alice", 10),
906            recent_contributors: Vec::new(),
907            suggested_reviewers: Vec::new(),
908            declared_owner: None,
909            unowned: Some(true),
910            ownership_state: OwnershipState::Unowned,
911            drift: false,
912            drift_reason: None,
913        });
914        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
915        let refactor = finding
916            .actions
917            .iter()
918            .find(|a| a.kind == HotspotActionType::RefactorFile)
919            .expect("refactor-file action present");
920        assert!(refactor.description.contains("src/api/users.ts"));
921        assert!(!refactor.description.contains('\\'));
922        let unowned = finding
923            .actions
924            .iter()
925            .find(|a| a.kind == HotspotActionType::UnownedHotspot)
926            .expect("unowned-hotspot action present");
927        assert_eq!(unowned.suggested_pattern.as_deref(), Some("/src/api/"));
928    }
929
930    #[test]
931    fn hotspot_drift_action_uses_provided_reason() {
932        let mut entry = sample_entry("/root/src/api.ts");
933        entry.ownership = Some(OwnershipMetrics {
934            bus_factor: 2,
935            contributor_count: 4,
936            top_contributor: contributor("alice", 10),
937            recent_contributors: Vec::new(),
938            suggested_reviewers: Vec::new(),
939            declared_owner: None,
940            unowned: Some(false),
941            ownership_state: OwnershipState::Drifting,
942            drift: true,
943            drift_reason: Some("top contributor changed in last 6 months".to_string()),
944        });
945        let finding = HotspotFinding::with_actions(entry, Path::new("/root"));
946        let drift = finding
947            .actions
948            .iter()
949            .find(|a| a.kind == HotspotActionType::OwnershipDrift)
950            .expect("ownership-drift action present");
951        assert!(
952            drift
953                .description
954                .contains("top contributor changed in last 6 months"),
955        );
956    }
957
958    #[test]
959    fn refactoring_target_finding_flattens_inner_fields_at_top_level() {
960        let target = sample_target();
961        let finding = RefactoringTargetFinding::with_actions(target);
962        let json = serde_json::to_value(&finding).unwrap();
963        let obj = json.as_object().unwrap();
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(crate::health_types::TargetEvidence {
996            unused_exports: Vec::new(),
997            complex_functions: vec![crate::health_types::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).unwrap();
1056            let serde_str = via_serde.as_str().unwrap();
1057            assert_eq!(
1058                serde_str,
1059                category_snake_case(cat),
1060                "category_snake_case for {cat:?} drifted from serde rename_all",
1061            );
1062        }
1063    }
1064}