Skip to content

test(execution): 28 testes para ResultAggregator (#52)#610

Open
nikolasdehor wants to merge 4 commits intoSynkraAI:mainfrom
nikolasdehor:test/result-aggregator-coverage
Open

test(execution): 28 testes para ResultAggregator (#52)#610
nikolasdehor wants to merge 4 commits intoSynkraAI:mainfrom
nikolasdehor:test/result-aggregator-coverage

Conversation

@nikolasdehor
Copy link
Contributor

@nikolasdehor nikolasdehor commented Mar 17, 2026

Resumo

Adiciona 28 testes unitarios para o modulo ResultAggregator sem cobertura.

Cobertura

Area Testes
Constructor 3
detectFileConflicts 3
assessConflictSeverity 8
suggestResolution 3
extractFilesFromOutput 3
aggregate 6
aggregateAll 2

Bug encontrado

waveIndex: waveResults.waveIndex || waveResults.wave usa || que trata 0 como falsy. Deveria usar ??.

Closes #52 (parcial)

Plano de teste

  • npx jest tests/core/execution/result-aggregator.test.js -- 28/28 passando

Summary by CodeRabbit

Release Notes

  • New Features

    • Added Reflection Engine enabling agents to learn from task outcomes, detect usage patterns, and generate strategic recommendations.
    • Agents can now receive context-enriched recommendations based on historical performance and patterns.
    • New analytics features including trend analysis and performance statistics by task type and outcome.
  • Tests

    • Added comprehensive test coverage for reflection and result aggregation systems.

gotchas-memory.js exporta named export { GotchasMemory, ... }, mas
ideation-engine.js atribuía o objeto module inteiro à variável.
Resultado: new GotchasMemory() lançava TypeError silenciado pelo
try/catch, e gotchasMemory ficava sempre null.

Corrige com destructuring: ({ GotchasMemory } = require(...))

Closes SynkraAI#517
Story 9.6 of Epic 9 (Persistent Memory Layer).
Phase 2 of Agent Immortality Protocol (SynkraAI#482).

The Reflection Engine enables agents to learn from past executions:
- Record execution reflections with outcome, strategy, duration
- Extract recurring patterns (success/failure clusters)
- Recommend strategies before similar tasks based on history
- Track performance trends per agent and task type
- Inject relevant reflections as context before execution
- Auto-prune stale reflections beyond retention window

44 unit tests covering all features including edge cases.
- Use nullish coalescing (??) instead of || for durationMs, lesson,
  context fields to correctly preserve falsy values like 0
- Add null check for p.tags in injectContext() to prevent TypeError
  when patterns lack tags
- Add _recomputePatterns() method that rebuilds patterns from remaining
  reflections after pruning
- Call _recomputePatterns() in prune() and after maxReflections eviction
  to keep patterns consistent with remaining data
- Copy reflection-engine.js to .aiox-core/core/memory/ for rebranded
  package path compatibility (keeps .aios-core version too)
Cobertura: constructor, detectFileConflicts, assessConflictSeverity
(8 padroes: critical/high/medium), suggestResolution, extractFilesFromOutput,
aggregate (single wave, conflitos, history, eventos), aggregateAll.
@github-actions github-actions bot added area: agents Agent system related area: workflows Workflow system related squad mcp type: test Test coverage and quality area: core Core framework (.aios-core/core/) area: installer Installer and setup (packages/installer/) area: synapse SYNAPSE context engine area: cli CLI tools (bin/, packages/aios-pro-cli/) area: pro Pro features (pro/) area: health-check Health check system area: docs Documentation (docs/) area: devops CI/CD, GitHub Actions (.github/) labels Mar 17, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 17, 2026

Walkthrough

This pull request introduces a new ReflectionEngine class for agent memory persistence, pattern detection, and strategy recommendations. It refactors GotchasMemory imports in three modules to use destructuring with explicit error handling, adds comprehensive test suites for ResultAggregator and ReflectionEngine, and updates the installation manifest with checksums.

Changes

Cohort / File(s) Summary
ReflectionEngine Implementation
.aios-core/core/memory/reflection-engine.js
New 538-line module providing reflection lifecycle management, pattern detection from similar reflections, strategy recommendations via weighted scoring, context injection enrichment, trend analysis with windowed success rates, and JSON persistence with schema versioning. Exports ReflectionEngine class plus Outcome, TaskType, Events, and CONFIG constants.
GotchasMemory Import Refactoring
.aiox-core/core/execution/context-injector.js, .aiox-core/core/execution/subagent-dispatcher.js, .aiox-core/core/ideation/ideation-engine.js
Converts direct require assignments to destructured imports for GotchasMemory; adds explicit error logging and null-assignment in catch blocks. Changes failure handling from silent to warning-logged.
Test Suites
tests/core/execution/result-aggregator.test.js, tests/core/memory/reflection-engine.test.js
New 298-line and 633-line test suites validating ResultAggregator conflict detection and resolution, and ReflectionEngine persistence, recommendations, pattern detection, context injection, trends, and statistics. Covers lifecycle, event emission, validation, and edge cases.
Installation Manifest
.aiox-core/install-manifest.yaml
Updated generated_at timestamp and sha256 hashes for multiple file entries across core execution, template, and data modules; reflects content changes from code modifications above.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant ReflectionEngine
    participant FileSystem
    participant EventEmitter

    rect rgba(100, 149, 237, 0.5)
    Note over Client,EventEmitter: recordReflection Flow
    Client->>ReflectionEngine: recordReflection(taskType, agentId, outcome, strategy)
    ReflectionEngine->>ReflectionEngine: validate required fields
    ReflectionEngine->>ReflectionEngine: assign ID & timestamp
    ReflectionEngine->>ReflectionEngine: add to reflections array
    ReflectionEngine->>ReflectionEngine: prune if exceeds maxReflections
    ReflectionEngine->>ReflectionEngine: detectPatterns from similar reflections
    ReflectionEngine->>EventEmitter: emit REFLECTION_RECORDED
    ReflectionEngine->>EventEmitter: emit PATTERN_DETECTED (if found)
    ReflectionEngine->>FileSystem: save() to reflections.json
    end

    rect rgba(144, 238, 144, 0.5)
    Note over Client,EventEmitter: getRecommendations Flow
    Client->>ReflectionEngine: getRecommendations(context, maxRecommendations)
    ReflectionEngine->>ReflectionEngine: filter reflections by taskType/agent/tags
    ReflectionEngine->>ReflectionEngine: score by outcome, tag overlap, recency
    ReflectionEngine->>ReflectionEngine: group and rank by strategy
    ReflectionEngine->>ReflectionEngine: compute aggregated metrics
    alt recommendations available
    ReflectionEngine->>EventEmitter: emit STRATEGY_RECOMMENDED
    end
    ReflectionEngine->>Client: return ranked recommendations
    end

    rect rgba(255, 165, 0, 0.5)
    Note over Client,EventEmitter: injectContext Flow
    Client->>ReflectionEngine: injectContext(taskContext)
    ReflectionEngine->>ReflectionEngine: getRecommendations
    ReflectionEngine->>ReflectionEngine: getTrends & relevant patterns
    ReflectionEngine->>Client: return enriched context with reflections
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Suggested labels

area: core, type: test

Suggested reviewers

  • Pedrovaleriolopez
🚥 Pre-merge checks | ✅ 2 | ❌ 3

❌ Failed checks (3 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title references ResultAggregator tests but the changeset includes ReflectionEngine tests, engine implementation, and GotchasMemory refactoring—not just ResultAggregator tests. Update title to reflect all major changes: e.g., 'test: Add ResultAggregator and ReflectionEngine tests with GotchasMemory refactoring' or split into focused PRs.
Out of Scope Changes check ⚠️ Warning PR includes GotchasMemory import refactoring in 3 files (context-injector, subagent-dispatcher, ideation-engine) and ReflectionEngine implementation (538 lines) beyond the stated ResultAggregator test objective. Separate implementation changes (ReflectionEngine, GotchasMemory refactoring) from test coverage PR to maintain scope clarity and facilitate independent review/testing.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR adds ResultAggregator tests (28 tests, 298 lines) and ReflectionEngine tests (633 lines) directly addressing issue #52's requirement to improve test coverage for execution and memory modules.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@vercel
Copy link

vercel bot commented Mar 17, 2026

@nikolasdehor is attempting to deploy a commit to the Pedro Valério Lopez's projects Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (1)
tests/core/execution/result-aggregator.test.js (1)

14-17: Use the repo's absolute import form for ResultAggregator.

This new suite introduces another relative require into the test tree, which makes the import brittle to file moves and breaks the repo import convention.

As per coding guidelines, **/*.{js,jsx,ts,tsx}: Use absolute imports instead of relative imports in all code.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/core/execution/result-aggregator.test.js` around lines 14 - 17, The
test imports ResultAggregator using a brittle relative path; change the require
for ResultAggregator to the repo's absolute import form (use the package/module
absolute path for core/execution/result-aggregator) so the test uses an absolute
require/import for ResultAggregator instead of
'../../../.aiox-core/core/execution/result-aggregator'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.aiox-core/core/ideation/ideation-engine.js:
- Around line 16-19: In the catch for loading GotchasMemory (the
require('../memory/gotchas-memory') block), make the error handling defensive by
logging a safe representation of the thrown value (e.g., use String(error) or
inspect the object rather than accessing error.message directly) and include the
full error object for context, and after the require succeeds validate the
export by checking typeof GotchasMemory === 'function' (or the expected type)
and if it’s not valid, log a clear warning including the invalid type/value and
set GotchasMemory = null as the safe fallback; update the catch and the
post-require validation around GotchasMemory to implement these checks and
improved logging.

In @.aiox-core/core/memory/reflection-engine.js:
- Around line 127-143: The save() method currently overwrites the file and can
drop concurrent writes; change save() (and use _getFilePath()) to perform a
read-merge-write with atomic replace: ensure the directory exists, if the file
exists read and JSON.parse it, merge its reflections and patterns with
this.reflections/this.patterns (e.g. upsert by reflection id/key and merge
arrays without losing entries), then write the merged JSON to a temporary file
(e.g. filePath + '.tmp') using fs.writeFileSync and fsync, and atomically
replace the target with fs.renameSync; this ensures that concurrent save()
callers merge the latest on-disk data instead of blindly overwriting and uses an
atomic rename to avoid partial writes.
- Around line 161-177: recordReflection currently assumes a well-formed payload
and can crash or corrupt analytics; before generating the entry in
recordReflection validate the input shape and types: reject null/undefined,
ensure required string fields taskType, agentId, outcome, strategy are present
and non-empty, enforce outcome/taskType against allowed enums (or a simple
pattern) to avoid poisoning analytics, ensure tags is an array (or coerce to []
if missing) and description is a string, and validate durationMs is a number
when provided; throw clear, descriptive errors from recordReflection when
validation fails so callers get immediate feedback and downstream methods like
_detectPatterns and getRecommendations receive only well-formed entries.

In @.aiox-core/install-manifest.yaml:
- Line 11: The regenerated manifest (.aiox-core/install-manifest.yaml) omitted
the newly added module core/memory/reflection-engine.js and did not bump
file_count; update the manifest generation logic so the repository scan includes
core/memory/reflection-engine.js (ensure the glob or scanner used by the
generator includes core/** and JS files), add the new filename to the files:
array, increment file_count accordingly, and rewrite generated_at with the
current timestamp so regenerations include the new file; reference the manifest
writer/serializer used in the generator to ensure it writes the updated files
list and count.

In `@tests/core/execution/result-aggregator.test.js`:
- Around line 225-230: The test is missing an assertion that a waveResults with
waveIndex === 0 is preserved; update the test that calls aggregate({ waveIndex:
0, results: [] }) to explicitly assert the stored entry has waveIndex 0 (e.g.,
assert agg.history[0].waveIndex === 0 or that agg.history contains an item with
waveIndex 0) so the implementation using waveResults.waveIndex ||
waveResults.wave fails if 0 is dropped; apply the same explicit-waveIndex-0
assertion to the corresponding event-emission test to cover that regression.

In `@tests/core/memory/reflection-engine.test.js`:
- Around line 10-11: The test imports the ReflectionEngine using a relative path
to `.aios-core`, violating the absolute-import guideline and inconsistent with
other tests; update the require call so it uses the canonical module path (e.g.
require('aiox-core/core/memory/reflection-engine')) and keep the existing
destructuring of ReflectionEngine (Outcome, TaskType, Events, CONFIG) intact so
the symbols import the same module from the correct package name.

---

Nitpick comments:
In `@tests/core/execution/result-aggregator.test.js`:
- Around line 14-17: The test imports ResultAggregator using a brittle relative
path; change the require for ResultAggregator to the repo's absolute import form
(use the package/module absolute path for core/execution/result-aggregator) so
the test uses an absolute require/import for ResultAggregator instead of
'../../../.aiox-core/core/execution/result-aggregator'.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 21fb17fe-4401-4236-8af0-699604c73236

📥 Commits

Reviewing files that changed from the base of the PR and between f74e3e7 and c1556c8.

📒 Files selected for processing (8)
  • .aios-core/core/memory/reflection-engine.js
  • .aiox-core/core/execution/context-injector.js
  • .aiox-core/core/execution/subagent-dispatcher.js
  • .aiox-core/core/ideation/ideation-engine.js
  • .aiox-core/core/memory/reflection-engine.js
  • .aiox-core/install-manifest.yaml
  • tests/core/execution/result-aggregator.test.js
  • tests/core/memory/reflection-engine.test.js

Comment on lines +16 to 19
({ GotchasMemory } = require('../memory/gotchas-memory'));
} catch (error) {
console.warn('[IdeationEngine] Failed to load GotchasMemory:', error.message);
GotchasMemory = null;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Locate and read the ideation-engine.js file to confirm the current code state
find . -name "ideation-engine.js" -type f

Repository: SynkraAI/aiox-core

Length of output: 107


🏁 Script executed:

# Check the memory module to understand the GotchasMemory export
find . -name "gotchas-memory.js" -type f

Repository: SynkraAI/aiox-core

Length of output: 104


🏁 Script executed:

# Look for test files related to ideation-engine
find . -path "*/test*" -name "*ideation*" -type f
find . -path "*/spec*" -name "*ideation*" -type f
find . -name "*.test.js" -o -name "*.spec.js" | grep -i ideation

Repository: SynkraAI/aiox-core

Length of output: 44


🏁 Script executed:

# Read ideation-engine.js to see the exact code at lines 16-19
cat -n ./.aiox-core/core/ideation/ideation-engine.js | head -30

Repository: SynkraAI/aiox-core

Length of output: 1085


🏁 Script executed:

# Read the GotchasMemory export definition
cat -n ./.aiox-core/core/memory/gotchas-memory.js | head -50

Repository: SynkraAI/aiox-core

Length of output: 2055


🏁 Script executed:

# Find and read context-injector.js to compare error handling pattern
find . -name "context-injector.js" -type f

Repository: SynkraAI/aiox-core

Length of output: 109


🏁 Script executed:

# Read context-injector.js to compare the error handling pattern
cat -n ./.aiox-core/core/execution/context-injector.js | grep -A 10 -B 2 "GotchasMemory"

Repository: SynkraAI/aiox-core

Length of output: 1373


🏁 Script executed:

# Check the module.exports at end of gotchas-memory.js
tail -30 ./.aiox-core/core/memory/gotchas-memory.js

Repository: SynkraAI/aiox-core

Length of output: 906


🏁 Script executed:

# Search for how GotchasMemory is used in ideation-engine.js
cat -n ./.aiox-core/core/ideation/ideation-engine.js | grep -A 5 -B 5 "GotchasMemory"

Repository: SynkraAI/aiox-core

Length of output: 1041


Add defensive validation to the GotchasMemory import.

The catch handler accesses error.message unsafely—any thrown value (including non-Error primitives) will fail. Additionally, there's no validation that the GotchasMemory export is actually a function before the fallback to null. While the current code gracefully degrades, a missing or malformed export goes silently undetected, which contradicts the guideline to ensure comprehensive error handling with proper context.

Suggested hardening
 try {
-  ({ GotchasMemory } = require('../memory/gotchas-memory'));
+  const loaded = require('../memory/gotchas-memory');
+  if (typeof loaded?.GotchasMemory !== 'function') {
+    throw new TypeError('GotchasMemory export is missing or invalid');
+  }
+  ({ GotchasMemory } = loaded);
 } catch (error) {
-  console.warn('[IdeationEngine] Failed to load GotchasMemory:', error.message);
+  const message = error instanceof Error ? error.message : String(error);
+  console.warn('[IdeationEngine] Failed to load GotchasMemory:', message);
   GotchasMemory = null;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.aiox-core/core/ideation/ideation-engine.js around lines 16 - 19, In the
catch for loading GotchasMemory (the require('../memory/gotchas-memory') block),
make the error handling defensive by logging a safe representation of the thrown
value (e.g., use String(error) or inspect the object rather than accessing
error.message directly) and include the full error object for context, and after
the require succeeds validate the export by checking typeof GotchasMemory ===
'function' (or the expected type) and if it’s not valid, log a clear warning
including the invalid type/value and set GotchasMemory = null as the safe
fallback; update the catch and the post-require validation around GotchasMemory
to implement these checks and improved logging.

Comment on lines +127 to +143
async save() {
const filePath = this._getFilePath();
const dir = path.dirname(filePath);

if (!fs.existsSync(dir)) {
fs.mkdirSync(dir, { recursive: true });
}

const data = {
schemaVersion: this.config.schemaVersion,
version: this.config.version,
savedAt: new Date().toISOString(),
reflections: this.reflections,
patterns: this.patterns,
};

fs.writeFileSync(filePath, JSON.stringify(data, null, 2), 'utf8');
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

save() will drop reflections under concurrent writers.

Each instance serializes its in-memory snapshot straight to disk with no lock, reload, or merge. Two agents doing load → recordReflection → save in parallel will race, and the later write can erase the earlier reflection set. This needs serialized/atomic merge semantics before it is safe as a shared persistence layer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.aiox-core/core/memory/reflection-engine.js around lines 127 - 143, The
save() method currently overwrites the file and can drop concurrent writes;
change save() (and use _getFilePath()) to perform a read-merge-write with atomic
replace: ensure the directory exists, if the file exists read and JSON.parse it,
merge its reflections and patterns with this.reflections/this.patterns (e.g.
upsert by reflection id/key and merge arrays without losing entries), then write
the merged JSON to a temporary file (e.g. filePath + '.tmp') using
fs.writeFileSync and fsync, and atomically replace the target with
fs.renameSync; this ensures that concurrent save() callers merge the latest
on-disk data instead of blindly overwriting and uses an atomic rename to avoid
partial writes.

Comment on lines +161 to +177
recordReflection(reflection) {
if (!reflection.taskType || !reflection.agentId || !reflection.outcome || !reflection.strategy) {
throw new Error('Required fields: taskType, agentId, outcome, strategy');
}

const entry = {
id: this._generateId(),
taskType: reflection.taskType,
agentId: reflection.agentId,
outcome: reflection.outcome,
strategy: reflection.strategy,
description: reflection.description || '',
tags: reflection.tags || [],
durationMs: reflection.durationMs ?? null,
lesson: reflection.lesson ?? null,
context: reflection.context ?? null,
createdAt: new Date().toISOString(),
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate the reflection payload before persisting it.

recordReflection(null) will throw before your own guard runs, unknown taskType/outcome values can poison analytics, and non-array tags will blow up later in _detectPatterns() / getRecommendations(). This is a public core API; reject bad shapes up front.

🛡️ Suggested validation hardening
   recordReflection(reflection) {
+    if (!reflection || typeof reflection !== 'object') {
+      throw new TypeError('reflection must be an object');
+    }
     if (!reflection.taskType || !reflection.agentId || !reflection.outcome || !reflection.strategy) {
       throw new Error('Required fields: taskType, agentId, outcome, strategy');
     }
+    if (!Object.values(TaskType).includes(reflection.taskType)) {
+      throw new Error(`Invalid taskType: ${reflection.taskType}`);
+    }
+    if (!Object.values(Outcome).includes(reflection.outcome)) {
+      throw new Error(`Invalid outcome: ${reflection.outcome}`);
+    }
+    if (reflection.tags != null && !Array.isArray(reflection.tags)) {
+      throw new Error('tags must be an array');
+    }
+    if (reflection.durationMs != null && !Number.isFinite(reflection.durationMs)) {
+      throw new Error('durationMs must be a finite number');
+    }
 
     const entry = {

As per coding guidelines, .aiox-core/core/**: Verify proper input validation on public API methods.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.aiox-core/core/memory/reflection-engine.js around lines 161 - 177,
recordReflection currently assumes a well-formed payload and can crash or
corrupt analytics; before generating the entry in recordReflection validate the
input shape and types: reject null/undefined, ensure required string fields
taskType, agentId, outcome, strategy are present and non-empty, enforce
outcome/taskType against allowed enums (or a simple pattern) to avoid poisoning
analytics, ensure tags is an array (or coerce to [] if missing) and description
is a string, and validate durationMs is a number when provided; throw clear,
descriptive errors from recordReflection when validation fails so callers get
immediate feedback and downstream methods like _detectPatterns and
getRecommendations receive only well-formed entries.

#
version: 5.0.3
generated_at: "2026-03-10T17:08:07.160Z"
generated_at: "2026-03-11T02:24:06.096Z"
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Regenerated manifest still omits core/memory/reflection-engine.js.

This PR adds .aiox-core/core/memory/reflection-engine.js, but the refreshed manifest never lists core/memory/reflection-engine.js under files: and file_count stays at 1089. Brownfield upgrades that rely on this manifest will miss the new module entirely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.aiox-core/install-manifest.yaml at line 11, The regenerated manifest
(.aiox-core/install-manifest.yaml) omitted the newly added module
core/memory/reflection-engine.js and did not bump file_count; update the
manifest generation logic so the repository scan includes
core/memory/reflection-engine.js (ensure the glob or scanner used by the
generator includes core/** and JS files), add the new filename to the files:
array, increment file_count accordingly, and rewrite generated_at with the
current timestamp so regenerations include the new file; reference the manifest
writer/serializer used in the generator to ensure it writes the updated files
list and count.

Comment on lines +225 to +230
it('deve adicionar ao history', async () => {
await agg.aggregate({ waveIndex: 0, results: [] });
await agg.aggregate({ waveIndex: 1, results: [] });

expect(agg.history).toHaveLength(2);
});
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add the missing waveIndex === 0 regression assertion.

These tests call aggregate({ waveIndex: 0, ... }), but never assert that 0 survives. With the current waveResults.waveIndex || waveResults.wave implementation, this suite still passes while the bug remains. The same blind spot also affects the event-emission case below.

🧪 Suggested assertion
   it('deve adicionar ao history', async () => {
-    await agg.aggregate({ waveIndex: 0, results: [] });
+    const first = await agg.aggregate({ waveIndex: 0, results: [] });
     await agg.aggregate({ waveIndex: 1, results: [] });
 
+    expect(first.waveIndex).toBe(0);
+    expect(agg.history[0].waveIndex).toBe(0);
     expect(agg.history).toHaveLength(2);
   });
📝 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
it('deve adicionar ao history', async () => {
await agg.aggregate({ waveIndex: 0, results: [] });
await agg.aggregate({ waveIndex: 1, results: [] });
expect(agg.history).toHaveLength(2);
});
it('deve adicionar ao history', async () => {
const first = await agg.aggregate({ waveIndex: 0, results: [] });
await agg.aggregate({ waveIndex: 1, results: [] });
expect(first.waveIndex).toBe(0);
expect(agg.history[0].waveIndex).toBe(0);
expect(agg.history).toHaveLength(2);
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/core/execution/result-aggregator.test.js` around lines 225 - 230, The
test is missing an assertion that a waveResults with waveIndex === 0 is
preserved; update the test that calls aggregate({ waveIndex: 0, results: [] })
to explicitly assert the stored entry has waveIndex 0 (e.g., assert
agg.history[0].waveIndex === 0 or that agg.history contains an item with
waveIndex 0) so the implementation using waveResults.waveIndex ||
waveResults.wave fails if 0 is dropped; apply the same explicit-waveIndex-0
assertion to the corresponding event-emission test to cover that regression.

Comment on lines +10 to +11
const ReflectionEngine = require('../../../.aios-core/core/memory/reflection-engine');
const { Outcome, TaskType, Events, CONFIG } = ReflectionEngine;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Explore project structure and root directory
cd /tmp && ls -la | head -20
git ls-files | grep -E '^\.(aios-core|aiox-core)' | head -20

Repository: SynkraAI/aiox-core

Length of output: 556


🏁 Script executed:

# Check for both directories
git ls-files | grep -E '(aios-core|aiox-core)' | head -30

Repository: SynkraAI/aiox-core

Length of output: 1321


🏁 Script executed:

# Check package.json for entry points and absolute import configuration
find . -maxdepth 2 -name 'package.json' -o -name 'tsconfig.json' -o -name '.babelrc*' -o -name 'jest.config.js' | head -10

Repository: SynkraAI/aiox-core

Length of output: 135


🏁 Script executed:

# Look at the test file itself to understand context
cat -n tests/core/memory/reflection-engine.test.js | head -30

Repository: SynkraAI/aiox-core

Length of output: 1143


🏁 Script executed:

# Check jest configuration
cat jest.config.js

Repository: SynkraAI/aiox-core

Length of output: 5632


🏁 Script executed:

# Check package.json for entry points and imports configuration
cat package.json | head -50

Repository: SynkraAI/aiox-core

Length of output: 1204


🏁 Script executed:

# Check tsconfig for path mappings
cat tsconfig.json

Repository: SynkraAI/aiox-core

Length of output: 1205


🏁 Script executed:

# Check how other test files import modules
find tests -name '*.test.js' -o -name '*.spec.js' | head -10 | xargs -I {} head -20 {}

Repository: SynkraAI/aiox-core

Length of output: 5539


Use absolute imports and point to the canonical module path.

This file violates the coding guideline requiring absolute imports for **/*.js files. Additionally, the import targets .aios-core while other test files use .aiox-core. Replace the relative import with an absolute path to the canonical location: require('aiox-core/core/memory/reflection-engine') (using the path alias from tsconfig.json), or align with other tests by using a relative import to .aiox-core consistently.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/core/memory/reflection-engine.test.js` around lines 10 - 11, The test
imports the ReflectionEngine using a relative path to `.aios-core`, violating
the absolute-import guideline and inconsistent with other tests; update the
require call so it uses the canonical module path (e.g.
require('aiox-core/core/memory/reflection-engine')) and keep the existing
destructuring of ReflectionEngine (Outcome, TaskType, Events, CONFIG) intact so
the symbols import the same module from the correct package name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: agents Agent system related area: cli CLI tools (bin/, packages/aios-pro-cli/) area: core Core framework (.aios-core/core/) area: devops CI/CD, GitHub Actions (.github/) area: docs Documentation (docs/) area: health-check Health check system area: installer Installer and setup (packages/installer/) area: pro Pro features (pro/) area: synapse SYNAPSE context engine area: workflows Workflow system related mcp squad type: test Test coverage and quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(coverage): Improve global test coverage after ADE implementation

1 participant