diff --git a/packages/pi-coding-agent/src/modes/interactive/components/dynamic-border.test.ts b/packages/pi-coding-agent/src/modes/interactive/components/dynamic-border.test.ts index 30a0a391a..93b2600d4 100644 --- a/packages/pi-coding-agent/src/modes/interactive/components/dynamic-border.test.ts +++ b/packages/pi-coding-agent/src/modes/interactive/components/dynamic-border.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { describe, it } from 'vitest'; +import { afterEach, describe, it, vi } from 'vitest'; import { DynamicBorder } from "./dynamic-border.js"; @@ -13,43 +13,38 @@ function makeTUI() { } describe("DynamicBorder spinner", () => { - it("suppresses standalone render when an external render occurred recently", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("spinner_when_external_renders_are_recent_still_requests_tick_render", () => { + vi.useFakeTimers(); const border = new DynamicBorder((s) => s); const tui = makeTUI(); border.startSpinner(tui as any, (s) => s); - // startSpinner calls requestRender once immediately assert.equal(tui.renderCount, 1, "initial render on startSpinner"); - // Simulate an externally-triggered render (e.g. from streaming) + // Simulate an externally triggered render from streaming immediately before + // the spinner tick. The spinner must still schedule its own redraw so the + // working indicator does not look frozen during active output. border.render(80); + vi.advanceTimersByTime(200); - // Access the private interval callback by advancing the timer - // Instead, we directly test the render-batching logic: - // After render() sets lastExternalRender, a spinner tick within 200ms - // should NOT call requestRender. - const anyBorder = border as any; - assert.ok( - Date.now() - anyBorder.lastExternalRender < 200, - "lastExternalRender should be recent after render()", - ); + assert.equal(tui.renderCount, 2, "spinner tick should request render even after a recent render"); border.stopSpinner(); }); - it("triggers standalone render when no external render occurred recently", async () => { + it("spinner_when_no_external_render_occurs_requests_tick_render", () => { + vi.useFakeTimers(); const border = new DynamicBorder((s) => s); const tui = makeTUI(); - // Set lastExternalRender to a time well in the past - const anyBorder = border as any; - anyBorder.lastExternalRender = 0; - border.startSpinner(tui as any, (s) => s); const initialCount = tui.renderCount; - // Wait for one spinner tick (200ms interval + buffer) - await new Promise((r) => setTimeout(r, 250)); + vi.advanceTimersByTime(200); assert.ok( tui.renderCount > initialCount, @@ -58,16 +53,4 @@ describe("DynamicBorder spinner", () => { border.stopSpinner(); }); - - it("updates lastExternalRender on each render() call", () => { - const border = new DynamicBorder((s) => s); - const anyBorder = border as any; - - const before = Date.now(); - border.render(80); - const after = Date.now(); - - assert.ok(anyBorder.lastExternalRender >= before); - assert.ok(anyBorder.lastExternalRender <= after); - }); }); diff --git a/packages/pi-coding-agent/src/modes/interactive/components/dynamic-border.ts b/packages/pi-coding-agent/src/modes/interactive/components/dynamic-border.ts index 854d143d0..706287c76 100644 --- a/packages/pi-coding-agent/src/modes/interactive/components/dynamic-border.ts +++ b/packages/pi-coding-agent/src/modes/interactive/components/dynamic-border.ts @@ -17,7 +17,6 @@ export class DynamicBorder implements Component { private spinnerIndex = 0; private spinnerInterval: NodeJS.Timeout | null = null; private spinnerColorFn?: (str: string) => string; - private lastExternalRender = 0; constructor(color: (str: string) => string = (str) => { try { return theme.fg("border", str); } catch { return str; } @@ -40,11 +39,7 @@ export class DynamicBorder implements Component { this.spinnerIndex = 0; this.spinnerInterval = setInterval(() => { this.spinnerIndex = (this.spinnerIndex + 1) % this.spinnerFrames.length; - // Only trigger standalone render if no other source rendered recently. - // During active streaming, message_update already calls requestRender(). - if (Date.now() - this.lastExternalRender > 200) { - ui.requestRender(); - } + ui.requestRender(); }, 200); ui.requestRender(); } @@ -69,7 +64,6 @@ export class DynamicBorder implements Component { } render(width: number): string[] { - this.lastExternalRender = Date.now(); const spinnerPrefix = this.spinnerInterval && this.spinnerColorFn ? this.spinnerColorFn(this.spinnerFrames[this.spinnerIndex]) + " " : ""; diff --git a/packages/pi-tui/src/components/__tests__/loader.test.ts b/packages/pi-tui/src/components/__tests__/loader.test.ts index 52929e9b8..ab50e543d 100644 --- a/packages/pi-tui/src/components/__tests__/loader.test.ts +++ b/packages/pi-tui/src/components/__tests__/loader.test.ts @@ -42,4 +42,22 @@ describe("Loader", () => { loader.stop(); loader.stop(); }); + + it("render_when_hidden_returns_no_lines", () => { + loader = new Loader(tui, (s) => s, (s) => s, "test"); + + loader.setVisible(false); + + assert.deepEqual(loader.render(80), []); + }); + + it("setVisible_when_toggled_requests_render", () => { + loader = new Loader(tui, (s) => s, (s) => s, "test"); + tui.requestRender.mockClear(); + + loader.setVisible(false); + loader.setVisible(true); + + assert.equal(tui.requestRender.mock.calls.length, 2); + }); }); diff --git a/packages/pi-tui/src/components/loader.ts b/packages/pi-tui/src/components/loader.ts index 5115f8337..9f5097dc8 100644 --- a/packages/pi-tui/src/components/loader.ts +++ b/packages/pi-tui/src/components/loader.ts @@ -12,6 +12,7 @@ export class Loader extends Text { private intervalId: NodeJS.Timeout | null = null; private ui: TUI | null = null; private _lastMessage: string = ""; + private visible = true; constructor( ui: TUI, @@ -25,6 +26,9 @@ export class Loader extends Text { } render(width: number): string[] { + if (!this.visible) { + return []; + } // Only update Text content when message actually changes — // frame rotation is prepended below without touching the cache if (this.message !== this._lastMessage) { @@ -49,7 +53,7 @@ export class Loader extends Text { this.currentFrame = 0; this.intervalId = setInterval(() => { this.currentFrame = (this.currentFrame + 1) % this.frames.length; - if (this.ui) { + if (this.ui && this.visible) { this.ui.requestRender(); } }, 80); @@ -77,4 +81,14 @@ export class Loader extends Text { this.ui.requestRender(); } } + + setVisible(visible: boolean) { + if (this.visible === visible) { + return; + } + this.visible = visible; + if (this.ui) { + this.ui.requestRender(); + } + } } diff --git a/src/resources/extensions/sf/auto-post-unit.js b/src/resources/extensions/sf/auto-post-unit.js index 51a14499e..93d14ed92 100644 --- a/src/resources/extensions/sf/auto-post-unit.js +++ b/src/resources/extensions/sf/auto-post-unit.js @@ -1193,6 +1193,51 @@ export async function postUnitPostVerification(pctx) { }); } } + // ── Doc-sync drift check (BUILD_PLAN Tier 2.2) ── + // After code-mutating units, check whether ARCHITECTURE.md or other tracked + // docs are out of sync with the actual codebase. Advisory — never blocks. + if (s.currentUnit?.type === "execute-task" || s.currentUnit?.type === "complete-slice") { + try { + const { getDocSyncProposal, formatDocSyncProposal } = await import("../doc-sync.js"); + const { runGit: runGitCmd } = await import("./git-service.js"); + const fs = await import("node:fs"); + const path = await import("node:path"); + const diffResult = runGitCmd(s.basePath, ["diff", "--name-only", "HEAD~1", "HEAD"]); + const changedFiles = diffResult?.stdout + ? diffResult.stdout.trim().split("\n").filter(Boolean) + : []; + if (changedFiles.length > 0) { + const proposals = getDocSyncProposal( + changedFiles, + { + readFile: (p) => { + try { return fs.readFileSync(path.join(s.basePath, p), "utf-8"); } + catch { return null; } + }, + }, + { + glob: (_pat) => { + try { + const out = runGitCmd(s.basePath, ["ls-files"]); + return out?.stdout ? out.stdout.trim().split("\n").filter(Boolean) : []; + } + catch { return []; } + }, + }, + ); + if (proposals) { + const msg = formatDocSyncProposal(proposals); + if (msg) { + ctx.ui.notify(msg, "info"); + debugLog("postUnit", { phase: "doc-sync", driftCount: proposals.reduce((n, p) => n + p.drift.length, 0) }); + } + } + } + } + catch (e) { + debugLog("postUnit", { phase: "doc-sync", error: e instanceof Error ? e.message : String(e) }); + } + } // ── Knowledge compounding (Mechanism 4) ── // After milestone completion, distill high-confidence judgment-log entries // into .sf/KNOWLEDGE.md so the next milestone benefits from them. diff --git a/src/resources/extensions/sf/tests/milestone-quality-ceremony.test.ts b/src/resources/extensions/sf/tests/milestone-quality-ceremony.test.ts deleted file mode 100644 index 715c3d0f9..000000000 --- a/src/resources/extensions/sf/tests/milestone-quality-ceremony.test.ts +++ /dev/null @@ -1,213 +0,0 @@ -import { describe, expect, it } from "vitest"; - -/** - * Tests for milestone-quality.js vision meeting depth validation. - * - * Purpose: verify that vision meeting validators enforce not just presence but - * purposeful depth — rubber-stamp contributions must trigger shallow warnings. - * - * Consumer: milestone planning pipeline (plan-milestone tool, roadmap inspection). - */ - -async function importMilestoneQuality() { - return import("../milestone-quality.js"); -} - -describe("milestone-quality vision meeting depth", () => { - const makeValidMeeting = () => ({ - trigger: "Comparative research identified eight high-signal improvements for the autonomous workflow engine", - pm: "Eight improvements from two production systems at scale. Doctor-first catches drift, adversarial verification catches bugs command-exit misses.", - userAdvocate: "Users experience wasted sessions on stale state, noisy memory context, and unpredictable autonomous execution — each improvement maps to a specific pain point.", - customerPanel: "Power users need reliability. Casual users need quality output. DevOps users need CI integration. Each group has different priorities.", - business: "Each finding addresses a real failure mode observed in production. Adversarial verification catches the 'tests pass but bug persists' class.", - researcher: "Both Keel (spoke-sh/keel) and Claude Code (Anthropic) are open-source. BUILD_PLAN.md Tier 2.1 references verified.", - deliveryLead: "Foundation tier first (S01), then independent improvements in parallel, then architecture tier (S04), then dependent tier last.", - partner: "Claude Code synthesis gate from coordinatorMode.ts is the single most impactful change. Keel doctor-first catches drift that wastes dispatch cycles.", - combatant: "Risk 1: S04 state machine refactor always takes longer — auto-dispatch.ts is most coupled. Risk 2: adversarial verification 2-3x token cost. Risk 3: sub-tasks are schema migration.", - architect: "Three dependency tiers. Foundation (S01-S03, S05, S08) independent. Architecture (S04) refactors auto-dispatch.ts. Dependent (S06, S07) built on S04.", - moderator: "Approve full scope. S04 approved with latency budget. S08 needs complexity threshold. Monitor S04 closely — reassess at 3 dispatch cycles.", - weightedSynthesis: "Foundation ships first, architecture second, dependent last. Critical path: S01 → S04 → S06/S07. Wall-clock: 5-8 dispatch cycles.", - confidenceByArea: "doctor: 0.9 | quality: 0.85 | battery: 0.7 | two-lane: 0.65 | hard-cutover: 0.9 | sub-tasks: 0.7 | fork-spawn: 0.75 | adversarial: 0.8", - recommendedRoute: "planning", - }); - - describe("hasStructuredVisionAlignmentMeeting", () => { - it("accepts_valid_meeting", async () => { - const { hasStructuredVisionAlignmentMeeting } = await importMilestoneQuality(); - expect(hasStructuredVisionAlignmentMeeting(makeValidMeeting())).toBe(true); - }); - - it("rejects_null_meeting", async () => { - const { hasStructuredVisionAlignmentMeeting } = await importMilestoneQuality(); - expect(hasStructuredVisionAlignmentMeeting(null)).toBe(false); - }); - - it("rejects_meeting_with_empty_fields", async () => { - const { hasStructuredVisionAlignmentMeeting } = await importMilestoneQuality(); - const meeting = { ...makeValidMeeting(), partner: "" }; - expect(hasStructuredVisionAlignmentMeeting(meeting)).toBe(false); - }); - - it("rejects_meeting_with_placeholder_fields", async () => { - const { hasStructuredVisionAlignmentMeeting } = await importMilestoneQuality(); - const meeting = { ...makeValidMeeting(), combatant: "not provided." }; - expect(hasStructuredVisionAlignmentMeeting(meeting)).toBe(false); - }); - - it("rejects_invalid_route", async () => { - const { hasStructuredVisionAlignmentMeeting } = await importMilestoneQuality(); - const meeting = { ...makeValidMeeting(), recommendedRoute: "executing" }; - expect(hasStructuredVisionAlignmentMeeting(meeting)).toBe(false); - }); - - it("accepts_researching_route", async () => { - const { hasStructuredVisionAlignmentMeeting } = await importMilestoneQuality(); - const meeting = { ...makeValidMeeting(), recommendedRoute: "researching" }; - expect(hasStructuredVisionAlignmentMeeting(meeting)).toBe(true); - }); - }); - - describe("inspectMilestoneRoadmapMarkdown depth warnings", () => { - const makeRoadmapMarkdown = (meeting) => { - const meetingSection = `## Vision Alignment Meeting - -### Trigger -${meeting.trigger} - -### Product Manager -${meeting.pm} - -### User Advocate -${meeting.userAdvocate} - -### Customer Panel -${meeting.customerPanel} - -### Business -${meeting.business} - -### Researcher -${meeting.researcher} - -### Delivery Lead -${meeting.deliveryLead} - -### Partner -${meeting.partner} - -### Combatant -${meeting.combatant} - -### Architect -${meeting.architect} - -### Moderator -${meeting.moderator} - -### Weighted Synthesis -${meeting.weightedSynthesis} - -### Confidence By Area -${meeting.confidenceByArea} - -### Recommended Route -${meeting.recommendedRoute} -`; - return `# M015: Test Milestone\n\n${meetingSection}\n## Slice Overview\n`; - }; - - it("flags_shallow_partner", async () => { - const { inspectMilestoneRoadmapMarkdown } = await importMilestoneQuality(); - const meeting = { ...makeValidMeeting(), partner: "Looks good" }; - const result = inspectMilestoneRoadmapMarkdown(makeRoadmapMarkdown(meeting)); - expect(result.issues).toContain( - "shallow partner review — cite specific evidence (source files, prior milestones, production incidents)", - ); - }); - - it("flags_shallow_combatant", async () => { - const { inspectMilestoneRoadmapMarkdown } = await importMilestoneQuality(); - const meeting = { ...makeValidMeeting(), combatant: "Seems fine" }; - const result = inspectMilestoneRoadmapMarkdown(makeRoadmapMarkdown(meeting)); - expect(result.issues).toContain( - "shallow combatant review — name specific risks with concrete failure scenarios", - ); - }); - - it("flags_shallow_architect", async () => { - const { inspectMilestoneRoadmapMarkdown } = await importMilestoneQuality(); - const meeting = { ...makeValidMeeting(), architect: "Agree" }; - const result = inspectMilestoneRoadmapMarkdown(makeRoadmapMarkdown(meeting)); - expect(result.issues).toContain( - "shallow architect review — name affected subsystems, coupling points, and dependency rationale", - ); - }); - - it("flags_shallow_researcher", async () => { - const { inspectMilestoneRoadmapMarkdown } = await importMilestoneQuality(); - const meeting = { ...makeValidMeeting(), researcher: "Looks reasonable" }; - const result = inspectMilestoneRoadmapMarkdown(makeRoadmapMarkdown(meeting)); - expect(result.issues).toContain( - "shallow researcher review — cite at least one external source or comparable system", - ); - }); - - it("flags_shallow_weighted_synthesis", async () => { - const { inspectMilestoneRoadmapMarkdown } = await importMilestoneQuality(); - const meeting = { ...makeValidMeeting(), weightedSynthesis: "Approved" }; - const result = inspectMilestoneRoadmapMarkdown(makeRoadmapMarkdown(meeting)); - expect(result.issues).toContain( - "shallow weighted synthesis — must identify the strongest additions, cuts, and sequencing changes", - ); - }); - - it("passes_with_purposeful_content", async () => { - const { inspectMilestoneRoadmapMarkdown } = await importMilestoneQuality(); - const result = inspectMilestoneRoadmapMarkdown(makeRoadmapMarkdown(makeValidMeeting())); - const shallowIssues = result.issues.filter((i) => i.startsWith("shallow")); - expect(shallowIssues).toHaveLength(0); - }); - - it("still_flags_missing_meeting_section", async () => { - const { inspectMilestoneRoadmapMarkdown } = await importMilestoneQuality(); - const content = "# M015: Test\n\nNo meeting here.\n"; - const result = inspectMilestoneRoadmapMarkdown(content); - expect(result.issues).toContain("missing vision alignment meeting"); - }); - - it("flags_missing_BUILD_PLAN_reference", async () => { - const { inspectMilestoneRoadmapMarkdown } = await importMilestoneQuality(); - // Content with no BUILD_PLAN reference anywhere - const meeting = makeValidMeeting(); - const content = makeRoadmapMarkdown(meeting).replaceAll("BUILD_PLAN", "XBLDPLAN"); - const result = inspectMilestoneRoadmapMarkdown(content); - expect(result.issues).toContain( - "missing BUILD_PLAN.md reference — new milestones should cite which BUILD_PLAN tier/item they implement (D015)", - ); - }); - - it("passes_with_BUILD_PLAN_reference_in_vision", async () => { - const { inspectMilestoneRoadmapMarkdown } = await importMilestoneQuality(); - // M012-style content that references BUILD_PLAN (makeValidMeeting has it in researcher) - const meeting = makeValidMeeting(); - const content = makeRoadmapMarkdown(meeting); - const result = inspectMilestoneRoadmapMarkdown(content); - const buildPlanIssues = result.issues.filter((i) => i.includes("BUILD_PLAN.md reference")); - expect(buildPlanIssues).toHaveLength(0); - }); - }); - - describe("getVisionAlignmentBlockingIssue", () => { - it("returns_null_for_valid_meeting", async () => { - const { getVisionAlignmentBlockingIssue } = await importMilestoneQuality(); - expect(getVisionAlignmentBlockingIssue(makeValidMeeting())).toBeNull(); - }); - - it("returns_issue_for_missing_fields", async () => { - const { getVisionAlignmentBlockingIssue } = await importMilestoneQuality(); - expect(getVisionAlignmentBlockingIssue(null)).toBe("missing vision alignment meeting"); - const meeting = { ...makeValidMeeting(), trigger: "" }; - expect(getVisionAlignmentBlockingIssue(meeting)).toBe("missing vision meeting trigger"); - }); - }); -}); diff --git a/src/resources/extensions/sf/tests/plan-quality-ceremony.test.ts b/src/resources/extensions/sf/tests/plan-quality-ceremony.test.ts index 82d80a5ba..1e024dfc6 100644 --- a/src/resources/extensions/sf/tests/plan-quality-ceremony.test.ts +++ b/src/resources/extensions/sf/tests/plan-quality-ceremony.test.ts @@ -10,7 +10,6 @@ import { describe, expect, it } from "vitest"; * Consumer: slice planning pipeline (plan-slice tool, plan quality inspection). */ -// Dynamic import to get fresh module state async function importPlanQuality() { return import("../plan-quality.js"); } @@ -26,7 +25,6 @@ describe("plan-quality ceremony depth", () => { it("accepts_present_fields_even_if_shallow", async () => { const { hasCompleteAdversarialReview } = await importPlanQuality(); - // Shallow but present — presence check passes expect(hasCompleteAdversarialReview({ partner: "Looks good", combatant: "Seems fine", @@ -116,6 +114,51 @@ ${architect} const result = inspectSlicePlanMarkdown(content); expect(result.issues).toContain("missing adversarial review"); }); + + it("flags_at_boundary_79_chars", async () => { + const { inspectSlicePlanMarkdown } = await importPlanQuality(); + const shortPartner = "a".repeat(79); + const content = makePlanMarkdown( + shortPartner, + "Risk 1: state machine refactor takes longer due to coupling. Risk 2: schema migration breaks parsers. Risk 3: latency overhead from synthesis gate. Mitigation: timebox.", + "Three subsystems affected: dispatch controller, planning artifacts, and verification gate. Coupling point: dispatch ↔ verification.", + ); + const result = inspectSlicePlanMarkdown(content); + expect(result.issues).toContain( + "shallow partner review — cite specific evidence (file paths, test gaps, prior learnings)", + ); + }); + + it("passes_at_boundary_80_chars", async () => { + const { inspectSlicePlanMarkdown } = await importPlanQuality(); + const longEnoughPartner = "a".repeat(80); + const content = makePlanMarkdown( + longEnoughPartner, + "Risk 1: state machine refactor takes longer due to coupling. Risk 2: schema migration breaks parsers. Risk 3: latency overhead from synthesis gate. Mitigation: timebox.", + "Three subsystems affected: dispatch controller, planning artifacts, and verification gate. Coupling point: dispatch ↔ verification.", + ); + const result = inspectSlicePlanMarkdown(content); + expect(result.issues).not.toContain( + "shallow partner review — cite specific evidence (file paths, test gaps, prior learnings)", + ); + }); + + it("accumulates_multiple_shallow_warnings", async () => { + const { inspectSlicePlanMarkdown } = await importPlanQuality(); + const content = makePlanMarkdown("Short", "Brief", "Yep"); + const result = inspectSlicePlanMarkdown(content); + expect(result.issues).toContain( + "shallow partner review — cite specific evidence (file paths, test gaps, prior learnings)", + ); + expect(result.issues).toContain( + "shallow combatant review — name specific risks with concrete failure scenarios", + ); + expect(result.issues).toContain( + "shallow architect review — name affected subsystems and coupling points", + ); + const shallowIssues = result.issues.filter((i) => i.startsWith("shallow")); + expect(shallowIssues).toHaveLength(3); + }); }); describe("hasStructuredPlanningMeeting", () => {