Skip to content

1.9.79: a report breakdown says when it has more rows than it returned - #284

Open
tracking202 wants to merge 5 commits into
1.9.79from
1.9.79-breakdown-has-more
Open

tracking202 wants to merge 5 commits into
1.9.79from
1.9.79-breakdown-has-more

Conversation

@tracking202

Copy link
Copy Markdown
Owner

Backlog: TX1Q

What and why

GET /reports/breakdown answered 50 rows unless limit asked for more (500 at most), and its answer held only data, breakdown and available_breakdowns: nothing said rows remained. Every reader of one page read it as the whole breakdown. Measured on a live instance with 654 keyword rows: p202 report losers --breakdown keyword listed 48 losing keywords costing $4.80, of 650 costing $37.15.

Server

  • The breakdown reads one row past the page (LIMIT limit+1) and answers truncated (true when rows remain), limit and offset beside data, the spelling and shape GET /reports/timeseries already uses for truncated/limit. offset is added because a breakdown pages; the next page is offset + limit.
  • No total. A count of the groups as a second query cost about 60% of the page's own query (median of 5: 4.95 s against 8.0 s, 300,000 clicks over 60,000 keywords, MariaDB 10.11, this shared sandbox). COUNT(*) OVER () in the same query cost nothing measurable there (7.3 s), but it is unmeasured on MySQL 8.0 (CI's server), and a page past the end has no row to carry it, so it would need the second query exactly where it is asked for. Every reader we have decides only whether to read on, which truncated answers.
  • The other list reports in ReportsController do not share the shape: daypart (24 rows) and weekpart (7) are complete, rotator stats lists every rule, groups refuses past 5000 rather than cutting, and timeseries already says truncated.

Go CLI (and MCP)

Rule for the signal: --json passes the API's keys through; --ndjson stamps "truncated": true on every line (as masked is, in internal/output, so report timeseries --ndjson carries it too); a table, --csv or --quiet gets one note on stderr naming the flag that reads more; --json/--ndjson keep stderr for errors, as they do for masked. stdout stays data.

