Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/application/interfaces/ILiberateService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,24 @@
* with structured memory notes.
*/

/**
* Progress update emitted during liberate processing.
*/
export interface ILiberateProgress {
/** Current phase of processing. */
readonly phase: 'triage' | 'processing' | 'complete';
/** Current commit index (1-based during 'processing'; 0 for triage/complete). */
readonly current: number;
Comment on lines +12 to +15
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify current semantics in progress docs.

LiberateService emits current = total on the complete phase, but the comment says 0 for triage/complete. Align the doc (or the emitter) to avoid ambiguity for consumers.

✏️ Suggested doc tweak
-  /** Current commit index (1-based during 'processing'; 0 for triage/complete). */
+  /** Current commit index (1-based during 'processing'; 0 for triage; equals total for complete). */
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/** Current phase of processing. */
readonly phase: 'triage' | 'processing' | 'complete';
/** Current commit index (1-based during 'processing'; 0 for triage/complete). */
readonly current: number;
/** Current phase of processing. */
readonly phase: 'triage' | 'processing' | 'complete';
/** Current commit index (1-based during 'processing'; 0 for triage; equals total for complete). */
readonly current: number;
🤖 Prompt for AI Agents
In `@src/application/interfaces/ILiberateService.ts` around lines 12 - 15, Update
the ILiberateService.docs for the current property to match runtime behavior:
state that current is 0 during 'triage', is a 1-based commit index during
'processing', and equals total when phase is 'complete' (instead of saying 0 for
complete); reference the ILiberateService interface and its phase and current
properties so consumers see the exact semantics.

/** Total high-interest commits to process. */
readonly total: number;
/** Current commit SHA (empty for triage/complete). */
readonly sha: string;
/** Current commit subject (empty for triage/complete). */
readonly subject: string;
/** Running total of facts extracted so far. */
readonly factsExtracted: number;
}

/**
* Options for liberate operation.
*/
Expand All @@ -21,6 +39,8 @@ export interface ILiberateOptions {
readonly cwd?: string;
/** Enable LLM enrichment (requires API key). */
readonly enrich?: boolean;
/** Optional progress callback for UI feedback. */
readonly onProgress?: (progress: ILiberateProgress) => void;
}

/**
Expand Down
8 changes: 8 additions & 0 deletions src/application/services/LiberateService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,14 @@ export class LiberateService implements ILiberateService {

this.logger?.debug('Triage complete', { total: triageResult.totalCommits, highInterest: triageResult.highInterest.length });

const highInterestTotal = triageResult.highInterest.length;
options?.onProgress?.({ phase: 'triage', current: 0, total: highInterestTotal, sha: '', subject: '', factsExtracted: 0 });

// Process each high-interest commit
let commitIndex = 0;
for (const scored of triageResult.highInterest) {
commitIndex++;
options?.onProgress?.({ phase: 'processing', current: commitIndex, total: highInterestTotal, sha: scored.commit.sha, subject: scored.commit.subject, factsExtracted: totalFactsExtracted });
const text = `${scored.commit.subject}\n${scored.commit.body}`.trim();
const heuristicMatches = extractPatternMatches(text);

Expand Down Expand Up @@ -150,6 +156,8 @@ export class LiberateService implements ILiberateService {
});
}

options?.onProgress?.({ phase: 'complete', current: highInterestTotal, total: highInterestTotal, sha: '', subject: '', factsExtracted: totalFactsExtracted });

const result: ILiberateResult = {
commitsScanned: triageResult.totalCommits,
commitsAnnotated: annotations.length,
Expand Down
15 changes: 11 additions & 4 deletions src/commands/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
} from './init-hooks';
import { buildMcpConfig } from './init-mcp';
import { createContainer } from '../infrastructure/di';
import { createStderrProgressHandler, liberateWithProgress } from './progress';

interface IInitCommandOptions {
yes?: boolean;
Expand Down Expand Up @@ -195,10 +196,16 @@ export async function initCommand(options: IInitCommandOptions, logger?: ILogger
const container = createContainer({ logger, scope: 'init', enrich: true });
const { liberateService } = container.cradle;

const result = await liberateService.liberate({
maxCommits: commitCount,
enrich: true,
});
const onProgress = createStderrProgressHandler();

const result = await liberateWithProgress(
() => liberateService.liberate({
maxCommits: commitCount,
enrich: true,
onProgress,
}),
onProgress,
);

console.log(
`Commits scanned: ${result.commitsScanned} | ` +
Expand Down
21 changes: 14 additions & 7 deletions src/commands/liberate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import { createContainer } from '../infrastructure/di';
import type { ILogger } from '../domain/interfaces/ILogger';
import { createStderrProgressHandler, liberateWithProgress } from './progress';

interface ILiberateCommandOptions {
since?: string;
Expand Down Expand Up @@ -48,13 +49,19 @@ export async function liberateCommand(options: ILiberateCommandOptions, logger?:
console.log('Dry run — no notes will be written.\n');
}

const result = await liberateService.liberate({
since,
maxCommits,
dryRun: options.dryRun,
threshold,
enrich: options.enrich,
});
const onProgress = createStderrProgressHandler();

const result = await liberateWithProgress(
() => liberateService.liberate({
since,
maxCommits,
dryRun: options.dryRun,
threshold,
enrich: options.enrich,
onProgress,
}),
onProgress,
);

console.log(`Commits scanned: ${result.commitsScanned}`);
console.log(`Commits annotated: ${result.commitsAnnotated}`);
Expand Down
58 changes: 58 additions & 0 deletions src/commands/progress.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/**
* Shared stderr progress handler for liberate operations.
*/

