fix(synapse): dynamic context window from model registry instead of hardcoded 200K#606
Conversation
LeandroMachadoAveiro
requested review from
Pedrovaleriolopez and
oalanicolas
as code owners
|
@LeandroMachadoAveiro 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
WalkthroughAdds a new model registry configuration block under the boundary section in the core configuration, specifying context window sizes and average token consumption for three Claude model variants with Opus designated as the active model. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
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: 1
🧹 Nitpick comments (1)
.aios-core/core/synapse/context/context-tracker.js (1)
108-111: Silent error swallowing may obscure configuration issues.The catch block falls back to defaults without any logging or indication that an error occurred. This could make it difficult to diagnose issues such as YAML syntax errors, permission problems, or unexpected config structures.
🔧 Proposed improvement
Consider logging a warning when falling back due to an error (not just missing config):
} catch (_err) { + if (process.env.DEBUG || process.env.NODE_ENV !== 'production') { + console.warn('[context-tracker] Failed to load model config, using defaults:', _err.message); + } _modelConfigCache = DEFAULTS; return _modelConfigCache; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aios-core/core/synapse/context/context-tracker.js around lines 108 - 111, The catch block that assigns _modelConfigCache = DEFAULTS is silently swallowing errors; modify the catch in the function that loads/parses the model config so it logs a warning (including the caught error object/message and context like the config path or source) before falling back to DEFAULTS; specifically update the catch that currently uses ( _err ) and references _modelConfigCache and DEFAULTS to call the existing logger (or console.warn if none) with a clear message and the error details to aid debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.aios-core/core/synapse/context/context-tracker.js:
- Around line 79-82: The code uses process.cwd() to construct configPath which
can break when the working directory is changed; update context-tracker.js to
resolve the config file relative to the module or an explicit base path instead:
replace process.cwd() usage in the configPath construction with a
module-relative resolution (e.g., using __dirname or an injected basePath
parameter) and implement a small upward directory walk from that base to locate
'.aios-core/core-config.yaml' (with the same fallback to
'.aiox-core/core-config.yaml') so the lookup is robust to chdir or test changes;
key symbols to modify are the configPath assignment and any initializer that
imports this module so it can optionally accept a basePath.
---
Nitpick comments:
In @.aios-core/core/synapse/context/context-tracker.js:
- Around line 108-111: The catch block that assigns _modelConfigCache = DEFAULTS
is silently swallowing errors; modify the catch in the function that
loads/parses the model config so it logs a warning (including the caught error
object/message and context like the config path or source) before falling back
to DEFAULTS; specifically update the catch that currently uses ( _err ) and
references _modelConfigCache and DEFAULTS to call the existing logger (or
console.warn if none) with a clear message and the error details to aid
debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fac3c96c-4242-4de6-a7d0-8683944fe054
📒 Files selected for processing (2)
.aios-core/core-config.yaml.aios-core/core/synapse/context/context-tracker.js
| let configPath = path.join(process.cwd(), '.aios-core', 'core-config.yaml'); | ||
| if (!fs.existsSync(configPath)) { | ||
| configPath = path.join(process.cwd(), '.aiox-core', 'core-config.yaml'); | ||
| } |
There was a problem hiding this comment.
Consider using module-relative path resolution instead of process.cwd().
The context snippets show that bin/aios.js uses process.chdir(targetPath) before running commands, and tests may change cwd to temporary directories. Using process.cwd() here could cause the config lookup to fail unexpectedly when:
- The CLI is invoked from a subdirectory
- Tests change the working directory before importing this module
- The module is cached before a
chdiroccurs
The fallback to DEFAULTS prevents crashes, but users may not realize they're not using their intended model configuration.
💡 Suggested approach
Consider accepting an optional base path parameter or using __dirname to resolve relative to the module's location, then walking up to find the project root:
-function getModelConfig() {
+function getModelConfig(basePath = null) {
if (_modelConfigCache) return _modelConfigCache;
try {
const yaml = require('js-yaml');
- let configPath = path.join(process.cwd(), '.aios-core', 'core-config.yaml');
+ const root = basePath || process.cwd();
+ let configPath = path.join(root, '.aios-core', 'core-config.yaml');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.aios-core/core/synapse/context/context-tracker.js around lines 79 - 82, The
code uses process.cwd() to construct configPath which can break when the working
directory is changed; update context-tracker.js to resolve the config file
relative to the module or an explicit base path instead: replace process.cwd()
usage in the configPath construction with a module-relative resolution (e.g.,
using __dirname or an injected basePath parameter) and implement a small upward
directory walk from that base to locate '.aios-core/core-config.yaml' (with the
same fallback to '.aiox-core/core-config.yaml') so the lookup is robust to chdir
or test changes; key symbols to modify are the configPath assignment and any
initializer that imports this module so it can optionally accept a basePath.
…ardcoded 200K The context-tracker was hardcoded to 200K tokens, causing premature bracket escalation on Opus 4.6 (1M context). Now reads active model from core-config.yaml models.registry with graceful fallback to 200K default. Closes SynkraAI#605 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address CodeRabbit review findings: - Replace process.cwd() with __dirname-based resolution for config path lookup - Add optional basePath parameter to getModelConfig() for test injection - Log warning when config loading fails (only in DEBUG/AIOX_DEBUG mode) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
LeandroMachadoAveiro
force-pushed
the
fix/synapse-dynamic-context-window
branch
from
cddbd00 to
192b3c3
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.aiox-core/core-config.yaml (1)
388-391: Minor: Comment references MIS-2+ but this serves context-tracker.Line 388 mentions "Memory Intelligence System (Epic MIS) configuration placeholder — MIS-2+" as a section header, but the
modelssection is being introduced forcontext-tracker.jsto read dynamic context windows (per PR#606/ Issue#605). Consider updating the comment to reflect the actual purpose, or adding a separate comment for clarity.📝 Suggested comment update
# Memory Intelligence System (Epic MIS) configuration placeholder — MIS-2+ +# Model registry for dynamic context window configuration (Issue `#605`) models: active: claude-sonnet-4-6🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.aiox-core/core-config.yaml around lines 388 - 391, Update the YAML section comment to accurately describe that this `models` block is used by context-tracker.js to provide dynamic context window configuration (per PR `#606` / Issue `#605`) rather than the generic "Memory Intelligence System (Epic MIS) — MIS-2+" label; edit the header above `models:` (the lines mentioning "Memory Intelligence System..." and/or "MIS-2+") so it clearly states purpose and scope (e.g., "Context-tracker dynamic model/config registry for context windowing") and optionally mention `active: claude-sonnet-4-6` and `registry:` as the fields consumed by context-tracker.js.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.aiox-core/core-config.yaml:
- Around line 388-391: Update the YAML section comment to accurately describe
that this `models` block is used by context-tracker.js to provide dynamic
context window configuration (per PR `#606` / Issue `#605`) rather than the generic
"Memory Intelligence System (Epic MIS) — MIS-2+" label; edit the header above
`models:` (the lines mentioning "Memory Intelligence System..." and/or "MIS-2+")
so it clearly states purpose and scope (e.g., "Context-tracker dynamic
model/config registry for context windowing") and optionally mention `active:
claude-sonnet-4-6` and `registry:` as the fields consumed by context-tracker.js.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2828c818-1c64-4683-a242-d9ae61f2049d
📒 Files selected for processing (1)
.aiox-core/core-config.yaml
|
Excelente fix. O hardcode de 200K no context-tracker era uma bomba-relógio — qualquer modelo com janela diferente ia ter estimativas completamente erradas. Observações:
Aprovado da minha parte — o fallback gracioso garante retrocompatibilidade. |
2 participants
Summary
maxContext: 200000causes premature bracket escalation on Opus 4.6 (1M context) — CRITICAL fires at ~66 prompts when 80%+ context remainsmodelssection tocore-config.yamlwith model registry (context window + avg tokens per model)getModelConfig()reads active model dynamically, cached per process, graceful fallback to 200KImpact
Files Changed
.aios-core/core-config.yaml— addedmodelssection with registry.aios-core/core/synapse/context/context-tracker.js— v1.0.0 → v1.1.0, dynamic config readerMigration Guide
Add to your project
core-config.yaml:Without this section, the tracker falls back to the previous 200K default (backward-compatible).
Test plan
estimateContextPercent(100)returns ~82% with Opus config (1M)modelssection is missingresetModelConfigCache()clears cache for test isolation.aios-coreand.aiox-corepaths both resolveCloses #605
🤖 Generated with Claude Code
Summary by CodeRabbit