diff --git a/tools/audit_stored_article_languages.py b/tools/audit_stored_article_languages.py index 93955ed95..40d9a7d7d 100644 --- a/tools/audit_stored_article_languages.py +++ b/tools/audit_stored_article_languages.py @@ -14,7 +14,7 @@ parent's — which is what makes this safe for cross-language shares: article.summary on originals - ArticleLevelSummary.summary the per-level feed-card blurbs; these only + LevelAdaptedArticleText.summary the per-level feed-card blurbs; these only ever exist on originals (the assess step skips children), so they are always in the original's language @@ -45,7 +45,7 @@ - article.summary is nulled, along with the cached tokenized copy of it (that cache is only ever written when empty, so a stale one would outlive the regeneration and keep rendering); - - bad ArticleLevelSummary rows are deleted, with the bookmark anchors that + - bad LevelAdaptedArticleText rows are deleted, with the bookmark anchors that reference them (a real FK, no cascade) removed first; - child articles are marked broken (LLM_WRONG_LANGUAGE), which takes a simplified one out of available_simplified_versions and a translated one out of @@ -71,8 +71,8 @@ from zeeguu.core.language.language_check import language_mismatch, describe_mismatches from zeeguu.core.model.article import Article from zeeguu.core.model.article_broken_code_map import LowQualityTypes -from zeeguu.core.model.article_level_summary import ArticleLevelSummary -from zeeguu.core.model.article_level_summary_context import ArticleLevelSummaryContext +from zeeguu.core.model.level_adapted_article_text import LevelAdaptedArticleText +from zeeguu.core.model.level_adapted_article_summary_context import LevelAdaptedArticleSummaryContext from zeeguu.core.model.article_tokenization_cache import ArticleTokenizationCache from zeeguu.core.model.language import Language @@ -188,12 +188,12 @@ def rows_for(articles, rows_by_article): def audit_level_summaries(articles): - """ArticleLevelSummary rows — the tappable per-level feed-card blurbs.""" + """LevelAdaptedArticleText rows — the tappable per-level feed-card blurbs.""" by_id = {article.id: article for article in articles} if not by_id: return [], [] - rows = ArticleLevelSummary.query.filter( - ArticleLevelSummary.article_id.in_(list(by_id)) + rows = LevelAdaptedArticleText.query.filter( + LevelAdaptedArticleText.article_id.in_(list(by_id)) ).all() wrong, wrong_rows = [], [] @@ -334,7 +334,7 @@ def is_level_adaptation(child): "these want marking broken", wrong_language_rows) wrong_level_summaries, level_summary_rows = audit_level_summaries(originals) - report("ArticleLevelSummary.summary", wrong_level_summaries) + report("LevelAdaptedArticleText.summary", wrong_level_summaries) wrong_simplified, simplified_articles = audit_child_articles(simplified) report("simplified children (same language as parent)", wrong_simplified) @@ -399,14 +399,14 @@ def is_level_adaptation(child): cache.tokenized_summary = None session.add(cache) - # Bookmarks anchor to a SPECIFIC level summary (ArticleLevelSummaryContext + # Bookmarks anchor to a SPECIFIC level summary (LevelAdaptedArticleSummaryContext # holds a real FK with no cascade), so the anchors have to go first or the # delete fails on the constraint and takes the whole batch down with it. # They anchor into text we are throwing away, so there is nothing to keep. if level_summary_rows: summary_ids = [row.id for row in level_summary_rows] - anchors = ArticleLevelSummaryContext.query.filter( - ArticleLevelSummaryContext.article_level_summary_id.in_(summary_ids) + anchors = LevelAdaptedArticleSummaryContext.query.filter( + LevelAdaptedArticleSummaryContext.level_adapted_article_text_id.in_(summary_ids) ).all() for anchor in anchors: session.delete(anchor) diff --git a/tools/backfill_reassess_summaries.py b/tools/backfill_reassess_summaries.py index 3c1d60160..525e71c7f 100644 --- a/tools/backfill_reassess_summaries.py +++ b/tools/backfill_reassess_summaries.py @@ -10,7 +10,7 @@ Both are fixed in code now. This tool re-runs the SAME crawl-time assessment (assess_summarize_and_classify) on the affected originals, which overwrites -cefr_level, article_type, summary, and every ArticleLevelSummary row (find_or_ +cefr_level, article_type, summary, and every LevelAdaptedArticleText row (find_or_ create updates in place) — in the correct language, once 7cd9d6ca is DEPLOYED. IMPORTANT: run this only AFTER the prompt fix (7cd9d6ca) is live on the server, diff --git a/tools/migrations/26-08-28--level_adapted_article_text.sql b/tools/migrations/26-08-28--level_adapted_article_text.sql new file mode 100644 index 000000000..7d8539c94 --- /dev/null +++ b/tools/migrations/26-08-28--level_adapted_article_text.sql @@ -0,0 +1,63 @@ +-- Per-level headlines, plus the rename that makes room for them. +-- +-- WHY THE RENAME. article_level_summary reads as "the article's level" or +-- "article-level granularity"; the thing it stores is an article's text ADAPTED +-- TO a level. Adding a title to the same row makes "..._summary" wrong outright, +-- so the table becomes level_adapted_article_text and the two context types +-- become LevelAdaptedArticleSummary / LevelAdaptedArticleTitle. +-- +-- WHY THE TITLES. Before on-demand simplification the feed card DID show a +-- level-appropriate headline: the overlay borrowed title and summary off the +-- level-matched simplified child article. When the crawl stopped generating +-- those children (commit 0d047e32), the per-level summary replaced the summary +-- half and nothing replaced the title half. That matters more than it sounds: +-- the default feed view is Headlines, which renders the title and NO summary, so +-- with no per-level title the CEFR selector changes nothing a default-view +-- reader can see. +-- +-- Renaming is cheap here: bookmark_context rows point at context_type by id, and +-- UPDATE preserves the id, so the existing rows need no migration at all. +-- +-- IF YOU ALREADY APPLIED 26-08-28--add_article_level_title.sql (an earlier draft +-- of this file, superseded by the rename), undo it first — it created an empty +-- table and empty columns that this file recreates under the right names: +-- DROP TABLE article_level_title_context; +-- ALTER TABLE article_level_summary DROP COLUMN title, DROP COLUMN tokenized_title; +-- DELETE FROM context_type WHERE type = 'ArticleLevelTitle'; + +RENAME TABLE article_level_summary TO level_adapted_article_text; +RENAME TABLE article_level_summary_context TO level_adapted_article_summary_context; + +ALTER TABLE level_adapted_article_summary_context + CHANGE COLUMN article_level_summary_id level_adapted_article_text_id INT; + +-- The level's headline and its token stream. Nullable on purpose: every row +-- written before per-level titles has none, and a title dropped by the +-- wrong-language check leaves its summary in place. Both fall back to the +-- article's own title. +ALTER TABLE level_adapted_article_text + ADD COLUMN title TEXT AFTER tokenized_summary, + ADD COLUMN tokenized_title JSON AFTER title; + +-- The tap-to-translate join for those headlines. Separate from the summary +-- context even though both point at the same level_adapted_article_text row: a +-- level's title and its summary are two different token streams, and one join +-- table for both would return the title's bookmarks when highlighting the +-- summary. +CREATE TABLE level_adapted_article_title_context ( + id INT AUTO_INCREMENT PRIMARY KEY, + bookmark_id INT NOT NULL, + level_adapted_article_text_id INT, + CONSTRAINT fk_latc_bookmark FOREIGN KEY (bookmark_id) REFERENCES bookmark (id), + CONSTRAINT fk_latc_text FOREIGN KEY (level_adapted_article_text_id) REFERENCES level_adapted_article_text (id) ON DELETE CASCADE, + -- One context row per (bookmark, level title): makes a concurrent-insert + -- race fail with IntegrityError (which find_or_create catches + re-queries) + -- instead of silently creating a duplicate that later breaks .one(). + UNIQUE KEY uq_latc_bookmark_title (bookmark_id, level_adapted_article_text_id) +); + +-- Same row, new spelling: the id is preserved, so every bookmark_context already +-- pointing at it stays valid. +UPDATE context_type SET type = 'LevelAdaptedArticleSummary' WHERE type = 'ArticleLevelSummary'; + +INSERT INTO context_type (type) VALUES ('LevelAdaptedArticleTitle'); diff --git a/zeeguu/core/account_management/user_account_deletion.py b/zeeguu/core/account_management/user_account_deletion.py index e079d1d33..ab2cb059e 100644 --- a/zeeguu/core/account_management/user_account_deletion.py +++ b/zeeguu/core/account_management/user_account_deletion.py @@ -39,7 +39,8 @@ from zeeguu.core.model.user_word_interaction_history import UserWordInteractionHistory from zeeguu.core.model.example_sentence_context import ExampleSentenceContext from zeeguu.core.model.article_summary_context import ArticleSummaryContext -from zeeguu.core.model.article_level_summary_context import ArticleLevelSummaryContext +from zeeguu.core.model.level_adapted_article_summary_context import LevelAdaptedArticleSummaryContext +from zeeguu.core.model.level_adapted_article_title_context import LevelAdaptedArticleTitleContext from zeeguu.core.model.article_fragment_context import ArticleFragmentContext from zeeguu.core.model.article_title_context import ArticleTitleContext from zeeguu.core.model.video_caption_context import VideoCaptionContext @@ -68,7 +69,8 @@ bookmark_context_tables = [ ExampleSentenceContext, ArticleSummaryContext, - ArticleLevelSummaryContext, + LevelAdaptedArticleSummaryContext, + LevelAdaptedArticleTitleContext, ArticleFragmentContext, ArticleTitleContext, VideoCaptionContext, diff --git a/zeeguu/core/content_recommender/elastic_recommender.py b/zeeguu/core/content_recommender/elastic_recommender.py index 0fd80d47b..49b57b931 100644 --- a/zeeguu/core/content_recommender/elastic_recommender.py +++ b/zeeguu/core/content_recommender/elastic_recommender.py @@ -531,27 +531,32 @@ def get_user_info_from_content_recommendations(user, content_list): def _apply_simplified_display_overlay(user, results): """ Overlay a CEFR-level-matched preview *summary* onto feed-card result dicts - that point at an original article, using the per-level ArticleLevelSummary + that point at an original article, using the per-level LevelAdaptedArticleText rows (on-demand simplification means there are no simplified child articles to borrow a summary from anymore). Sets both the plain ``summary`` teaser (Preview mode) and the tappable ``interactiveSummary`` payload (Interactive mode), the latter anchored to the specific level-summary row so tap-to-translate and past-bookmark highlighting - land on the right tokens. Titles are left as the original — we don't produce - per-level titles. Falls back silently to the article's own summary when the - learner's level has no simpler summary. + land on the right tokens. The level's ``title``/``interactiveTitle`` are + overlaid the same way when the row has one — which is what makes the level + selector visible in Headlines mode, where the title is the only text on the + card. Falls back silently to the article's own title/summary when the + learner's level has no simpler one. Batched in two queries: a columns-only pick of the best level per article, then a load of just those chosen rows (so the heavy tokenized_summary JSON is deserialized once per article, not once per level). """ - from zeeguu.core.model.article_level_summary import ( - ArticleLevelSummary, + from zeeguu.core.model.level_adapted_article_text import ( + LevelAdaptedArticleText, CEFR_ORDER, ) - from zeeguu.core.model.article_level_summary_context import ( - ArticleLevelSummaryContext, + from zeeguu.core.model.level_adapted_article_summary_context import ( + LevelAdaptedArticleSummaryContext, + ) + from zeeguu.core.model.level_adapted_article_title_context import ( + LevelAdaptedArticleTitleContext, ) from zeeguu.core.model.context_identifier import ContextIdentifier from zeeguu.core.model.context_type import ContextType @@ -573,21 +578,28 @@ def _apply_simplified_display_overlay(user, results): if not candidate_ids: return - allowed = ArticleLevelSummary.allowed_levels(user_cefr_level) + allowed = LevelAdaptedArticleText.allowed_levels(user_cefr_level) # Two steps so we deserialize the heavy tokenized_summary JSON for only the ONE # best row per article, never every level: first a columns-only query to pick # the best-matching level per article, then load just those chosen rows. + # Article.cefr_level rides along so pick_best can tell "this learner reads at + # or above the article's own level" (→ use the article's own summary) from + # "this learner needs a simpler one". Joined here rather than read off the + # result dicts, whose metrics.cefr_level is the *effective* level and can come + # back compound ("B1/B2") — see article_info. lightweight = ( - ArticleLevelSummary.query + LevelAdaptedArticleText.query .with_entities( - ArticleLevelSummary.id, - ArticleLevelSummary.article_id, - ArticleLevelSummary.cefr_level, + LevelAdaptedArticleText.id, + LevelAdaptedArticleText.article_id, + LevelAdaptedArticleText.cefr_level, + Article.cefr_level.label("article_own_level"), ) + .join(Article, Article.id == LevelAdaptedArticleText.article_id) .filter( - ArticleLevelSummary.article_id.in_(candidate_ids), - ArticleLevelSummary.cefr_level.in_(allowed), + LevelAdaptedArticleText.article_id.in_(candidate_ids), + LevelAdaptedArticleText.cefr_level.in_(allowed), ) .all() ) @@ -595,12 +607,16 @@ def _apply_simplified_display_overlay(user, results): return by_article = {} + own_level_by_article = {} for row in lightweight: by_article.setdefault(row.article_id, []).append(row) + own_level_by_article[row.article_id] = row.article_own_level chosen_id_by_article = {} for article_id, rows in by_article.items(): - best_row = ArticleLevelSummary.pick_best(rows, user_cefr_level) + best_row = LevelAdaptedArticleText.pick_best( + rows, user_cefr_level, own_level_by_article.get(article_id) + ) if best_row: chosen_id_by_article[article_id] = best_row.id if not chosen_id_by_article: @@ -608,8 +624,8 @@ def _apply_simplified_display_overlay(user, results): full_by_id = { als.id: als - for als in ArticleLevelSummary.query.filter( - ArticleLevelSummary.id.in_(chosen_id_by_article.values()) + for als in LevelAdaptedArticleText.query.filter( + LevelAdaptedArticleText.id.in_(chosen_id_by_article.values()) ).all() } @@ -626,28 +642,55 @@ def _apply_simplified_display_overlay(user, results): if not display: continue + # The bookmark mapping keys on level_adapted_article_text_id; article_id is + # carried only for the client's MWE-ungroup path (parent article id). + def _payload(tokens, context_type, past_bookmarks): + overrides_by_hash = overrides_by_article.get(display.article_id) + if overrides_by_hash: + # Returns a cleared copy; never mutates the ORM-loaded token list. + tokens = UserArticle._apply_mwe_overrides_to_summary_tokens( + tokens, overrides_by_hash + ) + ctx = ContextIdentifier( + context_type, + article_id=display.article_id, + level_adapted_article_text_id=display.id, + ) + return { + "tokens": tokens, + "context_identifier": ctx.as_dictionary(), + "past_bookmarks": past_bookmarks, + } + if display.summary and len(display.summary.strip()) > 10: result["summary"] = display.summary.strip() - tokens = display.get_tokenized_summary() - if not tokens: - continue - # Apply the user's MWE ungroup overrides to the overlaid summary tokens - # (returns a cleared copy; never mutates the ORM-loaded token list). - overrides_by_hash = overrides_by_article.get(display.article_id) - if overrides_by_hash: - tokens = UserArticle._apply_mwe_overrides_to_summary_tokens(tokens, overrides_by_hash) - # The bookmark mapping keys on article_level_summary_id; article_id is - # carried only for the client's MWE-ungroup path (parent article id). - ctx = ContextIdentifier( - ContextType.ARTICLE_LEVEL_SUMMARY, - article_id=display.article_id, - article_level_summary_id=display.id, - ) - result["interactiveSummary"] = { - "tokens": tokens, - "context_identifier": ctx.as_dictionary(), - "past_bookmarks": ArticleLevelSummaryContext.get_all_user_bookmarks_for_article_level_summary( - user.id, display.id - ), - } + summary_tokens = display.get_tokenized_summary() + if summary_tokens: + result["interactiveSummary"] = _payload( + summary_tokens, + ContextType.LEVEL_ADAPTED_ARTICLE_SUMMARY, + LevelAdaptedArticleSummaryContext.get_all_user_bookmarks_for_level_adapted_summary( + user.id, display.id + ), + ) + + # Title is overlaid independently of the summary: a row can carry one and + # not the other (rows written before per-level titles existed have no + # title; the language check can drop a title while keeping its summary). + if display.title and display.title.strip(): + result["title"] = display.title.strip() + title_tokens = display.get_tokenized_title() + if title_tokens: + result["interactiveTitle"] = _payload( + title_tokens, + ContextType.LEVEL_ADAPTED_ARTICLE_TITLE, + LevelAdaptedArticleTitleContext.get_all_user_bookmarks_for_level_adapted_title( + user.id, display.id + ), + ) + else: + # Plain title replaced but no tokens to tap: drop any bundled + # interactiveTitle rather than leave the ORIGINAL title's tokens + # sitting under the level title — they'd translate the wrong words. + result.pop("interactiveTitle", None) diff --git a/zeeguu/core/elastic/elastic_query_builder.py b/zeeguu/core/elastic/elastic_query_builder.py index e08cb02e9..77af1e567 100644 --- a/zeeguu/core/elastic/elastic_query_builder.py +++ b/zeeguu/core/elastic/elastic_query_builder.py @@ -95,7 +95,7 @@ def build_elastic_recommender_query( NOT filtered by CEFR level, deliberately. With on-demand simplification every article is readable at the learner's level — the feed card carries a - level-appropriate summary (ArticleLevelSummary) and the body simplifies on + level-appropriate summary (LevelAdaptedArticleText) and the body simplifies on request (POST /simplify_article/) — so an article being "too hard" is no longer a reason to hide it. Filtering on available_cefr_levels used to mean the opposite: that field lists the levels for which a version was already diff --git a/zeeguu/core/llm_services/prompts/article_simplification.py b/zeeguu/core/llm_services/prompts/article_simplification.py index 133e5c5fe..aad3b04ac 100644 --- a/zeeguu/core/llm_services/prompts/article_simplification.py +++ b/zeeguu/core/llm_services/prompts/article_simplification.py @@ -135,7 +135,7 @@ def get_assessment_and_summary_prompt(language: str) -> str: return f"""You are an expert {language_name} language teacher. Your task is to assess an article's CEFR level and write a short summary. Do NOT rewrite or simplify the article — only assess and summarize it. -CRITICAL — OUTPUT LANGUAGE: These instructions are written in English, but every summary you produce (ORIGINAL_SUMMARY and each [LEVEL]_SUMMARY) MUST be written in {language_name}, the article's own language — NEVER in English (unless {language_name} is itself English). A summary written in English is a failure, even though this prompt is in English. +CRITICAL — OUTPUT LANGUAGE: These instructions are written in English, but every summary and title you produce (ORIGINAL_SUMMARY, each [LEVEL]_SUMMARY and each [LEVEL]_TITLE) MUST be written in {language_name}, the article's own language — NEVER in English (unless {language_name} is itself English). A summary or title written in English is a failure, even though this prompt is in English. CEFR Level Guidelines: - A1: Very basic vocabulary (1000 most common words), simple present tense, basic sentence structures @@ -145,7 +145,9 @@ def get_assessment_and_summary_prompt(language: str) -> str: - C1: Sophisticated vocabulary, complex grammar, idiomatic expressions - C2: Near-native level, literary devices, specialized terminology -IMPORTANT: If the article appears to be incomplete due to a paywall, simply respond with: "unfinished". This includes: +ALWAYS produce every field listed below, for every article. NEVER reply with a single bare word: the two rejection signals are FIELDS (INCOMPLETE_ARTICLE, ADVERTORIAL_CONTENT), not a way to end your answer early. + +IMPORTANT: If the article appears to be incomplete due to a paywall, set INCOMPLETE_ARTICLE to YES. This includes: - Articles with fewer than 3 paragraphs (very likely incomplete) - Articles that end abruptly without a proper conclusion - Articles that appear to be only the first paragraph(s) of a longer piece @@ -157,7 +159,7 @@ def get_assessment_and_summary_prompt(language: str) -> str: - Articles that have audio elements mentioned ("Lyt til artiklen", "Læst op af") but very little text content - Articles that appear to be just a teaser or introduction without the main content -IMPORTANT: If the article appears to be promotional/advertorial content rather than genuine news, simply respond with: "advertorial". This includes: +IMPORTANT: If the article appears to be promotional/advertorial content rather than genuine news, set ADVERTORIAL_CONTENT to YES. This includes: - Articles primarily promoting specific products with pricing/discounts ("Ne laissez pas passer cette offre", "à -30%", "en promotion") - Articles with affiliate marketing language ("meilleure offre", "bon plan", "code promo") - Articles focused on shopping recommendations rather than journalistic news content @@ -167,8 +169,8 @@ def get_assessment_and_summary_prompt(language: str) -> str: - Articles that read like shopping guides or product catalogs rather than news reporting LEVELS TO SUMMARIZE: -- Also write a level-appropriate summary for EVERY CEFR level simpler than the original. -- If original is A1, write no extra summaries. +- Also write a level-appropriate TITLE and summary for EVERY CEFR level simpler than the original. +- If original is A1, write no extra titles or summaries. - If original is A2, write A1. - If original is B1, write A1 and A2. - If original is B2, write A1, A2, and B1. @@ -178,6 +180,10 @@ def get_assessment_and_summary_prompt(language: str) -> str: You must respond in the exact format shown below. Do NOT include any explanations, comments, or meta-text. Do NOT produce full simplified versions of the article — summaries only. +INCOMPLETE_ARTICLE: [YES or NO - is the article text cut off by a paywall, per the rules above? If YES, still fill in every other field as best you can.] + +ADVERTORIAL_CONTENT: [YES or NO - is this promotional/advertorial content rather than journalism, per the rules above? If YES, still fill in every other field as best you can.] + DISTURBING_CONTENT: [YES or NO - would a reader who has asked to avoid disturbing material be upset by this article? Answer YES if EITHER (a) the article's focus is violence, death, or disaster AS AN EVENT - violent crimes, war, terrorism, accidents with casualties, tragic deaths, or graphic violence; OR (b) the article dwells on death/burial imagery and themes even when nobody was harmed - corpses, coffins, being buried (alive or dead), funerary, mortuary, or embalming practices, graphic bodily harm or medical gore. A lifestyle, wellness, or novelty piece whose SUBJECT is death or burial (e.g. "coffin therapy", being buried alive for relaxation) is YES. Note: brief incidental mentions in an otherwise neutral article do NOT count, and purely historical or educational treatment of a difficult topic is acceptable (NO).] ARTICLE_TYPE: [NEWS or GENERAL - NEWS = current events tied to a specific time (politics, breaking news, weather, sports results, someone visiting somewhere today, elections, daily events). GENERAL = evergreen content you could read months later (science explainers, cultural topics, how-to guides, historical articles, general knowledge, health/lifestyle advice).] @@ -186,9 +192,11 @@ def get_assessment_and_summary_prompt(language: str) -> str: ORIGINAL_SUMMARY: [plain text summary (NO markdown formatting, no bold/italic) in {language_name}, 2-4 sentences scaled to the article (short items 1-2 sentences; longer or denser articles up to 4; hard cap ~70 words). It MUST ADD information the title does not already contain — the specific names, numbers, reasons, consequences, or context the headline implies but does not state; if the title assumes a referent (a person, event, acronym, "a swap with X"), briefly say what it is IF the article explains it. NEVER restate or paraphrase the title: if a reader could guess the summary from the title alone, rewrite it. Use ONLY facts stated in the article — do not add details from your own knowledge. Lead with the concrete facts (who/what/how many/outcome). Paraphrase in your own words: no verbatim sentences or phrases lifted from the article, and no direct quotes. DO NOT use meta-preambles like "The article tells about...", "This article is about...", "Artiklen fortæller om...", "L'article parle de...", "Der Artikel handelt von...", "El artículo trata de...". Just state the content as if reporting it yourself.] -SIMPLIFIED_LEVELS: [comma-separated list of the levels simpler than the original for which you wrote a summary below, e.g. "A1,A2" — leave empty if original is A1] +SIMPLIFIED_LEVELS: [comma-separated list of the levels simpler than the original for which you wrote a title and summary below, e.g. "A1,A2" — leave empty if original is A1] + +[For each level in SIMPLIFIED_LEVELS, include BOTH of these sections:] -[For each level in SIMPLIFIED_LEVELS, include this section:] +[LEVEL]_TITLE: [plain text headline (NO markdown formatting, no bold/italic) in {language_name}, on ONE line, rewriting the article's own title with vocabulary and sentence structure appropriate for this level. It names the SAME subject and event as the original title — this is a headline the reader will tap to open that very article, so it must not promise something the article does not deliver. Keep it headline-length (comparable to the original, hard cap ~12 words); do not turn it into a sentence-long summary, do not add facts the original title does not carry, and do not add clickbait or a question form the original did not have.] [LEVEL]_SUMMARY: [plain text summary (NO markdown formatting, no bold/italic) in {language_name} using vocabulary appropriate for this level, same length limits and same content/no-meta-preamble rules as ORIGINAL_SUMMARY above.] diff --git a/zeeguu/core/llm_services/simplification_and_classification.py b/zeeguu/core/llm_services/simplification_and_classification.py index e93fab2bb..b36400c39 100644 --- a/zeeguu/core/llm_services/simplification_and_classification.py +++ b/zeeguu/core/llm_services/simplification_and_classification.py @@ -113,7 +113,13 @@ def _call_simplification_llm(prompt, provider, api_key, max_tokens, timeout=180) def _raise_if_paywall_or_advertorial(result): - """Both prompts answer with a bare token when the article is junk — reject it.""" + """ + The adaptive-simplification prompt answers with a bare token when the article + is junk — reject it. Kept for the assess prompt too, which now asks for fields + instead (see _raise_if_flagged): a model that ignores that and answers in the + old shape anyway must still be caught rather than parsed into an empty + assessment. + """ if result.lower().strip() == "unfinished": raise Exception("PAYWALL: Article appears to be incomplete due to paywall") if result.lower().strip() == "advertorial": @@ -122,6 +128,26 @@ def _raise_if_paywall_or_advertorial(result): ) +def _raise_if_flagged(assessment): + """ + Same rejection, read off the parsed fields instead of a bare one-word reply. + + The bare word was the ONLY way the assess prompt could report junk, and it let + a model end the whole response with one token — which deepseek-chat did for + 100% of articles (6/6 complete Danish articles rejected as "unfinished", in + under a second each), making it look like a broken model rather than a prompt + it could not follow. As fields, the same six articles assess correctly and + truncated copies of them are still flagged: the signal survives, the escape + hatch doesn't. + """ + if assessment.get("is_incomplete"): + raise Exception("PAYWALL: Article appears to be incomplete due to paywall") + if assessment.get("is_advertorial"): + raise Exception( + "ADVERTORIAL: Article appears to be advertorial/promotional content" + ) + + def _clean_text(text): return text.strip("[](){}\"'") @@ -146,17 +172,29 @@ def _level_summary_label(level: str) -> str: return f"{level} summary" +def _level_title_label(level: str) -> str: + return f"{level} title" + + def _summaries_to_check(assessment: dict) -> list: """ - The summaries out of an assessment, labelled — the only part of it that has - a language. Each label is also the key we drop that summary by, so they have + The generated text out of an assessment, labelled — the only part of it that + has a language. Each label is also the key we drop that piece by, so they have to match the ones _without_wrong_language_summaries looks for. + + Per-level titles are included, but a headline is short enough that the + language check will often return "can't judge" (None) rather than a verdict; + that is the check failing open on purpose, not a pass. """ fields = [(ORIGINAL_SUMMARY_LABEL, assessment.get("original_summary", ""))] fields += [ (_level_summary_label(level), text) for level, text in assessment.get("level_summaries", {}).items() ] + fields += [ + (_level_title_label(level), text) + for level, text in assessment.get("level_titles", {}).items() + ] return fields @@ -178,6 +216,14 @@ def _without_wrong_language_summaries(assessment: dict, mismatches: list) -> dic for level, text in result.get("level_summaries", {}).items() if _level_summary_label(level) not in wrong } + # A level's title and summary are dropped independently: an English title + # next to a good Danish summary should cost us the title, not the summary. + # The overlay falls back to the article's own title when a level has none. + result["level_titles"] = { + level: text + for level, text in result.get("level_titles", {}).items() + if _level_title_label(level) not in wrong + } return result @@ -230,7 +276,11 @@ def generate(correction): prompt + correction, provider, api_key, max_tokens=2000, timeout=120 ) _raise_if_paywall_or_advertorial(result) - return _parse_assessment_and_summary(result, provider, model_name) + assessment = _parse_assessment_and_summary(result, provider, model_name) + # Raised, not returned, so junk propagates out of the language-check retry + # loop exactly as the bare-word rejection always has. + _raise_if_flagged(assessment) + return assessment try: return generate_in_language( @@ -258,6 +308,8 @@ def _parse_assessment_and_summary(result: str, provider: str, model_name: str) - is_field = ":" in line and any( line.startswith(prefix) for prefix in [ + "INCOMPLETE_ARTICLE", + "ADVERTORIAL_CONTENT", "DISTURBING_CONTENT", "ARTICLE_TYPE", "ORIGINAL_LEVEL", @@ -273,7 +325,15 @@ def _parse_assessment_and_summary(result: str, provider: str, model_name: str) - "C1", "C2", ] - if is_field or is_level_summary: + is_level_title = "_TITLE:" in line and line.split("_TITLE:")[0] in [ + "A1", + "A2", + "B1", + "B2", + "C1", + "C2", + ] + if is_field or is_level_summary or is_level_title: if current_section: sections[current_section] = "\n".join(current_content).strip() current_section = line.split(":")[0] @@ -284,6 +344,12 @@ def _parse_assessment_and_summary(result: str, provider: str, model_name: str) - sections[current_section] = "\n".join(current_content).strip() is_disturbing = _clean_text(sections.get("DISTURBING_CONTENT", "NO")).upper() == "YES" + # Absent → NO: a model still answering in the old bare-word shape is caught by + # _raise_if_paywall_or_advertorial before we ever get here. + is_incomplete = _clean_text(sections.get("INCOMPLETE_ARTICLE", "NO")).upper() == "YES" + is_advertorial = ( + _clean_text(sections.get("ADVERTORIAL_CONTENT", "NO")).upper() == "YES" + ) article_type_raw = _clean_text(sections.get("ARTICLE_TYPE", "")).upper() article_type = article_type_raw if article_type_raw in ["NEWS", "GENERAL"] else None original_level = _clean_text(sections.get("ORIGINAL_LEVEL", "")) @@ -300,6 +366,17 @@ def _parse_assessment_and_summary(result: str, provider: str, model_name: str) - if text: level_summaries[level] = text + # Per-level headlines, same shape as the summaries. A title is one line by + # construction, so collapse any stray wrapping the section parser picked up. + level_titles = {} + for level in ["A1", "A2", "B1", "B2", "C1", "C2"]: + raw = sections.get(f"{level}_TITLE") + if raw: + text = _strip_markdown_from_summary(_clean_text(raw)) + text = " ".join(text.split()) + if text: + level_titles[level] = text + return { "original_cefr_level": original_level, "original_summary": original_summary, @@ -310,7 +387,10 @@ def _parse_assessment_and_summary(result: str, provider: str, model_name: str) - # parser in simplify_and_classify below. "article_type": article_type.lower() if article_type else None, "is_disturbing": is_disturbing, + "is_incomplete": is_incomplete, + "is_advertorial": is_advertorial, "level_summaries": level_summaries, + "level_titles": level_titles, "provider": provider, "model_name": model_name, } @@ -369,6 +449,7 @@ def assess_summarize_and_classify( original_article, result.get("level_summaries", {}), result.get("model_name"), + result.get("level_titles", {}), ) classifications = [] @@ -382,14 +463,23 @@ def assess_summarize_and_classify( ASSESS_SUMMARY_PROMPT_VERSION = "assess_summary_v1" -def _store_level_summaries(session, article, level_summaries, model_name): - """Create/refresh ArticleLevelSummary rows for an article, tokenizing each.""" +def _store_level_summaries(session, article, level_summaries, model_name, level_titles=None): + """ + Create/refresh LevelAdaptedArticleText rows for an article, tokenizing each. + + A level's title is optional and stored alongside its summary: the LLM may not + have produced one, or the language check may have dropped it while keeping the + summary. The card falls back to the article's own title in that case, so a + missing title is never a reason to skip the row. + """ if not level_summaries: return - from zeeguu.core.model.article_level_summary import ArticleLevelSummary + from zeeguu.core.model.level_adapted_article_text import LevelAdaptedArticleText from zeeguu.core.model.ai_generator import AIGenerator from zeeguu.core.mwe import tokenize_for_reading + level_titles = level_titles or {} + ai_generator_id = None if model_name: ai_generator = AIGenerator.find_or_create( @@ -397,23 +487,36 @@ def _store_level_summaries(session, article, level_summaries, model_name): ) ai_generator_id = ai_generator.id - for level, summary_text in level_summaries.items(): + def tokenized_or_none(text, what, level): try: - tokenized = tokenize_for_reading(summary_text, article.language, mode="stanza") + return tokenize_for_reading(text, article.language, mode="stanza") except Exception as e: - log(f" Could not tokenize {level} summary for article {article.id}: {e}") - tokenized = None - ArticleLevelSummary.find_or_create( + log(f" Could not tokenize {level} {what} for article {article.id}: {e}") + return None + + n_titles = 0 + for level, summary_text in level_summaries.items(): + title_text = level_titles.get(level) + tokenized_title = None + if title_text: + tokenized_title = tokenized_or_none(title_text, "title", level) + n_titles += 1 + LevelAdaptedArticleText.find_or_create( session, article, cefr_level=level, summary=summary_text, - tokenized_summary=tokenized, + tokenized_summary=tokenized_or_none(summary_text, "summary", level), ai_generator_id=ai_generator_id, commit=False, + title=title_text, + tokenized_title=tokenized_title, ) session.commit() - log(f" Stored {len(level_summaries)} per-level preview summaries for article {article.id}") + log( + f" Stored {len(level_summaries)} per-level preview summaries " + f"({n_titles} with titles) for article {article.id}" + ) def get_target_levels_for_original_level(original_level: str) -> list[str]: diff --git a/zeeguu/core/model/__init__.py b/zeeguu/core/model/__init__.py index 38ffbaabf..e3868ba84 100644 --- a/zeeguu/core/model/__init__.py +++ b/zeeguu/core/model/__init__.py @@ -32,8 +32,9 @@ from .article_fragment_context import ArticleFragmentContext from .article_title_context import ArticleTitleContext from .article_summary_context import ArticleSummaryContext -from .article_level_summary import ArticleLevelSummary -from .article_level_summary_context import ArticleLevelSummaryContext +from .level_adapted_article_text import LevelAdaptedArticleText +from .level_adapted_article_summary_context import LevelAdaptedArticleSummaryContext +from .level_adapted_article_title_context import LevelAdaptedArticleTitleContext from .example_sentence import ExampleSentence from .example_sentence_context import ExampleSentenceContext diff --git a/zeeguu/core/model/bookmark.py b/zeeguu/core/model/bookmark.py index 6cc1ac081..fd121c2af 100644 --- a/zeeguu/core/model/bookmark.py +++ b/zeeguu/core/model/bookmark.py @@ -152,15 +152,36 @@ def get_source_title(self): else: # Fallback: context mapping is missing return "[Title not available]" - if self.context.context_type.type == ContextType.ARTICLE_LEVEL_SUMMARY: - from zeeguu.core.model.article_level_summary_context import ( - ArticleLevelSummaryContext, + if self.context.context_type.type == ContextType.LEVEL_ADAPTED_ARTICLE_SUMMARY: + from zeeguu.core.model.level_adapted_article_summary_context import ( + LevelAdaptedArticleSummaryContext, ) - level_summary_context = ArticleLevelSummaryContext.find_by_bookmark(self) - if level_summary_context and level_summary_context.article_level_summary: + level_summary_context = LevelAdaptedArticleSummaryContext.find_by_bookmark(self) + if level_summary_context and level_summary_context.level_adapted_article_text: return Article.find_by_id( - level_summary_context.article_level_summary.article_id + level_summary_context.level_adapted_article_text.article_id + ).title + else: + # Fallback: context mapping is missing + return "[Title not available]" + if self.context.context_type.type == ContextType.LEVEL_ADAPTED_ARTICLE_TITLE: + from zeeguu.core.model.level_adapted_article_title_context import ( + LevelAdaptedArticleTitleContext, + ) + + # Deliberately the ARTICLE's title, not the level title the bookmark + # actually sits in. get_source_title answers "which document did this + # word come from", and the article title is the stable answer: level + # titles change with the learner's CEFR level, so a word list keyed on + # one would show a headline that no longer exists for that reader. + # Every other branch here resolves to Article.title for the same + # reason. Trade-off: an A1 learner sees the publisher's harder + # headline next to a word they met in the A1 one. + level_title_context = LevelAdaptedArticleTitleContext.find_by_bookmark(self) + if level_title_context and level_title_context.level_adapted_article_text: + return Article.find_by_id( + level_title_context.level_adapted_article_text.article_id ).title else: # Fallback: context mapping is missing @@ -443,9 +464,9 @@ def get_context_identifier(self): context_identifier.article_id = ( result.article_id if result else None ) - case ContextType.ARTICLE_LEVEL_SUMMARY: - context_identifier.article_level_summary_id = ( - result.article_level_summary_id if result else None + case ContextType.LEVEL_ADAPTED_ARTICLE_SUMMARY: + context_identifier.level_adapted_article_text_id = ( + result.level_adapted_article_text_id if result else None ) case ContextType.VIDEO_TITLE: context_identifier.video_id = result.video_id if result else None diff --git a/zeeguu/core/model/context_identifier.py b/zeeguu/core/model/context_identifier.py index 624d47936..20cc60e2d 100644 --- a/zeeguu/core/model/context_identifier.py +++ b/zeeguu/core/model/context_identifier.py @@ -10,7 +10,7 @@ def __init__( video_id=None, video_caption_id=None, example_sentence_id=None, - article_level_summary_id=None, + level_adapted_article_text_id=None, ): self.context_type = context_type self.article_fragment_id = article_fragment_id @@ -18,24 +18,51 @@ def __init__( self.video_id = video_id self.video_caption_id = video_caption_id self.example_sentence_id = example_sentence_id - self.article_level_summary_id = article_level_summary_id + self.level_adapted_article_text_id = level_adapted_article_text_id def __repr__(self): return f"" + # Pre-rename spellings, still arriving from clients. The context identifier is + # opaque to the app: the server hands it out with a feed payload and the client + # posts the same blob back when a word is translated. An app holding a payload + # built before the rename — a cached feed, a backgrounded tab, an older release + # — will post the old keys, and dropping them would silently lose the anchor so + # the bookmark could not be highlighted again. Safe to delete once no client can + # still be holding a pre-rename payload. + LEGACY_KEYS = { + "level_adapted_article_text_id": "article_level_summary_id", + } + LEGACY_CONTEXT_TYPES = { + "ArticleLevelSummary": "LevelAdaptedArticleSummary", + "ArticleLevelTitle": "LevelAdaptedArticleTitle", + } + + @classmethod + def _get(cls, dictionary, key): + value = dictionary.get(key, None) + if value is None and key in cls.LEGACY_KEYS: + return dictionary.get(cls.LEGACY_KEYS[key], None) + return value + @classmethod def from_dictionary(cls, dictionary): assert dictionary is not None assert "context_type" in dictionary, f"Context type must be provided" + context_type = dictionary.get("context_type", None) + context_type = cls.LEGACY_CONTEXT_TYPES.get(context_type, context_type) + return ContextIdentifier( - dictionary.get("context_type", None), + context_type, dictionary.get("article_fragment_id", None), dictionary.get("article_id", None), video_id=dictionary.get("video_id", None), video_caption_id=dictionary.get("video_caption_id", None), example_sentence_id=dictionary.get("example_sentence_id", None), - article_level_summary_id=dictionary.get("article_level_summary_id", None), + level_adapted_article_text_id=cls._get( + dictionary, "level_adapted_article_text_id" + ), ) @classmethod @@ -50,7 +77,7 @@ def as_dictionary(self): "video_id": self.video_id, "video_caption_id": self.video_caption_id, "example_sentence_id": self.example_sentence_id, - "article_level_summary_id": self.article_level_summary_id, + "level_adapted_article_text_id": self.level_adapted_article_text_id, } def create_context_mapping(self, session, bookmark, commit=False): @@ -98,12 +125,15 @@ def create_context_mapping(self, session, bookmark, commit=False): ) session.add(mapped_context) - case ContextType.ARTICLE_LEVEL_SUMMARY: - if self.article_level_summary_id is None: + # Both level cases resolve the same LevelAdaptedArticleText row; they + # differ only in which join table context_specific_table resolved to, + # so the level's title and its summary keep separate bookmark sets. + case ContextType.LEVEL_ADAPTED_ARTICLE_SUMMARY | ContextType.LEVEL_ADAPTED_ARTICLE_TITLE: + if self.level_adapted_article_text_id is None: return None - from zeeguu.core.model.article_level_summary import ArticleLevelSummary - level_summary = ArticleLevelSummary.find_by_id( - self.article_level_summary_id + from zeeguu.core.model.level_adapted_article_text import LevelAdaptedArticleText + level_summary = LevelAdaptedArticleText.find_by_id( + self.level_adapted_article_text_id ) if level_summary is None: return None diff --git a/zeeguu/core/model/context_type.py b/zeeguu/core/model/context_type.py index 16b18207e..3519fafe9 100644 --- a/zeeguu/core/model/context_type.py +++ b/zeeguu/core/model/context_type.py @@ -14,7 +14,8 @@ class ContextType(db.Model): ARTICLE_FRAGMENT = "ArticleFragment" ARTICLE_TITLE = "ArticleTitle" ARTICLE_SUMMARY = "ArticleSummary" - ARTICLE_LEVEL_SUMMARY = "ArticleLevelSummary" + LEVEL_ADAPTED_ARTICLE_SUMMARY = "LevelAdaptedArticleSummary" + LEVEL_ADAPTED_ARTICLE_TITLE = "LevelAdaptedArticleTitle" VIDEO_TITLE = "VideoTitle" VIDEO_CAPTION = "VideoCaption" WEB_FRAGMENT = "WebFragment" @@ -26,7 +27,8 @@ class ContextType(db.Model): ARTICLE_FRAGMENT, ARTICLE_TITLE, ARTICLE_SUMMARY, - ARTICLE_LEVEL_SUMMARY, + LEVEL_ADAPTED_ARTICLE_SUMMARY, + LEVEL_ADAPTED_ARTICLE_TITLE, VIDEO_TITLE, VIDEO_CAPTION, WEB_FRAGMENT, @@ -73,8 +75,11 @@ def get_table_corresponding_to_type(cls, type: str): from zeeguu.core.model.article_fragment_context import ArticleFragmentContext from zeeguu.core.model.article_title_context import ArticleTitleContext from zeeguu.core.model.article_summary_context import ArticleSummaryContext - from zeeguu.core.model.article_level_summary_context import ( - ArticleLevelSummaryContext, + from zeeguu.core.model.level_adapted_article_summary_context import ( + LevelAdaptedArticleSummaryContext, + ) + from zeeguu.core.model.level_adapted_article_title_context import ( + LevelAdaptedArticleTitleContext, ) from zeeguu.core.model.video_title_context import VideoTitleContext from zeeguu.core.model.video_caption_context import VideoCaptionContext @@ -87,8 +92,10 @@ def get_table_corresponding_to_type(cls, type: str): return ArticleTitleContext case cls.ARTICLE_SUMMARY: return ArticleSummaryContext - case cls.ARTICLE_LEVEL_SUMMARY: - return ArticleLevelSummaryContext + case cls.LEVEL_ADAPTED_ARTICLE_SUMMARY: + return LevelAdaptedArticleSummaryContext + case cls.LEVEL_ADAPTED_ARTICLE_TITLE: + return LevelAdaptedArticleTitleContext case cls.VIDEO_TITLE: return VideoTitleContext case cls.VIDEO_CAPTION: diff --git a/zeeguu/core/model/article_level_summary_context.py b/zeeguu/core/model/level_adapted_article_summary_context.py similarity index 63% rename from zeeguu/core/model/article_level_summary_context.py rename to zeeguu/core/model/level_adapted_article_summary_context.py index 7dc285ed1..7f2a5f79e 100644 --- a/zeeguu/core/model/article_level_summary_context.py +++ b/zeeguu/core/model/level_adapted_article_summary_context.py @@ -2,10 +2,10 @@ import sqlalchemy -class ArticleLevelSummaryContext(db.Model): +class LevelAdaptedArticleSummaryContext(db.Model): """ A context that is found in a per-level preview summary of an Article - (see ArticleLevelSummary). Anchors a bookmark to a SPECIFIC level's summary + (see LevelAdaptedArticleText). Anchors a bookmark to a SPECIFIC level's summary so past-bookmark highlighting lands on the right tokens — summaries differ by level, so token coordinates are not shared across levels. @@ -16,7 +16,9 @@ class ArticleLevelSummaryContext(db.Model): # At most one context row per (bookmark, level summary) — see find_or_create. db.UniqueConstraint( "bookmark_id", - "article_level_summary_id", + "level_adapted_article_text_id", + # Keeps its pre-rename name: RENAME TABLE carries index names across + # unchanged, and renaming an index buys nothing. name="uq_alsc_bookmark_summary", ), {"mysql_collate": "utf8_bin"}, @@ -29,19 +31,19 @@ class ArticleLevelSummaryContext(db.Model): bookmark_id = db.Column(db.Integer, db.ForeignKey(Bookmark.id), nullable=False) bookmark = db.relationship(Bookmark) - from zeeguu.core.model.article_level_summary import ArticleLevelSummary + from zeeguu.core.model.level_adapted_article_text import LevelAdaptedArticleText - article_level_summary_id = db.Column( - db.Integer, db.ForeignKey(ArticleLevelSummary.id) + level_adapted_article_text_id = db.Column( + db.Integer, db.ForeignKey(LevelAdaptedArticleText.id) ) - article_level_summary = db.relationship(ArticleLevelSummary) + level_adapted_article_text = db.relationship(LevelAdaptedArticleText) - def __init__(self, bookmark, article_level_summary): + def __init__(self, bookmark, level_adapted_article_text): self.bookmark = bookmark - self.article_level_summary = article_level_summary + self.level_adapted_article_text = level_adapted_article_text def __repr__(self): - return f"" + return f"" @classmethod def find_by_bookmark(cls, bookmark): @@ -51,10 +53,10 @@ def find_by_bookmark(cls, bookmark): return None @classmethod - def find_or_create(cls, session, bookmark, article_level_summary, commit=True): + def find_or_create(cls, session, bookmark, level_adapted_article_text, commit=True): existing = cls.query.filter( cls.bookmark == bookmark, - cls.article_level_summary == article_level_summary, + cls.level_adapted_article_text == level_adapted_article_text, ).one_or_none() if existing: return existing @@ -64,14 +66,14 @@ def find_or_create(cls, session, bookmark, article_level_summary, commit=True): # constraint fires and we roll back just this insert (not the caller's # whole transaction, which may still be uncommitted when commit=False) and # return the row the other request created. - new = cls(bookmark, article_level_summary) + new = cls(bookmark, level_adapted_article_text) try: with session.begin_nested(): session.add(new) except sqlalchemy.exc.IntegrityError: return cls.query.filter( cls.bookmark == bookmark, - cls.article_level_summary == article_level_summary, + cls.level_adapted_article_text == level_adapted_article_text, ).one() if commit: @@ -79,15 +81,15 @@ def find_or_create(cls, session, bookmark, article_level_summary, commit=True): return new @classmethod - def get_all_user_bookmarks_for_article_level_summary( - cls, user_id: int, article_level_summary_id: int, as_json_serializable: bool = True + def get_all_user_bookmarks_for_level_adapted_summary( + cls, user_id: int, level_adapted_article_text_id: int, as_json_serializable: bool = True ): from zeeguu.core.model import Bookmark, UserWord result = ( Bookmark.query.join(cls) .join(UserWord, Bookmark.user_word_id == UserWord.id) - .filter(cls.article_level_summary_id == article_level_summary_id) + .filter(cls.level_adapted_article_text_id == level_adapted_article_text_id) .filter(UserWord.user_id == user_id) ).all() diff --git a/zeeguu/core/model/article_level_summary.py b/zeeguu/core/model/level_adapted_article_text.py similarity index 53% rename from zeeguu/core/model/article_level_summary.py rename to zeeguu/core/model/level_adapted_article_text.py index 0cad0702b..386856a62 100644 --- a/zeeguu/core/model/article_level_summary.py +++ b/zeeguu/core/model/level_adapted_article_text.py @@ -10,16 +10,32 @@ CEFR_ORDER = ["A1", "A2", "B1", "B2", "C1", "C2"] -class ArticleLevelSummary(db.Model): +def _parsed_tokens(value): + """A db.JSON column round-trips as a list/dict, but rows written when the + column held a JSON *string* still exist — accept both, and never raise.""" + if not value: + return None + if isinstance(value, str): + try: + return json.loads(value) + except (ValueError, TypeError): + return None + return value + + +class LevelAdaptedArticleText(db.Model): """ - A short, CEFR-level-specific summary of an Article, used as the tappable - preview blurb on feed cards. + The CEFR-level-specific card text for an Article: a short summary and the + headline that goes above it, used as the tappable preview on feed cards. On-demand simplification means the crawl no longer creates a simplified child - article per level, so the level-appropriate summaries live here directly - instead of on child-article rows. There is at most one row per - (article, cefr_level), for levels simpler than the article's own level; the - article's own-level summary stays on ``Article.summary``. + article per level, so the level-appropriate text lives here directly instead + of on child-article rows. There is at most one row per (article, cefr_level), + for levels simpler than the article's own level; the article's own-level text + stays on ``Article.summary`` / ``Article.title``. + + (The table kept its ``level_adapted_article_text`` name when the title columns were + added — renaming it would have churned the two context joins and every FK.) """ __table_args__ = {"mysql_collate": "utf8_bin"} @@ -34,6 +50,12 @@ class ArticleLevelSummary(db.Model): # Cached token stream (same shape as ArticleTokenizationCache.tokenized_summary) # so the tappable preview renders without re-tokenizing on the request path. tokenized_summary = db.Column(db.JSON) + # The level's headline, and its token stream. Nullable on purpose: every row + # written before per-level titles existed has none, and a level whose title + # came back in the wrong language is dropped while its summary is kept — both + # fall back to the article's own title. + title = db.Column(db.UnicodeText) + tokenized_title = db.Column(db.JSON) # First-class generator entity (model_name + prompt_version), same as # Article.simplification_ai_generator_id — not a raw model-name string. ai_generator_id = db.Column(db.Integer, db.ForeignKey("ai_generator.id")) @@ -41,16 +63,25 @@ class ArticleLevelSummary(db.Model): created_at = db.Column(db.DateTime, server_default=db.func.now()) def __init__( - self, article, cefr_level, summary, tokenized_summary=None, ai_generator_id=None + self, + article, + cefr_level, + summary, + tokenized_summary=None, + ai_generator_id=None, + title=None, + tokenized_title=None, ): self.article = article self.cefr_level = cefr_level self.summary = summary self.tokenized_summary = tokenized_summary + self.title = title + self.tokenized_title = tokenized_title self.ai_generator_id = ai_generator_id def __repr__(self): - return f"" + return f"" @classmethod def find_by_id(cls, id: int): @@ -69,6 +100,8 @@ def find_or_create( tokenized_summary=None, ai_generator_id=None, commit=True, + title=None, + tokenized_title=None, ): try: existing = cls.query.filter( @@ -77,13 +110,23 @@ def find_or_create( ).one() existing.summary = summary existing.tokenized_summary = tokenized_summary + existing.title = title + existing.tokenized_title = tokenized_title existing.ai_generator_id = ai_generator_id session.add(existing) if commit: session.commit() return existing except sqlalchemy.orm.exc.NoResultFound: - new = cls(article, cefr_level, summary, tokenized_summary, ai_generator_id) + new = cls( + article, + cefr_level, + summary, + tokenized_summary, + ai_generator_id, + title, + tokenized_title, + ) session.add(new) if commit: session.commit() @@ -98,29 +141,46 @@ def allowed_levels(user_level: str): return set(CEFR_ORDER[: CEFR_ORDER.index(user_level) + 1]) @staticmethod - def pick_best(candidates, user_level: str): + def pick_best(candidates, user_level: str, article_own_level: str = None): """ From ``candidates`` (any objects with a ``.cefr_level``), return the one at the highest level still at or below ``user_level``, or None. This is the single source of truth for level selection, shared by the single-article lookup and the batched feed overlay so they can't drift apart. + + ``article_own_level`` is the article's own CEFR level, and a learner at or + above it gets None — meaning "use the article's own summary". Rows exist + only for levels BELOW the article's own, so without this the highest + *stored* row wins and every learner from the article's level upwards + collapses onto it: for a Danish B1 article (rows A1, A2) the A2 row was + served to A2, B1, B2, C1 and C2 readers alike, so changing level changed + nothing. Passing it None keeps the old highest-row-wins behaviour for + callers that genuinely have no article level to compare against. """ - allowed = ArticleLevelSummary.allowed_levels(user_level) + allowed = LevelAdaptedArticleText.allowed_levels(user_level) if not allowed: return None + if ( + article_own_level in CEFR_ORDER + and CEFR_ORDER.index(user_level) >= CEFR_ORDER.index(article_own_level) + ): + return None eligible = [c for c in candidates if c.cefr_level in allowed] if not eligible: return None return max(eligible, key=lambda c: CEFR_ORDER.index(c.cefr_level)) @classmethod - def best_for_user_level(cls, article_id: int, user_level: str): + def best_for_user_level(cls, article_id: int, user_level: str, article_own_level: str = None): """ - Return the ArticleLevelSummary best matching a learner's CEFR level: the + Return the LevelAdaptedArticleText best matching a learner's CEFR level: the highest stored level that is still at or below ``user_level`` (rows only exist for levels below the article's own, so a learner at or above the article level gets None and the caller falls back to Article.summary). Returns None when there's no suitable per-level summary. + + Pass ``article_own_level`` for that at-or-above check to actually happen — + see pick_best. """ allowed = cls.allowed_levels(user_level) if not allowed: @@ -128,15 +188,13 @@ def best_for_user_level(cls, article_id: int, user_level: str): rows = cls.query.filter( cls.article_id == article_id, cls.cefr_level.in_(allowed) ).all() - return cls.pick_best(rows, user_level) + return cls.pick_best(rows, user_level, article_own_level) def get_tokenized_summary(self): """Parse the cached token stream, tolerating either JSON text or a dict.""" - if not self.tokenized_summary: - return None - if isinstance(self.tokenized_summary, str): - try: - return json.loads(self.tokenized_summary) - except (ValueError, TypeError): - return None - return self.tokenized_summary + return _parsed_tokens(self.tokenized_summary) + + def get_tokenized_title(self): + """Same, for the level's headline. None when this row predates per-level + titles or its title was dropped by the language check.""" + return _parsed_tokens(self.tokenized_title) diff --git a/zeeguu/core/model/level_adapted_article_title_context.py b/zeeguu/core/model/level_adapted_article_title_context.py new file mode 100644 index 000000000..72780cdd8 --- /dev/null +++ b/zeeguu/core/model/level_adapted_article_title_context.py @@ -0,0 +1,98 @@ +from zeeguu.core.model.db import db +import sqlalchemy + + +class LevelAdaptedArticleTitleContext(db.Model): + """ + A context that is found in a per-level headline of an Article (the ``title`` + on LevelAdaptedArticleText). Anchors a bookmark to a SPECIFIC level's title so + past-bookmark highlighting lands on the right tokens — titles differ by level, + so token coordinates are not shared across levels, nor with the publisher's + own headline (which keeps using ArticleTitleContext). + + Separate from LevelAdaptedArticleSummaryContext even though both point at the same + LevelAdaptedArticleText row: a level's title and its summary are two different + token streams, and one join table for both would return the title's bookmarks + when highlighting the summary. + """ + + __table_args__ = ( + # At most one context row per (bookmark, level title) — see find_or_create. + db.UniqueConstraint( + "bookmark_id", + "level_adapted_article_text_id", + name="uq_latc_bookmark_title", + ), + {"mysql_collate": "utf8_bin"}, + ) + + id = db.Column(db.Integer, primary_key=True) + + from zeeguu.core.model.bookmark import Bookmark + + bookmark_id = db.Column(db.Integer, db.ForeignKey(Bookmark.id), nullable=False) + bookmark = db.relationship(Bookmark) + + from zeeguu.core.model.level_adapted_article_text import LevelAdaptedArticleText + + level_adapted_article_text_id = db.Column( + db.Integer, db.ForeignKey(LevelAdaptedArticleText.id) + ) + level_adapted_article_text = db.relationship(LevelAdaptedArticleText) + + def __init__(self, bookmark, level_adapted_article_text): + self.bookmark = bookmark + self.level_adapted_article_text = level_adapted_article_text + + def __repr__(self): + return f"" + + @classmethod + def find_by_bookmark(cls, bookmark): + try: + return cls.query.filter(cls.bookmark == bookmark).one() + except sqlalchemy.orm.exc.NoResultFound: + return None + + @classmethod + def find_or_create(cls, session, bookmark, level_adapted_article_text, commit=True): + existing = cls.query.filter( + cls.bookmark == bookmark, + cls.level_adapted_article_text == level_adapted_article_text, + ).one_or_none() + if existing: + return existing + + # Insert inside a SAVEPOINT so that if a concurrent request created the + # same (bookmark, level title) between our SELECT and INSERT, the unique + # constraint fires and we roll back just this insert (not the caller's + # whole transaction, which may still be uncommitted when commit=False) and + # return the row the other request created. + new = cls(bookmark, level_adapted_article_text) + try: + with session.begin_nested(): + session.add(new) + except sqlalchemy.exc.IntegrityError: + return cls.query.filter( + cls.bookmark == bookmark, + cls.level_adapted_article_text == level_adapted_article_text, + ).one() + + if commit: + session.commit() + return new + + @classmethod + def get_all_user_bookmarks_for_level_adapted_title( + cls, user_id: int, level_adapted_article_text_id: int, as_json_serializable: bool = True + ): + from zeeguu.core.model import Bookmark, UserWord + + result = ( + Bookmark.query.join(cls) + .join(UserWord, Bookmark.user_word_id == UserWord.id) + .filter(cls.level_adapted_article_text_id == level_adapted_article_text_id) + .filter(UserWord.user_id == user_id) + ).all() + + return [each.to_json(True) if as_json_serializable else each for each in result] diff --git a/zeeguu/core/model/user_article.py b/zeeguu/core/model/user_article.py index 5fc40da63..ff5063f56 100644 --- a/zeeguu/core/model/user_article.py +++ b/zeeguu/core/model/user_article.py @@ -723,19 +723,14 @@ def _apply_mwe_overrides_to_summary_tokens(summary_tokens, overrides_by_hash): return tokens @classmethod - def _level_matched_summary_payload(cls, user, article): + def _level_matched_row(cls, user, article): """ - Return the tappable-summary payload for the ArticleLevelSummary matching - this learner's CEFR level, or None if there's no suitable per-level - summary (caller falls back to the article's own-level summary). + The LevelAdaptedArticleText matching this learner's CEFR level, or None when + there is no suitable per-level row (caller falls back to the article's own + title/summary). """ from sqlalchemy.orm.exc import NoResultFound - from zeeguu.core.model.article_level_summary import ArticleLevelSummary - from zeeguu.core.model.article_level_summary_context import ( - ArticleLevelSummaryContext, - ) - from zeeguu.core.model.context_identifier import ContextIdentifier - from zeeguu.core.model.context_type import ContextType + from zeeguu.core.model.level_adapted_article_text import LevelAdaptedArticleText try: user_level = user.cefr_level_for_learned_language() @@ -744,26 +739,76 @@ def _level_matched_summary_payload(cls, user, article): if not user_level: return None - level_summary = ArticleLevelSummary.best_for_user_level(article.id, user_level) + return LevelAdaptedArticleText.best_for_user_level( + article.id, user_level, article.cefr_level + ) + + @classmethod + def _level_matched_summary_payload(cls, user, article, level_row=None): + """ + Return the tappable-summary payload for the LevelAdaptedArticleText matching + this learner's CEFR level, or None if there's no suitable per-level + summary (caller falls back to the article's own-level summary). + """ + from zeeguu.core.model.level_adapted_article_summary_context import ( + LevelAdaptedArticleSummaryContext, + ) + from zeeguu.core.model.context_identifier import ContextIdentifier + from zeeguu.core.model.context_type import ContextType + + level_summary = level_row or cls._level_matched_row(user, article) if not level_summary: return None tokens = level_summary.get_tokenized_summary() if not tokens: return None - # The bookmark mapping still keys on article_level_summary_id + # The bookmark mapping still keys on level_adapted_article_text_id # (create_context_mapping switches on context_type); article_id is carried # only so the client's MWE-ungroup path can address the override by # parent article id. context_id = ContextIdentifier( - ContextType.ARTICLE_LEVEL_SUMMARY, + ContextType.LEVEL_ADAPTED_ARTICLE_SUMMARY, + article_id=article.id, + level_adapted_article_text_id=level_summary.id, + ) + return { + "tokens": tokens, + "context_identifier": context_id.as_dictionary(), + "past_bookmarks": LevelAdaptedArticleSummaryContext.get_all_user_bookmarks_for_level_adapted_summary( + user.id, level_summary.id + ), + } + + @classmethod + def _level_matched_title_payload(cls, user, article, level_row=None): + """ + Same as _level_matched_summary_payload, for the level's headline. None + when the row has no title (rows predating per-level titles, or a title the + language check dropped) — the caller then keeps the article's own title. + """ + from zeeguu.core.model.level_adapted_article_title_context import ( + LevelAdaptedArticleTitleContext, + ) + from zeeguu.core.model.context_identifier import ContextIdentifier + from zeeguu.core.model.context_type import ContextType + + level_summary = level_row or cls._level_matched_row(user, article) + if not level_summary or not level_summary.title: + return None + tokens = level_summary.get_tokenized_title() + if not tokens: + return None + + context_id = ContextIdentifier( + ContextType.LEVEL_ADAPTED_ARTICLE_TITLE, article_id=article.id, - article_level_summary_id=level_summary.id, + level_adapted_article_text_id=level_summary.id, ) return { "tokens": tokens, "context_identifier": context_id.as_dictionary(), - "past_bookmarks": ArticleLevelSummaryContext.get_all_user_bookmarks_for_article_level_summary( + "past_bookmarks": LevelAdaptedArticleTitleContext.get_all_user_bookmarks_for_level_adapted_title( user.id, level_summary.id ), } @@ -803,8 +848,10 @@ def user_article_summary_info(cls, user: User, article: Article, tokenization_ca # Build summary response — prefer a CEFR-level-matched preview summary if # one exists for this learner's level; otherwise fall back to the article's - # own-level summary. - level_summary = cls._level_matched_summary_payload(user, article) + # own-level summary. Resolved once and shared with the title branch below + # so the two can't pick different levels (and to save a query). + level_row = cls._level_matched_row(user, article) + level_summary = cls._level_matched_summary_payload(user, article, level_row) if level_summary: result["tokenized_summary"] = level_summary elif article.summary and cache.tokenized_summary: @@ -829,7 +876,7 @@ def user_article_summary_info(cls, user: User, article: Article, tokenization_ca # Apply the user's MWE ungroup overrides to whichever summary branch # produced the payload (per-level OR the fallback own-summary). The lookup # keys on the PARENT article.id: for level summaries that's how overrides - # are stored (see ContextIdentifier.article_id on ARTICLE_LEVEL_SUMMARY), + # are stored (see ContextIdentifier.article_id on LEVEL_ADAPTED_ARTICLE_SUMMARY), # and the summary sentence text differs from body sentences so the # sentence hash still scopes the override to the summary. Mutates in place. if "tokenized_summary" in result: @@ -847,8 +894,12 @@ def user_article_summary_info(cls, user: User, article: Article, tokenization_ca result["tokenized_summary"].get("tokens"), overrides_by_hash ) - # Build title response - if cache.tokenized_title: + # Build title response — same precedence as the summary above: the + # learner's level headline if this row has one, else the article's own. + level_title = cls._level_matched_title_payload(user, article, level_row) + if level_title: + result["tokenized_title"] = level_title + elif cache.tokenized_title: try: tokenized_title = json.loads(cache.tokenized_title) title_context_id = ContextIdentifier( diff --git a/zeeguu/core/test/test_assess_and_summarize.py b/zeeguu/core/test/test_assess_and_summarize.py index 35606a65a..034aa291e 100644 --- a/zeeguu/core/test/test_assess_and_summarize.py +++ b/zeeguu/core/test/test_assess_and_summarize.py @@ -1,4 +1,5 @@ -"""Regression tests for assess_and_summarize's article_type handling. +"""Regression tests for assess_and_summarize: article_type handling, and how an +article gets rejected as junk. The article.article_type column is enum('news','general') with a case-sensitive utf8mb4_bin collation. If the parser returns an UPPERCASE 'NEWS'/'GENERAL', the @@ -63,3 +64,72 @@ def test_missing_article_type_is_none(self): ), ): self.assertIsNone(sac.assess_and_summarize("T", "C", "da")["article_type"]) + + +class AssessAndSummarizeRejectionTest(TestCase): + """How an article is rejected as junk. + + The prompt used to offer a bare one-word reply ("unfinished") as the ONLY + way to report a paywall, which let a model end the whole response with one + token. deepseek-chat took that exit on 100% of articles — including complete + ones — which read as a broken model for weeks. The signal is a field now; + the bare word is still honoured for any model that ignores the change. + """ + + def _assess(self, llm_response): + with patch.object( + sac, "_select_provider_and_key", return_value=("anthropic", "fake-key") + ), patch.object( + sac, "get_assessment_and_summary_prompt", return_value="{title}\n{content}" + ), patch.object( + sac, "_call_simplification_llm", return_value=(llm_response, "fake-model") + ): + return sac.assess_and_summarize("Titel", "Indhold", "da") + + def test_the_incomplete_field_rejects_the_article(self): + """The paywall signal is a field now, not a bare one-word reply — it must + still reject. (The bare word kept working too; see + test_a_bare_unfinished_still_rejects.)""" + response = ( + "INCOMPLETE_ARTICLE: YES\n" + "ADVERTORIAL_CONTENT: NO\n" + "DISTURBING_CONTENT: NO\n" + "ARTICLE_TYPE: News\n" + "ORIGINAL_LEVEL: B1\n" + "ORIGINAL_SUMMARY: Noget dansk tekst her.\n" + ) + with self.assertRaises(Exception) as raised: + self._assess(response) + assert str(raised.exception).startswith("PAYWALL") + + def test_the_advertorial_field_rejects_the_article(self): + response = ( + "INCOMPLETE_ARTICLE: NO\n" + "ADVERTORIAL_CONTENT: YES\n" + "DISTURBING_CONTENT: NO\n" + "ARTICLE_TYPE: General\n" + "ORIGINAL_LEVEL: B1\n" + "ORIGINAL_SUMMARY: Noget dansk tekst her.\n" + ) + with self.assertRaises(Exception) as raised: + self._assess(response) + assert str(raised.exception).startswith("ADVERTORIAL") + + def test_a_bare_unfinished_still_rejects(self): + """Backward compatibility: the prompt no longer offers the bare word, but a + model that answers with it anyway must not be parsed into an empty + assessment.""" + with self.assertRaises(Exception) as raised: + self._assess("unfinished") + assert str(raised.exception).startswith("PAYWALL") + + def test_a_clean_article_is_not_rejected_when_the_flags_are_absent(self): + """Rows from a model that omits the new fields entirely must default to + NO — absent must never read as "junk", or every such article is dropped.""" + response = ( + "DISTURBING_CONTENT: NO\n" + "ARTICLE_TYPE: News\n" + "ORIGINAL_LEVEL: B1\n" + "ORIGINAL_SUMMARY: Regeringen har fremlagt et nyt forslag om klimaet i dag.\n" + ) + assert self._assess(response)["original_cefr_level"] == "B1" diff --git a/zeeguu/core/test/test_bookmark_context_uniqueness.py b/zeeguu/core/test/test_bookmark_context_uniqueness.py index faf1eb72f..93f5bde0f 100644 --- a/zeeguu/core/test/test_bookmark_context_uniqueness.py +++ b/zeeguu/core/test/test_bookmark_context_uniqueness.py @@ -6,7 +6,7 @@ for the same (bookmark, anchor) -- which later broke find_by_bookmark/.one() with MultipleResultsFound. -These tests mirror test_article_level_summary.py's +These tests mirror test_level_adapted_article_text.py's test_context_find_or_create_is_idempotent / _rejected_by_unique_constraint, run across all six affected models at once: diff --git a/zeeguu/core/test/test_article_level_summary.py b/zeeguu/core/test/test_level_adapted_article_text.py similarity index 53% rename from zeeguu/core/test/test_article_level_summary.py rename to zeeguu/core/test/test_level_adapted_article_text.py index ff77b6a30..7c2f471d0 100644 --- a/zeeguu/core/test/test_article_level_summary.py +++ b/zeeguu/core/test/test_level_adapted_article_text.py @@ -1,7 +1,7 @@ """Per-level preview summaries (on-demand simplification flow). Covers the level-selection logic and that the feed's summary payload anchors to -the CEFR-matched ArticleLevelSummary (with the ArticleLevelSummary context type) +the CEFR-matched LevelAdaptedArticleText (with the LevelAdaptedArticleText context type) so the tappable preview / bookmark highlighting targets the right level. """ from unittest import TestCase @@ -11,8 +11,9 @@ from zeeguu.core.test.rules.article_rule import ArticleRule from zeeguu.core.test.rules.user_rule import UserRule -from zeeguu.core.model.article_level_summary import ArticleLevelSummary -from zeeguu.core.model.article_level_summary_context import ArticleLevelSummaryContext +from zeeguu.core.model.level_adapted_article_text import LevelAdaptedArticleText +from zeeguu.core.model.level_adapted_article_summary_context import LevelAdaptedArticleSummaryContext +from zeeguu.core.model.context_identifier import ContextIdentifier from zeeguu.core.model.context_type import ContextType from zeeguu.core.model.user_article import UserArticle from zeeguu.core.model.user_language import UserLanguage @@ -25,6 +26,10 @@ # tokens (same nesting tokenize_for_reading produces). DUMMY_TOKENS = [[[{"text": "hej", "sentence_i": 0, "token_i": 0}]]] +# Distinct from DUMMY_TOKENS so a test can tell a served title apart from a +# served summary rather than matching whichever ran last. +DUMMY_TITLE_TOKENS = [[[{"text": "overskrift", "sentence_i": 0, "token_i": 0}]]] + # A summary token stream (paragraphs -> sentences -> tokens) whose first sentence # groups "har lavet" into an MWE. The metadata keys mirror what # UserArticle._clear_mwe_metadata_for_expressions reads and clears. @@ -51,7 +56,7 @@ def _mwe_tokens(): CEFR_TO_INT = {"A1": 1, "A2": 2, "B1": 3, "B2": 4, "C1": 5, "C2": 6} -class ArticleLevelSummaryTest(ModelTestMixIn, TestCase): +class LevelAdaptedArticleTextTest(ModelTestMixIn, TestCase): def setUp(self): super().setUp() self.user = UserRule().user @@ -63,16 +68,27 @@ def setUp(self): session.commit() def _add_level_summary(self, level): - return ArticleLevelSummary.find_or_create( + return LevelAdaptedArticleText.find_or_create( + session, + self.article, + cefr_level=level, + summary=f"summary at {level}", + tokenized_summary=DUMMY_TOKENS, + ) + + def _add_level_summary_with_title(self, level): + return LevelAdaptedArticleText.find_or_create( session, self.article, cefr_level=level, summary=f"summary at {level}", tokenized_summary=DUMMY_TOKENS, + title=f"title at {level}", + tokenized_title=DUMMY_TITLE_TOKENS, ) def _add_mwe_level_summary(self, level): - return ArticleLevelSummary.find_or_create( + return LevelAdaptedArticleText.find_or_create( session, self.article, cefr_level=level, @@ -91,24 +107,66 @@ def test_best_for_user_level_picks_highest_at_or_below(self): b1 = self._add_level_summary("B1") # B2 learner → highest available at/below is B1 - assert ArticleLevelSummary.best_for_user_level(self.article.id, "B2").id == b1.id + assert LevelAdaptedArticleText.best_for_user_level(self.article.id, "B2").id == b1.id # C1 learner → still B1 (nothing higher stored) - assert ArticleLevelSummary.best_for_user_level(self.article.id, "C1").id == b1.id + assert LevelAdaptedArticleText.best_for_user_level(self.article.id, "C1").id == b1.id # A2 learner → only A1 qualifies assert ( - ArticleLevelSummary.best_for_user_level(self.article.id, "A2").cefr_level + LevelAdaptedArticleText.best_for_user_level(self.article.id, "A2").cefr_level == "A1" ) # A1 learner → A1 assert ( - ArticleLevelSummary.best_for_user_level(self.article.id, "A1").cefr_level + LevelAdaptedArticleText.best_for_user_level(self.article.id, "A1").cefr_level == "A1" ) def test_no_summary_at_or_below_returns_none(self): self._add_level_summary("B1") # An A1 learner has nothing at/below B1... wait, B1 > A1, so None. - assert ArticleLevelSummary.best_for_user_level(self.article.id, "A1") is None + assert LevelAdaptedArticleText.best_for_user_level(self.article.id, "A1") is None + + def test_learner_at_or_above_article_level_gets_the_articles_own_summary(self): + """ + Rows exist only BELOW the article's own level, so a learner reading at or + above it must fall through to Article.summary (None here) rather than be + handed the highest stored row. + + This is the Danish B1 collapse: 366 of 368 recent Danish B1 articles carry + only A1 and A2 rows, so before this every reader from B1 up was served the + A2 summary and changing CEFR level changed nothing on the card. + """ + self._add_level_summary("A1") + self._add_level_summary("A2") + + for level_at_or_above in ("B1", "B2", "C1", "C2"): + assert ( + LevelAdaptedArticleText.best_for_user_level( + self.article.id, level_at_or_above, "B1" + ) + is None + ), f"{level_at_or_above} learner should get the article's own summary" + + # ...while learners below the article's level still get their own row, + # and crucially A2 and B1 no longer collapse onto the same text. + assert ( + LevelAdaptedArticleText.best_for_user_level(self.article.id, "A2", "B1").cefr_level + == "A2" + ) + assert ( + LevelAdaptedArticleText.best_for_user_level(self.article.id, "A1", "B1").cefr_level + == "A1" + ) + + def test_own_level_summary_preferred_over_a_simpler_row(self): + """An A2 learner on an A2 article gets the article's own A2 summary, not + the A1 row that happens to be the highest stored one at/below A2.""" + self._add_level_summary("A1") + assert LevelAdaptedArticleText.best_for_user_level(self.article.id, "A2", "A2") is None + assert ( + LevelAdaptedArticleText.best_for_user_level(self.article.id, "A1", "A2").cefr_level + == "A1" + ) def test_summary_info_anchors_to_level_summary(self): self._add_level_summary("A1") @@ -119,10 +177,43 @@ def test_summary_info_anchors_to_level_summary(self): payload = info.get("tokenized_summary") assert payload is not None ctx = payload["context_identifier"] - assert ctx["context_type"] == ContextType.ARTICLE_LEVEL_SUMMARY - assert ctx["article_level_summary_id"] == b1.id + assert ctx["context_type"] == ContextType.LEVEL_ADAPTED_ARTICLE_SUMMARY + assert ctx["level_adapted_article_text_id"] == b1.id assert payload["tokens"] == DUMMY_TOKENS + def test_title_info_anchors_to_level_title(self): + """A level row with a title serves that title, under its own context type + — not ARTICLE_TITLE, whose token coordinates belong to the publisher's + headline and would highlight the wrong words.""" + b1 = self._add_level_summary_with_title("B1") + self._set_user_level("B2") + + info = UserArticle.user_article_summary_info(self.user, self.article) + payload = info.get("tokenized_title") + assert payload is not None + ctx = payload["context_identifier"] + assert ctx["context_type"] == ContextType.LEVEL_ADAPTED_ARTICLE_TITLE + assert ctx["level_adapted_article_text_id"] == b1.id + assert ctx["article_id"] == self.article.id + assert payload["tokens"] == DUMMY_TITLE_TOKENS + + def test_level_row_without_a_title_falls_back_to_the_articles_own(self): + """Rows written before per-level titles existed — and rows whose title the + language check dropped — keep serving the article's own headline.""" + self._add_level_summary("B1") # summary only, no title + self._set_user_level("B2") + + info = UserArticle.user_article_summary_info(self.user, self.article) + # The level summary is still served... + assert ( + info["tokenized_summary"]["context_identifier"]["context_type"] + == ContextType.LEVEL_ADAPTED_ARTICLE_SUMMARY + ) + # ...while the title falls back to the article's own. + title_ctx = info.get("tokenized_title", {}).get("context_identifier") + if title_ctx is not None: + assert title_ctx["context_type"] == ContextType.ARTICLE_TITLE + def test_level_summary_context_carries_parent_article_id(self): # The served level-summary context now also carries the parent article id # so the client's MWE-ungroup path can address the override. @@ -131,7 +222,7 @@ def test_level_summary_context_carries_parent_article_id(self): info = UserArticle.user_article_summary_info(self.user, self.article) ctx = info["tokenized_summary"]["context_identifier"] - assert ctx["context_type"] == ContextType.ARTICLE_LEVEL_SUMMARY + assert ctx["context_type"] == ContextType.LEVEL_ADAPTED_ARTICLE_SUMMARY assert ctx["article_id"] == self.article.id def test_mwe_override_clears_metadata_on_level_summary(self): @@ -175,7 +266,7 @@ def test_mwe_override_untouched_when_expression_not_disabled(self): assert sentence[0]["mwe_group_id"] == 7 def test_mwe_override_does_not_mutate_stored_tokens(self): - # The served tokens are a cleared copy; the ArticleLevelSummary row's own + # The served tokens are a cleared copy; the LevelAdaptedArticleText row's own # tokenized_summary must be left intact (no accidental DB-side mutation). als = self._add_mwe_level_summary("B1") self._set_user_level("B2") @@ -216,7 +307,7 @@ def test_overlay_applies_mwe_override_and_carries_article_id(self): interactive = results[0].get("interactiveSummary") assert interactive is not None ctx = interactive["context_identifier"] - assert ctx["context_type"] == ContextType.ARTICLE_LEVEL_SUMMARY + assert ctx["context_type"] == ContextType.LEVEL_ADAPTED_ARTICLE_SUMMARY assert ctx["article_id"] == self.article.id # MWE metadata cleared on the overlaid tokens. sentence = interactive["tokens"][0][0] @@ -241,12 +332,12 @@ def test_context_find_or_create_is_idempotent(self): als = self._add_level_summary("B1") bookmark = BookmarkRule(self.user).bookmark - c1 = ArticleLevelSummaryContext.find_or_create(session, bookmark, als) - c2 = ArticleLevelSummaryContext.find_or_create(session, bookmark, als) + c1 = LevelAdaptedArticleSummaryContext.find_or_create(session, bookmark, als) + c2 = LevelAdaptedArticleSummaryContext.find_or_create(session, bookmark, als) assert c1.id == c2.id - count = ArticleLevelSummaryContext.query.filter_by( - bookmark_id=bookmark.id, article_level_summary_id=als.id + count = LevelAdaptedArticleSummaryContext.query.filter_by( + bookmark_id=bookmark.id, level_adapted_article_text_id=als.id ).count() assert count == 1 @@ -257,8 +348,55 @@ def test_duplicate_context_rejected_by_unique_constraint(self): bookmark = BookmarkRule(self.user).bookmark # Two raw rows for the same (bookmark, level summary) must not coexist. - session.add(ArticleLevelSummaryContext(bookmark, als)) - session.add(ArticleLevelSummaryContext(bookmark, als)) + session.add(LevelAdaptedArticleSummaryContext(bookmark, als)) + session.add(LevelAdaptedArticleSummaryContext(bookmark, als)) with self.assertRaises(IntegrityError): session.flush() session.rollback() + + +class LegacyContextIdentifierKeysTest(TestCase): + """ + Clients round-trip the context identifier as an opaque blob: the server hands + it out with a feed payload, the client posts the same thing back when a word + is translated. So an app holding a payload built before the + ArticleLevelSummary -> LevelAdaptedArticleText rename posts the OLD spellings, + and dropping them would silently lose the anchor — the bookmark would be + created but never highlighted again. + """ + + def test_the_old_id_key_is_still_understood(self): + ctx = ContextIdentifier.from_dictionary( + {"context_type": "LevelAdaptedArticleSummary", "article_level_summary_id": 42} + ) + assert ctx.level_adapted_article_text_id == 42 + + def test_the_old_context_types_are_mapped_to_the_new_ones(self): + summary = ContextIdentifier.from_dictionary( + {"context_type": "ArticleLevelSummary", "article_level_summary_id": 7} + ) + assert summary.context_type == ContextType.LEVEL_ADAPTED_ARTICLE_SUMMARY + assert summary.level_adapted_article_text_id == 7 + + title = ContextIdentifier.from_dictionary( + {"context_type": "ArticleLevelTitle", "article_level_summary_id": 9} + ) + assert title.context_type == ContextType.LEVEL_ADAPTED_ARTICLE_TITLE + assert title.level_adapted_article_text_id == 9 + + def test_the_new_key_wins_when_both_are_present(self): + ctx = ContextIdentifier.from_dictionary( + { + "context_type": "LevelAdaptedArticleSummary", + "level_adapted_article_text_id": 1, + "article_level_summary_id": 2, + } + ) + assert ctx.level_adapted_article_text_id == 1 + + def test_unrelated_context_types_are_untouched(self): + ctx = ContextIdentifier.from_dictionary( + {"context_type": "ArticleTitle", "article_id": 5} + ) + assert ctx.context_type == ContextType.ARTICLE_TITLE + assert ctx.article_id == 5 diff --git a/zeeguu/core/test/test_llm_output_language_policy.py b/zeeguu/core/test/test_llm_output_language_policy.py index 022d65837..b9d7632b6 100644 --- a/zeeguu/core/test/test_llm_output_language_policy.py +++ b/zeeguu/core/test/test_llm_output_language_policy.py @@ -23,6 +23,15 @@ "The government has presented a new climate proposal that would make it " "cheaper to drive an electric car and more expensive to fly within the country." ) +# Long enough to clear language_check.MIN_CHARS_TO_JUDGE (60), so these two +# exercise the drop path. Real headlines are usually shorter than that and are +# therefore NOT judged — see test_a_short_wrong_language_title_is_not_caught. +DANISH_TITLE = ( + "Regeringen fremlægger nyt klimaforslag om elbiler og indenrigsflyvninger" +) +ENGLISH_TITLE = ( + "Government presents new climate proposal on electric cars and domestic flights" +) DANISH_BODY = ( "Regeringen vil gøre det billigere at køre i elbil. Mange partier er enige " "i planen. Forslaget skal nu behandles i Folketinget, og det sker efter " @@ -61,7 +70,7 @@ def simplifying(response): return _llm_returning(response, "get_adaptive_simplification_prompt") -def an_assessment(summary, level_summary=None): +def an_assessment(summary, level_summary=None, level_title=None): """A well-formed assess+summarize response, with the summaries we want to test.""" response = ( "ORIGINAL_LEVEL: B1\n" @@ -69,6 +78,8 @@ def an_assessment(summary, level_summary=None): "DISTURBING_CONTENT: NO\n" f"ORIGINAL_SUMMARY: {summary}\n" ) + if level_title: + response += f"A2_TITLE: {level_title}\n" if level_summary: response += f"A2_SUMMARY: {level_summary}\n" return response @@ -120,6 +131,43 @@ def test_only_the_wrong_summary_is_dropped(self): self.assertEqual(result["original_summary"], DANISH_SUMMARY) self.assertEqual(result["level_summaries"], {}) + def test_a_level_title_is_parsed_alongside_its_summary(self): + with assessing( + an_assessment(DANISH_SUMMARY, DANISH_SUMMARY, level_title=DANISH_TITLE) + ): + result = sac.assess_and_summarize("Titel", "Indhold", "da") + + self.assertEqual(result["level_titles"]["A2"], DANISH_TITLE) + self.assertEqual(result["level_summaries"]["A2"], DANISH_SUMMARY) + + def test_an_english_level_title_is_dropped_without_costing_its_summary(self): + """A title and its summary are judged independently: an English headline + should not take a good Danish summary down with it — the card falls back + to the article's own title and keeps the level-appropriate blurb.""" + with assessing( + an_assessment(DANISH_SUMMARY, DANISH_SUMMARY, level_title=ENGLISH_TITLE) + ): + result = sac.assess_and_summarize("Titel", "Indhold", "da") + + self.assertEqual(result["level_titles"], {}) + self.assertEqual(result["level_summaries"]["A2"], DANISH_SUMMARY) + self.assertEqual(result["original_summary"], DANISH_SUMMARY) + + def test_a_short_wrong_language_title_is_not_caught(self): + """Honest limit, not an aspiration: language_check needs + MIN_CHARS_TO_JUDGE (60) characters, and most real headlines are shorter, + so a wrong-language TITLE alone ships. It is a small residual risk because + one LLM call produces the whole response — an English title almost always + arrives beside English summaries, which ARE long enough to be judged and + which trigger the retry. Do not read the drop test above as a guarantee + that titles are language-guarded in production.""" + with assessing( + an_assessment(DANISH_SUMMARY, DANISH_SUMMARY, level_title="Climate deal") + ): + result = sac.assess_and_summarize("Titel", "Indhold", "da") + + self.assertEqual(result["level_titles"]["A2"], "Climate deal") + class AdaptiveSimplificationLanguageTest(TestCase): """Policy: ask again once, then fail the run — nothing is salvaged."""