Skip to content

feat: Cost Control Center#23

Open
dreamwing wants to merge 55 commits intomasterfrom
feature/cost-control-center
Open

feat: Cost Control Center#23
dreamwing wants to merge 55 commits intomasterfrom
feature/cost-control-center

Conversation

@dreamwing
Copy link
Owner

Implementation of Cost Control Center frontend and backend as specified in the PRD. Includes diagnostic engine, optimizer service, and dashboard UI.

dreamwing added 30 commits March 1, 2026 00:18
…urity hardening, config backup, test coverage

Changes across 9 files (588 additions, 238 deletions):

## Diagnostics Engine (diagnostics.js)
- REMOVED: mock fallback data — returns zeroed struct + noData flag instead
- FIXED: D06 cache hit formula to PRD spec: cacheRead/(input+cacheRead)
- FIXED: D05 thinking savings uses actual output tokens × cost ratio
- FIXED: D09 output verbosity uses output/input > 10% threshold
- FIXED: currentMonthlyCost extrapolated by activeDays
- ADDED: _computeTokenCostRatio() for precise per-token costing
- ADDED: 60s result cache with invalidateCache() method
- ADDED: cacheHitRate in API response

## Optimizer Service (optimizer.js)
- ADDED: config backup before every optimization (data/backups/)
- ADDED: action_id whitelist validation
- ADDED: dynamic A01 model target via meta.alternative
- ADDED: diagnostics cache invalidation after apply
- FIXED: JSONL parse tolerance for corrupted lines
- FIXED: savings rounded to 2 decimal places

## Security (auth/middleware.js, auth/routes.js)
- ADDED: crypto.timingSafeEqual() for all key comparisons
- Prevents timing attacks on SECRET_KEY

## Frontend (dashboard.js)
- FIXED: duplicate savings variable declaration
- FIXED: field name alignment (totalMonthlySavings)
- ADDED: loading/disabled state on Apply button
- ADDED: meta passthrough for dynamic A01
- ADDED: savings amount in history timeline entries
- REPLACED: alert() with inline toast notifications
- ADDED: noData state handling

## Route (optimize.js)
- ADDED: meta passthrough from request body

## Tests (3 files)
- diagnostics: 5 → 9 tests (added D07, D09, empty stats, caching)
- optimizer: 4 → 13 tests (added failure, unknown action, backup, corrupted JSONL)
- integration: 3 → 6 tests (added meta, invalid action, cost fields)
…culation

Instead of binary on/off, A02 now offers multiple interval options:
- Every 30 min, 1h, 2h, 4h, or Disable completely
- Each option shows precise savings based on token difference vs current interval
- Radio selector in frontend updates savings tag dynamically on selection
- Optimizer accepts custom interval via meta.interval (not just '0m')

Changes:
- diagnostics.js: generate options array with per-interval savings calculation
- optimizer.js: A02 accepts custom interval value, dynamic title
- dashboard.js: radio selector UI with dynamic savings tag
- dashboard.css: opt-radio styling + .applying button state
- Tests: updated D02 and integration tests for new format
…anced guidance

Five UX improvements from persona analysis report:

1. Layered Copy System (all user levels)
   - plainTitle: beginner-friendly headers (e.g., 'Switch to a cheaper AI model')
   - plainSideEffect: scenario-language side effects
   - Collapsible 'Technical Details' section with codeTag + calcDetail formula
   - Removed hardcoded title overrides — titles now come from backend

2. Config Diff Preview (intermediate users)
   - Each action shows configDiff: { key, from, to }
   - Rendered as: heartbeat.every: 15m → 0m (with strikethrough + green)

3. History Undo (all user levels)
   - backupPath now stored in optimizations.jsonl entries
   - Added restoreBackup() method in optimizer.js
   - POST /api/optimizations/undo endpoint
   - 'Undo' button on most recent history entry
   - Restores all config keys from backup + logs UNDO action

4. Enhanced No-Data Guidance (beginners)
   - Updated message: 'Start chatting with your AI agent...'

5. Calculation Detail Formulas (advanced users)
   - calcDetail field shows the exact formula used for savings
   - e.g., '10K output × 40% thinking × 75% cut × .00/M × 5.0x'

Changes: 5 files, +249/-32
…te card (WIP)

