Skip to content

Feature/semantic error precision#10

Open
IfTrueEqualsEqualsTrue wants to merge 2 commits intoadandeigor:mainfrom
IfTrueEqualsEqualsTrue:feature/SemanticErrorPrecision
Open

Feature/semantic error precision#10
IfTrueEqualsEqualsTrue wants to merge 2 commits intoadandeigor:mainfrom
IfTrueEqualsEqualsTrue:feature/SemanticErrorPrecision

Conversation

@IfTrueEqualsEqualsTrue
Copy link
Copy Markdown

📋 Description

Ajout d'une variable suggérée dans les messages d'erreur pour les variables non déclarées. Ajout du test associé pour vérifier la présence de la suggestion.

🔗 Issue liée

Closes #4

🔄 Type de changement

  • 🐛 Bug fix
  • ✨ Nouvelle fonctionnalité
  • 📝 Documentation
  • 🧪 Tests
  • ♻️ Refactoring
  • 🎨 Extension VS Code

✅ Checklist

  • Mon code suit le style du projet (ruff check passe)
  • Les tests passent (pytest)
  • J'ai ajouté des tests pour les nouveaux comportements (si applicable)
  • J'ai mis à jour la documentation (si applicable)
  • Ma PR est focalisée sur un seul sujet

📸 Captures d'écran (si applicable)

Un exemple avec une faute de frappe sur une variable courte

algolab -c 'Variable x : Entier Debut y <- 42 Ecrire x Fin'
Erreur Semantique (Ligne 1, Colonne 27) : Variable non declaree: y. Vouliez-vous dire 'x' ?

Un autre avec un caractère manquant en début de mot

algolab -c 'Variable temperature : Entier Debut emperature <- 42 Ecrire x Fin'
Erreur Semantique (Ligne 1, Colonne 37) : Variable non declaree: emperature. Vouliez-vous dire 'temperature' ?

📝 Notes pour les reviewers

Ruff se plaint à propos des mix indentations / espaces dans certains fichiers, problèmes déjà détectés avant la PR.

@adandeigor
Copy link
Copy Markdown
Owner

adandeigor commented Apr 19, 2026

Merci pour cette contribution, elle est bien dans l'esprit du projet et l'implémentation est globalement propre. Quelques remarques avant de merger.


✅ Ce qui est bien

  • Utilisation de difflib (stdlib) → zéro dépendance externe ajoutée, bon choix.
  • Le cas names vide est géré proprement dans suggest_var, pas de risque de crash si aucune variable n'est déclarée.
  • Impact minimal sur le code existant : 3 lignes touchées dans environment.py, la logique de suggestion est bien isolée dans utils.py.
  • Test ajouté et pertinent.

⚠️ Point bloquant : le fallback SequenceMatcher est trop agressif

Dans suggest_var, si get_close_matches ne trouve rien (similarité insuffisante), tu tombes sur un fallback qui retourne toujours la variable la plus proche, même si elle n'a aucun rapport avec la faute de frappe.

Exemple concret :

Variable temperature : Entier
Debut
  z <- 42
Fin

Avec le code actuel, l'erreur affichera :

Variable non declaree: z. Vouliez-vous dire 'temperature' ?

Ce qui est trompeur pour un étudiant débutant — une mauvaise suggestion est pire qu'aucune suggestion.

get_close_matches a précisément un seuil de similarité (~0.6) pour éviter ce cas. Le fallback contourne ce garde-fou.

Fix proposé (one-liner) :

def suggest_var(typo: str, names) -> str:
    if not names:
        return ""
    matches = get_close_matches(typo, names, n=1)
    return matches[0] if matches else ""

Si get_close_matches ne trouve rien, on retourne ""build_suggestion_string retourne "" → le message d'erreur reste sobre, sans suggestion hasardeuse.


📝 Point mineur : tests insuffisants

Le test ajouté couvre le cas nominal (faute de frappe proche). Il manque :

  • Aucune variable déclarée → pas de suggestion, pas de crash
  • Variable complètement différente → pas de suggestion affichée (ce cas valide précisément le fix ci-dessus)

🔧 Ruff

Tu mentionnes que les warnings ruff existaient avant ta PR, c'est noté. Mais idéalement, une PR ne devrait pas introduire de nouveaux problèmes. Peux-tu vérifier que ruff check src/algolab/utils.py est propre sur ton fichier au moins ?


Une fois le fallback corrigé et les deux tests supplémentaires ajoutés, je merge sans hésiter. Bon travail 🙌

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.

Améliorer les messages d'erreur pour les variables non déclarées

2 participants