From 053730b9692bfe28b8ab000356597571620c9858 Mon Sep 17 00:00:00 2001 From: rafaelscosta Date: Thu, 7 May 2026 12:11:04 -0300 Subject: [PATCH] fix: preserve result aggregator falsy defaults (#611) --- .../core/execution/result-aggregator.js | 30 ++--- .aiox-core/data/entity-registry.yaml | 6 +- .aiox-core/install-manifest.yaml | 10 +- ...123.13-result-aggregator-falsy-defaults.md | 51 ++++++++ package-lock.json | 4 +- package.json | 2 +- tests/core/result-aggregator.test.js | 109 ++++++++++++++++++ 7 files changed, 187 insertions(+), 25 deletions(-) create mode 100644 docs/stories/epic-123/STORY-123.13-result-aggregator-falsy-defaults.md diff --git a/.aiox-core/core/execution/result-aggregator.js b/.aiox-core/core/execution/result-aggregator.js index 80956fb053..b0e6f51077 100644 --- a/.aiox-core/core/execution/result-aggregator.js +++ b/.aiox-core/core/execution/result-aggregator.js @@ -15,15 +15,15 @@ class ResultAggregator extends EventEmitter { super(); // Root path for reports - this.rootPath = config.rootPath || process.cwd(); - this.reportDir = config.reportDir || path.join(this.rootPath, 'plan'); + this.rootPath = config.rootPath ?? process.cwd(); + this.reportDir = config.reportDir ?? path.join(this.rootPath, 'plan'); // Conflict detection settings this.detectConflicts = config.detectConflicts !== false; // Aggregation history this.history = []; - this.maxHistory = config.maxHistory || 50; + this.maxHistory = config.maxHistory ?? 50; } /** @@ -36,8 +36,8 @@ class ResultAggregator extends EventEmitter { const aggregation = { id: `agg-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`, - waveIndex: waveResults.waveIndex || waveResults.wave, - startedAt: waveResults.startedAt || new Date().toISOString(), + waveIndex: waveResults.waveIndex ?? waveResults.wave, + startedAt: waveResults.startedAt ?? new Date().toISOString(), completedAt: new Date().toISOString(), tasks: [], conflicts: [], @@ -50,11 +50,13 @@ class ResultAggregator extends EventEmitter { for (const result of results) { aggregation.tasks.push({ taskId: result.taskId, - agentId: result.agentId || 'unknown', + agentId: result.agentId ?? 'unknown', success: result.success, - filesModified: result.filesModified || this.extractFilesFromOutput(result.output), - duration: result.duration || 0, - output: this.summarizeOutput(result.output || result.result?.output), + filesModified: Array.isArray(result.filesModified) + ? result.filesModified + : this.extractFilesFromOutput(result.output ?? result.result?.output), + duration: result.duration ?? 0, + output: this.summarizeOutput(result.output ?? result.result?.output), error: result.error, }); } @@ -366,7 +368,7 @@ class ResultAggregator extends EventEmitter { * @returns {Promise} - Report file path */ async generateReport(aggregation, filename = null) { - const reportName = filename || `wave-results-${aggregation.waveIndex || 'all'}.json`; + const reportName = filename || `wave-results-${aggregation.waveIndex ?? 'all'}.json`; const reportPath = path.join(this.reportDir, reportName); // Ensure directory exists @@ -394,7 +396,7 @@ class ResultAggregator extends EventEmitter { let md = '# Wave Results Report\n\n'; md += `> **Generated:** ${aggregation.completedAt}\n`; - md += `> **Success Rate:** ${metrics.successRate || metrics.overallSuccessRate}%\n\n`; + md += `> **Success Rate:** ${metrics.successRate ?? metrics.overallSuccessRate}%\n\n`; md += '## Summary\n\n'; md += '| Metric | Value |\n'; @@ -403,8 +405,8 @@ class ResultAggregator extends EventEmitter { md += `| Successful | ${metrics.successful} |\n`; md += `| Failed | ${metrics.failed} |\n`; md += `| Duration | ${Math.round(metrics.totalDuration / 1000)}s |\n`; - md += `| Conflicts | ${metrics.conflictCount || metrics.totalConflicts || 0} |\n`; - md += `| Files Modified | ${metrics.filesModified || 'N/A'} |\n\n`; + md += `| Conflicts | ${metrics.conflictCount ?? metrics.totalConflicts ?? 0} |\n`; + md += `| Files Modified | ${metrics.filesModified ?? 'N/A'} |\n\n`; // Tasks section if (aggregation.tasks && aggregation.tasks.length > 0) { @@ -414,7 +416,7 @@ class ResultAggregator extends EventEmitter { for (const task of aggregation.tasks) { const status = task.success ? '✅' : '❌'; - const duration = task.duration ? `${Math.round(task.duration / 1000)}s` : '-'; + const duration = task.duration != null ? `${Math.round(task.duration / 1000)}s` : '-'; md += `| ${task.taskId} | ${task.agentId} | ${status} | ${duration} |\n`; } md += '\n'; diff --git a/.aiox-core/data/entity-registry.yaml b/.aiox-core/data/entity-registry.yaml index 2ae99f1937..37826faa16 100644 --- a/.aiox-core/data/entity-registry.yaml +++ b/.aiox-core/data/entity-registry.yaml @@ -1,6 +1,6 @@ metadata: version: 1.0.0 - lastUpdated: '2026-05-07T14:31:05.482Z' + lastUpdated: '2026-05-07T15:02:59.792Z' entityCount: 746 checksumAlgorithm: sha256 resolutionRate: 100 @@ -8822,8 +8822,8 @@ entities: score: 0.4 constraints: [] extensionPoints: [] - checksum: sha256:fc662bbc649bf704a8f6e70d0203fab389c664a6b4b2932510f84c251ae26610 - lastVerified: '2026-03-11T00:48:55.878Z' + checksum: sha256:bcd102ca46e74d61af151aa7877921807c9bf5aba45bfba73f9f5c8cf714d8e2 + lastVerified: '2026-05-07T15:02:59.785Z' semantic-merge-engine: path: .aiox-core/core/execution/semantic-merge-engine.js layer: L1 diff --git a/.aiox-core/install-manifest.yaml b/.aiox-core/install-manifest.yaml index 9e9f49abdb..bd917b7c1b 100644 --- a/.aiox-core/install-manifest.yaml +++ b/.aiox-core/install-manifest.yaml @@ -7,8 +7,8 @@ # - SHA256 hashes for change detection # - File types for categorization # -version: 5.1.8 -generated_at: "2026-05-07T14:31:25.534Z" +version: 5.1.9 +generated_at: "2026-05-07T15:10:49.402Z" generator: scripts/generate-install-manifest.js file_count: 1103 files: @@ -429,9 +429,9 @@ files: type: core size: 9033 - path: core/execution/result-aggregator.js - hash: sha256:fc662bbc649bf704a8f6e70d0203fab389c664a6b4b2932510f84c251ae26610 + hash: sha256:bcd102ca46e74d61af151aa7877921807c9bf5aba45bfba73f9f5c8cf714d8e2 type: core - size: 14553 + size: 14643 - path: core/execution/semantic-merge-engine.js hash: sha256:26f2a057407cf114a0611944960a8e6d58d93c5e03abe905489e6b699cb98a75 type: core @@ -1229,7 +1229,7 @@ files: type: data size: 9590 - path: data/entity-registry.yaml - hash: sha256:277cd2b2973a8b33b7b512794106c6e5e8a5536ed8aecc3c269f7c8ae84c9cf0 + hash: sha256:16fcaa219a4c1364337ae2d534494fe4f058570ba6f3db797e373d5cd0d7e672 type: data size: 522368 - path: data/learned-patterns.yaml diff --git a/docs/stories/epic-123/STORY-123.13-result-aggregator-falsy-defaults.md b/docs/stories/epic-123/STORY-123.13-result-aggregator-falsy-defaults.md new file mode 100644 index 0000000000..7cda61b2ec --- /dev/null +++ b/docs/stories/epic-123/STORY-123.13-result-aggregator-falsy-defaults.md @@ -0,0 +1,51 @@ +# STORY-123.13: Preservar valores falsy válidos no ResultAggregator + +## Status + +Done + +## Story + +Como operador do executor paralelo, quero que o ResultAggregator preserve valores válidos como `waveIndex: 0`, `duration: 0`, `filesModified: 0` e strings vazias intencionais, para que relatórios, histórico e markdown não tratem dados reais como ausentes. + +## Acceptance Criteria + +- [x] AC1. `waveIndex: 0` é preservado durante `aggregate()` e no nome padrão do relatório. +- [x] AC2. Defaults usam fallback apenas para `null`/`undefined` quando valores falsy são válidos. +- [x] AC3. `filesModified` continua seguro: valores não-array não quebram a agregação e caem para extração por output. +- [x] AC4. Markdown renderiza métricas zero em vez de trocar para valores alternativos ou `N/A`. +- [x] AC5. Há regressões automatizadas cobrindo o comportamento. + +## Tasks + +- [x] Atualizar defaults falsy-safe em `.aiox-core/core/execution/result-aggregator.js`. +- [x] Adicionar regressões focadas em `tests/core/result-aggregator.test.js`. +- [x] Bump de patch para `@aiox-squads/core@5.1.9`. +- [x] Regenerar manifest antes do PR. + +## Dev Notes + +- Esta story substitui o escopo útil do PR histórico #611 sem reaproveitar o branch antigo, que estava com CI/manifest obsoleto e review pendente. +- O ajuste evita uma troca mecânica cega de `||` por `??`: campos que precisam ser array continuam normalizados antes do uso. + +## Validation + +- `node -c .aiox-core/core/execution/result-aggregator.js` -> PASS. +- `npm test -- tests/core/result-aggregator.test.js --runInBand` -> PASS, 1 suite / 36 tests. +- `npm test -- tests/core/result-aggregator.test.js tests/core/wave-executor.test.js tests/core/build-state-manager.test.js --runInBand --forceExit` -> PASS, 3 suites / 117 tests. +- `npm run validate:manifest` -> PASS. +- `npm run validate:publish` -> PASS. +- `npm run lint -- --quiet` -> PASS. +- `npm run typecheck` -> PASS. +- `git diff --check` -> PASS. +- `npm run test:ci` -> PASS, 315 suites / 7.846 tests, 149 skipped. + +## File List + +- `.aiox-core/core/execution/result-aggregator.js` +- `tests/core/result-aggregator.test.js` +- `docs/stories/epic-123/STORY-123.13-result-aggregator-falsy-defaults.md` +- `package.json` +- `package-lock.json` +- `.aiox-core/install-manifest.yaml` +- `.aiox-core/data/entity-registry.yaml` diff --git a/package-lock.json b/package-lock.json index 703e08157e..77eac6e1b6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@aiox-squads/core", - "version": "5.1.8", + "version": "5.1.9", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@aiox-squads/core", - "version": "5.1.8", + "version": "5.1.9", "license": "MIT", "workspaces": [ "packages/*" diff --git a/package.json b/package.json index bc92115b5d..afce7b8c48 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@aiox-squads/core", - "version": "5.1.8", + "version": "5.1.9", "description": "Synkra AIOX: AI-Orchestrated System for Full Stack Development - Core Framework", "bin": { "aiox": "bin/aiox.js", diff --git a/tests/core/result-aggregator.test.js b/tests/core/result-aggregator.test.js index a0590f4999..cb21b43477 100644 --- a/tests/core/result-aggregator.test.js +++ b/tests/core/result-aggregator.test.js @@ -43,6 +43,13 @@ describe('ResultAggregator', () => { expect(ra.maxHistory).toBe(10); }); + test('preserves falsy but intentional config values', () => { + const ra = new ResultAggregator({ rootPath: '', reportDir: '', maxHistory: 0 }); + expect(ra.rootPath).toBe(''); + expect(ra.reportDir).toBe(''); + expect(ra.maxHistory).toBe(0); + }); + test('extends EventEmitter', () => { const ra = new ResultAggregator(); expect(typeof ra.on).toBe('function'); @@ -145,6 +152,51 @@ describe('ResultAggregator', () => { expect(ra.history.length).toBe(1); }); + test('preserves wave zero and other falsy task values', async () => { + const ra = new ResultAggregator({ rootPath: tmpDir }); + const result = await ra.aggregate({ + waveIndex: 0, + startedAt: '', + results: [ + { + taskId: 't0', + agentId: '', + success: true, + duration: 0, + output: '', + result: { output: 'fallback should not be used' }, + filesModified: [], + }, + ], + }); + + expect(result.waveIndex).toBe(0); + expect(result.startedAt).toBe(''); + expect(result.tasks[0]).toMatchObject({ + agentId: '', + duration: 0, + output: '', + filesModified: [], + }); + }); + + test('falls back safely when filesModified is not an array', async () => { + const ra = new ResultAggregator({ rootPath: tmpDir }); + const result = await ra.aggregate({ + waveIndex: 1, + results: [ + { + taskId: 't1', + success: true, + filesModified: false, + output: 'Created `src/app.js`', + }, + ], + }); + + expect(result.tasks[0].filesModified).toEqual(['src/app.js']); + }); + test('trims history to maxHistory', async () => { const ra = new ResultAggregator({ rootPath: tmpDir, maxHistory: 2 }); await ra.aggregate({ waveIndex: 1, results: [] }); @@ -152,6 +204,12 @@ describe('ResultAggregator', () => { await ra.aggregate({ waveIndex: 3, results: [] }); expect(ra.history.length).toBe(2); }); + + test('allows maxHistory zero without retaining entries', async () => { + const ra = new ResultAggregator({ rootPath: tmpDir, maxHistory: 0 }); + await ra.aggregate({ waveIndex: 1, results: [] }); + expect(ra.history.length).toBe(0); + }); }); // ── aggregateAll ────────────────────────────────────────────────────── @@ -303,6 +361,30 @@ describe('ResultAggregator', () => { expect(fs.existsSync(reportPath)).toBe(true); expect(fs.existsSync(reportPath.replace('.json', '.md'))).toBe(true); }); + + test('uses wave zero in default report filename', async () => { + const reportDir = path.join(tmpDir, 'plan'); + const ra = new ResultAggregator({ reportDir }); + const agg = { + waveIndex: 0, + completedAt: new Date().toISOString(), + tasks: [], + conflicts: [], + warnings: [], + metrics: { + totalTasks: 0, + successful: 0, + failed: 0, + successRate: 100, + totalDuration: 0, + conflictCount: 0, + filesModified: 0, + }, + }; + + const reportPath = await ra.generateReport(agg); + expect(path.basename(reportPath)).toBe('wave-results-0.json'); + }); }); // ── formatMarkdown ──────────────────────────────────────────────────── @@ -335,6 +417,33 @@ describe('ResultAggregator', () => { expect(md).toContain('Conflicts'); expect(md).toContain('app.js'); }); + + test('renders zero metrics instead of falling back to alternate labels', () => { + const ra = new ResultAggregator(); + const agg = { + completedAt: new Date().toISOString(), + tasks: [{ taskId: 't0', agentId: '@dev', success: true, duration: 0 }], + conflicts: [], + warnings: [], + metrics: { + totalTasks: 1, + successful: 1, + failed: 0, + successRate: 0, + overallSuccessRate: 99, + totalDuration: 0, + conflictCount: 0, + totalConflicts: 12, + filesModified: 0, + }, + }; + + const md = ra.formatMarkdown(agg); + expect(md).toContain('> **Success Rate:** 0%'); + expect(md).toContain('| Conflicts | 0 |'); + expect(md).toContain('| Files Modified | 0 |'); + expect(md).toContain('| t0 | @dev | ✅ | 0s |'); + }); }); // ── History ───────────────────────────────────────────────────────────