Partial implementation of remaining UX persona analysis items:
- helpText tooltip field on all 6 actions
- diagnostics.config.json custom threshold loading
- ?verbose=true API param for raw data export
- Cache hit rate card render + tooltip ? icon in frontend
- All diagnostic rules now use configurable thresholds
Items implemented:

#3 — Tooltip micro-interaction
  - helpText field on all 8 actions (A01-A09)
  - ? icon with native title tooltip in opt-header

#8 — 7-day Effect Tracking
  - preOptCostSnapshot stored in optimization log entries
  - Frontend compares predicted vs actual savings for >7d entries
  - Color-coded tag: green (>80% achieved), amber (partial)

#9 — Custom Thresholds (diagnostics.config.json)
  - _getThresholds() loads from data/diagnostics.config.json
  - Falls back to built-in defaults
  - All 6 rules use configurable thresholds
  - Example file: data/diagnostics.config.json.example
  - invalidateCache() clears threshold cache

#10 — D03/D04 Diagnostic Rules
  - D03: Session Reset detection (short sessions + high frequency)
  - D04: Idle Skill detection (>3 Skills installed in workspace)
  - Both include full layered copy (plainTitle, helpText, calcDetail, configDiff)

#11 — Pricing Service (from existing pricing.json)
  - NEW: src/services/pricing.js
  - Loads from data/config/pricing.json (no remote fetch)
  - Curated KNOWN_REPLACEMENTS with dynamic savingsRatio
  - MODEL_REPLACEMENTS now dynamically built from actual prices
  - Process-lifetime cache (invalidated on demand)

#12 — Verbose API Export
  - GET /api/diagnostics?verbose=true returns _rawData
  - Includes thresholds, per-model costs, token ratios
  - Non-verbose mode auto-strips _rawData

#13 — Cache Hit Rate Card
  - HTML element in index.html optimizer panel
  - Color-coded: green (>50%), amber (>10%), red (<10%)
  - Hidden until data is available

Changes: 8 files, +619/-337
D04 now scans each Skill folder's modification time instead of just
counting total Skills. Groups results into:
- Idle (>7d): strongly recommended for removal, shown in red badges
- Quiet (>3d): listed for user review, shown in amber badges

Backend: diagnostics.js D04 rule rewritten with fs.stat() per-folder,
returns _meta.idleSkills / _meta.quietSkills arrays with name + daysSince.

Frontend: renderSkillAuditList() renders grouped badge lists inside A04.
CSS: .skill-audit-list, .skill-group, .skill-badge with idle/quiet variants.
Previously D04 was incorrectly scanning ~/.openclaw/workspace/ which
contains project folders, not Skills. Now scans the correct directories:
- ~/.openclaw/skills/ (primary, Skills installed here, may be symlinks)
- ~/.openclaw/workspace/skills/ (secondary, legacy location)

Also handles symbolic links (e.g. backlink-pilot -> ...) and deduplicates
by name when the same Skill appears in both directories.
Based on openclaw/src/agents/skills/workspace.ts loadSkillEntries():
- Skills are loaded from 6 sources with precedence order
- D04 now only scans 'managed' skills (user-installed via openclaw CLI)
- Managed dir: (OPENCLAW_STATE_DIR || ~/.openclaw)/skills
- Validates SKILL.md exists in each subfolder (not just any directory)
- Handles symlinks like OpenClaw's listChildDirectories()
- Skips node_modules and dotfiles

Previously incorrectly scanned ~/.openclaw/workspace/ (project dirs).
Integrated robust directory resolution logic from openclaw/src/infra/home-dir.ts:
- Added support for OPENCLAW_HOME environment variable.
- Added os.homedir() fallback for HOME/USERPROFILE.
- Unified resolution via _resolveHomeDir() and _resolveConfigDir() helpers.
- Applied fixed resolution to Heartbeat (D02) and Skill (D04) detection.
P1: Add MODEL_ALIAS_MAP for pricing fallback — maps provider-native
    model names (e.g. gemini-3-pro-high) to OpenRouter pricing keys.
    Prevents 20x cost underestimation when usage.cost is missing.

P2: Add OPENCLAW_HOME env var support in analyze.js path discovery,
    aligned with OpenClaw's src/infra/home-dir.ts resolution order.

