Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 80 additions & 0 deletions apps/skillnet-api/src/agents/content/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

62 changes: 41 additions & 21 deletions apps/skillnet-api/src/agents/content/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand All @@ -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")
Expand Down Expand Up @@ -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),
)
)
Expand Down
19 changes: 17 additions & 2 deletions apps/skillnet-api/src/llm/prompts/generation.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
76 changes: 76 additions & 0 deletions apps/skillnet-api/tests/test_exercise_content_validation.py
Original file line number Diff line number Diff line change
@@ -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", {})
51 changes: 51 additions & 0 deletions apps/skillnet-api/tests/test_generation_grounding.py
Original file line number Diff line number Diff line change
@@ -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
Loading