diff --git a/apps/skillnet-api/src/agents/content/helpers.py b/apps/skillnet-api/src/agents/content/helpers.py index 4e6452b..79e2381 100644 --- a/apps/skillnet-api/src/agents/content/helpers.py +++ b/apps/skillnet-api/src/agents/content/helpers.py @@ -208,3 +208,83 @@ def review_report(parsed: Any) -> dict: return parsed issues = _dict_items(parsed) return {"passed": False, "overall_score": 0.0, "issues": issues} + + +# --------------------------------------------------------------------------- # +# Exercise content: the answer key's shape, checked before it is published +# --------------------------------------------------------------------------- # +#: A run of underscores is a blank in a ``fill_blank`` template, whatever its length +#: (the generator prompt writes three, the runtime prompt four). Two is the floor so a +#: lone ``_`` used as a character is not mistaken for a hole. +_BLANK_RUN_RE = re.compile(r"_{2,}") + + +def exercise_content_problems(exercise_type: str, content: Any) -> list[str]: + """Why an exercise's ``content`` cannot be graded, or ``[]`` when it can. + + ``_EXERCISE_SHAPES`` in the prompt module states the exact jsonb of every exercise and + nothing downstream checked it. A ``test`` whose ``correct`` points past the end of its + options, or a ``fill_blank`` with fewer answers than holes, grades every learner 0.0 + forever: an exercise nobody can pass, which blocks the course. The pipeline's only + defence was the quality reviewer, and the reviewer is best-effort (it is skipped when + the provider fails) and reads prose, not shapes. + + Returns human-readable reasons, empty when the content is usable. ``publish`` drops an + exercise with problems instead of shipping one that cannot be answered. + """ + if not isinstance(content, dict): + return ["content is not an object"] + + problems: list[str] = [] + if exercise_type == "test": + options = content.get("options") + correct = content.get("correct") + if not isinstance(options, list) or len(options) < 2: + problems.append("test needs at least two options") + elif isinstance(correct, bool) or not isinstance(correct, int): + problems.append("test 'correct' must be an option index") + elif not 0 <= correct < len(options): + problems.append( + f"test 'correct' {correct} is outside its {len(options)} options" + ) + elif exercise_type == "true_false": + if not isinstance(content.get("correct"), bool): + problems.append("true_false 'correct' must be a boolean") + elif exercise_type == "fill_blank": + template = content.get("template") + blanks = content.get("blanks") + holes = len(_BLANK_RUN_RE.findall(template)) if isinstance(template, str) else 0 + if holes == 0: + problems.append("fill_blank template has no blank (___)") + if not isinstance(blanks, list) or not all(isinstance(b, str) for b in blanks): + problems.append("fill_blank 'blanks' must be a list of strings") + elif holes and len(blanks) != holes: + problems.append( + f"fill_blank has {holes} hole(s) but {len(blanks)} blank answer(s)" + ) + elif exercise_type == "order_steps": + steps = content.get("steps") + order = content.get("correct_order") + if not isinstance(steps, list) or len(steps) < 2: + problems.append("order_steps needs at least two steps") + elif not isinstance(order, list) or sorted(order) != list(range(len(steps))): + problems.append( + "order_steps 'correct_order' must be a permutation of the steps" + ) + elif exercise_type == "practical_case": + if not str(content.get("context") or "").strip(): + problems.append("practical_case needs a context") + if not str(content.get("question") or "").strip(): + problems.append("practical_case needs a question") + if not isinstance(content.get("rubric"), list) or not content.get("rubric"): + problems.append("practical_case needs a non-empty rubric") + elif exercise_type == "dialogue": + if not str(content.get("system_prompt") or "").strip(): + problems.append("dialogue needs a system_prompt") + turns = content.get("max_turns") + if isinstance(turns, bool) or not isinstance(turns, int) or turns < 1: + problems.append("dialogue 'max_turns' must be a positive integer") + else: + problems.append(f"unknown exercise type {exercise_type!r}") + return problems + diff --git a/apps/skillnet-api/src/agents/content/nodes.py b/apps/skillnet-api/src/agents/content/nodes.py index ba71a39..351cb78 100644 --- a/apps/skillnet-api/src/agents/content/nodes.py +++ b/apps/skillnet-api/src/agents/content/nodes.py @@ -20,6 +20,7 @@ assemble_chunk_text as _assemble_chunk_text, describe_payload as _describe_payload, estimate_pages as _estimate_pages, + exercise_content_problems as _exercise_content_problems, module_payload as _module_payload, outline_dict as _outline_dict, review_report as _review_report, @@ -272,25 +273,28 @@ async def design_structure(state: GenerationState) -> dict: async def _module_context( state: GenerationState, spec: dict, embeddings: EmbeddingService | None ) -> str: + """The source material that belongs to one module. + + A short document (``full_text``) goes in whole: there is nothing to scope and every + module can see the material it needs. A long one is retrieved **for this module**, on + its title and summary, so a module is not handed a fact from a section it never + teaches. Before 2026-09-30 this function loaded *every* chunk and then ran a + similarity search that could add nothing (each hit was already in the map), so every + module received the whole document and a question could be written about any part of + it — the "pregunta que no va con el temario" the testers reported. The retrieval now + does what it says, and the whole document is only the fallback for when embeddings are + unavailable. + """ context = "\n\n".join((state.get("full_texts") or {}).values()) if context.strip(): return context org_id = uuid.UUID(state["org_id"]) doc_ids = _uuids(state.get("source_document_ids")) - items: dict[uuid.UUID, dict] = {} + query_text = f"{spec.get('title', '')} {spec.get('summary', '')}".strip() async with async_session_factory() as db: repo = DocumentChunkRepository(db) - all_chunks = list(await repo.list_for_documents_ordered(doc_ids)) - for chunk in all_chunks: - items[chunk.id] = { - "doc": str(chunk.document_id), - "idx": chunk.chunk_index, - "content": chunk.content, - } - - query_text = f"{spec.get('title', '')} {spec.get('summary', '')}".strip() if embeddings is not None and query_text: try: vector = await embeddings.embed_query(query_text) @@ -300,18 +304,20 @@ async def _module_context( top_k=SEMANTIC_TOP_K, document_ids=doc_ids, ) - for hit in hits: - if hit["chunk_id"] not in items: - items[hit["chunk_id"]] = { - "doc": str(hit["document_id"]), - "idx": 10**9, - "content": hit["content"], - } except LLMError: - logger.warning("Semantic supplement skipped (embedding unavailable)") + hits = [] + logger.warning( + "Semantic retrieval skipped for module %r (embedding unavailable); " + "falling back to the whole document", + spec.get("title", ""), + ) + if hits: + return "\n\n".join( + _strip_chunk_prefix(hit["content"]) for hit in hits + ) - ordered = sorted(items.values(), key=lambda it: (it["doc"], it["idx"])) - return "\n\n".join(_strip_chunk_prefix(it["content"]) for it in ordered) + chunks = list(await repo.list_for_documents_ordered(doc_ids)) + return _assemble_chunk_text(chunks) if chunks else "" @node_error_wrapper("generate_modules") @@ -660,11 +666,25 @@ async def publish(state: GenerationState) -> dict: etype = ExerciseType(gen_exercise.get("type")) except ValueError: continue + content = gen_exercise.get("content", {}) + problems = _exercise_content_problems(etype.value, content) + if problems: + # An exercise whose key cannot be graded is unpassable, and an + # unpassable exercise blocks the whole course. Dropping it is the + # lesser failure, and the warning is how it stops being silent. + logger.warning( + "Dropping ungradeable %s exercise from module %r for job %s: %s", + etype.value, + spec.get("title", ""), + job_id, + "; ".join(problems), + ) + continue db.add( Exercise( lesson_id=last_lesson.id, type=etype, - content=gen_exercise.get("content", {}), + content=content, position=gen_exercise.get("position", position), ) ) diff --git a/apps/skillnet-api/src/llm/prompts/generation.py b/apps/skillnet-api/src/llm/prompts/generation.py index 5ff1ee2..59a1595 100644 --- a/apps/skillnet-api/src/llm/prompts/generation.py +++ b/apps/skillnet-api/src/llm/prompts/generation.py @@ -112,6 +112,14 @@ material educativo independiente, sin revelar la procedencia interna del texto. 6. Escribe en el mismo idioma que el material de origen. 7. Formatea el contenido de las lecciones como Markdown. +8. ALINEACION PREGUNTA-LECCION: cada ejercicio se responde con lo que ensenan las + lecciones de ESTE modulo. Antes de escribir una pregunta, comprueba que el dato, el + plazo, la regla o el paso que la resuelve aparece en alguna de las lecciones que acabas + de escribir para este modulo. Si no aparece, anadelo primero a una leccion o cambia la + pregunta. No preguntes algo que el modulo no ha explicado, aunque si este en el material + de origen: preguntar lo que no se explico no mide aprendizaje. +9. La "explanation" de un ejercicio cerrado cita el hecho concreto de la leccion que lo + justifica, no una afirmacion generica ni una copia literal del enunciado. """ + _EXERCISE_SHAPES + """ Responde en JSON valido con la forma: @@ -144,10 +152,17 @@ si el curso es demasiado teorico. 6. LENGUAJE: contenido claro, profesional y en el idioma de la fuente. 7. CONSISTENCIA: los modulos no deben contradecirse entre si. +8. ALINEACION PREGUNTA-LECCION: cada ejercicio debe poder responderse con lo que ensenan + las lecciones del MISMO modulo, no con un hecho del material de origen que la leccion no + llego a explicar. Recorre las lecciones del modulo y despues lee sus ejercicios: el dato, + el plazo, la regla o el paso que resuelve cada pregunta tiene que aparecer en una de esas + lecciones. Si la respuesta solo se justifica por la fuente y no por lo ensenado, marcalo + como "critical": es una pregunta que no se explico. Comprueba tambien que la + "explanation" corresponda al hecho concreto que la leccion ensena. Para cada problema encontrado especifica: -- severity: "critical" (error factual, respuesta erronea), "major" (omision - significativa, artefactos de cita) o "minor" (estilo, claridad) +- severity: "critical" (error factual, respuesta erronea, pregunta que no se explico), + "major" (omision significativa, artefactos de cita) o "minor" (estilo, claridad) - module_index: indice del modulo (null para problema a nivel de curso) - description: que esta mal - suggestion: como corregirlo diff --git a/apps/skillnet-api/tests/test_exercise_content_validation.py b/apps/skillnet-api/tests/test_exercise_content_validation.py new file mode 100644 index 0000000..1515149 --- /dev/null +++ b/apps/skillnet-api/tests/test_exercise_content_validation.py @@ -0,0 +1,76 @@ +"""``exercise_content_problems`` — the answer key's shape, checked before publish. + +An exercise whose key cannot be graded grades every learner 0.0 forever, so it can never +be passed and it blocks the course. The generator prompt states the shape of each type but +nothing verified it; the quality reviewer is best-effort and reads prose, not shapes. This +is the deterministic gate ``publish`` uses to drop an ungradeable exercise. +""" + +from __future__ import annotations + +import pytest + +from src.agents.content.helpers import exercise_content_problems + + +def test_a_well_formed_test_has_no_problems() -> None: + content = { + "question": "El descuadre es de seis euros. Que haces?", + "options": ["Cerrar", "Avisar al encargado", "Repetir", "Dejarlo"], + "correct": 1, + "explanation": "Por encima de cinco euros se avisa al encargado.", + } + assert exercise_content_problems("test", content) == [] + + +def test_a_test_whose_correct_index_is_out_of_range_is_rejected() -> None: + content = {"options": ["A", "B", "C"], "correct": 3} + problems = exercise_content_problems("test", content) + assert problems and "outside" in problems[0] + + +def test_a_test_whose_correct_is_a_boolean_is_rejected() -> None: + # ``isinstance(True, int)`` is True in Python, so the bool check has to come first. + content = {"options": ["A", "B"], "correct": True} + assert exercise_content_problems("test", content) + + +def test_a_fill_blank_with_the_wrong_number_of_answers_is_rejected() -> None: + content = {"template": "Se avisa al ____ y se anota en el ____.", "blanks": ["encargado"]} + problems = exercise_content_problems("fill_blank", content) + assert problems and "2 hole(s) but 1 blank" in problems[0] + + +def test_a_fill_blank_with_no_hole_is_rejected() -> None: + assert exercise_content_problems("fill_blank", {"template": "Sin hueco.", "blanks": []}) + + +def test_a_fill_blank_with_matching_holes_is_accepted() -> None: + content = {"template": "Se avisa al ____ antes de cerrar.", "blanks": ["encargado"]} + assert exercise_content_problems("fill_blank", content) == [] + + +def test_order_steps_must_be_a_permutation() -> None: + content = {"steps": ["Uno", "Dos", "Tres"], "correct_order": [0, 1]} + assert exercise_content_problems("order_steps", content) + good = {"steps": ["Uno", "Dos", "Tres"], "correct_order": [2, 0, 1]} + assert exercise_content_problems("order_steps", good) == [] + + +def test_true_false_needs_a_boolean() -> None: + assert exercise_content_problems("true_false", {"correct": "si"}) + assert exercise_content_problems("true_false", {"correct": False}) == [] + + +def test_practical_case_needs_a_context_a_question_and_a_rubric() -> None: + content = {"context": "Un cliente...", "question": "Que haces?", "rubric": []} + assert exercise_content_problems("practical_case", content) + + +@pytest.mark.parametrize("content", [None, "texto", []]) +def test_non_object_content_is_rejected(content: object) -> None: + assert exercise_content_problems("test", content) + + +def test_an_unknown_type_is_reported() -> None: + assert exercise_content_problems("quiz", {}) diff --git a/apps/skillnet-api/tests/test_generation_grounding.py b/apps/skillnet-api/tests/test_generation_grounding.py new file mode 100644 index 0000000..2200b54 --- /dev/null +++ b/apps/skillnet-api/tests/test_generation_grounding.py @@ -0,0 +1,51 @@ +"""The v1 generation prompts tie each question to the module that teaches it. + +The v2 episode path learned this the hard way (``test_episode_prompt_grounding.py``): an +assessment anchored only to the source asks about material the screen never explained, +because the source always carries more than a module can teach. The v1 prompts had the +same gap — ``MODULE_GENERATOR_SYSTEM`` only said "based on the source" and +``QUALITY_REVIEWER_SYSTEM`` only checked answers against the source — so a generated +exercise could quote any fact of the document, whatever the module covered. These tests +pin the two rules that close it. + +They read the system prompts, which is the level the model sees; a rule that exists only +in a comment does not pass. +""" + +from __future__ import annotations + +from src.llm.prompts.generation import ( + MODULE_GENERATOR_SYSTEM, + QUALITY_REVIEWER_SYSTEM, +) + + +def _normalized(prompt: str) -> str: + return " ".join(prompt.split()).lower() + + +def test_the_generator_requires_the_answer_to_live_in_the_lesson() -> None: + """The module writer has to teach a fact before it may ask about it.""" + normalized = _normalized(MODULE_GENERATOR_SYSTEM) + + assert "alineacion pregunta-leccion" in normalized + assert "cada ejercicio se responde con lo que ensenan las lecciones de este modulo" in normalized + # The narrowing against the source is what the old "fundamentado en el material de + # origen" rule was missing: the source is a ceiling, not a licence to ask. + assert "aunque si este en el material de origen" in normalized + + +def test_the_generator_binds_the_explanation_to_the_taught_fact() -> None: + normalized = _normalized(MODULE_GENERATOR_SYSTEM) + assert "la \"explanation\" de un ejercicio cerrado cita el hecho concreto" in normalized + + +def test_the_reviewer_checks_each_question_against_its_own_module() -> None: + """The gate the generator's rule needs to be enforced after the fact.""" + normalized = _normalized(QUALITY_REVIEWER_SYSTEM) + + assert "alineacion pregunta-leccion" in normalized + assert "cada ejercicio debe poder responderse con lo que ensenan las lecciones del mismo modulo" in normalized + # The failure has to be critical, or a passing score would ship it anyway. + assert "marcalo como \"critical\"" in normalized + assert "pregunta que no se explico" in normalized