P3: Add OPENCLAW_HOME env var support in config.js HOME_DIR,
    same resolution chain: OPENCLAW_HOME → HOME → USERPROFILE → os.homedir()

P4: Track session count per day in history aggregation.
    Each JSONL file = 1 session. Enables D03 session-reset detection.
Extract resolveHomeDir and resolveConfigDir from diagnostics.js, config.js,
and analyze.js into a single shared utility module (src/utils/paths.js).
This removes duplicate implementations, guarantees consistent OS-specific
path resolution across the app, and avoids triggering env-var constraints
when standalone scripts import the logic.
- Added tests for D03 (Session Reset) and D04 (Idle Skill) in diagnostics.
- Added tests for restoreBackup (Undo) and backupConfig in optimizer.
- Created new test suite for local pricing fallback and replacement generation.
…ify paths, add input validation

- Extract duplicated safeCompare() to shared src/auth/utils.js
- Sanitize backupPath in undo endpoint to prevent path traversal
- Replace os.homedir() with resolveHomeDir()/resolveConfigDir() in optimizer and openclaw_config
- Add input validation (savings, meta) on POST /api/optimize/:action_id
- D05: only trigger on explicit thinkingDefault config, skip undefined
- D01: add comment clarifying intentional single-model recommendation
- Remove unused cacheWrite from diagnostics fallback
- Add OPENCLAW_STATE_DIR override in integration tests
The bottom-nav, view-settings, and script tag were incorrectly nested
inside the external-trigger-container due to missing closing </div> tags
introduced by the Optimizer flip-card feature. This caused the bottom
tabs to only be visible on the Tokens view. Added 3 missing closing tags
to restore proper HTML structure.
- Separated external-trigger-container and flip-container as siblings
  inside view-tokens (they were incorrectly nested inside each other)
- Removed extra 'card' class from card-front that conflicted with
  the dashboard's .card styles (padding, border, background)
- Removed 'card-wrapper' class from flip-container
- Added perspective: 1500px to body for proper 3D flip animation
- Fixed maxWidth -> max-width CSS property
dreamwing added 25 commits March 6, 2026 00:10
- Added box-sizing: border-box to .card-face (width:100% + padding
  was causing overflow)
- Added overflow-x: hidden to body as safety net
- Include updated app-icon.png
- Added missing 'Optimize' button (et-btn) to external trigger
- Added missing CSS variables: --card-bg, --accent-green, --opt-glow,
  --opt-purple for proper text/icon colors
Moved perspective: 1500px from body to #view-tokens. CSS perspective
on body creates a containing block that breaks position:fixed on all
descendants (including bottom-nav), causing tabs to scroll away.
Added border: 1px solid var(--border) to .card-front to match the
subtle dark border from master's .card class.
- Changed card-front to use var(--card) instead of var(--card-bg)
- Removed transform: rotateY(0deg) from card-front default state
  (GPU compositing was subtly altering the card appearance)
- Removed redundant --card-bg variable, unified to --card
- Updated all remaining --card-bg refs in dashboard.js and CSS
Added padding-bottom to card-back to account for the fixed bottom
nav bar (64px). Since card-back uses position:absolute, it doesn't
benefit from body's padding-bottom.
- Removed all 4 hardcoded opt-items (A06, A09, A02, A07) from HTML
- Replaced fake dollar amounts ($38.72, $169.20, $130.48) with '--'
- Changed trigger text to 'Loading optimization data...' instead of
  hardcoded '5 actions available'
- opt-list div is now empty, populated only by renderOptimizerList()
  with real API data from /diagnostics endpoint
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@dreamwing
Copy link
Owner Author

@codex review please~

@dreamwing
Copy link
Owner Author

@greptileai review thx~

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 13, 2026

Greptile Summary

This PR implements the Cost Control Center — a diagnostic engine + optimizer UI that scans token usage data, surfaces 7–10 cost-saving recommendations, and lets users apply or roll back configuration changes with one tap. The backend adds DiagnosticsEngine (src/services/diagnostics.js), OptimizerService (src/services/optimizer.js), a pricing.js model database, two new API route files, and a shared utils/paths.js module. The frontend adds a 3D flip-card UI over the existing Token Economy view, a live diagnostic trigger banner, and an optimization history timeline.

