Skip to content

fix(docs): Remove AI instructions to multiply percentage metrics by 100#10607

Merged
keydunov merged 1 commit intomasterfrom
cursor/CUB-2081-ai-percentage-multiplier-removal-d8b3
Apr 16, 2026
Merged

fix(docs): Remove AI instructions to multiply percentage metrics by 100#10607
keydunov merged 1 commit intomasterfrom
cursor/CUB-2081-ai-percentage-multiplier-removal-d8b3

Conversation

@keydunov
Copy link
Copy Markdown
Member

@keydunov keydunov commented Apr 1, 2026

Description

This PR removes incorrect AI instructions and examples that suggest multiplying percentage metrics by 100. The Cube frontend now automatically handles percentage formatting when format: percent is specified, so multiplication by 100 in the SQL is no longer needed and results in incorrect values (e.g., showing 5000% instead of 50%).

Changes

Documentation Updates

  • Getting Started Guides: Updated completed_percentage examples in Cloud and Databricks tutorials
  • Data Modeling Overview: Updated generated SQL example for paying_percentage
  • Recipes: Updated percentage calculations in:
    • Event Analytics (bounce rate, repeat rate)
    • Cohort Retention (retention percentage)
    • Using Dynamic Measures (order status percentages)
    • Calculated Members (purchase percentage)
  • API Documentation: Updated SQL API query format examples

Code Changes

  • Funnels Extension: Updated conversionsPercent measure to use 1.0 * instead of 100.0 *
  • Example Schemas:
    • examples/recipes/active-users: Fixed WAU to MAU ratio
    • examples/recipes/referencing-dynamic-measures: Fixed percentage measures
  • Test Fixtures: Updated calendar_orders.yml test fixture

Key Changes

Before (Incorrect):

- name: completed_percentage
  type: number
  sql: "(100.0 * {CUBE.completed_count} / NULLIF({CUBE.count}, 0))"
  format: percent

After (Correct):

- name: completed_percentage
  type: number
  sql: "(1.0 * {CUBE.completed_count} / NULLIF({CUBE.count}, 0))"
  format: percent

Rationale

  • The frontend automatically multiplies by 100 when displaying values with format: percent
  • Multiplying by 100 in SQL causes double multiplication (100 * 100 = 10,000x)
  • Keeping 1.0 * ensures proper floating-point division while avoiding the multiplication issue
  • This aligns with the existing Databricks tutorial which already uses the correct pattern

Testing

  • Verified all documentation examples are consistent
  • Updated test fixtures to match new pattern
  • Confirmed Funnels extension generates correct SQL

Related Issue

Resolves CUB-2081

Linear Issue: CUB-2081

Open in Web Open in Cursor 

@github-actions github-actions bot added the javascript Pull requests that update Javascript code label Apr 1, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.03%. Comparing base (867a236) to head (89658f0).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #10607   +/-   ##
=======================================
  Coverage   58.03%   58.03%           
=======================================
  Files         215      215           
  Lines       16683    16683           
  Branches     3357     3357           
=======================================
  Hits         9682     9682           
  Misses       6511     6511           
  Partials      490      490           
Flag Coverage Δ
cube-backend 58.03% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- Updated documentation to use 1.0 * instead of 100.0 * for percentage calculations
- Frontend now automatically multiplies by 100 when format: percent is used
- Changed examples in getting started guides, recipes, and API documentation
- Updated Funnels extension conversionsPercent measure
- Updated test fixtures and example schemas
- Kept decimal point multiplication (1.0 *) to ensure proper floating point division

Resolves CUB-2081

Co-authored-by: Artyom Keydunov <artyom.keydunov@gmail.com>
@keydunov keydunov force-pushed the cursor/CUB-2081-ai-percentage-multiplier-removal-d8b3 branch from 493b641 to 89658f0 Compare April 16, 2026 04:15
@keydunov keydunov marked this pull request as ready for review April 16, 2026 04:15
@keydunov keydunov requested a review from a team as a code owner April 16, 2026 04:15
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 16, 2026

Claude finished @keydunov's task in 4m 32s —— View job


Code Review

  • Gather context and review the diff
  • Review source code change (Funnels.ts)
  • Review test fixture changes
  • Review documentation changes
  • Review example schema changes
  • Verify consistency and check for missed occurrences
  • Verify the frontend auto-formatting claim
  • Post final review

Summary

The core rationale of this PR is correct. I verified that packages/cubejs-client-core/src/format.ts:11 defines DEFAULT_PERCENT_FORMAT = '.2%', which is a d3-format specifier that automatically multiplies the value by 100 before displaying. So a SQL value of 0.5 with format: percent correctly displays as 50.00%. The old 100.0 * multiplication caused double-counting (e.g., 50.05000.00%).

The Funnels.ts change and all documentation updates are correct and consistent.

Issues Found

1. Precision loss in ROUND examples (minor)

Three files use ROUND(ratio, 2) after removing the * 100.0:

  • docs-mintlify/recipes/data-modeling/using-dynamic-measures.mdx:42-44
  • docs/content/product/data-modeling/recipes/using-dynamic-measures.mdx:39-41
  • examples/recipes/referencing-dynamic-measures/schema/Orders.js:25

