-
Notifications
You must be signed in to change notification settings - Fork 26
Enhance static report generation guide with detailed usage #3367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ns and environment verification - Expanded the guide to cater to different audiences: operations teams, hands-on developers, and data engineers. - Added sections on verifying the environment setup for Node.js and Docker. - Included a comprehensive overview of the benefits of automating report generation, highlighting cost savings and efficiency improvements. - Introduced data validation recommendations to ensure data quality before ingestion. - Updated troubleshooting tips for common setup and query issues. This update aims to provide users with a clearer path to implementing automated report generation using MooseStack.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughExpanded the static-report-generation guide with onboarding and environment verification, updated data model typings (nullable/optional fields and string|Date handling), added a new public API export for Changes
(*Paths compacted with ellipsis where multiple similar files were edited.) Sequence Diagram(s)sequenceDiagram
participant Client
participant API as Sales API (/api/sales-report)
participant CH as ClickHouse
participant PDF as PdfGenerator
Client->>API: GET /api/sales-report?start=&end=®ion=
API->>CH: Query materialized view / tables for aggregated data
CH-->>API: Return aggregated rows (JSON)
API->>PDF: generatePdf(data) (async Promise)
PDF-->>API: Resolve with PDF bytes or URL
API-->>Client: 200 + PDF (application/pdf) or JSON with URL/status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx (1)
237-238: ClarifycustomerIdoptionality in the comment.The comment states "(optional)" but the field is typed as
string(required). While line 435 documents using empty strings for missing loyalty cards, the inline comment should match this pattern to avoid confusion.Suggested fix
- // Customer (optional - from loyalty card) - customerId: string; + // Customer ID (use empty string when no loyalty card) + customerId: string;
🤖 Fix all issues with AI agents
In `@apps/framework-docs-v2/content/guides/static-report-generation.mdx`:
- Around line 473-507: In validateRecord, numeric fields (totalAmount, quantity,
unitPrice) can be NaN/undefined and bypass the current checks; add explicit
finite-number guards using Number.isFinite(...) before doing comparisons or
math. Concretely: for totalAmount and quantity, first push a validation error if
!Number.isFinite(record.totalAmount) or !Number.isFinite(record.quantity); only
then perform the negative/positive checks and only compute expectedTotal =
record.quantity * record.unitPrice after confirming
Number.isFinite(record.unitPrice) as well. Update the existing amount/quantity
checks and the Amount mismatch block in validateRecord to early-fail on
non-finite values to avoid NaN results.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-16T23:09:10.917Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:10.917Z
Learning: Applies to apps/framework-cli/**/*.rs : Document all public APIs and breaking changes
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/ingest/models.ts : Data models in ingest/models.ts should define interfaces using the Model naming convention, and create corresponding IngestPipeline instances with table, stream, and ingestApi options enabled
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/views/*.ts : Use SummingMergeTree engine for materialized views that require incremental updates, and wrap aggregated metrics with SUM() when querying to properly merge partial rows
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: For slow dashboard queries (>500ms) that aggregate across multiple tables or need cohort-based breakdowns, implement a materialized view instead of real-time queries
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🪛 LanguageTool
apps/framework-docs-v2/content/guides/static-report-generation.mdx
[style] ~105-~105: To elevate your writing, try using a synonym here.
Context: ...ripts** | No streaming, no type safety, hard to extend | **This approach gives you:...
(HARD_TO)
[style] ~625-~625: To elevate your writing, try using an alternative expression here.
Context: ...ggregates into final totals Why this matters for performance: | Query Target | Ro...
(MATTERS_RELEVANT)
[uncategorized] ~1286-~1286: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n real-time - API serves any date range on demand - PDF generation takes < 1 second - **T...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~1549-~1549: Ensure spelling is correct
Context: ...y is slow:** If queries take more than 100ms, check: - Are you querying the material...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
apps/framework-docs-v2/content/guides/static-report-generation.mdx
Outdated
Show resolved
Hide resolved
- Updated numeric validation to use `Number.isFinite()` for `totalAmount`, `quantity`, and `unitPrice` to ensure they are valid numbers. - Adjusted business rule validation to only check for amount mismatches if all numeric fields are valid. - Improved error messages for invalid inputs to enhance user experience and data integrity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/framework-docs-v2/content/guides/static-report-generation.mdx`:
- Around line 534-541: The docs mention a "Future dates" check but
validateRecord doesn't implement it; update the validateRecord function to
detect saleTimestamp > now by parsing record.saleTimestamp to a Date, comparing
it to a captured now (const now = new Date()), and when true push an error into
the existing errors array (use the same shape as other errors, referencing
row/index and field "saleTimestamp" with an appropriate message); alternatively,
if you prefer not to add runtime validation, add a short note next to
validateRecord that the future-date check is intentionally omitted and left as
an exercise.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/ingest/models.ts : Data models in ingest/models.ts should define interfaces using the Model naming convention, and create corresponding IngestPipeline instances with table, stream, and ingestApi options enabled
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/views/*.ts : Use SummingMergeTree engine for materialized views that require incremental updates, and wrap aggregated metrics with SUM() when querying to properly merge partial rows
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: For slow dashboard queries (>500ms) that aggregate across multiple tables or need cohort-based breakdowns, implement a materialized view instead of real-time queries
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🪛 LanguageTool
apps/framework-docs-v2/content/guides/static-report-generation.mdx
[style] ~105-~105: To elevate your writing, try using a synonym here.
Context: ...ripts** | No streaming, no type safety, hard to extend | **This approach gives you:...
(HARD_TO)
[style] ~636-~636: To elevate your writing, try using an alternative expression here.
Context: ...ggregates into final totals Why this matters for performance: | Query Target | Ro...
(MATTERS_RELEVANT)
[uncategorized] ~1297-~1297: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n real-time - API serves any date range on demand - PDF generation takes < 1 second - **T...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~1560-~1560: Ensure spelling is correct
Context: ...y is slow:** If queries take more than 100ms, check: - Are you querying the material...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (7)
apps/framework-docs-v2/content/guides/static-report-generation.mdx (7)
18-32: Clear audience segmentation improves discoverability.The path indicators help users quickly find relevant sections. Good addition.
41-57: Prerequisite verification commands are correct.The Docker running check correctly redirects stderr and provides actionable guidance.
184-213: Stack verification steps are practical and well-structured.Including expected outputs and the 10-15 second wait guidance will reduce support friction.
492-519: Numeric validation correctly usesNumber.isFinite()— LGTM.The fix from the previous review is properly implemented. The
else ifpattern prevents duplicate errors when values are non-finite.Two optional enhancements for completeness:
unitPriceisn't checked for> 0(prices are typically non-negative)- The table at line 540 mentions catching "Future dates" but no validation implements this
Neither is critical for a documentation example.
1174-1208: SummingMergeTree explanation is accurate and emphasizes the critical GROUP BY requirement.The example correctly shows that queries must use
sum()aggregation to consolidate partial rows:SELECT saleDate, region, sum(transactionCount), sum(totalRevenue) FROM DailySalesSummary GROUP BY saleDate, regionThis aligns with best practices for querying materialized views with SummingMergeTree. Based on learnings, wrapping metrics with
SUM()is essential for correctness.
1522-1540: Query troubleshooting correctly demonstrates the MV pitfall and solution.The "Wrong" vs "Correct" pattern effectively illustrates why
GROUP BYis mandatory with SummingMergeTree. This is a common gotcha for ClickHouse newcomers.
1283-1309: Before/After comparison effectively quantifies ROI.The concrete metrics (500+ analyst hours/year, zero delayed reports) give stakeholders actionable justification for implementing the pipeline. Well-structured for operations audiences mentioned in the guide intro.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Added a check to prevent future dates in the `saleTimestamp` field, ensuring transactions are not recorded with dates beyond the current date. - This update improves data integrity and user experience by providing clearer error messages for invalid date inputs.
Wrap `<1M rows` in backticks to prevent MDX from parsing it as a JSX tag. The `<1M` pattern was causing "Unexpected character 1 before name" compilation errors. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/framework-docs-v2/content/guides/static-report-generation.mdx`:
- Around line 640-644: The docs claim the MV uses SummingMergeTree but the
example actually defaults to MergeTree; either set the MV's engine explicitly by
adding engine: ClickHouseEngines.SummingMergeTree to the materialized view
configuration that writes into DailySalesSummary (so the background merges do
summing behavior), or change the bullet text to state that the example uses the
default MergeTree and explain that SummingMergeTree must be set explicitly in
the materialized view config for automatic summing of partial aggregates from
sales_SalesTransaction.
- Around line 487-499: Replace the loose Date.parse checks with strict ISO
parsing: add an import for parseISO and isValid from date-fns, then in the
validation for record.saleTimestamp parse once using
parseISO(record.saleTimestamp), check isValid(parsed) to push a "Wrong Format"
error when invalid, and if valid compare parsed to new Date() to push the
"Future dates not allowed" error; remove uses of Date.parse/isNaN and
consolidate the two blocks into a single validation that uses parseISO, isValid,
and the existing errors.push calls (referencing record.saleTimestamp, errors,
saleDate/new Date comparisons).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/ingest/models.ts : Data models in ingest/models.ts should define interfaces using the Model naming convention, and create corresponding IngestPipeline instances with table, stream, and ingestApi options enabled
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/views/*.ts : Use SummingMergeTree engine for materialized views that require incremental updates, and wrap aggregated metrics with SUM() when querying to properly merge partial rows
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: For slow dashboard queries (>500ms) that aggregate across multiple tables or need cohort-based breakdowns, implement a materialized view instead of real-time queries
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🪛 LanguageTool
apps/framework-docs-v2/content/guides/static-report-generation.mdx
[style] ~105-~105: To elevate your writing, try using a synonym here.
Context: ...ripts** | No streaming, no type safety, hard to extend | **This approach gives you:...
(HARD_TO)
[style] ~645-~645: To elevate your writing, try using an alternative expression here.
Context: ...ggregates into final totals Why this matters for performance: | Query Target | Ro...
(MATTERS_RELEVANT)
[uncategorized] ~1306-~1306: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n real-time - API serves any date range on demand - PDF generation takes < 1 second - **T...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~1569-~1569: Ensure spelling is correct
Context: ...y is slow:** If queries take more than 100ms, check: - Are you querying the material...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
apps/framework-docs-v2/content/guides/static-report-generation.mdx
Outdated
Show resolved
Hide resolved
apps/framework-docs-v2/content/guides/static-report-generation.mdx
Outdated
Show resolved
Hide resolved
- Replaced the existing date validation logic with `date-fns` for stricter ISO 8601 parsing, improving error handling for invalid date formats. - Updated documentation to emphasize the necessity of using `GROUP BY` when querying materialized views, clarifying aggregation behavior in ClickHouse. - Revised sections to better explain how materialized views aggregate data and the implications for querying, ensuring users understand the importance of accurate data retrieval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx (1)
652-656: Remove SummingMergeTree reference from the query step description.The MV example doesn't explicitly set SummingMergeTree, so the phrase is misleading. The doc later states GROUP BY is the safe practice regardless of engine. Use wording that applies to any MV result set:
Proposed fix
-Query the pre-aggregated data (always use GROUP BY with SummingMergeTree): +Query the pre-aggregated data (always use GROUP BY when reading MV results):
🤖 Fix all issues with AI agents
In `@apps/framework-docs-v2/content/guides/static-report-generation.mdx`:
- Around line 541-545: Update the "Invalid date format" row in the data-quality
table to reference parseISO and isValid from date-fns instead of Date.parse();
specifically change the Detection column text to mention using parseISO(...)
followed by isValid(...) (e.g., "Use parseISO(dateString) and
isValid(parsedDate)") and adjust the Impact if Missed wording if needed to
reflect silent failures when parseISO yields an invalid date; look for the table
row labeled "Invalid date format" to apply this change.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/views/*.ts : Use SummingMergeTree engine for materialized views that require incremental updates, and wrap aggregated metrics with SUM() when querying to properly merge partial rows
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2025-12-17T23:59:22.807Z
Learnt from: callicles
Repo: 514-labs/moosestack PR: 3143
File: apps/framework-docs/llm-docs/python/table-setup.md:331-336
Timestamp: 2025-12-17T23:59:22.807Z
Learning: In apps/framework-docs/llm-docs/**/*.md (LLM documentation), prefer exact class and type names (e.g., "MaterializedView", "OlapTable") over lowercase prose variants (e.g., "materialized view") to help LLMs generate correct code with proper imports and syntax.
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/ingest/models.ts : Data models in ingest/models.ts should define interfaces using the Model naming convention, and create corresponding IngestPipeline instances with table, stream, and ingestApi options enabled
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: For slow dashboard queries (>500ms) that aggregate across multiple tables or need cohort-based breakdowns, implement a materialized view instead of real-time queries
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🪛 LanguageTool
apps/framework-docs-v2/content/guides/static-report-generation.mdx
[style] ~105-~105: To elevate your writing, try using a synonym here.
Context: ...ripts** | No streaming, no type safety, hard to extend | **This approach gives you:...
(HARD_TO)
[style] ~643-~643: To elevate your writing, try using an alternative expression here.
Context: ...nsolidate results correctly Why this matters for performance: | Query Target | Ro...
(MATTERS_RELEVANT)
[uncategorized] ~1309-~1309: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n real-time - API serves any date range on demand - PDF generation takes < 1 second - **T...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~1572-~1572: Ensure spelling is correct
Context: ...y is slow:** If queries take more than 100ms, check: - Are you querying the material...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Changed the date validation logic to use `parseISO()` and `isValid()` from date-fns for improved accuracy in detecting invalid date formats. - This update enhances error handling and ensures better data integrity in report generation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/framework-docs-v2/content/guides/static-report-generation.mdx`:
- Around line 1517-1532: The docs currently advise using the FINAL modifier on
local.sales_SalesTransaction to deduplicate, but FINAL is not supported on plain
MergeTree and will error; update the guidance to state that FINAL only works on
MergeTree variants that implement row-level replacement/collapsing (e.g.,
ReplacingMergeTree, CollapsingMergeTree) and instruct the reader to use one of
those engines or an alternative deduplication strategy if their table uses a
plain MergeTree (reference FINAL and local.sales_SalesTransaction, and mention
ReplacingMergeTree/CollapsingMergeTree).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/views/*.ts : Use SummingMergeTree engine for materialized views that require incremental updates, and wrap aggregated metrics with SUM() when querying to properly merge partial rows
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2025-12-17T23:59:22.807Z
Learnt from: callicles
Repo: 514-labs/moosestack PR: 3143
File: apps/framework-docs/llm-docs/python/table-setup.md:331-336
Timestamp: 2025-12-17T23:59:22.807Z
Learning: In apps/framework-docs/llm-docs/**/*.md (LLM documentation), prefer exact class and type names (e.g., "MaterializedView", "OlapTable") over lowercase prose variants (e.g., "materialized view") to help LLMs generate correct code with proper imports and syntax.
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/ingest/models.ts : Data models in ingest/models.ts should define interfaces using the Model naming convention, and create corresponding IngestPipeline instances with table, stream, and ingestApi options enabled
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: For slow dashboard queries (>500ms) that aggregate across multiple tables or need cohort-based breakdowns, implement a materialized view instead of real-time queries
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🪛 LanguageTool
apps/framework-docs-v2/content/guides/static-report-generation.mdx
[style] ~105-~105: To elevate your writing, try using a synonym here.
Context: ...ripts** | No streaming, no type safety, hard to extend | **This approach gives you:...
(HARD_TO)
[style] ~643-~643: To elevate your writing, try using an alternative expression here.
Context: ...nsolidate results correctly Why this matters for performance: | Query Target | Ro...
(MATTERS_RELEVANT)
[uncategorized] ~1309-~1309: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n real-time - API serves any date range on demand - PDF generation takes < 1 second - **T...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~1572-~1572: Ensure spelling is correct
Context: ...y is slow:** If queries take more than 100ms, check: - Are you querying the material...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (5)
apps/framework-docs-v2/content/guides/static-report-generation.mdx (5)
18-112: Clear audience segmentation and ROI framing.
The guide is easier to scan and self-select now.
184-213: Actionable verification steps and outcomes.
The health checks and “What Just Happened” callouts are crisp and practical.Also applies to: 270-291
458-784: Solid validation and aggregation walkthrough.
The validation block plus API narrative reads cleanly and is easy to follow.
1102-1342: Strong conceptual deep dive and impact framing.
Nice balance between theory and outcomes.
1446-1516: Troubleshooting coverage is comprehensive.
Setup, data, query, and PDF sections are well scoped.Also applies to: 1534-1600
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Revised the section on deduplication in queries to include examples using `GROUP BY` and subqueries for better clarity. - Added a note regarding the `FINAL` modifier's compatibility with specific MergeTree variants, enhancing user understanding of deduplication options in ClickHouse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx (1)
370-376: Don't drop rows that should fail validation.
The CSV loader filters out falsytransactionIdrows, so the new validation for missing IDs never triggers and those records are silently discarded. Filter only fully empty rows (or validate before filtering) so missing IDs are surfaced.🛠️ Proposed fix (keep rows with missing IDs)
- records = data.filter((r) => r.transactionId); // Filter empty rows + records = data.filter((r) => + Object.values(r).some((v) => v !== "" && v !== null && v !== undefined) + ); // Drop fully empty rows; let validation catch missing IDsAlso applies to: 474-537
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/views/*.ts : Use SummingMergeTree engine for materialized views that require incremental updates, and wrap aggregated metrics with SUM() when querying to properly merge partial rows
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2025-12-17T23:59:22.807Z
Learnt from: callicles
Repo: 514-labs/moosestack PR: 3143
File: apps/framework-docs/llm-docs/python/table-setup.md:331-336
Timestamp: 2025-12-17T23:59:22.807Z
Learning: In apps/framework-docs/llm-docs/**/*.md (LLM documentation), prefer exact class and type names (e.g., "MaterializedView", "OlapTable") over lowercase prose variants (e.g., "materialized view") to help LLMs generate correct code with proper imports and syntax.
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/ingest/models.ts : Data models in ingest/models.ts should define interfaces using the Model naming convention, and create corresponding IngestPipeline instances with table, stream, and ingestApi options enabled
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: For slow dashboard queries (>500ms) that aggregate across multiple tables or need cohort-based breakdowns, implement a materialized view instead of real-time queries
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🪛 LanguageTool
apps/framework-docs-v2/content/guides/static-report-generation.mdx
[style] ~105-~105: To elevate your writing, try using a synonym here.
Context: ...ripts** | No streaming, no type safety, hard to extend | **This approach gives you:...
(HARD_TO)
[style] ~643-~643: To elevate your writing, try using an alternative expression here.
Context: ...nsolidate results correctly Why this matters for performance: | Query Target | Ro...
(MATTERS_RELEVANT)
[uncategorized] ~1309-~1309: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n real-time - API serves any date range on demand - PDF generation takes < 1 second - **T...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~1585-~1585: Ensure spelling is correct
Context: ...y is slow:** If queries take more than 100ms, check: - Are you querying the material...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx (1)
774-782: Verify the SQL-injection guarantee ofsql.
The doc states thesqltemplate literal “prevents SQL injection automatically.” Please confirm this behavior in moose-lib; if it’s not a hard guarantee, soften the wording to avoid overpromising.✏️ Safer phrasing (if behavior isn’t guaranteed)
-4. **Uses parameterized SQL** - The `sql` template literal prevents SQL injection automatically +4. **Uses parameterized SQL** - Prefer bound parameters over string concatenation to reduce injection risk
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/framework-docs-v2/content/guides/static-report-generation.mdx`:
- Around line 512-515: In the validation function that checks saleTimestamp
(look for the block that pushes { row: index, field: "saleTimestamp", message:
"Invalid date type" }), remove the early "return errors" and instead push the
error and continue executing the rest of the validators so numeric and
business-rule checks still run; ensure the function only returns the aggregated
errors at the end (e.g., after all field checks) and not inside the
saleTimestamp type-check branch.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-16T23:09:10.917Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: apps/framework-cli/AGENTS.md:0-0
Timestamp: 2025-12-16T23:09:10.917Z
Learning: Applies to apps/framework-cli/**/*.rs : Document all public APIs and breaking changes
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/views/*.ts : Use SummingMergeTree engine for materialized views that require incremental updates, and wrap aggregated metrics with SUM() when querying to properly merge partial rows
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2025-12-17T23:59:22.807Z
Learnt from: callicles
Repo: 514-labs/moosestack PR: 3143
File: apps/framework-docs/llm-docs/python/table-setup.md:331-336
Timestamp: 2025-12-17T23:59:22.807Z
Learning: In apps/framework-docs/llm-docs/**/*.md (LLM documentation), prefer exact class and type names (e.g., "MaterializedView", "OlapTable") over lowercase prose variants (e.g., "materialized view") to help LLMs generate correct code with proper imports and syntax.
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/ingest/models.ts : Data models in ingest/models.ts should define interfaces using the Model naming convention, and create corresponding IngestPipeline instances with table, stream, and ingestApi options enabled
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: For slow dashboard queries (>500ms) that aggregate across multiple tables or need cohort-based breakdowns, implement a materialized view instead of real-time queries
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🪛 LanguageTool
apps/framework-docs-v2/content/guides/static-report-generation.mdx
[style] ~105-~105: To elevate your writing, try using a synonym here.
Context: ...ripts** | No streaming, no type safety, hard to extend | **This approach gives you:...
(HARD_TO)
[style] ~670-~670: To elevate your writing, try using an alternative expression here.
Context: ...nsolidate results correctly Why this matters for performance: | Query Target | Ro...
(MATTERS_RELEVANT)
[uncategorized] ~1347-~1347: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n real-time - API serves any date range on demand - PDF generation takes < 1 second - **T...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~1623-~1623: Ensure spelling is correct
Context: ...y is slow:** If queries take more than 100ms, check: - Are you querying the material...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (6)
apps/framework-docs-v2/content/guides/static-report-generation.mdx (6)
610-614: LGTM on the null guard.The defensive check ensures TypeScript can narrow the type. In production, you might prefer a more graceful error path, but for tutorial code this is clear and explicit.
1076-1089: LGTM on async PDF generation.The synchronous
createPdfKitDocumentcall is now correctly documented. The Promise wrapper properly handles the inherently async stream-based file writing with appropriate error propagation.
1140-1143: LGTM on the conceptual deep-dive.Part 7 accurately explains ClickHouse columnar storage, MV batch aggregation, and the critical
GROUP BYrequirement. The batch scenario example (lines 1230-1244) effectively illustrates why partial aggregates exist.
1566-1584: LGTM on deduplication guidance.The documentation correctly explains that
FINALonly works with specific MergeTree variants (ReplacingMergeTree, CollapsingMergeTree) and provides practical query-based alternatives for standard MergeTree tables.
565-572: LGTM on data quality table.The table correctly references
parseISO()+isValid()from date-fns, consistent with the validation implementation above.
849-851: Use standard pdfmake server-side import pattern.Replace
require("pdfmake/js/Printer").defaultwithrequire("pdfmake")orrequire("pdfmake/src/printer"). The/js/Printerpath targets an ESM build and is not the recommended server-side pattern. Users following this guide will likely hit module resolution failures.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Updated the date validation method to ensure that only successfully parsed dates are validated, improving error handling for invalid formats and future dates. - Introduced a null check for parsed dates to enhance data integrity in report generation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
- Updated the JSON parsing logic to normalize records by filtering out empty rows and ensuring that `customerId` is an empty string when null, aligning with the expected API format. - This change improves data integrity and consistency in report generation.
…ucture - Changed the API endpoint from `/api/sales-report` to `/consumption/sales-report` in multiple sections of the documentation. - Updated example curl commands to align with the new endpoint, ensuring accurate usage instructions for users. - Enhanced clarity in the guide regarding the API's functionality and usage.
…ecture overview - Replaced the text-based architecture diagram with a mermaid flowchart for better visualization. - Enhanced clarity in the data flow representation, making it easier for users to understand the process from data ingestion to PDF export.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx (1)
357-395: Remove misleading comment and dead code in date handling.PapaParse's
dynamicTyping: trueonly converts numbers and booleans—it does not parse ISO date strings intoDateobjects. Theinstanceof Datecheck will never execute, and the current comment is false. Either add a custom transform parser to handle dates or remove the unnecessary date conversion logic.Suggested fix
-// Note: saleTimestamp can be string or Date because papaparse dynamicTyping -// converts ISO date strings to Date objects +// Note: PapaParse converts only numbers and booleans; dates remain strings. +// Add a transform callback if Date conversion is needed.And remove the unnecessary instanceof check:
records = data .filter((r) => r.transactionId) .map((r) => ({ ...r, - saleTimestamp: r.saleTimestamp instanceof Date - ? r.saleTimestamp.toISOString() - : r.saleTimestamp, + saleTimestamp: r.saleTimestamp,
🤖 Fix all issues with AI agents
In `@apps/framework-docs-v2/content/guides/static-report-generation.mdx`:
- Around line 863-865: The require for PdfPrinter is using the wrong path and
`.default`; update the import to require the correct module (e.g.,
require("pdfmake") or require("pdfmake/src/printer")) and remove the `.default`
usage so PdfPrinter references the actual constructor; locate the line that
defines PdfPrinter (const PdfPrinter = require("pdfmake/js/Printer").default;)
and replace it with a direct require of the correct path (e.g., const PdfPrinter
= require("pdfmake") or require("pdfmake/src/printer")) so PdfPrinter can be
used as the printer constructor alongside the existing
TDocumentDefinitions/TFontDictionary types.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/views/*.ts : Use SummingMergeTree engine for materialized views that require incremental updates, and wrap aggregated metrics with SUM() when querying to properly merge partial rows
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2025-12-17T23:59:22.807Z
Learnt from: callicles
Repo: 514-labs/moosestack PR: 3143
File: apps/framework-docs/llm-docs/python/table-setup.md:331-336
Timestamp: 2025-12-17T23:59:22.807Z
Learning: In apps/framework-docs/llm-docs/**/*.md (LLM documentation), prefer exact class and type names (e.g., "MaterializedView", "OlapTable") over lowercase prose variants (e.g., "materialized view") to help LLMs generate correct code with proper imports and syntax.
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: Applies to examples/cdp-analytics/app/ingest/models.ts : Data models in ingest/models.ts should define interfaces using the Model naming convention, and create corresponding IngestPipeline instances with table, stream, and ingestApi options enabled
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
📚 Learning: 2026-01-15T19:41:53.549Z
Learnt from: CR
Repo: 514-labs/moosestack PR: 0
File: examples/cdp-analytics/CLAUDE.md:0-0
Timestamp: 2026-01-15T19:41:53.549Z
Learning: For slow dashboard queries (>500ms) that aggregate across multiple tables or need cohort-based breakdowns, implement a materialized view instead of real-time queries
Applied to files:
apps/framework-docs-v2/content/guides/static-report-generation.mdx
🪛 LanguageTool
apps/framework-docs-v2/content/guides/static-report-generation.mdx
[style] ~105-~105: To elevate your writing, try using a synonym here.
Context: ...ripts** | No streaming, no type safety, hard to extend | **This approach gives you:...
(HARD_TO)
[style] ~684-~684: To elevate your writing, try using an alternative expression here.
Context: ...nsolidate results correctly Why this matters for performance: | Query Target | Ro...
(MATTERS_RELEVANT)
[uncategorized] ~1362-~1362: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...n real-time - API serves any date range on demand - PDF generation takes < 1 second - **T...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~1638-~1638: Ensure spelling is correct
Context: ...y is slow:** If queries take more than 100ms, check: - Are you querying the material...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (16)
apps/framework-docs-v2/content/guides/static-report-generation.mdx (16)
18-31: Clear audience segmentation.
35-56: Environment verification steps are crisp and actionable.
62-135: Scenario + ROI + architecture context is strong.
188-218: Stack health checks are practical and easy to follow.
224-260: String transaction IDs and ordering guidance look good.
276-297: Model detection guidance is clear and helpful.
328-331: ClickHouse verification command is straightforward.
398-406: JSON normalization parity with CSV looks good.
492-587: Validation block + data-quality table are well aligned.
624-654: Null guard and GROUP BY reminder are solid.
669-708: MV verification + query example are clear.
815-825: API “single source of truth” callout is strong.
941-1124: Async PDF generation with stream completion looks correct.
1155-1328: Conceptual deep dive is clear and technically accurate.
1348-1395: Before/after and next actions are compelling.
1430-1666: Reference + troubleshooting sections are comprehensive.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
@callicles there was merge conflict which was auto resolved. Need approval. |
instructions and environment verification
This update aims to provide users with a clearer path to implementing automated report generation using MooseStack.
Note
Significantly expands the
static-report-generation.mdxguide into an end-to-end, runnable tutorial.SalesTransaction(removeKey<>), uses MooseIngestPipeline, switches ClickHouse examples to HTTPcurl, and adds a file loader script with normalization and optional validationDailySalesSummarymaterialized view with guidance to alwaysGROUP BY; updates exports and verification steps/consumption/sales-reportendpoint with parameterized date range (optional region) querying the MVgenerate-pdf-report.tsusing pdfmake (async stream handling and typings) to create an executive summary PDFWritten by Cursor Bugbot for commit 967fa3a. This will update automatically on new commits. Configure here.