MWE ungroup: apply user MWE overrides to preview summaries - #699
Merged
Conversation
|
ArchLens detected architectural changes in the following views: |
mircealungu
marked this pull request as ready for review
August 13, 2026 06:34
The "ungroup a multi-word expression" action only affected the article body:
UserMweOverride clearing was applied to tokenized_fragments but never to the
per-level ArticleLevelSummary previews (nor to the article's own summary),
so tapping "ungroup" on a summary word had no effect.
- UserMweOverride.get_disabled_mwes_for_user_articles(user_id, article_ids):
batched {article_id: {sentence_hash: [expressions]}} in one query.
- UserArticle._apply_mwe_overrides_to_summary_tokens(): shared helper that
walks a summary's paragraphs -> sentences and clears disabled MWE metadata.
Applied in user_article_summary_info (both the level-summary and the
fallback own-summary paths) and in the feed overlay
_apply_simplified_display_overlay (batched, no N+1).
- Carry article_id on the ARTICLE_LEVEL_SUMMARY ContextIdentifier so the
client's ungroup path can address the override by parent article id; the
bookmark mapping still keys on article_level_summary_id.
- Tests for override clearing on a level summary and the article_id carry.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mircealungu
force-pushed
the
wt/mwe-summaries
branch
from
August 13, 2026 06:38
db39b43 to
11e2ecb
Compare
mircealungu
added a commit
that referenced
this pull request
Aug 13, 2026
MWE-on-summaries review fixes (follow-up to #699)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PR #698 added per-level tappable preview summaries (
article_level_summarytable +ArticleLevelSummarycontext type). Tapping a word in a summary translates/bookmarks fine, but the "ungroup a multi-word expression (MWE)" action did nothing on summaries.Root cause:
UserMweOverrideclearing was only applied to the article body (thetokenized_fragmentsblock inUserArticle.user_article_info, guarded byif with_content:). Summaries — both the per-level preview summaries and the article's ownsummary— never got that treatment.Change
Batched override fetch —
UserMweOverride.get_disabled_mwes_for_user_articles(user_id, article_ids)returns{article_id: {sentence_hash: [expressions]}}in one query (mirrors the existing single-article method).Shared apply helper —
UserArticle._apply_mwe_overrides_to_summary_tokens(summary_tokens, overrides_by_hash)walks a summary'sparagraphs -> sentences(one fewer loop than the body version, which also has the outer fragment wrapper) and clears disabled MWE metadata in place. Applied in:user_article_summary_info— for both the level-summary path and the fallbackarticle.summarypath (this also fixes the pre-existing gap for the original summary). Override lookup uses the parentarticle.id; summary sentence text differs from body sentences so the sentence hash still scopes the override to the summary._apply_simplified_display_overlay(feed overlay) — batch-fetches overrides for all candidate articles and applies to eachinteractiveSummary.article_idcarry — theARTICLE_LEVEL_SUMMARYContextIdentifiernow also passesarticle_id(in both_level_matched_summary_payloadand the overlay). The bookmark mapping still keys onarticle_level_summary_id(create_context_mappingswitches oncontext_type);article_idis only for the client's MWE-ungroup path. This was reverted from On-demand simplification: assess-only crawl + per-level preview summaries #698 because it was inert without this task.Tests — extended
test_article_level_summary.py: MWE metadata cleared on a served level summary when an override exists, the untouched-token sanity case, and thearticle_idcarry on the context identifier.Notes
wt/on-demand-simplify, not master). Should be rebased onto master after On-demand simplification: assess-only crawl + per-level preview summaries #698 merges.Tests
test_article_level_summary.py— 7 passed.🤖 Generated with Claude Code