From 5c99190ea7284f39191516cba6dcb9be5dda3bbe Mon Sep 17 00:00:00 2001 From: Lourince Daging Date: Thu, 16 Jul 2026 21:04:30 +0200 Subject: [PATCH] fix: correct four stale 'improvementSignal not wired yet' comments improvementSignal (epic #4737) shipped and its sub-issues (#4738-#4746) all closed, but four module/doc comments still described it as inert/test-only. src/queue/processors.ts is now its real caller (convergedFeatureActive + buildStructuralImprovementAssessment, gated on the resolved value). Correct the headers in improvement-signal-wire.ts and signals/improvement.ts, the improvementSignal field doc in services/ai-review.ts, and the CONVERGED_FEATURE_KEYS comment in engine/focus-manifest.ts to match. Comment-only; no runtime behavior, exports, types, or signatures change (the still-accurate sub-claims, e.g. improvement.ts's complexityDeltas note, are left as-is). Closes #6608. --- packages/loopover-engine/src/focus-manifest.ts | 6 +++--- src/review/improvement-signal-wire.ts | 6 +++--- src/services/ai-review.ts | 5 +++-- src/signals/improvement.ts | 8 ++++---- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/packages/loopover-engine/src/focus-manifest.ts b/packages/loopover-engine/src/focus-manifest.ts index 619180b4e5..440612f091 100644 --- a/packages/loopover-engine/src/focus-manifest.ts +++ b/packages/loopover-engine/src/focus-manifest.ts @@ -262,9 +262,9 @@ export type CopycatGateMode = "off" | "warn" | "label" | "block"; // `review.visual.enabled` keeps its own existing force-off-only semantics untouched by this change. // `improvementSignal` (#4738, foundation phase of the #4737 PR-improvement-signal epic) is likewise a plain // symmetric override: it is a READ-ONLY advisory quality-delta signal, not a security control, so a repo- -// level `false` behaves like any other plain override with no floor/ceiling. This is activation wiring only -// -- no tier reads the resolved value yet (sibling sub-issues #4739-#4746 build the deterministic/LLM/panel -// behavior that will gate on it). +// level `false` behaves like any other plain override with no floor/ceiling. The tiers that gate on it +// (sibling sub-issues #4739-#4746) have landed -- src/queue/processors.ts reads the resolved value to drive +// the deterministic/LLM improvement-signal behavior. export const CONVERGED_FEATURE_KEYS = [ "rag", "reputation", diff --git a/src/review/improvement-signal-wire.ts b/src/review/improvement-signal-wire.ts index 5634f0d226..5d24d37910 100644 --- a/src/review/improvement-signal-wire.ts +++ b/src/review/improvement-signal-wire.ts @@ -3,9 +3,9 @@ // counterpart to src/signals/slop.ts's risk-only score (see #4737 for the full design). This file is deliberately // minimal for now (just the env flag), mirroring the shape of `e2e-test-gen-wire.ts`/`rag-wire.ts` at the same // stage of their own rollout -- the deterministic (REES) tier, the LLM tier, and panel surfacing land in later, -// separate sub-issues (#4739-#4746) once this flag exists for them to gate on. This PR wires activation only: -// nothing calls resolveConvergedFeature(env, manifest, "improvementSignal", repoFullName) yet outside this -// feature's own tests. +// separate sub-issues (#4739-#4746) once this flag exists for them to gate on. Those tiers have since landed: +// src/queue/processors.ts now resolves this feature via convergedFeatureActive(env, repoFullName, +// "improvementSignal") (the async wrapper around resolveConvergedFeature) and gates real AI-review behavior on it. // // Single env switch: LOOPOVER_REVIEW_IMPROVEMENT_SIGNAL. Default OFF (unset/"false") -- when OFF the feature // never runs anywhere, regardless of any per-repo `.loopover.yml` override (see `resolveConvergedFeature` in diff --git a/src/services/ai-review.ts b/src/services/ai-review.ts index da1aa80ea6..093a7335d1 100644 --- a/src/services/ai-review.ts +++ b/src/services/ai-review.ts @@ -329,8 +329,9 @@ export type LoopOverAiReviewInput = { * caller-resolved rather than looked up internally, so a manifest already loaded once upstream for several * flags is never re-fetched per-flag inside this module. (The one exception, `safety`, resolves internally via * `convergedFeatureActive` because it is security-critical and has no upstream caller today; `improvementSignal` - * is a read-only advisory signal, not a security control, so it follows the majority pattern instead.) Absent/ - * false (the default, and the only reachable value until a caller starts resolving the feature) ⇒ no instruction + * is a read-only advisory signal, not a security control, so it follows the majority pattern instead. That + * caller is now live: src/queue/processors.ts resolves the feature and passes the resolved boolean here.) Absent/ + * false (the default) ⇒ no instruction * is appended and the model is never asked — byte-identical prompt, zero extra output tokens spent. */ improvementSignal?: boolean | undefined; diff --git a/src/signals/improvement.ts b/src/signals/improvement.ts index 1971e1c3df..d861c38521 100644 --- a/src/signals/improvement.ts +++ b/src/signals/improvement.ts @@ -6,10 +6,10 @@ // genuinely separate axis combined with this score at the SURFACING layer (a later sub-issue, #4744) — // never blended into `improvementScore` itself. // -// Activation wiring already exists (#4738/#4753: `isImprovementSignalEnabled` + the `improvementSignal` -// ConvergedFeatureKey) but nothing calls `resolveConvergedFeature` for it yet, and this module is not an -// exception — it is a pure, standalone computation consumed only by its own tests until the panel-surfacing -// sub-issue (#4744) wires a caller. It carries NO gate/blocker power (epic design constraint 2): unlike +// Activation wiring exists (#4738/#4753: `isImprovementSignalEnabled` + the `improvementSignal` +// ConvergedFeatureKey) and is now live: this remains a pure, standalone computation, but its +// `buildStructuralImprovementAssessment` is called by src/queue/processors.ts (gated on the resolved feature +// value), and the panel-surfacing sub-issue (#4744) has landed. It carries NO gate/blocker power (epic design constraint 2): unlike // slop.ts's header comment ("the ONLY thing allowed to gate"), `improvementScore` must never appear in // evaluateGateCheck or any blocker path. //