import type { ILiberateProgress } from '../application/interfaces/ILiberateService';

/**
* Create a progress callback that writes liberate progress to stderr.
* Uses in-place `\r` updates when stderr is a TTY, newline-based otherwise.
*/
export function createStderrProgressHandler(): (p: ILiberateProgress) => void {
const isTTY = process.stderr.isTTY;
let lastLineLength = 0;
let inPlaceStarted = false;

return (p: ILiberateProgress): void => {
if (p.phase === 'triage') {
process.stderr.write(`Found ${p.total} high-interest commits to analyze.\n`);
} else if (p.phase === 'processing') {
const sha = p.sha.slice(0, 7);
const subject = p.subject.length > 60 ? p.subject.slice(0, 57) + '...' : p.subject;
const line = ` [${p.current}/${p.total}] ${sha} ${subject} (${p.factsExtracted} facts)`;

if (isTTY) {
const padded = line.padEnd(lastLineLength, ' ');
lastLineLength = line.length;
inPlaceStarted = true;
process.stderr.write(`\r${padded}`);
} else {
process.stderr.write(`${line}\n`);
}
} else if (p.phase === 'complete') {
if (isTTY && inPlaceStarted) {
process.stderr.write('\n');
inPlaceStarted = false;
}
}
};
}

/**
* Wrap a liberate call so that if it throws after writing in-place progress,
* a trailing newline is still written to stderr.
*/
export async function liberateWithProgress<T>(
fn: () => Promise<T>,
onProgress: ReturnType<typeof createStderrProgressHandler>,
): Promise<T> {
try {
return await fn();
} finally {
// Ensure terminal is clean if progress was mid-line when error occurred.
// The 'complete' phase handler already writes \n, but if liberate threw
// before emitting 'complete', we need a fallback. We rely on the handler's
// internal state via one final 'complete' call.
onProgress({ phase: 'complete', current: 0, total: 0, sha: '', subject: '', factsExtracted: 0 });
}
}
48 changes: 48 additions & 0 deletions tests/unit/application/services/LiberateService.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, it, before, after } from 'node:test';
import assert from 'node:assert/strict';
import type { ILiberateProgress } from '../../../../src/application/interfaces/ILiberateService';
import { execFileSync } from 'child_process';
import { mkdtempSync, writeFileSync, rmSync } from 'fs';
import { join } from 'path';
Expand Down Expand Up @@ -93,5 +94,52 @@ describe('LiberateService', () => {
assert.equal(result.commitsAnnotated, 0);
assert.equal(result.factsExtracted, 0);
});

it('should call onProgress with triage, processing, and complete phases', async () => {
const events: ILiberateProgress[] = [];

await service.liberate({
cwd: repoDir,
dryRun: true,
threshold: 1,
onProgress: (p) => events.push({ ...p }),
});

assert.ok(events.length >= 2, 'should emit at least triage + complete');

// First event is triage
assert.equal(events[0].phase, 'triage');
assert.equal(events[0].current, 0);
assert.ok(events[0].total >= 0);

// Last event is complete
const last = events[events.length - 1];
assert.equal(last.phase, 'complete');

// All middle events are processing
const processingEvents = events.filter(e => e.phase === 'processing');
for (let i = 0; i < processingEvents.length; i++) {
assert.equal(processingEvents[i].current, i + 1, 'current should be 1-based');
assert.ok(processingEvents[i].sha.length > 0, 'sha should be non-empty');
assert.ok(processingEvents[i].subject.length > 0, 'subject should be non-empty');
}
});

it('should emit triage and complete even with zero high-interest commits', async () => {
const events: ILiberateProgress[] = [];

await service.liberate({
cwd: repoDir,
dryRun: true,
threshold: 100,
onProgress: (p) => events.push({ ...p }),
});

assert.equal(events.length, 2);
assert.equal(events[0].phase, 'triage');
assert.equal(events[0].total, 0);
assert.equal(events[1].phase, 'complete');
assert.equal(events[1].factsExtracted, 0);
});
Comment on lines +98 to +143
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Unit-test isolation: consider mocking Git/FS dependencies.

These new tests still exercise real git commands and filesystem via GitClient/MemoryRepository. For unit tests, prefer manual mocks or move this to tests/integration to keep unit tests isolated and fast.

As per coding guidelines: "tests/unit/**/*.{ts,tsx}: Create unit tests with manually mocked dependencies (no testing framework) in tests/unit/ directory".

🤖 Prompt for AI Agents
In `@tests/unit/application/services/LiberateService.test.ts` around lines 98 -
143, The tests in LiberateService.test.ts call service.liberate and indirectly
exercise real git/FS via GitClient/MemoryRepository; change them to be true unit
tests by mocking those dependencies (or move the file to tests/integration).
Replace real GitClient/MemoryRepository usage in the test setup with manual
mocks/stubs that implement the minimal behaviors used by
LiberateService.liberate (e.g., return controlled commit lists, shas, subjects,
and file contents) so ILiberateProgress events can be asserted
deterministically; ensure the test injects the mocked client into the service
instance (or the constructor/factory used by LiberateService) rather than
invoking real git commands. Also keep the same assertions on events
(triage/processing/complete) but driven from the mocked responses to keep the
test fast and isolated.

});
});