Command Decision Why
report breakdown, analytics one page; note Note: more rows remain after rows 1-50; --offset 50 reads the next page (--limit takes up to 500). they are the page reader; --min-clicks/--min-cost/--zero-leads/--having filter only the page, and the note says so (the filtered answer now keeps the server's other keys)
report breakeven one page: new --limit/--offset, ranked by margin within the page, says truncated its answer is a row per row, so it pages like a breakdown; reading every page would make it as large as the dimension, past the MCP connector's 60,000 characters with no way to ask for less
report losers, report winners read every page, 500 a request, up to the 5000 rows with the most clicks; --json always has truncated they select rows: a loser past the first 50 was missing with nothing to show it. The bound keeps them to 10 requests, each a full aggregation on the server
report crosstab rows: the server ranks by --metric over every row (sort=<metric>, one more than --limit-rows); columns: every value up to 5000; truncated + a note naming --limit-rows or the bound it ranked the first 50 rows by clicks and read 50 columns per row (a column past them read as no traffic)
campaign optimize reads every campaign by revenue, up to 5000; truncated same reader; it also dropped a failed read of the other campaigns (offers, _ :=, CLAUDE.md #1) and answered "no alternative": it fails now, naming it
analytics --split-at, list --with-stats unchanged already read every page (fetchAllRowsPaged)

fetchBreakdownRows is the pager: it asks for one row more than it still needs, so a server from before truncated is answered too (an exact 5000 is told from more by one extra request). The MCP tools report_breakdown, report_breakeven and report_crosstab say in their description what truncated: true means (kept short: with 1.9.79 merged the catalogue is 84,700 characters of its 85,000 budget, 307 more than 1.9.79; report_winners/report_losers carry the field without a note, since few reach 5000 rows).

bin/p202 report:breakdown prints the same note under its table.

Machine check

TestEveryBreakdownReadSaysWhetherRowsRemain (go-cli) walks every non-test Go file and holds each call naming "reports/breakdown" to a pager (fetchAllRowsPaged/fetchAllPages), fetchBreakdownRows itself, or a function that also calls breakdownMoreRowsNote. It does not see an endpoint held in a variable.

Agent eval

report-depth-004-a-keyword-count-reads-every-page: setup fires 60 keywords, the ask counts them; the reference agent pages until truncated is false. Run on 10/23 (see Live below).

Verification

.claude/skills/p202-verify/scripts/verify.sh --since origin/1.9.79 --changed, on 5413e6a (1.9.79 merged at 915a367; schema tier on the scratch database p202_tx1q_it; P202_FAKE_GOOGLE_PORT=8281, since the Play Integrity test's fixed port 8241 collided with another worktree's suite in an earlier run here):

Scope report
------------
  go         PASS  ran under CI's go1.25.14 via GOTOOLCHAIN (local go is 1.24)
  golangci   PASS  ran under CI's go1.25.14 via GOTOOLCHAIN (local go is 1.24)
  openapi    PASS
  patterns   PASS
  phpcs      PASS
  phpstan    PASS
  schema     PASS
  syntax     PASS
  unit       PASS

Not covered by this script:
  live      stand up an instance, drive the path a user would take
  agent     add and run a case under tests/fixtures/agent-eval/cases/
  ci        a workflow still in_progress has not passed; read the run

unit: 6108 tests, 5 skipped (the 4 documented UserTest DB-singleton skips, and CronEntryPointsFailLoudlyTest, which skips because the live instance's installer wrote a 202-config.php into this worktree; it is not committed). BreakdownTruncationIntegrationTest is @group integration and outside the schema tier: run by hand against p202_tx1q_it after it, OK (4 tests, 25 assertions).

After merging 1.9.79, TestSearchReadsEditVerbsAsUpdate failed: the help paragraphs this change added lengthened the long descriptions p202 search ranks on, and "set the api url" moved from config set-url (0.0328 on 1.9.79) to a tie with system integrations (0.0325 each). 5413e6a cuts each paragraph to one line; both score as on 1.9.79 again.

Tests added: ReportsControllerTest (more rows than the limit, exactly the limit at 7/50/500, every page of 120 rows, past the end), BreakdownTruncationIntegrationTest (@group integration, real MariaDB: 51 keywords, the default page, a page of exactly the rows, paging 3 × 17 where the last full page is the last, a sorted page), Go report_breakdown_truncation_test.go (fake servers with and without the flag, every command above, every output mode), output_test.go (NDJSON stamp), mcp_test.go, ReportBreakdownCommandTest.

Plants, each restored and checked with cmp, each caught by a named test (not a build failure): the server binding limit not limit+1, >= for >, the slice dropped (3 PHP unit + 3 integration tests); in Go, analytics' note removed, the pager reading one page, the old-server exact-count branch, the NDJSON stamp, the filter dropping the answer's keys, crosstab ranking client-side, the note printed under --json, campaign optimize dropping the error, breakeven dropping the page keys, a new one-page read with no note (the structural test), losers dropping the flag, the MCP note removed.

Live, in two parts (instance installed from this branch on 10/23, MariaDB 10.11, PHP 8.3; 749 clicks over 654 keywords fired through dl.php and dej.php run, 714 keywords after the eval case's 60).

On 10/23, on 1dd3663/b5c598d4 (before 1.9.79's #270, #280, #281, #282 and the help trim), with the licence stub up, every CLI path below ran end to end:

  • API: default page 50 rows truncated: true; limit=500&offset=154 (exactly the 500 remaining) false; offset=153 true; offset=654 0 rows false.
  • p202 report breakdown --breakdown keyword --table: the note on stderr, nothing on stdout; --limit 500 --offset 500: 154 rows, no note; --zero-leads: the note with the filter sentence; --json: 50 rows, truncated/limit/offset, empty stderr; --ndjson: 50 lines each with "truncated":true, empty stderr.
  • p202 analytics --group-by keyword --table: the note. p202 report breakeven --aff-campaign-id 2 --table: the note; --limit 500 --offset 600 --json: 54 rows truncated: false.
  • p202 report losers --breakdown keyword --no-attribution-check --json: 650 rows, $37.15, truncated: false (the base branch's binary on the same instance: 48 rows, $4.80, no flag).
  • p202 report crosstab --rows ppc_account --cols keyword --metric clicks --json: 654 columns read over two pages, truncated: false.
  • p202 mcp call report_breakdown with limit 5: 5 rows, truncated: true, limit: 5, offset: 0.
  • p202 eval run of report-depth-004 with the reference agent: deterministic graders pass (needs_judge for the rubric); with the server planted to answer truncated: false always it fails (reply does not contain "60", reply 50).

Today, on 5413e6a, with the licence stub down (the instance re-checked its licence and the Go CLI is refused 402 licence on every command):

  • API, driven with curl against the same instance, 714 keyword rows: default page 50 rows truncated: true, limit 50, offset 0; limit=500&offset=214 (exactly the 500 remaining) false; offset=213 true; offset=714 0 rows false; limit=20&offset=60 true; limit=3&sort=total_clicks&sort_dir=ASC true; limit=7&period=today (no clicks today) 0 rows false. Paging with limit=500 until truncated is false: 2 pages, 714 rows, 714 distinct ids, 869 clicks, which is what SQL counts on 202_dataengine (714 keywords, 869 clicks).
  • php bin/p202 report:breakdown --breakdown keyword --period alltime (not licence-gated): the note on stderr and 54 table lines on stdout; --limit 500 --offset 500 (214 rows, the last page): no note; --json: 50 rows, truncated: true, limit 50, offset 0, empty stderr.

Not run, and why

  • The Go CLI against the live instance after the merge. The licence stub is down, so the instance answers every Go CLI request 402: report breakdown (table, --csv, --json, --ndjson, the filters), analytics, report breakeven, report losers, report winners, report crosstab, campaign optimize, p202 mcp call and the report-depth-004 eval case through the reference agent could not run end to end on 5413e6a. They ran on 10/23 on the same Go code bar the help-text trim (above); since then they are covered by the cmd tests against fake servers only, which go in the ladder ran.
  • The hosted (js/wasm) MCP engine was not run. Under 1.9.79: a hosted call refuses answers with more JSON values than it decodes #281's bound a hosted call decodes at most 80,000 JSON values; 5000 breakdown rows are about 70,000 (each row an object of 13 values), so a hosted report_losers/report_winners/report_crosstab that reads that far, plus the attribution check's reads, can be refused by the guard with how to ask for less. That is the guard's refusal, never a cut answer.
  • CI: not yet observed. The @group instance suites and the rest of the agent eval suite were not run.
  • MySQL 8.0: the SQL ran on MariaDB 10.11 only; the statement is the one the breakdown already prepared, with limit + 1 bound.

For the reviewer

  • report losers/winners now make up to 11 requests where they made 1, each a full aggregation; on a large keyword set that is slower, and through MCP a complete losers list can pass the 60,000-character limit and be refused with "ask for less" (on the live instance: 710 rows, 120k characters; min_clicks: 2 brought it to 26k). Before, the same call answered the first 50 rows as all of them.
  • The 5000-row bound (breakdownReadRows) follows GROUPS_MAX_ROWS; past it the answer says what it covers and the note says to narrow the window or filter. There is no flag to raise it.
  • The stderr note is for people only (table, CSV, --quiet); report timeseries prints its warning in every mode, and losers'/winners' other notes print in every mode too. I followed the masked rule and the brief rather than those.
  • R083 / PR 1.9.79: the multi-profile TOTAL recomputes its ratios and never adds currencies #261: my edits in report_optimize.go are the pager, the breakeven RunE read and two flags, and the triage fetch/output lines; enrichBreakeven and the multi-profile code are untouched.

Found outside this item

  • GET /ltv/breakdown pages with limit/offset (50 by default) and says nothing of the rows after a page, the same gap.
  • No structural test holds the API's paged answers to saying whether rows remain (most say pagination.total, attribution meta.groups); a sweep could land one.

Generated by Claude Code

GET /reports/breakdown answered 50 rows unless limit asked for more (500
at most) and said nothing of the rest, so every reader of one page read
it as the whole breakdown. It now reads one row past the page and answers
truncated, limit and offset beside data, as GET /reports/timeseries says
truncated. No total: a count of the groups is a second aggregation of the
window, measured at about 60% of the page's own query (300,000 clicks over
60,000 keywords, MariaDB 10.11); COUNT(*) OVER () cost nothing there
but is unmeasured on MySQL 8.0 and has no row to carry it on a page past
the end; and the readers only decide whether to read on. The other list reports in
ReportsController are complete (daypart, weekpart, rotator stats),
refuse past their cap (groups) or already say truncated (timeseries).

The CLI:
- report breakdown and analytics pass the keys through under --json,
  stamp truncated on every --ndjson line (as masked), and under a table,
  CSV or --quiet print one stderr note naming the next --offset; the
  client-side filters are said to see only the page. bin/p202
  report:breakdown prints the same note.
- report breakeven takes --limit and --offset and reads one page, as
  report breakdown does, ranked by margin within it, and says the same.
- report losers, report winners and campaign optimize read one page of
  50 and classified or ranked only those (measured live: losers listed 48
  of 650 losing keywords, $4.80 of $37.15 spend). They read page after
  page, 500 a request, up to the 5000 rows with the most clicks, and say
  truncated: true when the answer covers only those.
- report crosstab ranked the first 50 rows by clicks: the server ranks
  them by --metric over every row now (one more than --limit-rows, to
  know whether more remain), each row's columns are read in full up to
  5000, and truncated (with a note) says what was left out.
- campaign optimize no longer drops a failed read of the other campaigns.
- The MCP tools report_breakdown, report_breakeven and report_crosstab
  say in their description what truncated: true means.

TestEveryBreakdownReadSaysWhetherRowsRemain holds every read of
reports/breakdown in go-cli to a pager or to a one-page answer that
prints breakdownMoreRowsNote. A new agent eval case
(report-depth-004) counts 60 keywords through the paging.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
# Conflicts:
#	202-config/changelogs.json
# Conflicts:
#	202-config/changelogs.json
The paragraphs the breakdown change added to the help of report breakeven,
losers, winners and crosstab lengthened the long descriptions p202 search
ranks on, and after #270 that moved "set the api url" from config set-url to
system integrations (TestSearchReadsEditVerbsAsUpdate: the two scored 0.0328
and 0.0323 on 1.9.79, 0.0325 each with the paragraphs). One line each says
the same, and the two score as on 1.9.79 again.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
Copilot AI balanced review requested due to automatic review settings October 28, 2026 05:15
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-28T05:15:40.867620Z
We couldn’t complete this review. Comment @codex review to try again.
5413e6a PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@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.

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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@github-actions

Copy link
Copy Markdown
Contributor

Code review: CLEAN

For 5413e6a: 0 issue(s). The review is encrypted for the maintainers in the review-findings artifact of this run.

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Adversarial review: NON-BLOCKING

For 2858b19: 4 finding(s). The report is encrypted for the maintainers in the review-findings artifact of this run.

Rewritten on every push; the check fails on BLOCKING.

Brings in 1.9.79's new commits. The conflicts were in 202-config/changelogs.json, changelogs.txt, where both sides added lines: the incoming lines are kept as they are and this branch's follow.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VRoS2SViuxaucFaKWsFnLr
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.

3 participants