feat(execution): Predictive Pipeline + CodeRabbit review fixes#579
Open
rafaelscosta wants to merge 3 commits intoSynkraAI:mainfrom
Open
feat(execution): Predictive Pipeline + CodeRabbit review fixes#579rafaelscosta wants to merge 3 commits intoSynkraAI:mainfrom
rafaelscosta wants to merge 3 commits intoSynkraAI:mainfrom
Conversation
…ning Story 9.5 of Epic 9 (Persistent Memory Layer). Implements Phase 2 of the Agent Immortality Protocol (SynkraAI#482) — Persistence layer. Features: - Record decisions with context, rationale, and alternatives - Track outcomes (success/partial/failure) with confidence scoring - Auto-detect categories from description keywords - Find relevant past decisions for context injection (AC7) - Pattern detection across recurring decisions (AC9) - Time-based confidence decay for relevance scoring - Persistence to .aiox/decisions.json 37 unit tests covering all features.
…ados de tasks Pipeline preditivo que estima resultados antes da execução usando padrões históricos. Implementa k-NN ponderado com vetores de features, EWMA para estimativa de duração, detecção de anomalias, avaliação de risco e motor de recomendação de agentes/estratégias. 89 testes unitários cobrindo todos os cenários.
…ion, atomic writes - Fix _enqueueWrite to rethrow errors after emitting, keeping the chain rejected - Add _recalculateModelStats after auto-prune splice to keep stats consistent - Sanitize numeric features with Number.isFinite guards in _extractFeatures - Honor minSamplesForPrediction threshold in _stagePredict - Reverse durationValues before EWMA so highest-similarity samples get largest weight - Add _ensureLoaded guard to recordDecision and save in decision-memory - Use atomicWriteSync instead of fs.writeFileSync in decision-memory save - Improve test assertions: verify unique IDs in concurrent test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
rafaelscosta
requested review from
Pedrovaleriolopez and
oalanicolas
as code owners
|
@rafaelscosta 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. |
4 tasks
WalkthroughThis PR introduces two new core modules for agentic decision-making: PredictivePipeline for forecasting task outcomes using historical data and k-NN similarity matching, and DecisionMemory for persistent cross-session decision recording and pattern detection. Includes a compatibility wrapper, comprehensive test suites, and manifest updates. Changes
Sequence DiagramssequenceDiagram
actor Client
participant Pipeline as PredictivePipeline
participant Persist as Persistence Layer
Client->>Pipeline: predict(taskSpec) or recordOutcome(outcome)
activate Pipeline
Pipeline->>Pipeline: 1. Preprocess: extract features
Pipeline->>Pipeline: 2. Match: compute similarity, find k-NN
Pipeline->>Pipeline: 3. Predict: weighted metrics, EWMA, estimates
Pipeline->>Pipeline: 4. Score: confidence, anomaly detection
Pipeline->>Pipeline: 5. Recommend: best agent/strategy
Pipeline->>Persist: save outcomes (lazy, sequenced writes)
Persist-->>Pipeline: persisted
Pipeline-->>Client: prediction result + events
deactivate Pipeline
sequenceDiagram
actor Client
participant Memory as DecisionMemory
participant Persist as Persistence Layer
Client->>Memory: recordDecision(spec) or findRelevant(taskSpec)
activate Memory
Memory->>Memory: auto-detect category & extract keywords
Memory->>Persist: save decision (atomic write, schema versioned)
Persist-->>Memory: persisted
Memory->>Memory: check pattern threshold
alt Pattern detected
Memory->>Memory: emit PATTERN_DETECTED event
end
Memory-->>Client: result + events (DECISION_RECORDED, PATTERN_DETECTED)
deactivate Memory
Client->>Memory: getRelevant(taskSpec)
activate Memory
Memory->>Memory: keyword similarity + time decay scoring
Memory->>Memory: filter by category, success rate, confidence
Memory-->>Client: sorted relevant decisions
deactivate Memory
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Welcome to aiox-core! Thanks for your first pull request.
What happens next?
- Automated checks will run on your PR
- A maintainer will review your changes
- Once approved, we'll merge your contribution!
PR Checklist:
- Tests pass (
npm test) - Linting passes (
npm run lint) - Commit messages follow Conventional Commits
Thanks for contributing!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
tests/core/execution/predictive-pipeline.test.js (2)
12-17: Use the repo’s absolute import form here.This relative
require(...)makes the test brittle to file moves and bypasses the project import convention. Switch it to the repository’s absolute module path.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/predictive-pipeline.test.js` around lines 12 - 17, The test imports core symbols using a brittle relative path; update the require call that pulls in PredictivePipeline, PipelineStage, RiskLevel, and DEFAULTS to use the repository’s absolute module path for the core execution predictive-pipeline module (follow the project's absolute import convention) so tests use the canonical module entry rather than a relative file path; locate the line importing those symbols and replace the relative require with the repo's absolute module specifier for the same module.
250-263: Assert the derived stats after auto-prune, not just the array length.The regression fixed in this PR was stale
taskType/agent/strategyaggregates after pruning. This test still goes green ifoutcomesis trimmed to 5 while those derived counters remain wrong. Seed at least one task/agent/strategy that should be fully pruned and assertgetStats()drops it too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/core/execution/predictive-pipeline.test.js` around lines 250 - 263, The test currently only checks outcomes length but not that derived aggregates were updated after pruning; update the test that creates PredictivePipeline (small) to seed outcomes with at least one unique taskType/agent/strategy that will be completely pruned when maxOutcomes is exceeded (use recordOutcome to create multiple entries so older entries for that task/agent/strategy are removed), then call getStats() and assert that the returned derived aggregates (the taskType/agent/strategy counts or lists provided by getStats()) no longer include the pruned task/agent/strategy; ensure you reference the same PredictivePipeline instance (small), use recordOutcome to insert the distinguishing entries, and assert on getStats() fields rather than only stats.outcomes length.
🤖 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/execution/predictive-pipeline.js:
- Around line 697-703: findSimilarTasks and assessRisk call _extractFeatures
directly and will throw a TypeError on null/undefined input; add an explicit
nullish guard at the start of each public method (findSimilarTasks and
assessRisk) to validate taskSpec (e.g., if (taskSpec == null) throw new
TypeError("taskSpec is required")); this ensures callers get a clear validation
error instead of a property-access crash and keeps _extractFeatures unchanged.
- Around line 300-314: Summary: recordOutcome() currently mutates in-memory
state before persistence, causing double-counting/inconsistent snapshots when
persistence fails; serialize the whole logical transaction and roll back on
failure. Fix: wrap the entire critical section in a mutex/serial lock (e.g.,
add/use this._transactionLock or a small async lock around recordOutcome),
perform the in-memory updates (push to this._outcomes, increment
this._stats.outcomesRecorded, call this._updateModelStats, prune via
this._recalculateModelStats when > this.maxOutcomes), then call
this._persistOutcomes and this._persistModel while still holding the lock; if
either persist rejects, revert the in-memory changes (pop the pushed records,
restore previous stats/model state) before releasing the lock and rethrow the
error; ensure the lock/serialization also prevents concurrent recordOutcome runs
to avoid race conditions.
- Around line 529-531: The prediction stage incorrectly uses `|| 3` which treats
an explicit minSamplesForPrediction = 0 as falsy and falls back to 3; update the
check inside the `_runStage(PipelineStage.PREDICT, ...)` callback so it uses a
nullish-coalescing style default (i.e., respect 0) when reading
`this.minSamplesForPrediction` before comparing to `neighbors.length`, and call
`_defaultPrediction(features)` only when the resolved minimum is >
neighbors.length.
In @.aiox-core/core/memory/decision-memory.js:
- Around line 252-254: updateOutcome currently reads this.decisions without
ensuring persisted decisions are loaded, causing valid IDs to return null for
fresh DecisionMemory instances; modify updateOutcome to await this.load() (same
guard used in recordDecision / save) before searching this.decisions, then
proceed to find the decision by id and update outcome/notes and call this.save()
as appropriate so cross-session behavior matches recordDecision/save.
- Around line 508-521: The recommendation for a new pattern is incorrectly set
to the "underperformed" message when all similar decisions are still
Outcome.PENDING because successCount and failureCount are both zero; update the
logic in the block that builds the pattern object (where similar, outcomes,
successCount, failureCount are computed and recommendation is assigned) to
detect the case outcomes.length === 0 and set a neutral recommendation like
"Insufficient resolved outcomes to recommend; monitor for more data." instead of
choosing success or failure branches.
- Around line 147-150: The loader currently only checks data.schemaVersion
before assigning this.decisions and this.patterns, which allows non-array types
to slip through and later crash; update the block that handles schemaVersion
(the code referencing schemaVersion, this.decisions, this.patterns) to validate
that data.decisions and data.patterns are arrays via Array.isArray; if either is
not an array, treat the file as corrupted by logging/warning (use the existing
logger if available), set the offending property to an empty array
(this.decisions = [] / this.patterns = []), and persist or mark the state
accordingly so downstream array-only operations are safe.
In `@tests/core/execution/predictive-pipeline.test.js`:
- Around line 324-335: Remove the test's registration of the spy on the shared
fixture "pipeline" and only attach it to the instance under test "p" (i.e., call
p.on('high-risk-detected', spy) only), then tighten the assertion to assert the
exact call count from the instance under test (e.g.,
expect(spy).toHaveBeenCalledTimes(1) or expect(spy.mock.calls.length).toBe(1))
after calling p.predict(...); this ensures only emissions from
PredictivePipeline p.predict() are measured.
---
Nitpick comments:
In `@tests/core/execution/predictive-pipeline.test.js`:
- Around line 12-17: The test imports core symbols using a brittle relative
path; update the require call that pulls in PredictivePipeline, PipelineStage,
RiskLevel, and DEFAULTS to use the repository’s absolute module path for the
core execution predictive-pipeline module (follow the project's absolute import
convention) so tests use the canonical module entry rather than a relative file
path; locate the line importing those symbols and replace the relative require
with the repo's absolute module specifier for the same module.
- Around line 250-263: The test currently only checks outcomes length but not
that derived aggregates were updated after pruning; update the test that creates
PredictivePipeline (small) to seed outcomes with at least one unique
taskType/agent/strategy that will be completely pruned when maxOutcomes is
exceeded (use recordOutcome to create multiple entries so older entries for that
task/agent/strategy are removed), then call getStats() and assert that the
returned derived aggregates (the taskType/agent/strategy counts or lists
provided by getStats()) no longer include the pruned task/agent/strategy; ensure
you reference the same PredictivePipeline instance (small), use recordOutcome to
insert the distinguishing entries, and assert on getStats() fields rather than
only stats.outcomes length.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8fdd3a3a-e9c6-4b49-be5d-b101c6803f54
📒 Files selected for processing (6)
.aios-core/core/execution/predictive-pipeline.js.aiox-core/core/execution/predictive-pipeline.js.aiox-core/core/memory/decision-memory.js.aiox-core/install-manifest.yamltests/core/execution/predictive-pipeline.test.jstests/core/memory/decision-memory.test.js
Comment on lines
+300
to
+314
| this._outcomes.push(record); | ||
| this._stats.outcomesRecorded++; | ||
|
|
||
| // Update model stats | ||
| this._updateModelStats(record); | ||
|
|
||
| // Auto-prune if exceeding max | ||
| if (this._outcomes.length > this.maxOutcomes) { | ||
| const excess = this._outcomes.length - this.maxOutcomes; | ||
| this._outcomes.splice(0, excess); | ||
| this._recalculateModelStats(); | ||
| } | ||
|
|
||
| await this._persistOutcomes(); | ||
| await this._persistModel(); |
There was a problem hiding this comment.
Serialize the whole recordOutcome() transaction, not just the file writes.
If either persistence step fails, the promise rejects after _outcomes, _model, and _stats have already been changed. A retry will double-count the same logical outcome, and a first-write/second-write split can leave disk snapshots inconsistent on the next load.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.aiox-core/core/execution/predictive-pipeline.js around lines 300 - 314,
Summary: recordOutcome() currently mutates in-memory state before persistence,
causing double-counting/inconsistent snapshots when persistence fails; serialize
the whole logical transaction and roll back on failure. Fix: wrap the entire
critical section in a mutex/serial lock (e.g., add/use this._transactionLock or
a small async lock around recordOutcome), perform the in-memory updates (push to
this._outcomes, increment this._stats.outcomesRecorded, call
this._updateModelStats, prune via this._recalculateModelStats when >
this.maxOutcomes), then call this._persistOutcomes and this._persistModel while
still holding the lock; if either persist rejects, revert the in-memory changes
(pop the pushed records, restore previous stats/model state) before releasing
the lock and rethrow the error; ensure the lock/serialization also prevents
concurrent recordOutcome runs to avoid race conditions.
Comment on lines
+529
to
+531
| return this._runStage(PipelineStage.PREDICT, () => { | ||
| if (neighbors.length < (this.minSamplesForPrediction || 3)) { | ||
| return this._defaultPrediction(features); |
There was a problem hiding this comment.
Respect an explicit minSamplesForPrediction: 0.
The constructor preserves 0 via ??, but || 3 converts it back to 3 here. Any caller that intentionally disables the sample threshold still gets default predictions.
Suggested fix
- if (neighbors.length < (this.minSamplesForPrediction || 3)) {
+ if (neighbors.length < this.minSamplesForPrediction) {
return this._defaultPrediction(features);
}📝 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
| return this._runStage(PipelineStage.PREDICT, () => { | |
| if (neighbors.length < (this.minSamplesForPrediction || 3)) { | |
| return this._defaultPrediction(features); | |
| return this._runStage(PipelineStage.PREDICT, () => { | |
| if (neighbors.length < this.minSamplesForPrediction) { | |
| return this._defaultPrediction(features); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.aiox-core/core/execution/predictive-pipeline.js around lines 529 - 531, The
prediction stage incorrectly uses `|| 3` which treats an explicit
minSamplesForPrediction = 0 as falsy and falls back to 3; update the check
inside the `_runStage(PipelineStage.PREDICT, ...)` callback so it uses a
nullish-coalescing style default (i.e., respect 0) when reading
`this.minSamplesForPrediction` before comparing to `neighbors.length`, and call
`_defaultPrediction(features)` only when the resolved minimum is >
neighbors.length.
Comment on lines
+697
to
+703
| findSimilarTasks(taskSpec, opts = {}) { | ||
| this._ensureLoaded(); | ||
|
|
||
| const limit = opts.limit ?? 10; | ||
| const minSimilarity = opts.minSimilarity ?? 0; | ||
| const features = this._extractFeatures(taskSpec); | ||
|
|
There was a problem hiding this comment.
Guard nullish taskSpec in the public read APIs.
findSimilarTasks() and assessRisk() call _extractFeatures() directly, so null / undefined inputs currently explode with a property-access TypeError instead of a targeted validation error.
Suggested fix
findSimilarTasks(taskSpec, opts = {}) {
this._ensureLoaded();
+ if (!taskSpec || typeof taskSpec !== 'object') {
+ throw new Error('taskSpec must be an object');
+ }
const limit = opts.limit ?? 10;
const minSimilarity = opts.minSimilarity ?? 0;
const features = this._extractFeatures(taskSpec);
@@
assessRisk(taskSpec) {
this._ensureLoaded();
+ if (!taskSpec || typeof taskSpec !== 'object') {
+ throw new Error('taskSpec must be an object');
+ }
const features = this._extractFeatures(taskSpec);
const neighbors = this._stageMatch(features);Also applies to: 747-750
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.aiox-core/core/execution/predictive-pipeline.js around lines 697 - 703,
findSimilarTasks and assessRisk call _extractFeatures directly and will throw a
TypeError on null/undefined input; add an explicit nullish guard at the start of
each public method (findSimilarTasks and assessRisk) to validate taskSpec (e.g.,
if (taskSpec == null) throw new TypeError("taskSpec is required")); this ensures
callers get a clear validation error instead of a property-access crash and
keeps _extractFeatures unchanged.
Comment on lines
+147
to
+150
| if (data.schemaVersion === this.config.schemaVersion) { | ||
| this.decisions = data.decisions || []; | ||
| this.patterns = data.patterns || []; | ||
| } |
There was a problem hiding this comment.
Treat non-array persisted collections as corruption.
schemaVersion is the only shape check here. If decisions or patterns is an object/string in an otherwise matching file, later calls hit array-only operations and crash.
Suggested fix
- if (data.schemaVersion === this.config.schemaVersion) {
- this.decisions = data.decisions || [];
- this.patterns = data.patterns || [];
- }
+ if (data.schemaVersion === this.config.schemaVersion) {
+ this.decisions = Array.isArray(data.decisions) ? data.decisions : [];
+ this.patterns = Array.isArray(data.patterns) ? data.patterns : [];
+ }📝 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
| if (data.schemaVersion === this.config.schemaVersion) { | |
| this.decisions = data.decisions || []; | |
| this.patterns = data.patterns || []; | |
| } | |
| if (data.schemaVersion === this.config.schemaVersion) { | |
| this.decisions = Array.isArray(data.decisions) ? data.decisions : []; | |
| this.patterns = Array.isArray(data.patterns) ? data.patterns : []; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.aiox-core/core/memory/decision-memory.js around lines 147 - 150, The loader
currently only checks data.schemaVersion before assigning this.decisions and
this.patterns, which allows non-array types to slip through and later crash;
update the block that handles schemaVersion (the code referencing schemaVersion,
this.decisions, this.patterns) to validate that data.decisions and data.patterns
are arrays via Array.isArray; if either is not an array, treat the file as
corrupted by logging/warning (use the existing logger if available), set the
offending property to an empty array (this.decisions = [] / this.patterns = []),
and persist or mark the state accordingly so downstream array-only operations
are safe.
Comment on lines
+252
to
+254
| updateOutcome(decisionId, outcome, notes = '') { | ||
| const decision = this.decisions.find(d => d.id === decisionId); | ||
| if (!decision) return null; |
There was a problem hiding this comment.
Lazy-load before looking up the decision ID.
A fresh DecisionMemory instance with an existing .aiox/decisions.json will return null for valid IDs here until callers remember to await load() manually. This mutator needs the same load guard as recordDecision() / save() to preserve the cross-session behavior the module is adding.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.aiox-core/core/memory/decision-memory.js around lines 252 - 254,
updateOutcome currently reads this.decisions without ensuring persisted
decisions are loaded, causing valid IDs to return null for fresh DecisionMemory
instances; modify updateOutcome to await this.load() (same guard used in
recordDecision / save) before searching this.decisions, then proceed to find the
decision by id and update outcome/notes and call this.save() as appropriate so
cross-session behavior matches recordDecision/save.
Comment on lines
+508
to
+521
| if (similar.length >= this.config.patternThreshold - 1) { | ||
| const outcomes = similar.map(d => d.outcome).filter(o => o !== Outcome.PENDING); | ||
| const successCount = outcomes.filter(o => o === Outcome.SUCCESS).length; | ||
| const failureCount = outcomes.filter(o => o === Outcome.FAILURE).length; | ||
|
|
||
| const pattern = { | ||
| id: `pattern-${this.patterns.length + 1}`, | ||
| category: newDecision.category, | ||
| description: `Recurring ${newDecision.category} decision: "${newDecision.description}"`, | ||
| occurrences: similar.length + 1, | ||
| successRate: outcomes.length > 0 ? successCount / outcomes.length : 0, | ||
| recommendation: successCount > failureCount | ||
| ? 'This approach has historically worked well. Consider reusing.' | ||
| : 'This approach has historically underperformed. Consider alternatives.', |
There was a problem hiding this comment.
Use a neutral recommendation when no resolved outcomes exist yet.
When every similar decision is still pending, successCount and failureCount are both 0, so this falls into the "underperformed" branch and emits misleading advice with zero evidence.
Suggested fix
- recommendation: successCount > failureCount
- ? 'This approach has historically worked well. Consider reusing.'
- : 'This approach has historically underperformed. Consider alternatives.',
+ recommendation: outcomes.length === 0
+ ? 'Pattern observed, but no completed outcomes exist yet.'
+ : successCount > failureCount
+ ? 'This approach has historically worked well. Consider reusing.'
+ : 'This approach has historically underperformed. Consider alternatives.',📝 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
| if (similar.length >= this.config.patternThreshold - 1) { | |
| const outcomes = similar.map(d => d.outcome).filter(o => o !== Outcome.PENDING); | |
| const successCount = outcomes.filter(o => o === Outcome.SUCCESS).length; | |
| const failureCount = outcomes.filter(o => o === Outcome.FAILURE).length; | |
| const pattern = { | |
| id: `pattern-${this.patterns.length + 1}`, | |
| category: newDecision.category, | |
| description: `Recurring ${newDecision.category} decision: "${newDecision.description}"`, | |
| occurrences: similar.length + 1, | |
| successRate: outcomes.length > 0 ? successCount / outcomes.length : 0, | |
| recommendation: successCount > failureCount | |
| ? 'This approach has historically worked well. Consider reusing.' | |
| : 'This approach has historically underperformed. Consider alternatives.', | |
| if (similar.length >= this.config.patternThreshold - 1) { | |
| const outcomes = similar.map(d => d.outcome).filter(o => o !== Outcome.PENDING); | |
| const successCount = outcomes.filter(o => o === Outcome.SUCCESS).length; | |
| const failureCount = outcomes.filter(o => o === Outcome.FAILURE).length; | |
| const pattern = { | |
| id: `pattern-${this.patterns.length + 1}`, | |
| category: newDecision.category, | |
| description: `Recurring ${newDecision.category} decision: "${newDecision.description}"`, | |
| occurrences: similar.length + 1, | |
| successRate: outcomes.length > 0 ? successCount / outcomes.length : 0, | |
| recommendation: outcomes.length === 0 | |
| ? 'Pattern observed, but no completed outcomes exist yet.' | |
| : successCount > failureCount | |
| ? 'This approach has historically worked well. Consider reusing.' | |
| : 'This approach has historically underperformed. Consider alternatives.', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.aiox-core/core/memory/decision-memory.js around lines 508 - 521, The
recommendation for a new pattern is incorrectly set to the "underperformed"
message when all similar decisions are still Outcome.PENDING because
successCount and failureCount are both zero; update the logic in the block that
builds the pattern object (where similar, outcomes, successCount, failureCount
are computed and recommendation is assigned) to detect the case outcomes.length
=== 0 and set a neutral recommendation like "Insufficient resolved outcomes to
recommend; monitor for more data." instead of choosing success or failure
branches.
Comment on lines
+324
to
+335
| it('should emit high-risk-detected for risky tasks', () => { | ||
| // No history → low confidence → potentially high risk | ||
| const spy = jest.fn(); | ||
| pipeline.on('high-risk-detected', spy); | ||
|
|
||
| // Predict with no history, anomaly threshold set low | ||
| const p = new PredictivePipeline(tmpDir, { highRiskThreshold: 0.3 }); | ||
| p.on('high-risk-detected', spy); | ||
| p.predict({ taskType: 'never-seen-before' }); | ||
|
|
||
| // Should have been called at least once (either from pipeline or p) | ||
| expect(spy.mock.calls.length).toBeGreaterThanOrEqual(1); |
There was a problem hiding this comment.
Listen only on the instance under test.
Registering the same spy on both pipeline and p makes this assertion non-specific; a future emission from the shared fixture could satisfy it even if p.predict() stops emitting. Attach the spy only to p and assert the exact call count.
Suggested tightening
it('should emit high-risk-detected for risky tasks', () => {
- // No history → low confidence → potentially high risk
const spy = jest.fn();
- pipeline.on('high-risk-detected', spy);
-
- // Predict with no history, anomaly threshold set low
const p = new PredictivePipeline(tmpDir, { highRiskThreshold: 0.3 });
p.on('high-risk-detected', spy);
p.predict({ taskType: 'never-seen-before' });
-
- // Should have been called at least once (either from pipeline or p)
- expect(spy.mock.calls.length).toBeGreaterThanOrEqual(1);
+ expect(spy).toHaveBeenCalledTimes(1);
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/core/execution/predictive-pipeline.test.js` around lines 324 - 335,
Remove the test's registration of the spy on the shared fixture "pipeline" and
only attach it to the instance under test "p" (i.e., call
p.on('high-risk-detected', spy) only), then tighten the assertion to assert the
exact call count from the instance under test (e.g.,
expect(spy).toHaveBeenCalledTimes(1) or expect(spy.mock.calls.length).toBe(1))
after calling p.predict(...); this ensures only emissions from
PredictivePipeline p.predict() are measured.
Contributor
|
@Pedrovaleriolopez @oalanicolas, o módulo Predictive Pipeline já está no nosso PR #575 (aberto 10/mar, antes deste). Mesma feature, implementação completa com 89 testes unitários. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CodeRabbit Fixes Applied
_emitSafeErrorto keep promise chain rejectedtaskTypeStats,agentStats,strategyStatsaftersplice(0, excess)Number.isFinite()forcomplexity,contextSize,agentExperience_stagePredictnow checksneighbors.length < minSamplesForPredictiondurationValuesbefore EWMA so highest-similarity neighbors get largest weightrecordDecision(now async) andsavecall_ensureLoaded()before mutating stateatomicWriteSync(write-to-tmp + rename) instead offs.writeFileSyncasync/awaitforrecordDecisionnew Set(results.map(r => r.id))Test Results
Supersedes #575
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests