Skip to content

fix(compression): record no-op compression runs so Stacked is visible in analytics (#4268) - #5277

Merged
diegosouzapw merged 1 commit into
release/v3.8.40from
fix/4268-stacked-observability
Jun 29, 2026
Merged

diegosouzapw merged 1 commit into
release/v3.8.40from
fix/4268-stacked-observability

Conversation

@diegosouzapw

Copy link
Copy Markdown
Owner

Closes #4268

Problem

Stacked (RTK→Caveman) compression is dispatched correctly on the /v1 path, but byMode.stacked.count stays flat and analytics show nothing — making it indistinguishable from "never ran". Root cause (confirmed with the maintainer's own diagnosis): a compression_analytics row is written only on a net-positive saving (chatCore.ts: result.compressed || fallbackApplied || cavemanOutputModeApplied). When a Stacked pipeline runs on already-compact context, both engines no-op → net 0 → no row at all. Ultra's heuristic almost always trims something, so its count climbs — exactly the asymmetry the reporter observed.

Fix (additive, no migration file needed)

  • New skip_reason column on compression_analytics, added through the existing ensureCompressionAnalyticsColumns ALTER path (same pattern as the other extended columns).
  • chatCore now records attempted-but-no-op runs via a new writeCompressionSkip (tokens_saved = 0, skip_reason: "no_savings"). The else only fires when result.stats is truthy — i.e. the pipeline genuinely ran (the early stats: null returns for empty/disabled bodies are untouched), so we don't record spurious skips.
  • Aggregation introduces one successWhere = appendCondition(whereClause, "skip_reason IS NULL") applied to every saving aggregate, so existing totals/averages are byte-identical (no skip rows exist for historical data). Skips are surfaced separately as per-mode skipped, plus totalSkipped and bySkipReason.
  • UI (Mode Breakdown) shows · N skipped (no-op), so a mode that only ever no-ops is now visible (count 0, skipped > 0).

Validation (Hard Rule #18 — TDD)

tests/unit/compression/compressionAnalytics.test.ts:

  • skip rows are counted separately without polluting saving aggregates (totalRequests/byMode.count/totalTokensSaved exclude them);
  • a skip-only mode still appears (count 0, skipped 2, totalSkipped, bySkipReason.no_savings).
  • Fail-before proven by reverting the successWhere exclusion → both new tests fail; restored → 17/17 green.

typecheck:core 0 errors, eslint clean, related compression suites green.

Note: active modes that no-op now write one (fire-and-forget, off-hot-path) row per request. That's the point of the transparency ask; net-saving accounting is unchanged.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

… in analytics (#4268)

A compression_analytics row was written only on a net-positive saving
(chatCore: result.compressed || fallbackApplied || cavemanOutputModeApplied).
A Stacked RTK->Caveman pipeline that ran on already-compact context saved
nothing -> no row -> indistinguishable from 'never dispatched'
(byMode.stacked.count stayed flat while Ultra climbed).

Attempted-but-no-op runs are now recorded via writeCompressionSkip with a
skip_reason. A new skip_reason column (added through the existing
ensureCompressionAnalyticsColumns ALTER path) marks these rows; the analytics
summary excludes them from every saving aggregate (via a single successWhere =
'skip_reason IS NULL' filter, so historical totals/averages are byte-identical)
and surfaces them as per-mode 'skipped' plus totalSkipped / bySkipReason. The
Mode Breakdown shows 'N skipped (no-op)'.

TDD: compressionAnalytics.test.ts gains coverage that skip rows are counted
separately without polluting saving aggregates, and that a skip-only mode still
appears (count 0, skipped > 0). Fail-before proven by reverting the successWhere
exclusion (both new tests fail). typecheck:core + eslint clean; 17/17 suite green.
@diegosouzapw
diegosouzapw force-pushed the fix/4268-stacked-observability branch from faa5416 to 3a0a0f5 Compare June 29, 2026 00:40
@diegosouzapw
diegosouzapw merged commit bc00e32 into release/v3.8.40 Jun 29, 2026
7 checks passed
@diegosouzapw
diegosouzapw deleted the fix/4268-stacked-observability branch June 29, 2026 01:05
@diegosouzapw diegosouzapw mentioned this pull request Jun 29, 2026
tkgo11 pushed a commit to tkgo11/OmniRoute that referenced this pull request Sep 23, 2026
… in analytics (diegosouzapw#4268) (diegosouzapw#5277)

A compression_analytics row was written only on a net-positive saving
(chatCore: result.compressed || fallbackApplied || cavemanOutputModeApplied).
A Stacked RTK->Caveman pipeline that ran on already-compact context saved
nothing -> no row -> indistinguishable from 'never dispatched'
(byMode.stacked.count stayed flat while Ultra climbed).

Attempted-but-no-op runs are now recorded via writeCompressionSkip with a
skip_reason. A new skip_reason column (added through the existing
ensureCompressionAnalyticsColumns ALTER path) marks these rows; the analytics
summary excludes them from every saving aggregate (via a single successWhere =
'skip_reason IS NULL' filter, so historical totals/averages are byte-identical)
and surfaces them as per-mode 'skipped' plus totalSkipped / bySkipReason. The
Mode Breakdown shows 'N skipped (no-op)'.

TDD: compressionAnalytics.test.ts gains coverage that skip rows are counted
separately without polluting saving aggregates, and that a skip-only mode still
appears (count 0, skipped > 0). Fail-before proven by reverting the successWhere
exclusion (both new tests fail). typecheck:core + eslint clean; 17/17 suite green.
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