ThinkR-open / ThinkR-open/datadiff

[robustesse] report.R : mémoïsation ignorée par datadiff_report_html, eval_error masqué, code mort et constantes dupliquées

Open
#29 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
R
Stars
6
Forks
0
PR merge metrics
No merged PRs in 30d

Description

Contexte

R/report.R manipule les internes de {pointblank} (16 champs de validation_set, extracts, classe has_intel) — c'est assumé et bien verrouillé par des tests de contrat. Mais l'audit relève quatre défauts propres au fichier :

  1. Mémoïsation ignorée : datadiff_report_html() reconstruit agent + rapport même si print(res$reponse) vient de peupler le cache, et ne l'alimente pas en retour → double construction systématique. datadiff_render_report(res$reponse) couvrirait le cas nominal. Par ailleurs le fallback de datadiff_render_report (attribut cache absent) crée un env local jamais rattaché → mémoïsation silencieusement perdue.
  2. eval_error masqué : la garde any(n_failed > 0, na.rm = TRUE) peut être FALSE alors que l'agent réel a échoué par eval_error (n_failed = NA) → branche synthétique choisie, extracts perdus ; et eval_error/eval_warning <- FALSE est forcé sur toutes les lignes, masquant une vraie erreur d'évaluation dans le rapport.
  3. Code mort : dans build_report_agent(), le bloc else d'injection de compteurs par ligne (~11 lignes : row$n, n_passed, f_passed, warn/stop/notify…) est intégralement réécrit par le bloc vectorisé « authoritative from coverage » qui suit. Seuls la copie du template et l'extract survivent.
  4. Constantes dupliquées : les défauts warn_at/stop_at = 1e-14 existent en 4 exemplaires (report.R ×3, compare_datasets_from_yaml.R) et les conventions de nommage __ok/__eq/__missing_col_/__type_mismatch_ sont redéclarées en dur dans report.R, pointblank_setup.R et coverage.R sans constante partagée — une dérive d'un seul côté casse le mapping silencieusement (le step réel n'est plus retrouvé, l'extract est perdu).

Reprex (point 1)

library(datadiff)
ref <- data.frame(id = 1:2, x = c(1, 2)); cand <- data.frame(id = 1:2, x = c(1, 3))
res <- compare_datasets_from_yaml(ref, cand, key = "id")
print(res$reponse)                      # construit + met en cache le rapport
system.time(datadiff_report_html(res, tempfile(fileext = ".html")))
# reconstruit tout au lieu de réutiliser le cache

Critères de succès

  • datadiff_report_html() passe par datadiff_render_report() (lecture et alimentation du cache) ; le second appel est quasi instantané.
  • Un eval_error sur l'agent réel est propagé au rapport (pas de bascule silencieuse sur la branche synthétique, pas de forçage à FALSE) + test.
  • Bloc mort supprimé de build_report_agent() (comportement identique, tests verts).
  • Constantes centralisées (fichier constants/objets internes) pour les suffixes/préfixes et les défauts warn_at/stop_at ; grep ne trouve plus qu'une définition de chaque.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in R/report.R with datadiff_report_html(), datadiff_render_report(), and build_report_agent(), then compare the related definitions in compare_datasets_from_yaml.R, pointblank_setup.R, and coverage.R. Run the existing contract and reporting tests while tracing cache reuse and eval_error propagation. Done means the cache is reused and populated, errors remain visible, dead code is removed, and each suffix, prefix, and threshold has one shared definition.

Written by the indexing model from the issue text.

Assessment

Tech stack
r
Domain
data
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
50/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.