Skip to content

fix(gooddata-eval): read a v2 text summary on a page with no takeaway - #1909

Closed
chrisbonilla95 wants to merge 1 commit into
masterfrom
pubops/LX-3326-4
Closed

chrisbonilla95 wants to merge 1 commit into
masterfrom
pubops/LX-3326-4

Conversation

@chrisbonilla95

@chrisbonilla95 chrisbonilla95 commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

gd-eval's agentic document evaluation now reads a v2 page's text widget summary as that page's summary when the page has no takeaway. Takeaways and v1 documents score as before.

Draft, held until the evaluator rule is settled. LX-3342 records that the document evaluation counts only takeaways, because a text summary is a static widget the user fills or removes and AI-written text is the takeaway's job. This PR implements the other rule and merges only if that decision changes.

PR 4 of 4 for LX-3326.

What this implements

gen-ai writes a page's summary into the page template's text widget with id summary whenever auto picks a template without a takeaway. That is any page auto cannot fit on viz2TakeawayRight (the only default template with a takeaway), or one where the model names a text-summary template. _summary_slots returned only takeaways for v2, so those pages had no summary slot. The narrative judge saw charts with no Summary: line and failed agent_document_skill item 7 for a missing summary, while document_summaries_present skipped the page. Judge input from a local item 7 run on the released gooddata-eval (page 3 is on a text-summary template holding "Customer totals, new customers, and returning customers for the first half of 2026."):

Period: 2026-01-01 to 2026-06-30

Page 2: Revenue performance
Charts: 3e16ebe3-7753-40c6-abc1-f549e7bf4d6c, 0c7ac154-1766-4d54-ba2b-b6f6913a59dd
Summary: Revenue trend and sales per customer for the first half of 2026.

Page 3: Customer activity
Charts: 9f2a657a-22e1-4791-8216-7a354bb8de5d, a347814a-a893-4354-915d-29699b92a6d7, 4cea4177-37c5-4196-8aab-c6bf60dc1f22

The same item on this branch, local stack (gen-ai on the Publisher v2 branch, api-gw with ENABLE_PUBLISHER_V2=true, this branch installed editable into gdc-nas tests/tavern-e2e). Both pages are on text-summary templates (viz1SummaryRight, viz4SummaryRight):

Period: 2026-01-01 to 2026-06-30

Page 2: Revenue
Charts: 3e16ebe3-7753-40c6-abc1-f549e7bf4d6c
Summary: Revenue trend during the first half of 2026.

Page 3: Customer activity
Charts: 9f2a657a-22e1-4791-8216-7a354bb8de5d, a347814a-a893-4354-915d-29699b92a6d7, dc8575f5-27e5-44be-a2dc-23d54b7777e7, 4cea4177-37c5-4196-8aab-c6bf60dc1f22
Summary: Customer totals and activity across the first half of 2026.
item7-r1 ... 'document_summaries_present': True, 'document_narrative_judged': False}. Failures: the judge failed the narrative: The summaries are placeholders and do not provide specific information on how revenue and customer numbers developed during the first half of 2026..
item7-r2 ... 'document_summaries_present': True, 'document_narrative_judged': False}
item7-r3 ... 'document_summaries_present': True, 'document_narrative_judged': False}
items0-6  ================= 7 passed, 1 deselected in 442.70s (0:07:22) ==================

Item 7 still fails the judge, now for a reason that sits in gen-ai rather than here. No summary in any run was written from the data (summaries_from_data: 0). A text-summary page holds only the copilot's summary_text, written without running a chart, so it describes the charts rather than how the numbers moved. On the local stack the one takeaway page fell back to that text as well, because mcp-server was not running.

Unit tests:

$ TEST_ENVS=py314 make -C packages/gooddata-eval test
======================= 2497 passed, 1 skipped in 26.94s =======================
$ make -C packages/gooddata-eval format-fix lint-fix type-check
141 files already formatted
All checks passed!
All checks passed!

Decisions

  1. A takeaway wins over a text summary on the same page.
    A v2 page with a takeaway widget is summarised by its takeaways alone. The text widget summary counts only when the page has none.
  • A written text summary does not stand in for an empty takeaway (test_a_written_summary_text_does_not_stand_in_for_an_empty_takeaway, unchanged).
  • An empty text summary beside a written takeaway is ignored.
  1. An unwritten text summary fails document_summaries_present.
    When the model gives no summary_text for a page on a text-summary template, the reader sees no summary on that page. The check names it as page 'X' has a text summary with no written text. A v2 document with neither slot reports the document has no takeaway or text summary.

Read _summary_slots, _summary_text and _slot_name in packages/gooddata-eval/src/gooddata_eval/core/agentic/document_skill.py closely.

What comes next

  • If the rule stands, this PR closes and text-summary pages get takeaways through the template set (LX-3428).
  • gdc-nas tests/tavern-e2e: bump the gooddata-eval pin once this is released.
  • Item 7's narrative passes only when its summaries come from the data. Text-summary pages never do, which is a gen-ai or fixture decision outside this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_014kgvNiaTSpvcsHvE8xfvYr

gen-ai writes a page's summary into its page template's `text` widget with id `summary` whenever `auto` picks a template without a takeaway, which is every page with three or more charts on the default templates. `_summary_slots` returned only takeaways for v2, so those pages had no summary slot: `render_document_text` gave the narrative judge no `Summary:` line for them and the judge failed `agent_document_skill` item 7 for a missing summary, while `document_summaries_present` skipped the page. A v2 page without a takeaway now has its `text` widget `summary` as its slot, `_summary_text` reads its `markdown`, and an unwritten one fails as `page 'X' has a text summary with no written text`. A takeaway still wins over a text summary on the same page, and a v2 document with neither reports `the document has no takeaway or text summary`.

jira: LX-3326
risk: low
@coderabbitai

coderabbitai Bot commented Oct 29, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.75%. Comparing base (a2147d1) to head (f037c0c).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1909   +/-   ##
=======================================
  Coverage   85.75%   85.75%           
=======================================
  Files         343      343           
  Lines       25182    25187    +5     
=======================================
+ Hits        21595    21600    +5     
  Misses       3587     3587           

☔ View full report in Codecov by Harness.
📢 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.

@chrisbonilla95

chrisbonilla95 commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor Author

Closing: this rule change is no longer needed. Pages with one chart, or three or more, get a takeaway from the revised default page templates instead (tracked in LX-3462), so the document evaluation keeps counting only takeaways.


🤖 Written by pub-ops, an AI agent, and approved by @chrisbonilla95.

@chrisbonilla95
chrisbonilla95 deleted the pubops/LX-3326-4 branch October 29, 2026 13:14
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