The backend logic is well-designed: path-traversal protection on the undo endpoint, execFile (not exec) in openclaw_config.js, pre-mutation config backups, and a solid test suite.

Issues found:

  • public/index.html contains 5 unresolved git merge conflict markers (lines 378, 384, 456, 474, 500). The literal <<<<<<< HEAD, =======, and >>>>>>> strings will be rendered as visible text, corrupting the daily detail drill-down panel and the "Fully Optimized" history section. This is a blocking issue.
  • Triple duplicate id="cache-hit-rate" in public/index.html (lines 309, 383, 434) — two are direct duplicates (front card and back card), the third was introduced by the unresolved conflict. getElementById always returns the first match, so the optimizer back card's cache hit rate will never be populated.
  • diagnosticsEngine_cache = null in dashboard.js (line 958) — references an undefined variable after an undo operation, a copy-paste artefact from the server-side service.
  • "push" npm script in package.json chains git push with ci-watch.sh, requiring gh CLI and jq for all contributors — better kept as a local alias.
  • D04_idleDaysThreshold is absent from the defaults object in diagnostics.js, making it unoverridable via diagnostics.config.json without knowing the undocumented key name.

Confidence Score: 2/5

  • Not safe to merge — unresolved merge conflict markers in public/index.html will visibly corrupt the dashboard UI for all users.
  • The backend services, route handlers, and test coverage are solid and well-structured. However, public/index.html ships with five unresolved git merge conflict markers that will render as literal text and break the daily detail panel and optimizer history section. The accompanying duplicate id="cache-hit-rate" and diagnosticsEngine_cache reference error are additional runtime bugs introduced in the same files. These frontend issues must be fixed before merging.
  • public/index.html requires immediate attention to resolve merge conflicts and deduplicate element IDs. public/js/dashboard.js line 958 needs the diagnosticsEngine_cache reference removed.

Important Files Changed

Filename Overview
public/index.html Contains 5 unresolved git merge conflict markers and 3 duplicate id="cache-hit-rate" elements — both are breaking bugs that corrupt page layout and silence the optimizer's cache-hit display.
public/js/dashboard.js Adds the full optimizer/diagnostics UI logic. One logic bug: diagnosticsEngine_cache = null references an undefined variable after undo. Otherwise well-structured with proper escaping, WS-driven refresh, and graceful error handling.
src/services/diagnostics.js New DiagnosticsEngine implementing 7 active rules (D01–D09). Cache TTL, empty-data guard, and format normalization are solid. Minor: D04_idleDaysThreshold is not included in the defaults object, making it non-configurable via the config file.
src/services/optimizer.js OptimizerService correctly backs up config before mutations, has path-traversal protection for skill names, and supports per-action undo via backup restoration. Well-tested.
src/routes/optimizations.js Undo endpoint uses path.basename to strip directory components and verifies the resolved path starts within BACKUPS_DIR, correctly preventing path-traversal attacks.
package.json Adds a "push" npm script that chains git push with ci-watch.sh; this is a developer convenience tool that shouldn't be in the shared package manifest as it requires gh CLI and jq and affects all contributors.
src/services/openclaw_config.js New service wrapping the openclaw CLI for config reads and writes, with a direct file-read fallback. Correctly uses execFile (not exec) to avoid shell injection.

Sequence Diagram

sequenceDiagram
    participant UI as Dashboard (Browser)
    participant DR as GET /api/diagnostics
    participant DE as DiagnosticsEngine
    participant OR as POST /api/optimize/:id
    participant OS as OptimizerService
    participant OC as OpenClawConfig (CLI)
    participant UN as POST /api/optimizations/undo

    UI->>DR: fetchDiagnostics()
    DR->>DE: runDiagnostics()
    DE->>OC: getRawConfig()
    OC-->>DE: agents config
    DE->>DE: Read latest.json stats
    DE->>DE: Run D01–D09 rules
    DE-->>DR: { actions, totalMonthlySavings }
    DR-->>UI: JSON (strips _rawData unless ?verbose)

    UI->>OR: handleOpt(actionId, meta)
    OR->>OS: applyAction(actionId, savings, meta)
    OS->>OS: backupConfig() → data/backups/
    OS->>OC: setConfig(key, value)
    OC-->>OS: { success }
    OS->>OS: logOptimization() → optimizations.jsonl
    OS->>DE: invalidateCache()
    OS-->>OR: { success, backupPath }
    OR-->>UI: result

    UI->>UN: handleUndo(backupPath)
    UN->>OS: restoreBackup(resolved path)
    OS->>OC: setConfig(key, value) × N
    OS->>OS: logOptimization(UNDO)
    OS->>DE: invalidateCache()
    OS-->>UN: { success, restoredKeys }
    UN-->>UI: result
