docs: aumenta cobertura de JSDoc nos módulos core (#89)#593
Open
nikolasdehor wants to merge 1 commit intoSynkraAI:mainfrom
Open
docs: aumenta cobertura de JSDoc nos módulos core (#89)#593nikolasdehor wants to merge 1 commit intoSynkraAI:mainfrom
nikolasdehor wants to merge 1 commit intoSynkraAI:mainfrom
Conversation
…, events, resilience) Melhora a cobertura de JSDoc em 5 módulos core, adicionando @param, @returns, @throws e @example a 25+ funções/métodos públicos: - config-loader.js: 9 funções com JSDoc expandido - master-orchestrator.js: 7 métodos + StubEpicExecutor documentado - recovery-handler.js: 7 métodos públicos com tags completos - gate-evaluator.js: 3 métodos com documentação melhorada - dashboard-emitter.js: classe e constructor documentados Ref: SynkraAI#89
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. |
github-actions
bot
added
area: agents
WalkthroughThis pull request enhances documentation across core configuration, event handling, and orchestration modules by adding and expanding JSDoc comments and docstrings to improve overall code documentation coverage without modifying functional logic or control flow. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.aiox-core/core/config/config-loader.js (1)
93-126:⚠️ Potential issue | 🟡 Minor
loadFullConfig()is documented as cache-aware, but it always hits disk.The TTL short-circuit only exists in
loadConfigSections(). Direct callers of the exportedloadFullConfig()API will still re-readcore-config.yamlon every call, so the new JSDoc currently overstates the behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aiox-core/core/config/config-loader.js around lines 93 - 126, loadFullConfig always reads from disk; update it to honor the same cache TTL used by loadConfigSections by checking configCache.full and configCache.lastLoad against the cache TTL before reading: if a valid cached config exists return it immediately (skipping file read and the performanceMetrics increment), otherwise proceed to read/parse the file, update performanceMetrics and then set configCache.full and configCache.lastLoad before returning; reference loadFullConfig, configCache.full, configCache.lastLoad, and performanceMetrics in your change..aiox-core/core/orchestration/master-orchestrator.js (1)
1540-1579: 🛠️ Refactor suggestion | 🟠 MajorKeep the stub result shape compatible with real executors.
The fallback currently returns only
{ status, epicNum, message, timestamp }, but the standard executor contract in.aiox-core/core/orchestration/executors/epic-executor.js:64-81also carriessuccess,artifacts,errors,duration*,startTime,endTime, andlogs. When this path is hit, downstream code has to special-case the stub or risk breaking on missing fields.As per coding guidelines, "Ensure backwards compatibility — core modules are consumed by all agents."Proposed contract-compatible stub result
async execute(_context) { + const timestamp = new Date().toISOString(); + console.log(chalk.yellow(` ⚠️ Using stub executor for Epic ${this.epicNum}`)); console.log(chalk.gray(` Real executor (${this.config.executor}) not yet implemented`)); console.log(chalk.gray(' See Story 0.3: Epic Executors')); // Return minimal success result for pipeline to continue return { status: 'stub', epicNum: this.epicNum, + success: true, + artifacts: [], + errors: [], + duration: '0m 0s', + durationMs: 0, + startTime: timestamp, + endTime: timestamp, + logs: [], message: `Stub executor for ${this.config.name}`, - timestamp: new Date().toISOString(), + timestamp, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aiox-core/core/orchestration/master-orchestrator.js around lines 1540 - 1579, The stub executor's execute method returns a minimal object missing fields required by the real executor contract; update StubEpicExecutor.execute to return the same shape as real executors (see epic-executor.js) by adding success (boolean), artifacts (array), errors (array), logs (array), startTime and endTime (ISO timestamps), and duration (numeric, e.g. 0) in addition to status, epicNum, message and timestamp; populate message using this.config.name and ensure success is true and artifacts/errors/logs are empty so downstream code can treat the stub result identically to real executor results.
🧹 Nitpick comments (1)
.aiox-core/core/orchestration/recovery-handler.js (1)
684-697: Avoid returning live attempt records fromgetAttemptHistory().
{ ...this.attempts }only clones the top-level map. Callers still get the original attempt arrays/objects, so mutating the returned value can changecanRetry()and escalation behavior from outside the handler. A deep copy or read-only view would make this accessor safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aiox-core/core/orchestration/recovery-handler.js around lines 684 - 697, getAttemptHistory currently returns a shallow copy of this.attempts so callers receive live arrays/objects that can mutate internal state; change getAttemptHistory to return a deep copy (e.g. use structuredClone(this.attempts) or JSON.parse(JSON.stringify(this.attempts)) depending on environment) or return frozen/read-only copies so external changes won't affect canRetry() or escalation logic; update the getAttemptHistory implementation to build and return cloned arrays/objects for each epic key (referencing the attempts property and getAttemptHistory function) to ensure immutability of returned attempt records.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.aiox-core/core/config/config-loader.js:
- Around line 93-126: loadFullConfig always reads from disk; update it to honor
the same cache TTL used by loadConfigSections by checking configCache.full and
configCache.lastLoad against the cache TTL before reading: if a valid cached
config exists return it immediately (skipping file read and the
performanceMetrics increment), otherwise proceed to read/parse the file, update
performanceMetrics and then set configCache.full and configCache.lastLoad before
returning; reference loadFullConfig, configCache.full, configCache.lastLoad, and
performanceMetrics in your change.
In @.aiox-core/core/orchestration/master-orchestrator.js:
- Around line 1540-1579: The stub executor's execute method returns a minimal
object missing fields required by the real executor contract; update
StubEpicExecutor.execute to return the same shape as real executors (see
epic-executor.js) by adding success (boolean), artifacts (array), errors
(array), logs (array), startTime and endTime (ISO timestamps), and duration
(numeric, e.g. 0) in addition to status, epicNum, message and timestamp;
populate message using this.config.name and ensure success is true and
artifacts/errors/logs are empty so downstream code can treat the stub result
identically to real executor results.
---
Nitpick comments:
In @.aiox-core/core/orchestration/recovery-handler.js:
- Around line 684-697: getAttemptHistory currently returns a shallow copy of
this.attempts so callers receive live arrays/objects that can mutate internal
state; change getAttemptHistory to return a deep copy (e.g. use
structuredClone(this.attempts) or JSON.parse(JSON.stringify(this.attempts))
depending on environment) or return frozen/read-only copies so external changes
won't affect canRetry() or escalation logic; update the getAttemptHistory
implementation to build and return cloned arrays/objects for each epic key
(referencing the attempts property and getAttemptHistory function) to ensure
immutability of returned attempt records.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 720c4ed3-e500-4d35-a5e7-d70cfdfb0c16
📒 Files selected for processing (6)
.aiox-core/core/config/config-loader.js.aiox-core/core/events/dashboard-emitter.js.aiox-core/core/orchestration/gate-evaluator.js.aiox-core/core/orchestration/master-orchestrator.js.aiox-core/core/orchestration/recovery-handler.js.aiox-core/install-manifest.yaml
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
1 participant
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
@param,@returns,@throwse@exampleconfig-loader.js,master-orchestrator.js,recovery-handler.js,gate-evaluator.js,dashboard-emitter.jsDetalhes por módulo
config-loader.js(9 funções)isCacheValid,loadFullConfig,loadConfigSections,loadAgentConfig,loadMinimalConfig,preloadConfig,clearCache,validateAgentConfig,getConfigSection@throwspara funções que lançam erros,@examplepara as mais usadasmaster-orchestrator.js(7+ métodos)startDashboard,stopDashboard,saveState,finalize,clearState,listSavedStatesStubEpicExecutor: constructor eexecuteagora documentados com@parame@returnsrecovery-handler.js(7 métodos)getLogs,getEpicLogs,getAttemptHistory,getAttemptCount,canRetry,resetAttempts,clear@exampleemgetLogs,getAttemptHistory,canRetrygate-evaluator.js(3 métodos)clear,_log,getLogscom tipos de retorno e parâmetros documentadosdashboard-emitter.js(classe + constructor)Test plan
npm run lintpassa sem errosnpm testpassa (nenhuma lógica alterada)Closes #89
Summary by CodeRabbit
Documentation
Improvements