feat(review): add the persistently-critical convergence advisory (land-with-residual-risk)

packages/cli/src/commands/review/lib/convergence.test.ts

@@ -12,8 +12,12 @@ import {
   renderConvergenceDiagnosis,
   renderMechanismHealth,
   MAX_RENDERED_CLUSTERS,
+  convergenceAssessment,
+  convergenceAdvisory,
+  LAND_WITH_RESIDUAL_RISK,
   type ConvergenceDiagnosis,
   type DraftedFinding,
+  type ConvergenceFacts,
 } from './convergence.js';
 import { LEDGER_MAX_ROUND, type LedgerFinding } from './ledger.js';
 
@@ -1131,3 +1135,268 @@ describe('isFreshDraft — a carried id no longer answers on its own', () => {
     ).toBe(true);
   });
 });
+
+// The persistently-critical signal is advisory telemetry: every input
+// degrades OPEN, so the tests pin both the firing conjunction and each
+// degraded arm individually — a false fire would tell an operator to land a
+// loop that is still converging, and a missed fire is the silent status quo
+// this module exists to end.
+
+const FIRE: ConvergenceFacts = {
+  prevHadCritical: true,
+  thisCriticals: 2,
+  fresh: 3,
+  prevFresh: 3,
+  floorEngaged: true,
+  prevFloor: 'c',
+  // The predecessor's work-list was Critical-only, which is what an engaged
+  // floor leaves behind — the stamp above cannot say so on its own.
+  prevPostedSuggestion: false,
+  // Equal to `thisCriticals`, so the backlog veto abstains and every other
+  // arm below is pinned on its own. A firing default whose backlog was
+  // already shrinking would make each `toBeNull()` below pass for the
+  // wrong reason.
+  prevCriticals: 2,
+  // A WHOLE predecessor list, so the two absence-derived readings above are
+  // evidence and the advisory publishes them unqualified.
+  prevTruncated: false,
+};
+
+describe('convergenceAssessment', () => {
+  it('fires on the full conjunction — persistent Criticals, fresh rate not falling', () => {
+    const a = convergenceAssessment(FIRE);
+    expect(a).not.toBeNull();
+    expect(a?.shape).toBe('persistently-critical');
+    expect(a?.recommendation).toBe(LAND_WITH_RESIDUAL_RISK);
+    expect(a?.criticals).toBe(2);
+    expect(a?.fresh).toBe(3);
+    expect(a?.prevFresh).toBe(3);
+  });
+
+  it('fires when the fresh rate is RISING — rising is not falling either', () => {
+    expect(
+      convergenceAssessment({ ...FIRE, fresh: 5, prevFresh: 3 }),
+    ).not.toBeNull();
+  });
+
+  it('suppresses when the previous round was NOT recovered — undefined is not false', () => {
+    // A second round introducing its first Critical must not read as
+    // "persistent": there is no prior work-list to have carried one.
+    expect(
+      convergenceAssessment({ ...FIRE, prevHadCritical: undefined }),
+    ).toBeNull();
+  });
+
+  it('suppresses when the previous work-list had no Critical', () => {
+    // Criticals appeared only THIS round — being worked for the first time,
+    // not persisted.
+    expect(
+      convergenceAssessment({ ...FIRE, prevHadCritical: false }),
+    ).toBeNull();
+  });
+
+  it('suppresses when this round posts no Critical', () => {
+    expect(convergenceAssessment({ ...FIRE, thisCriticals: 0 })).toBeNull();
+  });
+
+  it('suppresses when the severity floor is NOT engaged — its futility claim would be unprovable', () => {
+    // The advisory asserts the floor "will not converge" the loop; before
+    // the floor has run, the loop may still converge once it does, and a
+    // guess is the false fire this module must never ship.
+    expect(convergenceAssessment({ ...FIRE, floorEngaged: false })).toBeNull();
+  });
+
+  it('suppresses when floor engagement is UNKNOWN — absence degrades open', () => {
+    expect(
+      convergenceAssessment({ ...FIRE, floorEngaged: undefined }),
+    ).toBeNull();
+  });
+
+  it('suppresses when either fresh count is missing — a gap says nothing', () => {
+    // Reachable without tampering: a marker written before the fresh count
+    // shipped records only the total, and there is no honest way to read a
+    // trend off one end of a window.
+    expect(convergenceAssessment({ ...FIRE, fresh: undefined })).toBeNull();
+    expect(convergenceAssessment({ ...FIRE, prevFresh: undefined })).toBeNull();
+  });
+
+  it('suppresses when the fresh rate is FALLING — a converging loop', () => {
+    // Criticals present but the new ones drying up: the loop is settling.
+    // Measured on posting TOTALS this arm was unreachable — Step 6 re-posts
+    // every standing Critical, so the total only ever rises and a loop
+    // whose fresh findings fell 5 -> 4 still posted more comments than the
+    // round before, firing `land-with-residual-risk` over a converging
+    // loop.
+    expect(
+      convergenceAssessment({ ...FIRE, fresh: 1, prevFresh: 3 }),
+    ).toBeNull();
+  });
+
+  it('suppresses when the standing backlog is SHRINKING — the fresh window is blind to it', () => {
+    // The blind spot the fresh window alone leaves: a reviewer finding
+    // nothing new for two rounds while the author clears blockers sits at
+    // fresh 0 against fresh 0, which "not falling" reads as stuck. Only the
+    // Critical count coming down says the loop is moving.
+    expect(
+      convergenceAssessment({
+        ...FIRE,
+        fresh: 0,
+        prevFresh: 0,
+        thisCriticals: 3,
+        prevCriticals: 5,
+      }),
+    ).toBeNull();
+  });
+
+  it('abstains on the backlog when the previous count is unknown', () => {
+    // A veto on positive evidence only. The work-list the count comes off
+    // is the one the marker's byte budget may have shortened, and an
+    // undercount can only hide shrinkage — never invent it — so an unknown
+    // predecessor must not silence a loop that is genuinely stuck.
+    expect(
+      convergenceAssessment({ ...FIRE, prevCriticals: undefined }),
+    ).not.toBeNull();
+  });
+
+  it('fires at zero fresh on both rounds — the purest form of the shape', () => {
+    // Criticals standing round after round with nothing new found is not a
+    // quiet loop, it is the shape itself, so this must fire — which is why
+    // this signal does NOT carry the sibling diagnosis's `prev.fresh > 0`
+    // requirement. That module is about a loop GENERATING work; this one is
+    // about work that never clears. The backlog holding steady (not
+    // shrinking) is what separates it from a backlog being worked down.
+    expect(
+      convergenceAssessment({
+        prevHadCritical: true,
+        thisCriticals: 3,
+        fresh: 0,
+        prevFresh: 0,
+        floorEngaged: true,
+        prevFloor: 'c',
+        prevPostedSuggestion: false,
+        prevCriticals: 3,
+        prevTruncated: false,
+      }),
+    ).not.toBeNull();
+  });
+});
+
+it('suppresses when the previous round posted under a DIFFERENT floor', () => {
+  // The round the floor engages on compares a Critical-only window
+  // against a predecessor that was still posting Suggestions. That
+  // movement is the posture, not the loop — and "the severity floor will
+  // not converge it" is not a claim one round of the floor can support.
+  expect(convergenceAssessment({ ...FIRE, prevFloor: 'o' })).toBeNull();
+});
+
+it('suppresses when the predecessor still posted a Suggestion — the stamp lied', () => {
+  // The recorded floor is the REPORTING reading, which folds an absent
+  // `severityFloor` into `auto` and stamps `c` on any round >= 6 — even one
+  // the strict enforcement backstop never touched, where Suggestions posted
+  // normally. Paired against this round's enforcement reading, that stamp
+  // let an un-enforced predecessor pass as an engaged one and the advisory
+  // published "the severity floor will not converge it" against a window
+  // whose far end still included Suggestions. A Suggestion in the
+  // work-list is the fact the stamp cannot carry: enforcement moves drafted
+  // Suggestions out of the posting set before the marker is built, so an
+  // engaged round's list is Critical-only.
+  expect(
+    convergenceAssessment({ ...FIRE, prevPostedSuggestion: true }),
+  ).toBeNull();
+});
+
+it('still evaluates when the predecessor work-list is unreadable', () => {
+  // Unknown abstains, like every other fact read off that list — a marker
+  // this round could not recover says nothing about what the floor did.
+  expect(
+    convergenceAssessment({ ...FIRE, prevPostedSuggestion: undefined }),
+  ).not.toBeNull();
+});
+
+it('still evaluates when the previous floor was never recorded', () => {
+  // Read like the sibling diagnosis in this module: a floor that was not
+  // recorded is not a floor that DIFFERS. A marker written before the
+  // field existed must evaluate exactly as it did before this conjunct,
+  // or the advisory goes silent on every loop carrying an older marker.
+  expect(
+    convergenceAssessment({ ...FIRE, prevFloor: undefined }),
+  ).not.toBeNull();
+});
+
+it('fires on a truncated predecessor, and says the reading came off one', () => {
+  // The gate is deliberately NOT restored: a whole-list requirement would
+  // silence the advisory on exactly the deep-work-list rounds it exists for,
+  // which are the rounds the marker's byte budget shortens. What a shortened
+  // list changes is what may be CLAIMED — "no Suggestion, so the floor was
+  // enforcing" and "the backlog is not shrinking" are both read off absence,
+  // and absence in a shortened list is not evidence.
+  const a = convergenceAssessment({ ...FIRE, prevTruncated: true });
+  expect(a).not.toBeNull();
+  expect(a?.prevTruncated).toBe(true);
+  const { en, zh } = convergenceAdvisory(a!);
+  expect(en).toContain('truncated to fit the marker');
+  expect(en).toContain('read off a list known to be incomplete');
+  expect(zh).toContain('为适配 marker 被截断');
+  // And a WHOLE list publishes the readings unqualified.
+  const whole = convergenceAdvisory(convergenceAssessment(FIRE)!);
+  expect(whole.en).not.toContain('truncated to fit the marker');
+  expect(whole.zh).not.toContain('为适配 marker 被截断');
+});
+
+describe('convergenceAdvisory', () => {
+  it('renders a RISING window in the right direction, in both languages', () => {
+    // Every equal-count fixture reads the same number twice, so swapping
+    // the two interpolations keeps them all green while inverting the trend
+    // a maintainer reads when making the land decision. A rising window
+    // (fresh 5, previous 3) fires and must read this-round-first.
+    const a = convergenceAssessment({ ...FIRE, fresh: 5, prevFresh: 3 });
+    expect(a).not.toBeNull();
+    const { en, zh } = convergenceAdvisory(a!);
+    expect(en).toContain('this round 5, previous 3');
+    expect(zh).toContain('本轮 5');
+    expect(zh).toContain('上一轮 3');
+  });
+
+  it('names the recommendation code and disclaims itself, in both languages', () => {
+    const a = convergenceAssessment(FIRE);
+    expect(a).not.toBeNull();
+    const { en, zh } = convergenceAdvisory(a!);
+    for (const text of [en, zh]) {
+      expect(text).toContain(LAND_WITH_RESIDUAL_RISK);
+      expect(text).toContain('persistently');
+    }
+    // Advisory-only contract: it must say it blocks nothing.
+    expect(en).toContain('does not block');
+    expect(zh).toContain('不阻断');
+    // The scaffold names the three maintainer dimensions — in BOTH
+    // languages. Pinned only in English, a zh scaffold that lost a column
+    // shipped green, and the Chinese reader is the one who cannot fall back
+    // to the other half of the paragraph.
+    expect(en).toContain('attack surface');
+    expect(en).toContain('attacker-dependency');
+    expect(en).toContain('blast radius');
+    expect(zh).toContain('攻击面');
+    expect(zh).toContain('攻击者依赖性');
+    expect(zh).toContain('影响范围');
+    // The claim the recommendation rests on, positively, in both.
+    expect(en).toContain('The severity floor will not converge it');
+    expect(zh).toContain('severity floor 无法使其收敛');
+    // Bounded by construction: the facts ride as numbers, never model
+    // prose — and the zh Critical COUNT is its own interpolation slot, not
+    // a repeat of the volume beside it. `FIRE` is deliberately asymmetric
+    // (2 Criticals, volume 3/3) so a template reading the wrong slot shows.
+    expect(en).toContain('2 Critical(s)');
+    expect(en).toContain('this round 3, previous 3');
+    expect(zh).toContain('本轮 2 条 Critical');
+    expect(zh).toContain('本轮 3，上一轮 3');
+    // The numbers are FIRST-TIME findings, and the sentence must say so —
+    // reported as "the posting volume" they described a total the signal
+    // does not measure, which is the false record this pipeline refuses.
+    expect(en).toContain('the rate of first-time findings is not falling');
+    expect(en).toContain('the standing Critical backlog is not shrinking');
+    expect(en).not.toContain('posting volume');
+    expect(zh).toContain('首次发现的速率没有下降');
+    expect(zh).toContain('未决 Critical 积压没有减少');
+    expect(zh).not.toContain('发布音量');
+  });
+});