Loading

Last reviewed commit: 9fbc8c1

Comment on lines +378 to +500
=======
<div
style="background:rgba(34,211,238,0.1); border-radius:20px; padding:6px 14px; display:inline-flex; align-items:center; gap:8px;">
<span style="font-size:11px; color:var(--text-dim);">Cache Hit</span>
<span style="font-size:12px; font-weight:600; color:#22d3ee;"
id="cache-hit-rate">--</span>
>>>>>>> 3f45e78 (feat(tokens): optimize analysis report with WebSocket events, state
tracking, and enhanced analytics)
</div>
</div>
</div>

<div>
<button class="btn btn-danger" onclick="killAll()">
<span>🛑</span> Emergency Stop All Scripts
</button>
<div id="running-scripts-list"
style="font-size:11px; color:var(--text-dim); margin-top:8px; padding:0 5px;">
<!-- Scripts injected here -->
<!-- CARD SIDE 2: Optimizer Interface -->
<div class="card-face card-back">
<div class="back-header">
<div class="optimizer-brand">
<svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" width="22"
height="22">
<polygon points="13 2 3 14 12 14 11 22 21 10 12 10 13 2"></polygon>
</svg>
Optimizer
</div>
<button class="btn-close-back" onclick="flipToDashboard()">
<svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2" width="14"
height="14">
<line x1="18" y1="6" x2="6" y2="18"></line>
<line x1="6" y1="6" x2="18" y2="18"></line>
</svg>
</button>
</div>

<!-- Loader State -->
<div class="scan-progress" id="scan-progress">
<div class="scan-spinner"></div>
<div class="progress-title">Analyzing Usage Patterns</div>
<div class="progress-sub" id="scan-step-text">Reading 6 days of history...</div>
</div>

<!-- ACTIVE RESULTS VIEW -->
<div class="scan-results" id="scan-results">
<div class="savings-hero">
<div class="savings-hero-label">Potential Monthly Savings</div>
<div class="savings-hero-amount" id="main-savings-amount">--</div>
</div>
<div class="cost-comparison">
<div class="cost-compare-item">
<div class="cost-compare-label">Current</div>
<div class="cost-compare-val current">--</div>
</div>
<div class="cost-compare-arrow">→</div>
<div class="cost-compare-item">
<div class="cost-compare-label">Optimized</div>
<div class="cost-compare-val optimized">--</div>
</div>
<div class="cost-compare-item cache-rate-item" style="display:none;">
<div class="cost-compare-label">Cache Hit</div>
<div class="cost-compare-val" id="cache-hit-rate">—</div>
</div>
</div>
<div class="result-title">Cost-Saving Actions Found</div>
<div class="opt-list" id="opt-list">
<!-- Dynamically populated by renderOptimizerList() -->
</div>
</div>

<!-- SUCCESS STATE VIEW -->
<div class="success-state" id="success-state">
<div class="success-icon">
<svg viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="3" width="40"
height="40">
<path d="M22 11.08V12a10 10 0 1 1-5.93-9.14"></path>
<polyline points="22 4 12 14.01 9 11.01"></polyline>
</svg>
</div>
<div class="success-title">Fully Optimized</div>
<div class="success-desc">Great job! Your token consumption is now running at maximum
efficiency.</div>