The old code ROUND(ratio * 100.0, 2) gave 2 decimal places in the percentage space (e.g., 33.33). Now ROUND(ratio, 2) rounds the raw ratio to 2 decimal places (e.g., 0.33), and d3's .2% format displays it as 33.00% instead of 33.33%. Consider either:

  • Increasing to ROUND(..., 4) to preserve visible precision, or
  • Removing ROUND entirely and letting format: percent handle all rounding

See inline comments with suggested fixes.

2. Missed occurrences in integration test files (minor)

Several integration test files still contain the old * 100.0 pattern with format: percent. While these measures aren't directly asserted in test expectations, they should be updated for consistency:

File Line Measure
test/integration/postgres/calendars.test.ts 119 completed_percentage
test/integration/postgres/multi-stage.test.ts 63 return_rate
test/integration/postgres/calc-groups.test.ts 139 return_rate
test/integration/postgres/member-expression.test.ts 97 return_rate

These are inline schema definitions (not from the updated calendar_orders.yml fixture), so they were easy to miss.

What Looks Good

  • Funnels.ts (packages/cubejs-schema-compiler/src/extensions/Funnels.ts:48): The only production code change is correct. The CASE WHEN ... THEN 1.0 * conversions / firstStepConversions ELSE NULL END properly handles division-by-zero and ensures float division.
  • Test fixture (calendar_orders.yml): Updated correctly and also normalized the SQL expression style from ({completed_count} / NULLIF({count}, 0)) * 100.0 to 1.0 * {completed_count} / NULLIF({count}, 0) — cleaner.
  • Documentation: All 13 doc files are updated consistently across both docs/ and docs-mintlify/ trees, covering YAML and JavaScript variants.
  • Examples: Both recipe examples updated correctly.
  • No security concerns: Changes are purely arithmetic/formatting.
  • No snapshot tests broken: Verified no snapshot files reference the changed measures.

Verdict

The PR is solid and the fix is correct. The two minor issues above (ROUND precision and missed test files) are not blockers but worth addressing for completeness.

@keydunov keydunov merged commit bca52d9 into master Apr 16, 2026
14 of 15 checks passed
@keydunov keydunov deleted the cursor/CUB-2081-ai-percentage-multiplier-removal-d8b3 branch April 16, 2026 04:16
Comment on lines 42 to +44
`ROUND(${CUBE[`total_${status}_orders`]}::NUMERIC / ${
CUBE.total_orders
}::NUMERIC * 100.0, 2)`
}::NUMERIC, 2)`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Potential precision loss: With the old code, ROUND(ratio * 100.0, 2) gave 2 decimal places in the percentage space (e.g., 33.33). Now ROUND(ratio, 2) rounds the raw ratio to 2 decimal places (e.g., 0.33), and d3's .2% format will display it as 33.00% instead of 33.33%.

Since the format: percent (d3 .2%) already handles rounding when displaying, ROUND is arguably unnecessary now. But if you want to keep it for clean SQL output, consider increasing the precision:

Suggested change
`ROUND(${CUBE[`total_${status}_orders`]}::NUMERIC / ${
CUBE.total_orders
}::NUMERIC * 100.0, 2)`
}::NUMERIC, 2)`
`ROUND(${CUBE[`total_${status}_orders`]}::NUMERIC / ${
CUBE.total_orders
}::NUMERIC, 4)`

Comment on lines 39 to +41
`ROUND(${CUBE[`total_${status}_orders`]}::NUMERIC / ${
CUBE.total_orders
}::NUMERIC * 100.0, 2)`
}::NUMERIC, 2)`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same precision concern as the mintlify version — ROUND(ratio, 2) truncates to 2 decimal places before the frontend multiplies by 100, so 0.3333 becomes 0.33 → displayed as 33.00% instead of 33.33%.

Suggested change
`ROUND(${CUBE[`total_${status}_orders`]}::NUMERIC / ${
CUBE.total_orders
}::NUMERIC * 100.0, 2)`
}::NUMERIC, 2)`
`ROUND(${CUBE[`total_${status}_orders`]}::NUMERIC / ${
CUBE.total_orders
}::NUMERIC, 4)`

title: `Percentage of ${status} orders`,
sql: (CUBE) =>
`ROUND(${CUBE[`Total_${status}_orders`]}::numeric / ${CUBE.totalOrders}::numeric * 100.0, 2)`,
`ROUND(${CUBE[`Total_${status}_orders`]}::numeric / ${CUBE.totalOrders}::numeric, 2)`,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same precision concern as the docs: ROUND(ratio, 2) pre-truncates before the frontend's ×100 formatting. A raw ratio of 0.3333 becomes 0.33 → displayed as 33.00% rather than 33.33%. Consider ROUND(..., 4) to preserve two visible decimal places after the frontend multiplies, or remove the ROUND entirely and let the format: percent (d3 .2%) handle all rounding.

Suggested change
`ROUND(${CUBE[`Total_${status}_orders`]}::numeric / ${CUBE.totalOrders}::numeric, 2)`,
`ROUND(${CUBE[`Total_${status}_orders`]}::numeric / ${CUBE.totalOrders}::numeric, 4)`,

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

Labels

javascript Pull requests that update Javascript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants