Skip to content

chore: cap static analytics detail exports at top-n rows (#4924) - #4932

Merged
NoopDog merged 2 commits into
mainfrom
fran/4924-cap-detail-exports
Sep 2, 2026
Merged

chore: cap static analytics detail exports at top-n rows (#4924)#4932
NoopDog merged 2 commits into
mainfrom
fran/4924-cap-detail-exports

Conversation

@frano-m

@frano-m frano-m commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Closes #4924

What changed

export_df_as_json() gains an optional max_rows; when the frame exceeds it, the top rows are kept and the truncation is logged (Capped pageviews.json at top 100 of 1810 rows) so the cap is never silent. MAX_TABLE_EXPORT_ROWS = 100 is applied to the three exports the template renders as a fixed slice:

Export Rows the template renders Now exported
pageviews.json slice(0, 20) top 100
outbound_links.json slice(0, 20) top 100
filter_selected.json slice(0, 30) top 100

Left uncapped on purpose: event detail tables, access requests and search queries — the template renders every row of those (behind a "Show all N rows" toggle, or with no slice at all for search), so capping them would silently drop visible data. The constant's comment records both halves of that rule, and the README's output listing now annotates the three capped files.

Measured on a 1,810-row frame: 120KB → 7KB (~95% smaller), which is also thousands of fewer diff lines per monthly regen.

Why top-N is safe here

The cap keeps whichever rows come first, so it depends on the frames being sorted. That holds by construction, not by luck: all three producers pass explicit sort_results into get_one_period_change_df, which does a stable descending sort_values on the current-period metric (report_elements.pyget_page_views_change, get_outbound_links_change, get_index_filter_selected_change), and fetch.py's later exclude-pages / suspicious-path filtering preserves order. The deployed JSONs are fully sorted descending. export_df_as_json's docstring states the expectation for future callers.

Nothing consumes the full lists: within the template these three datasets are only ever sliced for their tables plus an emptiness check — no totals, counts or reductions are derived from them — and no other Python, notebook or TypeScript in the repo reads them. So no displayed aggregate can be skewed by the cap.

How verified

Manual local checks only — this PR adds no automated tests. The repo has no Python test harness: no test_*.py files, pytest is not a declared dependency, and no workflow runs Python tests. (Since #4943 there is an analytics CI job, but it runs ruff check, ruff format --check and an import analytics.static_site smoke test — no tests. It passes on this branch.) Flagging that explicitly because an earlier version of this description said "unit-tested", which wrongly implied in-repo coverage.

What I actually ran: an ad-hoc script in a throwaway virtualenv that imports export_df_as_json and exercises it against a synthetic 1,810-row frame — capped to exactly the top 100 with order preserved and NaN changes still nulled; a below-cap frame exported untouched; no max_rows still exports every row (the event-detail path); None and empty frames still emit []. That script was not committed.

Adding a real pytest suite for this module and wiring it into CI is worth doing, but it would introduce a Python test harness this repo doesn't have yet — happy to do that here or as a follow-up ticket, whichever is preferred.

🤖 Generated with Claude Code

Copilot AI left a comment

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.

Pull request overview

Caps the size of selected static analytics JSON exports to reduce page-load payload and monthly regeneration diffs, while keeping uncapped exports that the template renders in full.

Changes:

  • Added an optional max_rows parameter to export_df_as_json() to export only the top-N rows (with a console log when truncation happens).
  • Introduced MAX_TABLE_EXPORT_ROWS = 100 and applied it to pageviews.json, outbound_links.json, and filter_selected.json.
  • Annotated the README output listing to document which exports are capped.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
analytics/analytics_package/analytics/static_site/export.py Adds a configurable top-N row cap and applies it to the three sliced table exports.
analytics/analytics_package/analytics/static_site/README.md Documents the three capped export files in the output tree.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread analytics/analytics_package/analytics/static_site/export.py
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@frano-m
frano-m force-pushed the fran/4924-cap-detail-exports branch from cc02e97 to 07a2555 Compare September 1, 2026 04:25
note the export-side row cap at the template slice sites, so raising a
slice above max_table_export_rows can't silently truncate a table.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@frano-m
frano-m marked this pull request as ready for review September 1, 2026 05:05
@frano-m
frano-m requested a lite review from Copilot September 1, 2026 05:05

Copilot AI left a comment

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.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@NoopDog NoopDog left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at high effort. Verified the load-bearing claims rather than taking them on trust:

  • Sortedness holds by construction. All three producers pass sort_results into get_one_period_change_df, which does a stable descending sort_values (report_elements.py:282), and all nine committed JSONs under gh-pages/*/data/ are in fact sorted descending on their metric — so head(100) keeps the genuine top rows.
  • The cap runs after filtering, not before. exclude_pages and the suspicious-path filter execute in fetch.py:452-465 ahead of export, and both are order-preserving boolean masks.
  • No displayed aggregate is skewed. pageviews.json / outbound_links.json / filter_selected.json are read only by template/index.html and its deployed copies, and only for slice(0,20)/slice(0,20)/slice(0,30) plus an emptiness check — no totals, counts or "Show all" toggles. The toggles at lines 750 and 970 belong to event detail and access requests, which correctly stay uncapped.
  • Signature change is backward compatible (max_rows keyword-with-default, no callers of export_df_as_json outside export.py), and the empty/None paths still emit [].

One low-severity note, not blocking: export.py:42 — the cap silently depends on the caller having sorted the frame, and nothing enforces or detects it. If a follow-up under the same "fewer diff lines per regen" motivation adds a deterministic secondary sort (e.g. by page for stable diffs), or a future caller builds a frame without sort_results, df.head(100) would keep 100 arbitrary rows while the log line still reads Capped ... at top 100 of 1810 rows. Sorting inside the function on the metric column, or asserting it is monotonically non-increasing before slicing, would make the contract enforced rather than documented.

Appreciate the description correcting the earlier "unit-tested" claim — the absence of a Python test harness is worth its own ticket.

@NoopDog
NoopDog merged commit 96ae008 into main Sep 2, 2026
5 checks passed
@frano-m
frano-m deleted the fran/4924-cap-detail-exports branch September 2, 2026 05:45
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.

chore: cap static analytics detail exports at top-n rows

3 participants