feat(execution): Predictive Pipeline — previsão de resultados antes da execução#575
Closed
nikolasdehor wants to merge 0 commit intoSynkraAI:mainfrom
Closed
feat(execution): Predictive Pipeline — previsão de resultados antes da execução#575nikolasdehor wants to merge 0 commit intoSynkraAI:mainfrom
nikolasdehor wants to merge 0 commit intoSynkraAI:mainfrom
Conversation
nikolasdehor
requested review from
Pedrovaleriolopez and
oalanicolas
as code owners
|
@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. |
WalkthroughTwo new core modules are introduced: PredictivePipeline (1264 lines), a k-NN-based predictive system for task outcomes with five-stage processing and event emission; and DecisionMemory (549 lines), providing persistent cross-session decision tracking with pattern detection. A retrocompatibility wrapper forwards from the legacy API surface. Comprehensive test coverage and manifest updates accompany both modules. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Pipeline as PredictivePipeline
participant FeatureExt as Feature Extraction
participant KNN as k-NN Matcher
participant Predictor
participant Scorer
participant Recommender
Client->>Pipeline: predict(taskSpec)
Pipeline->>Pipeline: preprocess(validate features)
Pipeline->>FeatureExt: extract(complexity, taskType, agentExp)
FeatureExt-->>Pipeline: features
Pipeline->>KNN: findNearest(k=5)
KNN-->>Pipeline: similarTasks[]
Pipeline->>Predictor: estimateOutcome(weighted)
Predictor-->>Pipeline: successProb, durationEWMA, resources
Pipeline->>Scorer: computeConfidence(sampleSize, CV)
Scorer-->>Pipeline: confidence, anomalyFlags
Pipeline->>Recommender: selectAgent/Strategy()
Recommender-->>Pipeline: recommendation
Pipeline->>Client: prediction result + risk assessment
sequenceDiagram
participant Client
participant Memory as DecisionMemory
participant Extractor as Keyword Extractor
participant PatternDetector
participant Retriever
participant Storage
Client->>Memory: recordDecision(description, category?)
Memory->>Extractor: extractKeywords()
Extractor-->>Memory: keywords[]
Memory->>PatternDetector: _detectPatterns()
PatternDetector-->>Memory: patternMatch?
Memory->>Storage: save(decisions, patterns, stats)
Memory->>Client: DECISION_RECORDED event
Client->>Memory: getRelevantDecisions(context)
Memory->>Retriever: scoreByRelevance(keywordSim, timeDecay)
Retriever-->>Memory: rankedDecisions[]
Memory->>Client: relevantDecisions + scores
Client->>Memory: injectDecisionContext()
Memory->>Client: markdown formatted context + DECISIONS_INJECTED event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
tests/core/memory/decision-memory.test.js (1)
3-9: Use the repo's absolute import form in the test too.This reaches into the module through a long relative path, which is brittle and couples the test to the current directory layout instead of the intended import surface. 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/memory/decision-memory.test.js` around lines 3 - 9, The test currently imports DecisionMemory, DecisionCategory, Outcome, Events, and CONFIG via a brittle relative path; update the require to the repo's canonical absolute import (the package/library entry point) instead of '../../../.aiox-core/core/memory/decision-memory' so the test uses the public module surface; locate the import statement importing DecisionMemory, DecisionCategory, Outcome, Events, CONFIG and replace the relative require with the project's absolute module path for the decision-memory module so tests remain resilient to directory moves..aios-core/core/execution/predictive-pipeline.js (1)
1-2: Use the repo's absolute import form for the shim.This wrapper hardcodes a
../../../traversal to reach the canonical module, which makes the compatibility layer fragile to future layout changes. 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 @.aios-core/core/execution/predictive-pipeline.js around lines 1 - 2, The shim uses a fragile relative traversal in the module.exports require (require('../../../.aiox-core/core/execution/predictive-pipeline')); replace that relative path with the repository's canonical absolute import for the canonical module (use the repo absolute import form for .aiox-core core/execution/predictive-pipeline) so the wrapper exports the same module via an absolute require/import instead of ../../../ traversal.tests/core/execution/predictive-pipeline.test.js (1)
144-147: Keep the cwd fallback test inside the temp sandbox.
new PredictivePipeline(null)points this case at the real working directory. If constructor/load logic becomes eager, this will start creating.aioxunder the repo root and make the test environment-dependent. Stubprocess.cwd()to a temp path and assert against that value instead.🤖 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 144 - 147, The test currently constructs new PredictivePipeline(null) which uses the real process.cwd(); update the test to stub process.cwd() to a temporary sandbox path before creating the PredictivePipeline and restore the stub after the test — e.g. use jest.spyOn(process, 'cwd').mockReturnValue(tempPath) (or your test sandbox helper) so expect(p.projectRoot).toBe(tempPath) and ensure the spy is restored with mockRestore() or afterEach cleanup; reference PredictivePipeline and process.cwd() in the change.
🤖 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 177-181: The catch in _enqueueWrite currently swallows persistence
errors after calling this._emitSafeError, causing the promise chain to resolve
and higher-level callers like recordOutcome, retrain, prune, and subsequent
persists (_persistModel/_persistOutcomes) to proceed as if writes succeeded;
change _enqueueWrite so that after emitting the error it rethrows (or returns a
rejected promise) to keep the chain rejected, ensuring callers observe failure;
apply the same fix to the equivalent catch blocks around lines 189-205 so all
persistence failures propagate instead of being converted into successful
resolutions.
- Around line 372-378: The _extractFeatures function currently forwards
complexity, agentExperience, and contextSize verbatim which can produce
non-finite values and break _cosineSimilarity; validate and sanitize these
numeric features in _extractFeatures by coercing them to Numbers, rejecting or
replacing NaN/Infinity with safe defaults (e.g., complexity default 5,
contextSize default 0) and ensure _getAgentExperience(...) output is validated
the same way (clamp or default if not finite) so downstream similarity math
receives only finite numeric values.
- Around line 509-557: _stagePredict currently only falls back when
neighbors.length === 0; enforce the configured minimum by checking
this.minSamplesForPrediction (or default) and call
this._defaultPrediction(features) when neighbors.length <
this.minSamplesForPrediction so the pipeline honors the documented minimum
history requirement; update the early-return condition in _stagePredict, and
ensure any returned metadata (sampleSize, avgSimilarity) still reflect
neighbors.length when a prediction is produced.
- Around line 302-312: The code updates aggregates via _updateModelStats(record)
before pruning, so when you splice old entries from this._outcomes the
taskTypeStats, agentStats and strategyStats become inconsistent; after you
remove excess entries (the splice block) rebuild the aggregate statistics from
the remaining this._outcomes (or provide a method like _recalculateModelStats)
so taskTypeStats, agentStats and strategyStats reflect only the kept outcomes,
then call await this._persistOutcomes() and await this._persistModel(); use the
existing symbols _updateModelStats, this._outcomes, maxOutcomes, taskTypeStats,
agentStats, strategyStats, _persistOutcomes, and _persistModel to locate where
to add the recompute.
- Around line 523-552: The duration EWMA is currently computed from
durationValues without using neighbor similarity, so later (less-similar)
entries dominate; update the logic so duration is similarity-weighted: in
_stageMatch (which builds neighbors and durationValues) either (preferred)
compute a similarity-weighted duration estimate by calling a modified
_computeEwma that accepts paired arrays (durations and similarities) and applies
the EWMA using similarity as the per-sample weight, or (alternatively) reverse
durationValues before calling _computeEwma so the highest-similarity samples get
the largest EWMA influence; ensure the change affects estimatedDuration returned
from _stageMatch and keep symbol names _stageMatch, _computeEwma, neighbors,
durationValues, and estimatedDuration for locating the code.
In @.aiox-core/core/memory/decision-memory.js:
- Around line 141-155: The load() and save() logic in decision-memory.js is
currently vulnerable to partial overwrites and silently swallows errors; update
save() to write to a temporary file (e.g., filePath + '.tmp' or using a unique
temp name), fsync the temp file (or use a safe write API), then atomically
rename/replace the real decisions.json (fs.rename or fs.renameSync) to avoid
partial-file corruption, and update load() to throw or log errors with the
caught error object and contextual message instead of resetting state silently;
apply the same atomic-write and improved error reporting pattern to the other
save/load usage referenced around lines 164-181 so all file writes use
temp+rename and all catch blocks include the original error details and context
(function name, filePath, operation).
- Around line 125-132: The constructor leaves this._loaded false and
recordDecision (and other mutating methods around lines 195-227) mutates
this.decisions without first loading persisted state, which can overwrite
existing .aiox/decisions.json; add an async ensureLoaded/ensureInit helper that
checks this._loaded, reads the persisted decisions into this.decisions, and sets
this._loaded=true, then call await ensureLoaded() at the top of recordDecision
and any other mutating APIs (and/or make those APIs async) so reads/writes
always happen after initialization; make ensureLoaded idempotent and use it
before save() and any methods that read or write this.decisions.
In `@tests/core/execution/predictive-pipeline.test.js`:
- Around line 1085-1099: The concurrent test calls pipeline.recordOutcome(...)
in parallel but only asserts the final count, allowing ID collisions to go
unnoticed; update the test to collect the results (IDs) returned by
pipeline.recordOutcome when invoked in Promise.all, assert that the array length
equals 20 and that new Set(ids).size === 20 to guarantee uniqueness, and still
validate pipeline.getStats().outcomes; apply the same change to the similar test
at lines 1101-1112 so both concurrent paths verify returned IDs from
recordOutcome for uniqueness.
- Around line 12-17: The test imports internals via a relative path; change the
require for PredictivePipeline, PipelineStage, RiskLevel, DEFAULTS to use the
package's public absolute import instead of
"../../../.aiox-core/core/execution/predictive-pipeline" (e.g. the repository's
absolute module entry that exposes these symbols), so the test exercises the
public compatibility surface rather than internal files.
- Around line 442-452: The test "should respect minSimilarity" currently may
pass vacuously if findSimilarTasks returns an empty array; add an explicit
non-empty check before the loop: after calling pipeline.findSimilarTasks({
taskType: 'build' }, { minSimilarity: 0.9 }), assert that the returned array
(similar) has positive length (e.g. expect(similar.length).toBeGreaterThan(0))
so the for-loop that checks each s.similarity >= 0.9 actually runs; update the
test block containing the similar variable and the call to findSimilarTasks to
include this assertion.
---
Nitpick comments:
In @.aios-core/core/execution/predictive-pipeline.js:
- Around line 1-2: The shim uses a fragile relative traversal in the
module.exports require
(require('../../../.aiox-core/core/execution/predictive-pipeline')); replace
that relative path with the repository's canonical absolute import for the
canonical module (use the repo absolute import form for .aiox-core
core/execution/predictive-pipeline) so the wrapper exports the same module via
an absolute require/import instead of ../../../ traversal.
In `@tests/core/execution/predictive-pipeline.test.js`:
- Around line 144-147: The test currently constructs new
PredictivePipeline(null) which uses the real process.cwd(); update the test to
stub process.cwd() to a temporary sandbox path before creating the
PredictivePipeline and restore the stub after the test — e.g. use
jest.spyOn(process, 'cwd').mockReturnValue(tempPath) (or your test sandbox
helper) so expect(p.projectRoot).toBe(tempPath) and ensure the spy is restored
with mockRestore() or afterEach cleanup; reference PredictivePipeline and
process.cwd() in the change.
In `@tests/core/memory/decision-memory.test.js`:
- Around line 3-9: The test currently imports DecisionMemory, DecisionCategory,
Outcome, Events, and CONFIG via a brittle relative path; update the require to
the repo's canonical absolute import (the package/library entry point) instead
of '../../../.aiox-core/core/memory/decision-memory' so the test uses the public
module surface; locate the import statement importing DecisionMemory,
DecisionCategory, Outcome, Events, CONFIG and replace the relative require with
the project's absolute module path for the decision-memory module so tests
remain resilient to directory moves.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0453ee90-0372-4740-a92e-c19ad92a517e
📒 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
+177
to
+181
| _enqueueWrite(writeFn) { | ||
| this._writeChain = this._writeChain.then(() => writeFn()).catch((err) => { | ||
| this._emitSafeError({ type: 'persistence', error: err }); | ||
| }); | ||
| return this._writeChain; |
There was a problem hiding this comment.
Don't convert failed persistence into a successful API call.
The catch in _enqueueWrite() resolves the chain after emitting an error, so recordOutcome(), retrain(), and prune() can all report success even when nothing was written. It also allows _persistModel() to run after _persistOutcomes() failed, leaving model.json ahead of outcomes.json. As per coding guidelines, .aiox-core/core/**: Verify error handling is comprehensive with proper try/catch and error context.
Also applies to: 189-205
🤖 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 177 - 181, The
catch in _enqueueWrite currently swallows persistence errors after calling
this._emitSafeError, causing the promise chain to resolve and higher-level
callers like recordOutcome, retrain, prune, and subsequent persists
(_persistModel/_persistOutcomes) to proceed as if writes succeeded; change
_enqueueWrite so that after emitting the error it rethrows (or returns a
rejected promise) to keep the chain rejected, ensuring callers observe failure;
apply the same fix to the equivalent catch blocks around lines 189-205 so all
persistence failures propagate instead of being converted into successful
resolutions.
Comment on lines
+302
to
+312
| // 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); | ||
| } | ||
|
|
||
| await this._persistOutcomes(); | ||
| await this._persistModel(); |
There was a problem hiding this comment.
Rebuild aggregate stats when auto-pruning removes outcomes.
_updateModelStats(record) counts the new sample before pruning, but the splice() only removes it from _outcomes. Once maxOutcomes is exceeded, taskTypeStats, agentStats, and strategyStats still include pruned samples, so the persisted model no longer matches the persisted outcome set.
🤖 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 302 - 312, The
code updates aggregates via _updateModelStats(record) before pruning, so when
you splice old entries from this._outcomes the taskTypeStats, agentStats and
strategyStats become inconsistent; after you remove excess entries (the splice
block) rebuild the aggregate statistics from the remaining this._outcomes (or
provide a method like _recalculateModelStats) so taskTypeStats, agentStats and
strategyStats reflect only the kept outcomes, then call await
this._persistOutcomes() and await this._persistModel(); use the existing symbols
_updateModelStats, this._outcomes, maxOutcomes, taskTypeStats, agentStats,
strategyStats, _persistOutcomes, and _persistModel to locate where to add the
recompute.
Comment on lines
+372
to
+378
| _extractFeatures(task) { | ||
| return { | ||
| taskType: task.taskType ?? 'unknown', | ||
| complexity: task.complexity ?? 5, | ||
| agentExperience: this._getAgentExperience(task.agent), | ||
| contextSize: task.contextSize ?? 0, | ||
| }; |
There was a problem hiding this comment.
Sanitize numeric features before they reach the similarity math.
complexity, agentExperience, and contextSize are forwarded verbatim. A bad caller value like complexity: 'high' or contextSize: NaN turns _cosineSimilarity() into NaN, which then contaminates ranking and prediction scores. Normalize or reject non-finite values at extraction time. As per coding guidelines, .aiox-core/core/**: Check for 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/execution/predictive-pipeline.js around lines 372 - 378, The
_extractFeatures function currently forwards complexity, agentExperience, and
contextSize verbatim which can produce non-finite values and break
_cosineSimilarity; validate and sanitize these numeric features in
_extractFeatures by coercing them to Numbers, rejecting or replacing
NaN/Infinity with safe defaults (e.g., complexity default 5, contextSize default
0) and ensure _getAgentExperience(...) output is validated the same way (clamp
or default if not finite) so downstream similarity math receives only finite
numeric values.
Comment on lines
+509
to
+557
| _stagePredict(neighbors, features) { | ||
| return this._runStage(PipelineStage.PREDICT, () => { | ||
| if (neighbors.length === 0) { | ||
| return this._defaultPrediction(features); | ||
| } | ||
|
|
||
| // Weighted success probability | ||
| let weightSum = 0; | ||
| let successWeight = 0; | ||
| let durationEwma = 0; | ||
| let durationValues = []; | ||
| let resourceEstimates = { memory: 0, cpu: 0, apiCalls: 0 }; | ||
| let resourceCount = 0; | ||
|
|
||
| for (const { outcome, similarity } of neighbors) { | ||
| const weight = similarity; | ||
| weightSum += weight; | ||
| if (outcome.success) successWeight += weight; | ||
|
|
||
| durationValues.push(outcome.duration); | ||
|
|
||
| if (outcome.resources) { | ||
| resourceEstimates.memory += (outcome.resources.memory ?? 0) * weight; | ||
| resourceEstimates.cpu += (outcome.resources.cpu ?? 0) * weight; | ||
| resourceEstimates.apiCalls += (outcome.resources.apiCalls ?? 0) * weight; | ||
| resourceCount += weight; | ||
| } | ||
| } | ||
|
|
||
| const successProbability = weightSum > 0 ? successWeight / weightSum : 0.5; | ||
|
|
||
| // EWMA for duration | ||
| durationEwma = this._computeEwma(durationValues); | ||
|
|
||
| // Normalize resources | ||
| if (resourceCount > 0) { | ||
| resourceEstimates.memory /= resourceCount; | ||
| resourceEstimates.cpu /= resourceCount; | ||
| resourceEstimates.apiCalls /= resourceCount; | ||
| } | ||
|
|
||
| return { | ||
| successProbability, | ||
| estimatedDuration: Math.round(durationEwma), | ||
| resources: resourceEstimates, | ||
| sampleSize: neighbors.length, | ||
| avgSimilarity: weightSum / neighbors.length, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Honor minSamplesForPrediction before emitting a k-NN prediction.
The constructor documents this as the minimum history required, but _stagePredict() only falls back when neighbors.length === 0. With 1-2 historical outcomes and minSamplesForPrediction = 3, the pipeline still returns a full prediction instead of taking the insufficient-data path.
🤖 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 509 - 557,
_stagePredict currently only falls back when neighbors.length === 0; enforce the
configured minimum by checking this.minSamplesForPrediction (or default) and
call this._defaultPrediction(features) when neighbors.length <
this.minSamplesForPrediction so the pipeline honors the documented minimum
history requirement; update the early-return condition in _stagePredict, and
ensure any returned metadata (sampleSize, avgSimilarity) still reflect
neighbors.length when a prediction is produced.
Comment on lines
+523
to
+552
| for (const { outcome, similarity } of neighbors) { | ||
| const weight = similarity; | ||
| weightSum += weight; | ||
| if (outcome.success) successWeight += weight; | ||
|
|
||
| durationValues.push(outcome.duration); | ||
|
|
||
| if (outcome.resources) { | ||
| resourceEstimates.memory += (outcome.resources.memory ?? 0) * weight; | ||
| resourceEstimates.cpu += (outcome.resources.cpu ?? 0) * weight; | ||
| resourceEstimates.apiCalls += (outcome.resources.apiCalls ?? 0) * weight; | ||
| resourceCount += weight; | ||
| } | ||
| } | ||
|
|
||
| const successProbability = weightSum > 0 ? successWeight / weightSum : 0.5; | ||
|
|
||
| // EWMA for duration | ||
| durationEwma = this._computeEwma(durationValues); | ||
|
|
||
| // Normalize resources | ||
| if (resourceCount > 0) { | ||
| resourceEstimates.memory /= resourceCount; | ||
| resourceEstimates.cpu /= resourceCount; | ||
| resourceEstimates.apiCalls /= resourceCount; | ||
| } | ||
|
|
||
| return { | ||
| successProbability, | ||
| estimatedDuration: Math.round(durationEwma), |
There was a problem hiding this comment.
The duration EWMA is weighting the least similar neighbors the most.
_stageMatch() sorts neighbors by similarity descending, but _computeEwma(durationValues) gives the later entries the highest weight. That makes the least similar samples dominate estimatedDuration, and similarity never factors into duration at all. Use a similarity-weighted duration estimate, or reverse/order the series so the intended samples carry the larger weight.
🤖 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 523 - 552, The
duration EWMA is currently computed from durationValues without using neighbor
similarity, so later (less-similar) entries dominate; update the logic so
duration is similarity-weighted: in _stageMatch (which builds neighbors and
durationValues) either (preferred) compute a similarity-weighted duration
estimate by calling a modified _computeEwma that accepts paired arrays
(durations and similarities) and applies the EWMA using similarity as the
per-sample weight, or (alternatively) reverse durationValues before calling
_computeEwma so the highest-similarity samples get the largest EWMA influence;
ensure the change affects estimatedDuration returned from _stageMatch and keep
symbol names _stageMatch, _computeEwma, neighbors, durationValues, and
estimatedDuration for locating the code.
Comment on lines
+125
to
+132
| constructor(options = {}) { | ||
| super(); | ||
| this.projectRoot = options.projectRoot || process.cwd(); | ||
| this.config = { ...CONFIG, ...options.config }; | ||
| this.decisions = []; | ||
| this.patterns = []; | ||
| this._loaded = false; | ||
| } |
There was a problem hiding this comment.
Load persisted state before accepting new decisions.
recordDecision() mutates this.decisions even when the instance has never loaded the existing store. On a fresh process with an existing .aiox/decisions.json, the next save() can overwrite prior history with only the new in-memory entries. Add an internal eager/lazy load before any read or write, or make the mutating API async so callers cannot skip initialization.
Also applies to: 195-227
🤖 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 125 - 132, The
constructor leaves this._loaded false and recordDecision (and other mutating
methods around lines 195-227) mutates this.decisions without first loading
persisted state, which can overwrite existing .aiox/decisions.json; add an async
ensureLoaded/ensureInit helper that checks this._loaded, reads the persisted
decisions into this.decisions, and sets this._loaded=true, then call await
ensureLoaded() at the top of recordDecision and any other mutating APIs (and/or
make those APIs async) so reads/writes always happen after initialization; make
ensureLoaded idempotent and use it before save() and any methods that read or
write this.decisions.
Comment on lines
+141
to
+155
| try { | ||
| if (fs.existsSync(filePath)) { | ||
| const raw = fs.readFileSync(filePath, 'utf-8'); | ||
| const data = JSON.parse(raw); | ||
|
|
||
| if (data.schemaVersion === this.config.schemaVersion) { | ||
| this.decisions = data.decisions || []; | ||
| this.patterns = data.patterns || []; | ||
| } | ||
| } | ||
| } catch { | ||
| // Corrupted file — start fresh | ||
| this.decisions = []; | ||
| this.patterns = []; | ||
| } |
There was a problem hiding this comment.
Make the on-disk write atomic instead of overwriting decisions.json in place.
save() writes the target file directly, and load() treats any read/parse failure as “start fresh.” A partial write or process crash here can therefore erase the entire memory store on the next startup. Write to a temp file and rename it, and surface save/load failures with context instead of silently degrading. As per coding guidelines, .aiox-core/core/**: Verify error handling is comprehensive with proper try/catch and error context.
Also applies to: 164-181
🤖 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 141 - 155, The load()
and save() logic in decision-memory.js is currently vulnerable to partial
overwrites and silently swallows errors; update save() to write to a temporary
file (e.g., filePath + '.tmp' or using a unique temp name), fsync the temp file
(or use a safe write API), then atomically rename/replace the real
decisions.json (fs.rename or fs.renameSync) to avoid partial-file corruption,
and update load() to throw or log errors with the caught error object and
contextual message instead of resetting state silently; apply the same
atomic-write and improved error reporting pattern to the other save/load usage
referenced around lines 164-181 so all file writes use temp+rename and all catch
blocks include the original error details and context (function name, filePath,
operation).
Comment on lines
+12
to
+17
| const { | ||
| PredictivePipeline, | ||
| PipelineStage, | ||
| RiskLevel, | ||
| DEFAULTS, | ||
| } = require('../../../.aiox-core/core/execution/predictive-pipeline'); |
There was a problem hiding this comment.
Import the module through the public absolute path.
This suite reaches directly into ../../../.aiox-core/..., so it bypasses the compatibility surface added in this PR and can miss regressions there. It also breaks the repo rule for absolute imports.
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 internals via a relative path; change the require for
PredictivePipeline, PipelineStage, RiskLevel, DEFAULTS to use the package's
public absolute import instead of
"../../../.aiox-core/core/execution/predictive-pipeline" (e.g. the repository's
absolute module entry that exposes these symbols), so the test exercises the
public compatibility surface rather than internal files.
Comment on lines
+442
to
+452
| it('should respect minSimilarity option', async () => { | ||
| await seedOutcomes(pipeline, 10); | ||
|
|
||
| const similar = pipeline.findSimilarTasks( | ||
| { taskType: 'build' }, | ||
| { minSimilarity: 0.9 }, | ||
| ); | ||
|
|
||
| for (const s of similar) { | ||
| expect(s.similarity).toBeGreaterThanOrEqual(0.9); | ||
| } |
There was a problem hiding this comment.
Make the minSimilarity assertion non-vacuous.
If findSimilarTasks() incorrectly returns [] whenever minSimilarity is set, this still passes because the loop never runs. Add a positive-length assertion before iterating so the thresholded path is actually exercised.
Proposed tightening
const similar = pipeline.findSimilarTasks(
{ taskType: 'build' },
{ minSimilarity: 0.9 },
);
+ expect(similar.length).toBeGreaterThan(0);
for (const s of similar) {
expect(s.similarity).toBeGreaterThanOrEqual(0.9);
}🤖 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 442 - 452, The
test "should respect minSimilarity" currently may pass vacuously if
findSimilarTasks returns an empty array; add an explicit non-empty check before
the loop: after calling pipeline.findSimilarTasks({ taskType: 'build' }, {
minSimilarity: 0.9 }), assert that the returned array (similar) has positive
length (e.g. expect(similar.length).toBeGreaterThan(0)) so the for-loop that
checks each s.similarity >= 0.9 actually runs; update the test block containing
the similar variable and the call to findSimilarTasks to include this assertion.
Comment on lines
+1085
to
+1099
| it('should handle concurrent recordOutcome calls', async () => { | ||
| const promises = []; | ||
| for (let i = 0; i < 20; i++) { | ||
| promises.push(pipeline.recordOutcome({ | ||
| taskType: 'concurrent', | ||
| duration: 100 * i, | ||
| success: true, | ||
| })); | ||
| } | ||
|
|
||
| await Promise.all(promises); | ||
|
|
||
| const stats = pipeline.getStats(); | ||
| expect(stats.outcomes).toBe(20); | ||
| }); |
There was a problem hiding this comment.
Assert unique IDs in the concurrent path.
The race-prone case here is the parallel recordOutcome() calls. Right now the concurrent test only checks the final count, while uniqueness is verified separately in a sequential loop, so ID collisions under Promise.all() can slip through unnoticed.
Proposed tightening
- await Promise.all(promises);
+ const results = await Promise.all(promises);
const stats = pipeline.getStats();
expect(stats.outcomes).toBe(20);
+ expect(new Set(results.map((result) => result.id)).size).toBe(20);Also applies to: 1101-1112
🤖 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 1085 - 1099,
The concurrent test calls pipeline.recordOutcome(...) in parallel but only
asserts the final count, allowing ID collisions to go unnoticed; update the test
to collect the results (IDs) returned by pipeline.recordOutcome when invoked in
Promise.all, assert that the array length equals 20 and that new Set(ids).size
=== 20 to guarantee uniqueness, and still validate pipeline.getStats().outcomes;
apply the same change to the similar test at lines 1101-1112 so both concurrent
paths verify returned IDs from recordOutcome for uniqueness.
Contributor
|
All 10 CodeRabbit review items have been addressed in #579 (pushed from my fork since I don't have write access to Fixes applied:
All 126 tests pass (89 predictive-pipeline + 37 decision-memory). |
Contributor
Author
|
@Pedrovaleriolopez @oalanicolas, Predictive Pipeline — permite prever resultados de execução antes de rodar, usando k-NN com similaridade coseno, EWMA pra estimativa de duração, e risk assessment. 89 testes. |
nikolasdehor
force-pushed
the
feat/predictive-pipeline
branch
from
dfdc69b to
ff711c1
Compare
This was referenced Mar 11, 2026
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.
Resumo
Pipeline preditivo que estima resultados de tasks ANTES da execução. Usa padrões históricos para prever probabilidade de sucesso, duração, recursos e pontos de falha.
Funcionalidades
Arquivos
Test plan
Summary by CodeRabbit
New Features
Tests