<<<<<<< HEAD <div class="history-section">
<div class="history-title">Recent Optimizations</div>
<div class="timeline" id="timline-list">
<!-- Timeline items will be injected here -->
<div class="timeline-item">
<div class="timeline-dot"></div>
<div class="timeline-time">JUST NOW</div>
<div class="timeline-content">Applied 4 cost-saving policies
<span>-$35.02/mo</span>
</div>
</div>
<div class="timeline-item">
<div class="timeline-dot" style="border-color: var(--text-dim);"></div>
<div class="timeline-time">2 DAYS AGO</div>
<div class="timeline-content">Switched to Gemini 3.5 Sonnet</div>
</div>
</div>
</div>
=======
<div id="daily-detail"
style="display:none; background:rgba(255,255,255,0.03); border-radius:8px; padding:15px; margin-top:20px; border:1px solid var(--border);">
<div
style="display:flex; justify-content:space-between; align-items:center; margin-bottom:10px;">
<div style="display:flex; align-items:center; gap:10px;">
<span style="font-size:14px; font-weight:bold; color:var(--text)"
id="detail-date">--</span>
<span style="font-size:12px; font-weight:bold; display:none;"
id="detail-change"></span>
</div>
<span style="font-size:16px; font-weight:bold; color:var(--success)"
id="detail-cost">--</span>
</div>
<div
style="display:grid; grid-template-columns:1fr 1fr; gap:10px; font-size:12px; color:var(--text-dim);">
<div>In: <span id="detail-input"
style="color:var(--text); font-family:monospace;">--</span>
</div>
<div>Out: <span id="detail-output"
style="color:var(--text); font-family:monospace;">--</span>
</div>
<div>Cache R: <span id="detail-cache-read"
style="color:#22d3ee; font-family:monospace;">--</span></div>
<div>Cache W: <span id="detail-cache-write"
style="color:#22d3ee; font-family:monospace;">--</span></div>
>>>>>>> 3f45e78 (feat(tokens): optimize analysis report with WebSocket events, state
Copy link
Contributor

Choose a reason for hiding this comment

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

Unresolved git merge conflict markers

The file contains five unresolved merge conflict markers (<<<<<<< HEAD, =======, >>>>>>>) at lines 378, 384, 456, 474, and 500. These are not valid HTML and will be rendered as literal visible text in the browser, breaking the page layout in both the daily detail drill-down panel and the "Fully Optimized" success state's history section.

The conflict appears to be between the current branch's history-section UI (lines 456–473) and a daily-detail block from commit 3f45e78 (lines 474–500). You need to resolve which version of this content to keep, remove the conflict markers, and ensure there are no duplicate elements (e.g. id="daily-detail" and id="cache-hit-rate" already appear earlier in the file).

style="background:rgba(255,255,255,0.05); border-radius:20px; padding:6px 14px; display:inline-flex; align-items:center; gap:8px;">
<span style="font-size:11px; color:var(--text-dim);">Cache Hit</span>
<span style="font-size:12px; font-weight:600; color:var(--success);"
id="cache-hit-rate">--%</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Duplicate id="cache-hit-rate" causes stale display

The element ID "cache-hit-rate" is used three times in this file: line 309 (front card banner), line 383 (inside the merge conflict zone), and line 434 (optimizer back card's cost-comparison row).

document.getElementById('cache-hit-rate') always returns the first matching element. This means the renderOptimizerList() call in dashboard.js (which tries to update the cache hit rate on the optimizer back card) will silently update the front-card element instead, leaving the back card's display permanently showing .

After resolving the merge conflict, keep a single id="cache-hit-rate" element or use distinct IDs (e.g. "front-cache-hit-rate" vs "back-cache-hit-rate") and update the JS selectors accordingly.

showToast(`✓ Restored ${result.restoredKeys.length} settings from ${result.backupFile}`);
await renderHistoryList();
// Refresh diagnostics
diagnosticsEngine_cache = null; // Assuming this is a global cache variable
Copy link
Contributor

Choose a reason for hiding this comment

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

Reference to undefined variable diagnosticsEngine_cache

diagnosticsEngine_cache is not declared anywhere in the frontend JavaScript. This appears to be a copy-paste of the server-side cache-invalidation pattern from src/services/diagnostics.js, but it has no meaning in the browser context.

In non-strict mode this will silently create an unintentional global variable. In strict mode (or with a linter) it would throw a ReferenceError. The intent is presumably to ensure fresh data is fetched — the subsequent await fetchDiagnostics() already handles that. Remove this line:

Suggested change
diagnosticsEngine_cache = null; // Assuming this is a global cache variable
await fetchDiagnostics();

@@ -9,7 +9,8 @@
"test:coverage": "jest --forceExit --coverage",
"lint": "eslint src/ index.js",
"lint:fix": "eslint src/ index.js --fix",
Copy link
Contributor

Choose a reason for hiding this comment

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

Developer-workflow script committed to package.json

The "push" script (git push && ./.github/scripts/ci-watch.sh) is a personal developer convenience that runs the CI watcher after every push. Committing it to package.json means it applies to all contributors and CI environments, where ci-watch.sh requires the GitHub CLI (gh) and jq to be installed and authenticated. It also shadows any future npm publish-like tooling that might need a push hook.

Consider keeping this script local (e.g. in a personal .npmrc, a Makefile, or a git alias) rather than baking it into the shared package.json.

Comment on lines +26 to +35
const defaults = {
D01_modelCostRatio: 0.5, // trigger if single model > 50% of total cost
D06_cacheHitRateMin: 0.10, // trigger if cache hit rate < 10%
D06_cacheableRatio: 0.80, // assume 80% of input is cacheable
D09_outputRatioThreshold: 0.10, // trigger if output/input > 10%
D09_minOutputTokens: 1000, // minimum output tokens to trigger
D09_reductionFactor: 0.30, // concise mode reduces output by 30%
D05_thinkingProportion: 0.40, // 40% of output is thinking
D05_reductionRatio: 0.75 // minimal mode cuts thinking by 75%
};
Copy link
Contributor

Choose a reason for hiding this comment

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

D04_idleDaysThreshold missing from default thresholds

Every other threshold is declared in the defaults object so it's visible in data/diagnostics.config.json.example, but D04_idleDaysThreshold is only referenced inline as thresholds.D04_idleDaysThreshold || 7 on line 320. This means it can't be overridden via diagnostics.config.json without first knowing the undocumented key name.

Suggested change
const defaults = {
D01_modelCostRatio: 0.5, // trigger if single model > 50% of total cost
D06_cacheHitRateMin: 0.10, // trigger if cache hit rate < 10%
D06_cacheableRatio: 0.80, // assume 80% of input is cacheable
D09_outputRatioThreshold: 0.10, // trigger if output/input > 10%
D09_minOutputTokens: 1000, // minimum output tokens to trigger
D09_reductionFactor: 0.30, // concise mode reduces output by 30%
D05_thinkingProportion: 0.40, // 40% of output is thinking
D05_reductionRatio: 0.75 // minimal mode cuts thinking by 75%
};
const defaults = {
D01_modelCostRatio: 0.5, // trigger if single model > 50% of total cost
D06_cacheHitRateMin: 0.10, // trigger if cache hit rate < 10%
D06_cacheableRatio: 0.80, // assume 80% of input is cacheable
D09_outputRatioThreshold: 0.10, // trigger if output/input > 10%
D09_minOutputTokens: 1000, // minimum output tokens to trigger
D09_reductionFactor: 0.30, // concise mode reduces output by 30%
D05_thinkingProportion: 0.40, // 40% of output is thinking
D05_reductionRatio: 0.75, // minimal mode cuts thinking by 75%
D04_idleDaysThreshold: 7 // skills unused for > 7 days are flagged idle
};

Update data/diagnostics.config.json.example to include the key as well.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9fbc8c16c6

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +204 to +205
if (!entry.isDirectory() || entry.name.startsWith('.') || entry.name === 'node_modules') continue;
try {

Choose a reason for hiding this comment

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

P2 Badge Handle symlinked managed skills in A04 removal

The A04 optimizer only whitelists entries where entry.isDirectory() is true, so selected managed skills that are symlinked directories are treated as unknown and skipped; if the selection contains only such skills, the action fails with "Failed to move any skills." This conflicts with the diagnostics scanner, which explicitly includes symlinked skill folders (src/services/diagnostics.js handles isSymbolicLink()), so users can be shown removable skills that cannot actually be removed.

Useful? React with 👍 / 👎.

Comment on lines +422 to +423
if (cacheHitRate < thresholds.D06_cacheHitRateMin) {
const cacheableInput = totalInput * thresholds.D06_cacheableRatio;

Choose a reason for hiding this comment

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

P2 Badge Skip A06 recommendation when caching is already enabled

The caching recommendation is triggered solely by low observed cache-hit rate and does not check whether contextPruning.mode is already cache-ttl, so users who already enabled prompt caching can still be told to apply A06 and shown projected savings for a no-op change. This produces inaccurate optimization output and repeated suggestions that cannot materially reduce cost in that configuration.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant