feat(results): sous-commande results push (transcripts locaux → HF)#43
Conversation
Ajoute 'eval-transcript results push' pour verser les sorties des modeles locaux (WhisperX, Kyutai, Cohere/MLX) sur le dataset de resultats HF, afin que le WER et le juge couvrent tous les modeles. Garde-fou RGPD : l'allowlist est derivee du corpus PUBLIC (ground_truth/<id>) ; tout sample absent (reunion interne, sample retire) est ignore -> aucune donnee privee ne part sur le Hub. Dry-run, filtre --include, repos overridables par flag ou env. Ajoute huggingface-hub aux dependances + tests (FakeHfApi). Remplace le script local jetable par une vraie sous-commande, testee. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
There was a problem hiding this comment.
Code Review
This pull request introduces a new results push command to safely upload local model transcripts to a Hugging Face results dataset, using the public corpus as an allowlist to prevent private data leaks. It adds the huggingface-hub dependency, implements the ResultsClient and push planning logic, updates the documentation, and includes comprehensive unit tests. The reviewer feedback suggests several robustness and usability improvements, including wrapping Hugging Face API calls in try-except blocks to handle exceptions gracefully, using is_dir() and is_file() checks to prevent potential runtime errors, ignoring hidden directories, providing clearer CLI output when no files are available to push, and removing an unused helper function in the test suite.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| def plan( | ||
| self, *, transcriptions_dir: Path = DEFAULT_TRANSCRIPTIONS_DIR, include: str | None = None | ||
| ) -> PushPlan: | ||
| return build_push_plan( | ||
| api=self._get_api(), | ||
| corpus_repo=self.corpus_repo, | ||
| results_repo=self.results_repo, | ||
| transcriptions_dir=transcriptions_dir, | ||
| include=include, | ||
| ) | ||
|
|
||
| def push(self, plan: PushPlan, *, message: str = "Push local transcripts (officiels only)") -> None: | ||
| if not plan.uploads: | ||
| return | ||
| if not self.token: | ||
| raise ResultsError( | ||
| "HF_TOKEN is required to push results; set the HF_TOKEN environment variable or pass token=..." | ||
| ) | ||
| try: | ||
| from huggingface_hub import CommitOperationAdd # type: ignore[import-not-found] | ||
| except ImportError as exc: # pragma: no cover - exercised only in production | ||
| raise ResultsError("huggingface_hub is required for live HF Hub operations") from exc | ||
| operations = [ | ||
| CommitOperationAdd(path_in_repo=u.path_in_repo, path_or_fileobj=str(u.local_path)) | ||
| for u in plan.uploads | ||
| ] | ||
| self._get_api().create_commit( | ||
| repo_id=plan.results_repo, | ||
| repo_type=DATASET_REPO_TYPE, | ||
| operations=operations, | ||
| commit_message=message, | ||
| token=self.token, | ||
| ) |
There was a problem hiding this comment.
Les appels à l'API Hugging Face (list_repo_files via plan et create_commit via push) peuvent lever diverses exceptions en cas d'erreur réseau, de dépôt inexistant ou de jeton invalide (par exemple des exceptions de requests ou spécifiques à huggingface_hub). Comme la CLI ne capture pas ces exceptions (elle ne capture que ResultsError et httpx.HTTPError), cela provoquera un plantage avec une trace d'exécution (traceback) brute pour l'utilisateur.
Il est fortement recommandé d'envelopper ces appels dans des blocs try...except au sein de ResultsClient pour intercepter toutes les erreurs et les encapsuler dans une ResultsError claire.
def plan(
self, *, transcriptions_dir: Path = DEFAULT_TRANSCRIPTIONS_DIR, include: str | None = None
) -> PushPlan:
try:
return build_push_plan(
api=self._get_api(),
corpus_repo=self.corpus_repo,
results_repo=self.results_repo,
transcriptions_dir=transcriptions_dir,
include=include,
)
except Exception as exc:
if isinstance(exc, ResultsError):
raise
raise ResultsError(f"Impossible de planifier l'envoi des résultats : {exc}") from exc
def push(self, plan: PushPlan, *, message: str = "Push local transcripts (officiels only)") -> None:
if not plan.uploads:
return
if not self.token:
raise ResultsError(
"HF_TOKEN is required to push results; set the HF_TOKEN environment variable or pass token=..."
)
try:
from huggingface_hub import CommitOperationAdd # type: ignore[import-not-found]
except ImportError as exc: # pragma: no cover - exercised only in production
raise ResultsError("huggingface_hub is required for live HF Hub operations") from exc
operations = [
CommitOperationAdd(path_in_repo=u.path_in_repo, path_or_fileobj=str(u.local_path))
for u in plan.uploads
]
try:
self._get_api().create_commit(
repo_id=plan.results_repo,
repo_type=DATASET_REPO_TYPE,
operations=operations,
commit_message=message,
token=self.token,
)
except Exception as exc:
raise ResultsError(f"Échec de l'envoi des résultats vers Hugging Face : {exc}") from excReferences
- When designing API clients, perform early validation on required credentials and IDs to fail fast.
| if not transcriptions_dir.exists(): | ||
| raise ResultsError(f"Transcriptions directory not found: {transcriptions_dir}") |
There was a problem hiding this comment.
Si transcriptions_dir existe mais n'est pas un dossier (par exemple, si c'est un fichier), l'appel à iterdir() à la ligne 100 lèvera une exception NotADirectoryError. Il est plus robuste de vérifier is_dir() plutôt que exists().
| if not transcriptions_dir.exists(): | |
| raise ResultsError(f"Transcriptions directory not found: {transcriptions_dir}") | |
| if not transcriptions_dir.is_dir(): | |
| raise ResultsError(f"Transcriptions directory not found or is not a directory: {transcriptions_dir}") |
| for sample_dir in sorted(p for p in transcriptions_dir.iterdir() if p.is_dir()): | ||
| sid = sample_dir.name |
There was a problem hiding this comment.
Pour éviter de polluer la liste des dossiers ignorés avec des dossiers système ou cachés (comme .DS_Store, .ipynb_checkpoints ou .git), il est recommandé de filtrer les dossiers commençant par un point ..
| for sample_dir in sorted(p for p in transcriptions_dir.iterdir() if p.is_dir()): | |
| sid = sample_dir.name | |
| for sample_dir in sorted(p for p in transcriptions_dir.iterdir() if p.is_dir() and not p.name.startswith(".")): |
| for txt in sorted(sample_dir.glob(f"*{TRANSCRIPT_SUFFIX}")): | ||
| if include and include not in txt.name: | ||
| continue |
There was a problem hiding this comment.
Il est possible que glob retourne des dossiers s'ils correspondent au motif (par exemple, un sous-dossier nommé avec l'extension .txt). Pour éviter des erreurs lors de la création du commit Hugging Face (qui attend des fichiers), il est plus sûr de filtrer explicitement pour ne garder que les fichiers avec txt.is_file().
for txt in sorted(sample_dir.glob(f"*{TRANSCRIPT_SUFFIX}")):
if not txt.is_file():
continue
if include and include not in txt.name:
continue| if args.dry_run: | ||
| print("[dry-run] relancer sans --dry-run pour pousser.") | ||
| return | ||
| client.push(plan, message=args.message) | ||
| if plan.uploads: | ||
| print(f"Poussé {len(plan.uploads)} fichier(s) vers {plan.results_repo}") | ||
| return |
There was a problem hiding this comment.
Si aucun fichier n'est à pousser (liste plan.uploads vide), la commande se termine silencieusement sans informer explicitement l'utilisateur. De plus, il est inutile d'afficher le message de dry-run s'il n'y a rien à envoyer.
Il serait plus clair d'interrompre l'exécution tôt avec un message explicite si plan.uploads est vide.
| if args.dry_run: | |
| print("[dry-run] relancer sans --dry-run pour pousser.") | |
| return | |
| client.push(plan, message=args.message) | |
| if plan.uploads: | |
| print(f"Poussé {len(plan.uploads)} fichier(s) vers {plan.results_repo}") | |
| return | |
| if not plan.uploads: | |
| print("Aucun fichier à pousser.") | |
| return | |
| if args.dry_run: | |
| print("[dry-run] relancer sans --dry-run pour pousser.") | |
| return | |
| client.push(plan, message=args.message) | |
| print(f"Poussé {len(plan.uploads)} fichier(s) vers {plan.results_repo}") | |
| return |
| @contextlib.contextmanager | ||
| def chdir(path: Path): | ||
| previous = Path.cwd() | ||
| os.chdir(path) | ||
| try: | ||
| yield | ||
| finally: | ||
| os.chdir(previous) |
… de test mort - Emballe list_repo_files / create_commit : un HfHubHTTPError (token invalide, repo introuvable, reseau) remonte en ResultsError -> 'Error: ...' propre + exit 1 au lieu d'un traceback. Helper _hf_http_error_types() (import paresseux, tuple vide si hf absent -> garde les tests sans dependance dure). - Supprime le context manager chdir inutilise dans test_results.py. - Ajoute 2 tests du chemin d'erreur (lecture corpus + push). Repond aux points #1 et #2 de la review. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Transforme le helper local de push en sous-commande propre et testée
eval-transcript results push.Pourquoi
La CI couvre les modèles API (Albert, Voxtral). Les modèles locaux (WhisperX, Kyutai, Cohere/MLX) tournent sur la machine ; il faut verser leurs transcripts sur le dataset de résultats pour que le WER et le juge couvrent tous les modèles.
Comportement
ground_truth/<id>). Tout sample absent (réunion interne, sample retiré) est ignoré → aucune donnée privée ne part sur le Hub. Vérifié en dry-run réel : 40 fichiers (5 officiels × 8 modèles) poussables, 10 samples privés/retirés ignorés.--corpus/--results(ouEVAL_CORPUS_REPO/EVAL_RESULTS_REPO),--token(défautHF_TOKEN).Notes
huggingface-hubaux dépendances du projet (la commande en a besoin au runtime ; n'était pas sur main).results.pytestable via unProtocolHfApi (même pattern que le provider HF). 8 tests ajoutés, suite complète 117 tests verte.🤖 Generated with Claude Code