Skip to main content

fallow_api/
decision_surface.rs

1//! Decision-surface extractor (stage 6 / 6.G): THE product.
2//!
3//! The apex of the review brief. A change embeds many decisions; almost all are
4//! mechanical and a few are consequential enough to need human taste. This
5//! extractor lifts the consequential STRUCTURAL decisions out of the scattered
6//! diff, frames each as a judgment question, ranks by consequence (blast x
7//! reversibility), caps the surface to a working-memory-sized handful (4 plus or
8//! minus 1), collapses the mechanical remainder, and pairs each decision with the
9//! routed expert ("who to ask").
10//!
11//! ## The SOLID-3 (the ONLY categories that ship)
12//!
13//! Per the verdict (`.plans/agentic-review-e0-verdict.md`) the decision
14//! categories are NOT uniformly reliable on a syntactic engine (ADR-001). Exactly
15//! three are validated and shippable, each backed by a deterministic signal
16//! fallow already emits:
17//!
18//! 1. **coupling/boundary** (`boundary_introduced`): a new cross-zone edge.
19//! 2. **public-API/contract** (`public_api_added` + coordination gaps): a
20//!    new exports-aware public surface, or a changed contract consumed by modules
21//!    outside the diff.
22//! 3. **dependency**: a changed `package.json` that adds third-party entries or
23//!    moves one across a major version (`dependency_added` /
24//!    `dependency_major_bumped`), batch-consolidated per manifest per kind and
25//!    weighted by the graph's in-repo importers of the affected packages.
26//!
27//! The four CUT categories (abstraction-with-1-implementor, deletion-still-
28//! reachable, convention-divergence, irreversibility/migration) are CONFIRMED
29//! NOISE and MUST NOT ship. `DecisionCategory` has exactly three discriminants,
30//! so a cut category is not even representable: the type system is the guarantee.
31//!
32//! ## The trust mechanism (anti-hallucination)
33//!
34//! Post-validation closes on EXTRACTION, not on framing. Every decision carries a
35//! `signal_id` deterministically derived from the fallow-emitted candidate key it
36//! frames (a delta key or a coordination-gap key). The deterministic layer keeps
37//! the SET of signal_ids it emitted; `DecisionSurface::accept_signal_id` returns
38//! true iff an id is in that set. An agent-proposed decision whose `signal_id` was
39//! never emitted is REJECTED. The agent proposes; the graph disposes.
40
41pub use fallow_output::{
42    Decision, DecisionCategory, DecisionSurface, TruncationNote, build_decision_surface_output,
43};
44use xxhash_rust::xxh3::xxh3_64;
45
46use fallow_output::{ReviewDeltas, RoutingFacts};
47
48/// Default decision-surface cap (the working-memory limit). The surface holds at
49/// most this many ranked decisions; the rest collapse into a truncation note.
50pub const DEFAULT_DECISION_CAP: usize = 4;
51/// Lower bound on the configurable cap (4 minus 1).
52pub const MIN_DECISION_CAP: usize = 3;
53/// Upper bound on the configurable cap (4 plus 1).
54pub const MAX_DECISION_CAP: usize = 5;
55
56/// Derive a deterministic, content-addressed `signal_id` from a category tag plus
57/// the fallow-emitted candidate key. The tag namespaces the key so a boundary key
58/// and a public-API key sharing text never collide. Pure: same inputs always
59/// yield the same id (byte-identical across runs).
60#[must_use]
61pub fn derive_signal_id(category: DecisionCategory, candidate_key: &str) -> String {
62    let mut bytes = Vec::with_capacity(category.tag().len() + 1 + candidate_key.len());
63    bytes.extend_from_slice(category.tag().as_bytes());
64    bytes.push(0);
65    bytes.extend_from_slice(candidate_key.as_bytes());
66    format!("sig:{:016x}", xxh3_64(&bytes))
67}
68
69/// A representative boundary violation used to anchor a coupling/boundary
70/// decision to a file + line. Decoupled from the `fallow_types` finding type so
71/// the extractor unit-tests without constructing full findings.
72#[derive(Debug, Clone)]
73pub struct BoundaryAnchor {
74    /// The R2 zone-pair key (`"<from_zone>->-<to_zone>"`), matching
75    /// `ReviewDeltas::boundary_introduced`.
76    pub zone_pair_key: String,
77    /// Root-relative path of the importing file (the decision anchor).
78    pub from_file: String,
79    /// The `from_zone` of the edge (for the framed question).
80    pub from_zone: String,
81    /// The `to_zone` of the edge (for the framed question).
82    pub to_zone: String,
83    /// 1-based line of the offending import (the suppression anchor).
84    pub line: u32,
85}
86
87/// A coordination gap projected onto the public-API/contract decision shape: a
88/// changed contract consumed by a module outside the diff.
89#[derive(Debug, Clone)]
90pub struct CoordinationAnchor {
91    /// Root-relative path of the changed file whose contract is consumed elsewhere.
92    pub changed_file: String,
93    /// The consumed symbol names (the contract).
94    pub consumed_symbols: Vec<String>,
95    /// Count of distinct non-diff consumers of this changed file's contract.
96    pub consumer_count: u64,
97    /// 1-based line of the contract symbol's declaration in `changed_file`, so the
98    /// decision deep-links / inline-anchors to the exact export. `0` when the line
99    /// could not be resolved (graph not retained or file unreadable).
100    pub line: u32,
101}
102
103/// Which manifest change a dependency decision frames.
104#[derive(Debug, Clone, Copy, PartialEq, Eq)]
105pub enum DependencyChangeKind {
106    /// Entries the base manifest did not declare.
107    Added,
108    /// Entries whose range crossed a major version.
109    MajorBump,
110}
111
112/// One declared dependency entry inside a dependency decision.
113#[derive(Debug, Clone, PartialEq, Eq)]
114pub struct DependencyEntry {
115    /// The package name.
116    pub name: String,
117    /// The manifest section the entry lives in at head (`dependencies`,
118    /// `devDependencies`, `optionalDependencies`, `peerDependencies`).
119    pub section: String,
120    /// The base range; `None` for an added entry.
121    pub from: Option<String>,
122    /// The head range.
123    pub to: String,
124}
125
126/// A changed manifest's dependency candidates of one kind, batch-consolidated
127/// per manifest (rule R1): a reviewer reads "3 new dependencies", never one
128/// decision per package. The importer counts come from the graph's package
129/// usage, so the decision carries the modules the change actually reaches.
130#[derive(Debug, Clone, PartialEq, Eq)]
131pub struct DependencyAnchor {
132    /// Root-relative path of the `package.json` (the decision anchor).
133    pub manifest: String,
134    /// Added entries or major bumps.
135    pub kind: DependencyChangeKind,
136    /// The entries, name-sorted.
137    pub entries: Vec<DependencyEntry>,
138    /// In-repo modules importing any of the entries (the blast).
139    pub importers: u64,
140    /// The subset of `importers` outside the diff (the display number).
141    pub out_of_diff_importers: u64,
142    /// 1-based line of the first entry in the head manifest, `0` when unresolved.
143    pub line: u32,
144}
145
146/// All inputs the extractor needs, gathered from the assembled brief data.
147pub struct DecisionInputs<'a> {
148    /// Diff-aware deltas (boundary + public-API). The candidate source.
149    pub deltas: &'a ReviewDeltas,
150    /// Boundary anchors keyed by zone-pair, one representative per introduced edge.
151    pub boundary_anchors: &'a [BoundaryAnchor],
152    /// Coordination gaps projected to the contract decision shape.
153    pub coordination: &'a [CoordinationAnchor],
154    /// Dependency candidates per changed manifest, one per kind.
155    pub dependency_anchors: &'a [DependencyAnchor],
156    /// 1-based line of the first widened public-API export's declaration, so the
157    /// public-API-surface decision anchors to a real line. `0` when unresolved.
158    pub public_api_anchor_line: u32,
159    /// Project-wide fan-in beyond the diff (impact-closure `affected_not_shown`).
160    /// Used as the blast magnitude for boundary + public-API-surface decisions.
161    pub affected_not_shown: u64,
162    /// Ownership routing (routed expert per file).
163    pub routing: &'a RoutingFacts,
164    /// Per-anchor-file head source, for suppression checks. `None` for a file
165    /// whose head content could not be read (the decision is then not suppressed).
166    pub head_source: &'a dyn Fn(&str) -> Option<String>,
167    /// Resolve a head (post-rename) root-relative path to its pre-rename path, from
168    /// the diff's rename pairs. `None` when the file was not renamed. Lets each
169    /// decision carry a `previous_signal_id` so review memory survives a `git mv`.
170    pub rename_old_path: &'a dyn Fn(&str) -> Option<String>,
171    /// Honest per-anchor in-repo out-of-diff consumer count, precomputed from the
172    /// retained graph's reverse-deps before it was dropped. `0` for an anchor with
173    /// no recorded importers (a genuinely new file). The display number; distinct
174    /// from `affected_not_shown` (the project-wide ranking proxy).
175    pub internal_consumers: &'a dyn Fn(&str) -> u64,
176    /// The decision cap (default 4, clamped to [3, 5] by the caller).
177    pub cap: usize,
178}
179
180/// Resolve the routed expert(s) + bus-factor flag for a decision's anchor file.
181fn route_for(routing: &RoutingFacts, anchor_file: &str) -> (Vec<String>, bool) {
182    routing
183        .units
184        .iter()
185        .find(|unit| unit.file == anchor_file)
186        .map_or((Vec::new(), false), |unit| {
187            (unit.expert.clone(), unit.bus_factor_one)
188        })
189}
190
191/// Whether the head source of `anchor_file` suppresses a decision of `category`
192/// at (1-based) `line`. Honors a file-level `fallow-ignore-file` and a
193/// line-level `fallow-ignore-next-line` immediately above the anchor line, in
194/// both the category-scoped (`decision-surface` / category tag) and bare forms.
195fn is_decision_suppressed(
196    head_source: Option<&str>,
197    category: DecisionCategory,
198    line: u32,
199) -> bool {
200    let Some(source) = head_source else {
201        return false;
202    };
203    let lines: Vec<&str> = source.lines().collect();
204    let token_matches = |comment: &str| {
205        if !comment.contains("fallow-ignore") {
206            return false;
207        }
208        // A bare ignore (no kind) suppresses; a kinded ignore must name the
209        // decision-surface family or this decision's category tag.
210        let after = comment
211            .split_once("fallow-ignore-file")
212            .or_else(|| comment.split_once("fallow-ignore-next-line"))
213            .map(|(_, rest)| rest.trim());
214        match after {
215            None => false,
216            Some("") => true,
217            Some(rest) => {
218                rest.contains("decision-surface")
219                    || rest.contains("decision-surfaces")
220                    || rest.contains(category.tag())
221            }
222        }
223    };
224
225    // File-level: any line carrying a file-level ignore.
226    if lines
227        .iter()
228        .any(|l| l.contains("fallow-ignore-file") && token_matches(l))
229    {
230        return true;
231    }
232    // Line-level: the comment sits immediately above the 1-based anchor line.
233    if line >= 2
234        && let Some(prev) = lines.get((line - 2) as usize)
235        && prev.contains("fallow-ignore-next-line")
236        && token_matches(prev)
237    {
238        return true;
239    }
240    false
241}
242
243/// Frame a coupling/boundary decision as a judgment question.
244fn boundary_question(from_zone: &str, to_zone: &str) -> String {
245    format!(
246        "`{from_zone}` now imports `{to_zone}` for the first time. Intended coupling, or should this edge not exist?"
247    )
248}
249
250/// Frame the (batch-consolidated, R1) public-API-surface decision.
251fn public_api_question(count: usize) -> String {
252    format!(
253        "This change adds {count} export{} to the public API surface. Intended as maintained contracts, or should they stay internal?",
254        if count == 1 { "" } else { "s" }
255    )
256}
257
258/// Frame a coordination-gap (contract consumed outside the diff) decision.
259fn coordination_question(changed_file: &str, symbols: &[String], consumers: u64) -> String {
260    format!(
261        "`{changed_file}` changes {} ({}) imported by {consumers} {} outside this PR. Does this change break or alter what those callers expect?",
262        if symbols.len() == 1 {
263            "export"
264        } else {
265            "exports"
266        },
267        symbols.join(", "),
268        if consumers == 1 { "file" } else { "files" }
269    )
270}
271
272/// Pluralize "module" against a count.
273fn modules_word(n: u64) -> &'static str {
274    if n == 1 { "module" } else { "modules" }
275}
276
277/// Subject-verb agreement for the per-clause count: a singular subject takes the
278/// "-s" verb form ("1 module depends"), plural drops it ("2 modules depend").
279fn agrees(verb_plural: &str, n: u64) -> String {
280    if n == 1 {
281        format!("{verb_plural}s")
282    } else {
283        verb_plural.to_string()
284    }
285}
286
287/// The named structural sacrifice for a coupling/boundary decision, as a FACT.
288/// `consumers` is the honest in-repo out-of-diff count for the anchor.
289fn boundary_tradeoff(from_zone: &str, to_zone: &str, consumers: u64) -> String {
290    format!(
291        "Couples `{from_zone}` to `{to_zone}`; {consumers} in-repo {} already {} on this anchor.",
292        modules_word(consumers),
293        agrees("depend", consumers)
294    )
295}
296
297/// The named structural sacrifice for the public-API-surface decision, as a FACT.
298/// The internal count is internal-only, so the clause also names the external
299/// contract risk in prose (it cannot count a published library's downstream).
300fn public_api_tradeoff(count: usize, consumers: u64) -> String {
301    format!(
302        "Adds {count} maintained contract{}; {consumers} in-repo {} already {} this surface, and any external consumers become a contract you cannot remove without a breaking change.",
303        if count == 1 { "" } else { "s" },
304        modules_word(consumers),
305        agrees("consume", consumers)
306    )
307}
308
309/// The named structural sacrifice for a coordination-gap decision, as a FACT.
310fn coordination_tradeoff(consumers: u64) -> String {
311    format!(
312        "{consumers} {} outside the diff {} this contract; changing its shape requires coordinating them.",
313        modules_word(consumers),
314        agrees("consume", consumers)
315    )
316}
317
318/// The per-decision fields for [`build_decision`], distinct from the shared
319/// run context carried in [`DecisionInputs`].
320struct DecisionSpec {
321    category: DecisionCategory,
322    candidate_key: String,
323    question: String,
324    anchor_file: String,
325    anchor_line: u32,
326    blast: u64,
327    /// Honest per-decision in-repo out-of-diff consumer count (display number).
328    internal_consumer_count: u64,
329    /// The named-sacrifice clause, stated as a fact.
330    tradeoff: String,
331    /// Per-decision override of the category's reversibility weight. A
332    /// dependency ADDED is a permanent new surface (the category weight); a
333    /// MAJOR BUMP reverts with two files and a lockfile, so it ranks with a
334    /// public-API change rather than above it.
335    reversibility_weight: Option<u64>,
336}
337
338/// The reversibility weight a major bump carries: the public-API weight, not
339/// the added-dependency weight.
340const MAJOR_BUMP_REVERSIBILITY_WEIGHT: u64 = 3;
341
342/// Build one decision, resolving its routed expert and suppression state.
343fn build_decision(spec: DecisionSpec, inputs: &DecisionInputs<'_>) -> Decision {
344    let DecisionSpec {
345        category,
346        candidate_key,
347        question,
348        anchor_file,
349        anchor_line,
350        blast,
351        internal_consumer_count,
352        reversibility_weight,
353        tradeoff,
354    } = spec;
355    let signal_id = derive_signal_id(category, &candidate_key);
356    // Rename-durable review memory: if any path embedded in the candidate key was
357    // renamed, derive the signal_id this decision WOULD have had under the old
358    // path so the cloud can carry a prior dismissal across the move.
359    let previous_signal_id = remap_key_paths(&candidate_key, inputs.rename_old_path)
360        .map(|old_key| derive_signal_id(category, &old_key));
361    let (expert, bus_factor_one) = route_for(inputs.routing, &anchor_file);
362    let consequence = blast
363        .saturating_mul(reversibility_weight.unwrap_or_else(|| category.reversibility_weight()));
364    Decision {
365        signal_id,
366        category,
367        question,
368        anchor_file,
369        anchor_line,
370        signal_key: candidate_key,
371        previous_signal_id,
372        blast,
373        consequence,
374        expert,
375        bus_factor_one,
376        internal_consumer_count,
377        tradeoff,
378    }
379}
380
381/// Rebuild a candidate key with every embedded rel path swapped to its pre-rename
382/// form via `rename_old_path`. The key embeds paths as `contract:<path>` or as
383/// `|`-joined `<path>::<name>` components (boundary zone-pair keys carry no path).
384/// Returns the rebuilt, re-sorted key iff at least one path moved, else `None`.
385fn remap_key_paths(key: &str, rename_old_path: &dyn Fn(&str) -> Option<String>) -> Option<String> {
386    let mut moved = false;
387    let mut parts: Vec<String> = key
388        .split('|')
389        .map(|segment| {
390            if let Some(path) = segment.strip_prefix("contract:")
391                && let Some(old) = rename_old_path(path)
392            {
393                moved = true;
394                return format!("contract:{old}");
395            } else if let Some((path, name)) = segment.split_once("::")
396                && let Some(old) = rename_old_path(path)
397            {
398                moved = true;
399                return format!("{old}::{name}");
400            }
401            segment.to_string()
402        })
403        .collect();
404    if !moved {
405        return None;
406    }
407    // The public-API key is the SORTED added-key set joined; re-sort so the rebuilt
408    // key matches what the pre-rename change would have emitted.
409    parts.sort();
410    Some(parts.join("|"))
411}
412
413/// Classify the candidate signals into framed decisions (pre-rank, pre-cap).
414fn classify_candidates(inputs: &DecisionInputs<'_>) -> Vec<Decision> {
415    let mut decisions: Vec<Decision> = Vec::new();
416    append_boundary_decisions(&mut decisions, inputs);
417    append_public_api_decision(&mut decisions, inputs);
418    append_coordination_decisions(&mut decisions, inputs);
419    append_dependency_decisions(&mut decisions, inputs);
420    decisions
421}
422
423/// The candidate key for a dependency anchor: the manifest-scoped entry keys,
424/// name-sorted and `|`-joined, so one manifest yields one stable id per kind.
425/// Built from the same per-entry key the brief's `deltas` carry.
426fn dependency_candidate_key(anchor: &DependencyAnchor) -> String {
427    let keys: Vec<String> = anchor
428        .entries
429        .iter()
430        .map(|entry| {
431            crate::dependency_deltas::dependency_delta_key(&anchor.manifest, anchor.kind, entry)
432        })
433        .collect();
434    keys.join("|")
435}
436
437/// A short section tag for anything outside `dependencies`, so a dev tool
438/// never reads as a runtime dependency in the question.
439fn section_tag(section: &str) -> &'static str {
440    match section {
441        "devDependencies" => " (dev)",
442        "optionalDependencies" => " (optional)",
443        "peerDependencies" => " (peer)",
444        _ => "",
445    }
446}
447
448fn dependency_names(anchor: &DependencyAnchor) -> String {
449    anchor
450        .entries
451        .iter()
452        .map(|entry| {
453            let tag = section_tag(&entry.section);
454            match (&anchor.kind, &entry.from) {
455                (DependencyChangeKind::MajorBump, Some(from)) => {
456                    format!("`{}`{tag} {from} -> {}", entry.name, entry.to)
457                }
458                _ => format!("`{}`{tag}", entry.name),
459            }
460        })
461        .collect::<Vec<_>>()
462        .join(", ")
463}
464
465fn dependency_question(anchor: &DependencyAnchor) -> String {
466    let count = anchor.entries.len();
467    let names = dependency_names(anchor);
468    let plural = if count == 1 { "y" } else { "ies" };
469    // Zero importers means the package is reached through the build, a config
470    // file, or types only, so the question points there instead of at modules.
471    let reach = if anchor.importers == 0 {
472        "not imported by any in-repo module".to_string()
473    } else {
474        format!(
475            "imported by {} in-repo {}",
476            anchor.importers,
477            modules_word(anchor.importers)
478        )
479    };
480    match anchor.kind {
481        DependencyChangeKind::Added => format!(
482            "`{}` adds {count} third-party dependenc{plural} ({names}), {reach}. What does each replace, and who owns the new surface?",
483            anchor.manifest,
484        ),
485        DependencyChangeKind::MajorBump if anchor.importers == 0 => format!(
486            "`{}` moves {count} dependenc{plural} across a major version ({names}), {reach}. Which changelog-listed changes reach the build, config, or types?",
487            anchor.manifest,
488        ),
489        DependencyChangeKind::MajorBump => format!(
490            "`{}` moves {count} dependenc{plural} across a major version ({names}), {reach}. Which changelog-listed behavior changes reach those importers?",
491            anchor.manifest,
492        ),
493    }
494}
495
496fn dependency_tradeoff(anchor: &DependencyAnchor) -> String {
497    let count = anchor.entries.len();
498    match anchor.kind {
499        DependencyChangeKind::Added => format!(
500            "Takes on {count} new maintenance and supply-chain surface{}; {} in-repo {} outside this diff already {} the added packages.",
501            if count == 1 { "" } else { "s" },
502            anchor.out_of_diff_importers,
503            modules_word(anchor.out_of_diff_importers),
504            agrees("import", anchor.out_of_diff_importers),
505        ),
506        DependencyChangeKind::MajorBump => format!(
507            "A major bump is a behavior change nobody in this diff wrote; {} in-repo {} outside this diff {} the bumped packages and {} not in the review.",
508            anchor.out_of_diff_importers,
509            modules_word(anchor.out_of_diff_importers),
510            agrees("import", anchor.out_of_diff_importers),
511            if anchor.out_of_diff_importers == 1 {
512                "is"
513            } else {
514                "are"
515            },
516        ),
517    }
518}
519
520fn append_dependency_decisions(decisions: &mut Vec<Decision>, inputs: &DecisionInputs<'_>) {
521    for anchor in inputs.dependency_anchors {
522        if anchor.entries.is_empty() {
523            continue;
524        }
525        decisions.push(build_decision(
526            DecisionSpec {
527                category: DecisionCategory::Dependency,
528                candidate_key: dependency_candidate_key(anchor),
529                question: dependency_question(anchor),
530                tradeoff: dependency_tradeoff(anchor),
531                anchor_file: anchor.manifest.clone(),
532                anchor_line: anchor.line,
533                blast: anchor.importers,
534                internal_consumer_count: anchor.out_of_diff_importers,
535                reversibility_weight: match anchor.kind {
536                    DependencyChangeKind::Added => None,
537                    DependencyChangeKind::MajorBump => Some(MAJOR_BUMP_REVERSIBILITY_WEIGHT),
538                },
539            },
540            inputs,
541        ));
542    }
543}
544
545fn append_boundary_decisions(decisions: &mut Vec<Decision>, inputs: &DecisionInputs<'_>) {
546    for key in &inputs.deltas.boundary_introduced {
547        let anchor = inputs
548            .boundary_anchors
549            .iter()
550            .find(|a| &a.zone_pair_key == key);
551        let (anchor_file, anchor_line, from_zone, to_zone) = anchor.map_or_else(
552            || (String::new(), 0, key.clone(), String::new()),
553            |a| {
554                (
555                    a.from_file.clone(),
556                    a.line,
557                    a.from_zone.clone(),
558                    a.to_zone.clone(),
559                )
560            },
561        );
562        let internal_consumer_count = (inputs.internal_consumers)(&anchor_file);
563        decisions.push(build_decision(
564            DecisionSpec {
565                category: DecisionCategory::CouplingBoundary,
566                candidate_key: key.clone(),
567                question: boundary_question(&from_zone, &to_zone),
568                tradeoff: boundary_tradeoff(&from_zone, &to_zone, internal_consumer_count),
569                anchor_file,
570                anchor_line,
571                blast: inputs.affected_not_shown,
572                internal_consumer_count,
573                reversibility_weight: None,
574            },
575            inputs,
576        ));
577    }
578}
579
580fn append_public_api_decision(decisions: &mut Vec<Decision>, inputs: &DecisionInputs<'_>) {
581    if !inputs.deltas.public_api_added.is_empty() {
582        // The candidate key is the full sorted added-key set joined: one stable
583        // id per change, never one-per-symbol (kills the 111-export noise).
584        let key = inputs.deltas.public_api_added.join("|");
585        let anchor_file = inputs
586            .deltas
587            .public_api_added
588            .first()
589            .and_then(|k| k.split("::").next())
590            .map(str::to_string)
591            .unwrap_or_default();
592        let internal_consumer_count = (inputs.internal_consumers)(&anchor_file);
593        decisions.push(build_decision(
594            DecisionSpec {
595                category: DecisionCategory::PublicApiContract,
596                candidate_key: key,
597                question: public_api_question(inputs.deltas.public_api_added.len()),
598                tradeoff: public_api_tradeoff(
599                    inputs.deltas.public_api_added.len(),
600                    internal_consumer_count,
601                ),
602                anchor_file,
603                anchor_line: inputs.public_api_anchor_line,
604                blast: inputs.affected_not_shown,
605                internal_consumer_count,
606                reversibility_weight: None,
607            },
608            inputs,
609        ));
610    }
611}
612
613fn append_coordination_decisions(decisions: &mut Vec<Decision>, inputs: &DecisionInputs<'_>) {
614    for gap in inputs.coordination {
615        let key = format!("contract:{}", gap.changed_file);
616        decisions.push(build_decision(
617            DecisionSpec {
618                category: DecisionCategory::PublicApiContract,
619                candidate_key: key,
620                question: coordination_question(
621                    &gap.changed_file,
622                    &gap.consumed_symbols,
623                    gap.consumer_count,
624                ),
625                tradeoff: coordination_tradeoff(gap.consumer_count),
626                anchor_file: gap.changed_file.clone(),
627                anchor_line: gap.line,
628                blast: gap.consumer_count,
629                // The coordination arm already carries the honest per-decision
630                // count; no precomputed-map lookup needed.
631                internal_consumer_count: gap.consumer_count,
632                reversibility_weight: None,
633            },
634            inputs,
635        ));
636    }
637}
638
639/// Extract the full decision surface from the assembled brief inputs: classify
640/// the SOLID-3 candidates, anchor each `signal_id`, rank by consequence, cap to
641/// the working-memory limit, collapse the rest, and drop suppressed decisions.
642///
643/// The emitted-signal-id allowlist is built over EVERY classified decision
644/// (before the cap and before suppression drops), so `accept_signal_id` still
645/// recognizes a collapsed-or-suppressed decision's anchor as fallow-emitted.
646#[must_use]
647pub fn extract_decision_surface(inputs: &DecisionInputs<'_>) -> DecisionSurface {
648    let cap = inputs.cap.clamp(MIN_DECISION_CAP, MAX_DECISION_CAP);
649
650    let mut classified = classify_candidates(inputs);
651
652    // The allowlist: every signal_id the deterministic layer emitted.
653    let emitted_signal_ids: Vec<String> = classified.iter().map(|d| d.signal_id.clone()).collect();
654
655    // Drop suppressed decisions (suppression parity): a `// fallow-ignore` on the
656    // anchor hides the decision. Done BEFORE the cap so a suppressed decision does
657    // not consume a slot. The signal_id stays on the allowlist (anchor is still a
658    // real fallow signal), so an agent re-proposing it is not "hallucinating".
659    classified.retain(|d| {
660        let source = (inputs.head_source)(&d.anchor_file);
661        !is_decision_suppressed(source.as_deref(), d.category, d.anchor_line)
662    });
663
664    // Rank by consequence desc; stable, deterministic tiebreak on signal_id.
665    // Rank by consequence, then by the category's reversibility weight, then by
666    // anchor path, so a tie is broken by a stated policy a reviewer can predict
667    // across runs rather than by hash order. The id is the final, total order.
668    classified.sort_by(|a, b| {
669        b.consequence
670            .cmp(&a.consequence)
671            .then_with(|| {
672                b.category
673                    .reversibility_weight()
674                    .cmp(&a.category.reversibility_weight())
675            })
676            .then_with(|| a.anchor_file.cmp(&b.anchor_file))
677            .then_with(|| a.signal_id.cmp(&b.signal_id))
678    });
679
680    let total = classified.len();
681    let truncated = if total > cap {
682        let collapsed = total - cap;
683        classified.truncate(cap);
684        Some(TruncationNote {
685            collapsed,
686            reason: format!(
687                "{collapsed} more structural decision{} collapsed below the cap of {cap}",
688                if collapsed == 1 { "" } else { "s" }
689            ),
690        })
691    } else {
692        None
693    };
694
695    DecisionSurface {
696        decisions: classified,
697        truncated,
698        emitted_signal_ids,
699    }
700}
701
702#[cfg(test)]
703mod tests {
704    use super::*;
705    use fallow_output::RoutingUnit;
706
707    fn deltas(boundary: &[&str], public_api: &[&str]) -> ReviewDeltas {
708        ReviewDeltas {
709            boundary_introduced: boundary.iter().map(|s| (*s).to_string()).collect(),
710            cycle_introduced: Vec::new(),
711            public_api_added: public_api.iter().map(|s| (*s).to_string()).collect(),
712            dependency_added: Vec::new(),
713            dependency_major_bumped: Vec::new(),
714        }
715    }
716
717    fn no_source(_: &str) -> Option<String> {
718        None
719    }
720
721    fn no_consumers(_: &str) -> u64 {
722        0
723    }
724
725    fn inputs<'a>(
726        deltas: &'a ReviewDeltas,
727        boundary_anchors: &'a [BoundaryAnchor],
728        coordination: &'a [CoordinationAnchor],
729        routing: &'a RoutingFacts,
730        head_source: &'a dyn Fn(&str) -> Option<String>,
731        cap: usize,
732    ) -> DecisionInputs<'a> {
733        DecisionInputs {
734            deltas,
735            boundary_anchors,
736            coordination,
737            dependency_anchors: &[],
738            public_api_anchor_line: 0,
739            affected_not_shown: 3,
740            routing,
741            head_source,
742            rename_old_path: &no_source,
743            internal_consumers: &no_consumers,
744            cap,
745        }
746    }
747
748    fn empty_routing() -> RoutingFacts {
749        RoutingFacts::default()
750    }
751
752    // (d) None of the four cut categories can ever appear: the enum has exactly
753    // three discriminants, so this is a compile-time + runtime guarantee.
754    #[test]
755    fn dependency_anchor_becomes_one_batched_dependency_decision() {
756        let deltas = deltas(&[], &[]);
757        let routing = empty_routing();
758        let anchors = vec![
759            DependencyAnchor {
760                manifest: "package.json".to_string(),
761                kind: DependencyChangeKind::MajorBump,
762                entries: vec![
763                    DependencyEntry {
764                        name: "react".to_string(),
765                        section: "dependencies".to_string(),
766                        from: Some("^18.2.0".to_string()),
767                        to: "^19.0.0".to_string(),
768                    },
769                    DependencyEntry {
770                        name: "zod".to_string(),
771                        section: "dependencies".to_string(),
772                        from: Some("^3.0.0".to_string()),
773                        to: "^4.0.0".to_string(),
774                    },
775                ],
776                importers: 12,
777                out_of_diff_importers: 9,
778                line: 14,
779            },
780            DependencyAnchor {
781                manifest: "package.json".to_string(),
782                kind: DependencyChangeKind::Added,
783                entries: vec![DependencyEntry {
784                    name: "dayjs".to_string(),
785                    section: "dependencies".to_string(),
786                    from: None,
787                    to: "^1.11.0".to_string(),
788                }],
789                importers: 0,
790                out_of_diff_importers: 0,
791                line: 9,
792            },
793        ];
794        let surface = extract_decision_surface(&DecisionInputs {
795            deltas: &deltas,
796            boundary_anchors: &[],
797            coordination: &[],
798            dependency_anchors: &anchors,
799            public_api_anchor_line: 0,
800            affected_not_shown: 0,
801            routing: &routing,
802            head_source: &no_source,
803            rename_old_path: &no_source,
804            internal_consumers: &no_consumers,
805            cap: 4,
806        });
807        assert_eq!(
808            surface.decisions.len(),
809            2,
810            "one decision per manifest per kind"
811        );
812        let bump = &surface.decisions[0];
813        assert_eq!(bump.category, DecisionCategory::Dependency);
814        assert_eq!(
815            bump.signal_key,
816            "package.json::react@^18.2.0->^19.0.0|package.json::zod@^3.0.0->^4.0.0"
817        );
818        assert_eq!(bump.anchor_file, "package.json");
819        assert_eq!(bump.anchor_line, 14);
820        assert_eq!(bump.blast, 12);
821        assert_eq!(bump.internal_consumer_count, 9);
822        assert_eq!(
823            bump.consequence,
824            12 * 3,
825            "a major bump ranks with a public-API change, not above it"
826        );
827        assert!(bump.question.contains("`react` ^18.2.0 -> ^19.0.0"));
828        assert!(bump.question.ends_with('?'));
829        assert!(
830            bump.tradeoff
831                .contains("9 in-repo modules outside this diff import")
832        );
833        let added = &surface.decisions[1];
834        assert_eq!(added.signal_key, "package.json::dayjs");
835        assert!(
836            added
837                .question
838                .contains("adds 1 third-party dependency (`dayjs`)")
839        );
840        assert!(
841            added
842                .question
843                .contains("not imported by any in-repo module"),
844            "zero importers points at build, config, or types, not at modules"
845        );
846        assert!(surface.accept_signal_id(&added.signal_id));
847    }
848
849    #[test]
850    fn only_three_categories_exist_no_cut_category_representable() {
851        let all = [
852            DecisionCategory::CouplingBoundary,
853            DecisionCategory::PublicApiContract,
854            DecisionCategory::Dependency,
855        ];
856        assert_eq!(all.len(), 3);
857        // Serialized tags never include a cut-category name.
858        for c in all {
859            let tag = c.tag();
860            for cut in ["abstraction", "deletion", "convention", "irreversib"] {
861                assert!(!tag.contains(cut), "cut category {cut} leaked into {tag}");
862            }
863        }
864    }
865
866    // (a) Every surfaced decision has a signal_id fallow emitted.
867    #[test]
868    fn every_decision_signal_id_resolves_to_an_emitted_candidate() {
869        let d = deltas(&["ui->-db"], &["src/api.ts::Widget"]);
870        let anchors = vec![BoundaryAnchor {
871            zone_pair_key: "ui->-db".to_string(),
872            from_file: "src/ui/page.ts".to_string(),
873            from_zone: "ui".to_string(),
874            to_zone: "db".to_string(),
875            line: 4,
876        }];
877        let routing = empty_routing();
878        let surface = extract_decision_surface(&inputs(&d, &anchors, &[], &routing, &no_source, 4));
879        assert!(!surface.decisions.is_empty());
880        for decision in &surface.decisions {
881            assert!(
882                surface.accept_signal_id(&decision.signal_id),
883                "decision {} has an unanchored signal_id",
884                decision.question
885            );
886        }
887    }
888
889    // (b) An injected decision with no signal anchor is REJECTED.
890    #[test]
891    fn injected_unanchored_signal_id_is_rejected() {
892        let d = deltas(&["ui->-db"], &[]);
893        let anchors = vec![BoundaryAnchor {
894            zone_pair_key: "ui->-db".to_string(),
895            from_file: "src/ui/page.ts".to_string(),
896            from_zone: "ui".to_string(),
897            to_zone: "db".to_string(),
898            line: 1,
899        }];
900        let routing = empty_routing();
901        let surface = extract_decision_surface(&inputs(&d, &anchors, &[], &routing, &no_source, 4));
902        // A fabricated id the deterministic layer never emitted.
903        assert!(!surface.accept_signal_id("sig:deadbeefdeadbeef"));
904        assert!(!surface.accept_signal_id("sig:0000000000000000"));
905        // The real one is accepted.
906        let real = derive_signal_id(DecisionCategory::CouplingBoundary, "ui->-db");
907        assert!(surface.accept_signal_id(&real));
908    }
909
910    // (c) A >cap input is capped to 4 plus/minus 1 with a truncation reason.
911    #[test]
912    fn over_cap_input_is_capped_with_truncation_reason() {
913        // 6 boundary edges; default cap 4.
914        let d = deltas(&["a->-x", "b->-x", "c->-x", "d->-x", "e->-x", "f->-x"], &[]);
915        let routing = empty_routing();
916        let surface = extract_decision_surface(&inputs(&d, &[], &[], &routing, &no_source, 4));
917        assert_eq!(surface.decisions.len(), 4, "capped to default 4");
918        let note = surface.truncated.expect("truncation note present");
919        assert_eq!(note.collapsed, 2);
920        assert!(note.reason.contains("collapsed"));
921        assert!(note.reason.contains('2'));
922    }
923
924    #[test]
925    fn cap_is_clamped_to_the_4_plus_minus_1_band() {
926        let d = deltas(
927            &[
928                "a->-x", "b->-x", "c->-x", "d->-x", "e->-x", "f->-x", "g->-x",
929            ],
930            &[],
931        );
932        let routing = empty_routing();
933        // cap=10 clamps to MAX (5).
934        let high = extract_decision_surface(&inputs(&d, &[], &[], &routing, &no_source, 10));
935        assert_eq!(high.decisions.len(), MAX_DECISION_CAP);
936        // cap=1 clamps to MIN (3).
937        let low = extract_decision_surface(&inputs(&d, &[], &[], &routing, &no_source, 1));
938        assert_eq!(low.decisions.len(), MIN_DECISION_CAP);
939    }
940
941    // (e) A `// fallow-ignore` suppresses a flagged decision.
942    #[test]
943    fn fallow_ignore_suppresses_a_flagged_decision() {
944        let d = deltas(&["ui->-db"], &[]);
945        let anchors = vec![BoundaryAnchor {
946            zone_pair_key: "ui->-db".to_string(),
947            from_file: "src/ui/page.ts".to_string(),
948            from_zone: "ui".to_string(),
949            to_zone: "db".to_string(),
950            line: 3,
951        }];
952        let routing = empty_routing();
953
954        // No suppression: one decision surfaces.
955        let unsuppressed =
956            extract_decision_surface(&inputs(&d, &anchors, &[], &routing, &no_source, 4));
957        assert_eq!(unsuppressed.decisions.len(), 1);
958
959        // File-level suppression hides it.
960        let file_src = |f: &str| {
961            (f == "src/ui/page.ts").then(|| {
962                "// fallow-ignore-file decision-surface\nimport db from 'db';\n".to_string()
963            })
964        };
965        let suppressed =
966            extract_decision_surface(&inputs(&d, &anchors, &[], &routing, &file_src, 4));
967        assert!(
968            suppressed.decisions.is_empty(),
969            "file-level ignore hides it"
970        );
971        // But the signal id stays on the allowlist (the anchor is still real).
972        let id = derive_signal_id(DecisionCategory::CouplingBoundary, "ui->-db");
973        assert!(suppressed.accept_signal_id(&id));
974
975        // Line-level suppression immediately above the anchor line also hides it.
976        let line_src = |f: &str| {
977            (f == "src/ui/page.ts").then(|| {
978                "line1\n// fallow-ignore-next-line decision-surface\nimport db from 'db';\n"
979                    .to_string()
980            })
981        };
982        let line_suppressed =
983            extract_decision_surface(&inputs(&d, &anchors, &[], &routing, &line_src, 4));
984        assert!(
985            line_suppressed.decisions.is_empty(),
986            "line-level ignore hides it"
987        );
988    }
989
990    #[test]
991    fn bare_blanket_ignore_suppresses_without_a_kind() {
992        let d = deltas(&["ui->-db"], &[]);
993        let anchors = vec![BoundaryAnchor {
994            zone_pair_key: "ui->-db".to_string(),
995            from_file: "src/ui/page.ts".to_string(),
996            from_zone: "ui".to_string(),
997            to_zone: "db".to_string(),
998            line: 2,
999        }];
1000        let routing = empty_routing();
1001        let bare = |f: &str| {
1002            (f == "src/ui/page.ts")
1003                .then(|| "// fallow-ignore-next-line\nimport db from 'db';\n".to_string())
1004        };
1005        let surface = extract_decision_surface(&inputs(&d, &anchors, &[], &routing, &bare, 4));
1006        assert!(surface.decisions.is_empty(), "bare blanket ignore hides it");
1007    }
1008
1009    #[test]
1010    fn unrelated_kind_ignore_does_not_suppress() {
1011        let d = deltas(&["ui->-db"], &[]);
1012        let anchors = vec![BoundaryAnchor {
1013            zone_pair_key: "ui->-db".to_string(),
1014            from_file: "src/ui/page.ts".to_string(),
1015            from_zone: "ui".to_string(),
1016            to_zone: "db".to_string(),
1017            line: 2,
1018        }];
1019        let routing = empty_routing();
1020        let other = |f: &str| {
1021            (f == "src/ui/page.ts").then(|| {
1022                "// fallow-ignore-next-line unused-export\nimport db from 'db';\n".to_string()
1023            })
1024        };
1025        let surface = extract_decision_surface(&inputs(&d, &anchors, &[], &routing, &other, 4));
1026        assert_eq!(
1027            surface.decisions.len(),
1028            1,
1029            "an ignore naming a different kind must not suppress a decision"
1030        );
1031    }
1032
1033    #[test]
1034    fn routed_expert_is_paired_with_a_decision() {
1035        let d = deltas(&["ui->-db"], &[]);
1036        let anchors = vec![BoundaryAnchor {
1037            zone_pair_key: "ui->-db".to_string(),
1038            from_file: "src/ui/page.ts".to_string(),
1039            from_zone: "ui".to_string(),
1040            to_zone: "db".to_string(),
1041            line: 1,
1042        }];
1043        let routing = RoutingFacts {
1044            units: vec![RoutingUnit {
1045                file: "src/ui/page.ts".to_string(),
1046                expert: vec!["@team/ui".to_string()],
1047                bus_factor_one: true,
1048            }],
1049        };
1050        let surface = extract_decision_surface(&inputs(&d, &anchors, &[], &routing, &no_source, 4));
1051        assert_eq!(surface.decisions.len(), 1);
1052        assert_eq!(surface.decisions[0].expert, vec!["@team/ui".to_string()]);
1053        assert!(surface.decisions[0].bus_factor_one);
1054    }
1055
1056    #[test]
1057    fn public_api_is_batch_consolidated_to_one_decision_r1() {
1058        // 111 added export keys collapse to ONE public-API decision (R1).
1059        let keys: Vec<String> = (0..111).map(|i| format!("src/ui/index.ts::C{i}")).collect();
1060        let key_refs: Vec<&str> = keys.iter().map(String::as_str).collect();
1061        let d = deltas(&[], &key_refs);
1062        let routing = empty_routing();
1063        let surface = extract_decision_surface(&inputs(&d, &[], &[], &routing, &no_source, 4));
1064        let public_api_count = surface
1065            .decisions
1066            .iter()
1067            .filter(|dec| dec.category == DecisionCategory::PublicApiContract)
1068            .count();
1069        assert_eq!(
1070            public_api_count, 1,
1071            "R1: one public-API decision per change"
1072        );
1073        assert!(surface.decisions[0].question.contains("111"));
1074    }
1075
1076    #[test]
1077    fn public_api_decision_carries_honest_consumer_count_and_tradeoff() {
1078        // A public-API delta whose anchor has 7 in-repo out-of-diff consumers must
1079        // surface that honest number on the decision AND name it as a fact in the
1080        // trade-off clause, distinct from the project-wide ranking proxy (`blast`).
1081        let d = deltas(&[], &["src/ui/index.ts::Widget"]);
1082        let routing = empty_routing();
1083        let seven = |_: &str| 7u64;
1084        let surface = extract_decision_surface(&DecisionInputs {
1085            deltas: &d,
1086            boundary_anchors: &[],
1087            coordination: &[],
1088            dependency_anchors: &[],
1089            public_api_anchor_line: 0,
1090            // The project-wide proxy must NOT become the display number.
1091            affected_not_shown: 99,
1092            routing: &routing,
1093            head_source: &no_source,
1094            rename_old_path: &no_source,
1095            internal_consumers: &seven,
1096            cap: 4,
1097        });
1098        let dec = surface
1099            .decisions
1100            .iter()
1101            .find(|dec| dec.category == DecisionCategory::PublicApiContract)
1102            .expect("a public-API decision");
1103        assert_eq!(dec.internal_consumer_count, 7, "honest per-anchor count");
1104        assert_ne!(
1105            dec.internal_consumer_count, dec.blast,
1106            "display number must stay distinct from the ranking proxy"
1107        );
1108        assert!(
1109            dec.tradeoff.contains("7 in-repo"),
1110            "trade-off clause states the count as a fact: {}",
1111            dec.tradeoff
1112        );
1113        assert!(
1114            dec.question.ends_with('?'),
1115            "the decision stays a question (taste ownership)"
1116        );
1117    }
1118
1119    #[test]
1120    fn coordination_gap_becomes_a_public_api_contract_decision() {
1121        let d = deltas(&[], &[]);
1122        let coordination = vec![CoordinationAnchor {
1123            changed_file: "src/core.ts".to_string(),
1124            consumed_symbols: vec!["compute".to_string()],
1125            consumer_count: 4,
1126            line: 7,
1127        }];
1128        let routing = empty_routing();
1129        let surface =
1130            extract_decision_surface(&inputs(&d, &[], &coordination, &routing, &no_source, 4));
1131        assert_eq!(surface.decisions.len(), 1);
1132        assert_eq!(
1133            surface.decisions[0].category,
1134            DecisionCategory::PublicApiContract
1135        );
1136        assert_eq!(surface.decisions[0].blast, 4);
1137        // The contract symbol's declaration line flows onto the decision so a PR
1138        // review can anchor an inline comment to the exact export.
1139        assert_eq!(surface.decisions[0].anchor_line, 7);
1140        // No rename in this change -> no previous_signal_id (the default).
1141        assert!(surface.decisions[0].previous_signal_id.is_none());
1142    }
1143
1144    #[test]
1145    fn renamed_anchor_carries_a_previous_signal_id_for_review_memory() {
1146        // A coordination decision on a file renamed src/old.ts -> src/new.ts. The
1147        // signal_id keys on the NEW path; previous_signal_id keys on the OLD path,
1148        // so a cloud memory layer carries a prior dismissal across the `git mv`.
1149        let d = deltas(&[], &[]);
1150        let coordination = vec![CoordinationAnchor {
1151            changed_file: "src/new.ts".to_string(),
1152            consumed_symbols: vec!["compute".to_string()],
1153            consumer_count: 2,
1154            line: 0,
1155        }];
1156        let routing = empty_routing();
1157        let rename = |rel: &str| -> Option<String> {
1158            (rel == "src/new.ts").then(|| "src/old.ts".to_string())
1159        };
1160        let surface = extract_decision_surface(&DecisionInputs {
1161            deltas: &d,
1162            boundary_anchors: &[],
1163            coordination: &coordination,
1164            dependency_anchors: &[],
1165            public_api_anchor_line: 0,
1166            affected_not_shown: 2,
1167            routing: &routing,
1168            head_source: &no_source,
1169            rename_old_path: &rename,
1170            internal_consumers: &no_consumers,
1171            cap: 4,
1172        });
1173        assert_eq!(surface.decisions.len(), 1);
1174        let decision = &surface.decisions[0];
1175        assert_eq!(
1176            decision.signal_id,
1177            derive_signal_id(DecisionCategory::PublicApiContract, "contract:src/new.ts")
1178        );
1179        assert_eq!(
1180            decision.previous_signal_id,
1181            Some(derive_signal_id(
1182                DecisionCategory::PublicApiContract,
1183                "contract:src/old.ts"
1184            ))
1185        );
1186    }
1187
1188    #[test]
1189    fn signal_id_is_deterministic_and_namespaced_by_category() {
1190        let a = derive_signal_id(DecisionCategory::CouplingBoundary, "ui->-db");
1191        let b = derive_signal_id(DecisionCategory::CouplingBoundary, "ui->-db");
1192        assert_eq!(a, b, "deterministic");
1193        let c = derive_signal_id(DecisionCategory::PublicApiContract, "ui->-db");
1194        assert_ne!(a, c, "category namespaces the hash");
1195        assert!(a.starts_with("sig:"));
1196    }
1197
1198    #[test]
1199    fn consequence_ranks_less_reversible_categories_higher() {
1200        // Same blast: dependency > public-api > coupling on reversibility weight.
1201        let dep = DecisionCategory::Dependency.reversibility_weight();
1202        let api = DecisionCategory::PublicApiContract.reversibility_weight();
1203        let coupling = DecisionCategory::CouplingBoundary.reversibility_weight();
1204        assert!(dep > api && api > coupling);
1205    }
1206}