feat: new way of transport#872
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a new way to generate/refresh transport (“voiture”) equivalent values from Publicodes, extends the dataset with multiple car variants (size × fuel), and refactors existing Publicodes extraction logic into dedicated utility modules.
Changes:
- Add Publicodes extraction utilities for voiture, numérique, livraison, and chauffage; wire them into
getPublicodeValues.ts(including a newvoituremode). - Expand car equivalents (new slugs in
values.json+ new entries/updated ECVs indeplacement.ts). - Update translations / naming logic for carpooling labels and adjust tests/data to remove the unused
tilefield.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| testu/formatUsage.test.ts | Removes tile from test fixture data. |
| testu/computeECV.test.ts | Removes tile from test fixture data. |
| src/utils/publicodes/extractVoitureValues.ts | New extractor using Nos Gestes Climat rules to compute per-km usage/construction and write into deplacements. |
| src/utils/publicodes/extractNumeriqueValues.ts | New extractor module (moved out of the script) for numérique values. |
| src/utils/publicodes/extractLivraisonValues.ts | New extractor module (moved out of the script) for livraison values. |
| src/utils/publicodes/extractChauffageValues.ts | New extractor module (moved out of the script) for chauffage values. |
| src/utils/Equivalent/values.json | Updates existing car values and adds many new car variant slugs. |
| src/utils/Equivalent/equivalent.ts | Updates carpooling naming/translation logic (notably prefix/name handling). |
| src/scripts/getPublicodeValues.ts | Refactors to call extractor utilities and adds voiture category handling. |
| src/data/categories/deplacement.ts | Updates voiture ECVs and adds multiple new voiture variants; removes legacy tile/subtitle fields. |
| pnpm-lock.yaml | Adds @incubateur-ademe/nosgestesclimat lock entries. |
| package.json | Adds @incubateur-ademe/nosgestesclimat dependency. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const translation = value[language] as string | ||
| if (equivalent.carpool) { | ||
| const carpool = carpooling[language] | ||
| const [name, prefix] = translation.split(';') |
There was a problem hiding this comment.
Dans la branche equivalent.carpool, le split(';') est destructuré à l’envers (const [name, prefix] = ...). Or plus bas, le format ; est traité comme [prefix, name] (ex: "km en ;voiture thermique"). Tel quel, prefix/name seront inversés et l’affichage du préfixe sera incorrect (ex: préfixe = "voiture thermique"). Inverser la destructuration et gérer le cas où translation ne contient pas ; pour éviter un split inattendu.
| const [name, prefix] = translation.split(';') | |
| let prefix = '' | |
| let name = translation | |
| if (translation.includes(';')) { | |
| ;[prefix, name] = translation.split(';') | |
| } |
| engine.setSituation( | ||
| 'distance' in values | ||
| ? { | ||
| ...rules, | ||
| 'livraison colis . déplacement consommateur . mode de déplacement': "'voiture thermique'", | ||
| 'livraison colis . déplacement consommateur . distance': values.distance, | ||
| } | ||
| : { | ||
| ...rules, | ||
| 'livraison colis . déplacement consommateur . mode de déplacement': "'voiture thermique'", | ||
| } | ||
| ) |
There was a problem hiding this comment.
engine.setSituation(...) est exécuté hors du try/catch. Si la situation est invalide (clé de règle, type de valeur, etc.), l’exception interrompra toute l’extraction au lieu d’être loggée et de passer au slug suivant. Déplacer setSituation dans le try (ou ajouter un try/catch autour) pour rendre le script robuste.
| for (const [key, values] of Object.entries(livraisonEquipmentMapping)) { | ||
| engine.setSituation( | ||
| 'distance' in values | ||
| ? { | ||
| ...livraisonItemMapping[item as keyof typeof livraisonItemMapping], | ||
| 'livraison colis . déplacement consommateur . mode de déplacement': "'voiture thermique'", | ||
| 'livraison colis . déplacement consommateur . distance': values.distance, | ||
| } | ||
| : { | ||
| ...livraisonItemMapping[item as keyof typeof livraisonItemMapping], | ||
| 'livraison colis . déplacement consommateur . mode de déplacement': "'voiture thermique'", | ||
| } | ||
| ) | ||
|
|
||
| try { |
There was a problem hiding this comment.
Dans la boucle sur Object.keys(livraisonData), livraisonItemMapping[item as ...] est spreadé directement dans la situation. Si un item n’existe pas dans livraisonItemMapping, ça produira une exception (spread de undefined) avant même d’entrer dans le try/catch (et setSituation est aussi hors try). Ajouter un garde explicite sur l’existence du mapping (avec warn + continue) et inclure setSituation dans le bloc protégé.
| for (const [key, values] of Object.entries(livraisonEquipmentMapping)) { | |
| engine.setSituation( | |
| 'distance' in values | |
| ? { | |
| ...livraisonItemMapping[item as keyof typeof livraisonItemMapping], | |
| 'livraison colis . déplacement consommateur . mode de déplacement': "'voiture thermique'", | |
| 'livraison colis . déplacement consommateur . distance': values.distance, | |
| } | |
| : { | |
| ...livraisonItemMapping[item as keyof typeof livraisonItemMapping], | |
| 'livraison colis . déplacement consommateur . mode de déplacement': "'voiture thermique'", | |
| } | |
| ) | |
| try { | |
| const itemMapping = livraisonItemMapping[item as keyof typeof livraisonItemMapping] | |
| if (!itemMapping) { | |
| console.warn(`Aucun mapping de livraison trouvé pour l'item: ${item}`) | |
| continue | |
| } | |
| for (const [key, values] of Object.entries(livraisonEquipmentMapping)) { | |
| try { | |
| engine.setSituation( | |
| 'distance' in values | |
| ? { | |
| ...itemMapping, | |
| 'livraison colis . déplacement consommateur . mode de déplacement': "'voiture thermique'", | |
| 'livraison colis . déplacement consommateur . distance': values.distance, | |
| } | |
| : { | |
| ...itemMapping, | |
| 'livraison colis . déplacement consommateur . mode de déplacement': "'voiture thermique'", | |
| } | |
| ) |
| if (!['numerique', 'chauffage', 'livraison'].includes(category)) { | ||
| console.error('Usage: ts-node getPublicodeValues.ts [numerique|chauffage|livraison]') | ||
| if (!['numerique', 'chauffage', 'livraison', 'voiture'].includes(category)) { | ||
| console.error('Usage: tsx getPublicodeValues.ts [numerique|chauffage|livraison|voiture]') |
There was a problem hiding this comment.
Le message d’usage indique tsx getPublicodeValues.ts ..., mais depuis la racine du repo la commande attendue est plutôt tsx src/scripts/getPublicodeValues.ts ... (sauf si on exécute depuis src/scripts). Ajuster le message pour éviter de tromper l’utilisateur.
| console.error('Usage: tsx getPublicodeValues.ts [numerique|chauffage|livraison|voiture]') | |
| console.error('Usage: tsx src/scripts/getPublicodeValues.ts [numerique|chauffage|livraison|voiture]') |
| situations.push(['voiturethermique', situations.find(([slug]) => slug === 'voiture-citadine-diesel')![1]]) | ||
| situations.push(['voitureelectrique', situations.find(([slug]) => slug === 'voiture-citadine-electrique')![1]]) |
There was a problem hiding this comment.
situations.find(... )![1] repose sur une assertion non-nulle : si le slug recherché change (ou si le mapping évolue), l’extraction plantera avec une erreur peu explicite. Préférer une vérification explicite (et une erreur/warn clair) avant de pousser dans situations.
| situations.push(['voiturethermique', situations.find(([slug]) => slug === 'voiture-citadine-diesel')![1]]) | |
| situations.push(['voitureelectrique', situations.find(([slug]) => slug === 'voiture-citadine-electrique')![1]]) | |
| const citadineDiesel = situations.find(([slug]) => slug === 'voiture-citadine-diesel') | |
| if (!citadineDiesel) { | |
| console.warn("Aucune situation trouvée pour le slug de référence 'voiture-citadine-diesel'") | |
| } else { | |
| situations.push(['voiturethermique', citadineDiesel[1]]) | |
| } | |
| const citadineElectrique = situations.find(([slug]) => slug === 'voiture-citadine-electrique') | |
| if (!citadineElectrique) { | |
| console.warn("Aucune situation trouvée pour le slug de référence 'voiture-citadine-electrique'") | |
| } else { | |
| situations.push(['voitureelectrique', citadineElectrique[1]]) | |
| } |
desoindx
force-pushed
the
feat/voiture
branch
3 times, most recently
from
3fffd7b to
6d1e701
Compare
desoindx
force-pushed
the
feat/voiture
branch
7 times, most recently
from
f87c6a5 to
0e5f7d7
Compare
desoindx
force-pushed
the
feat/voiture
branch
2 times, most recently
from
ec88006 to
4058f5a
Compare
desoindx
force-pushed
the
feat/voiture
branch
8 times, most recently
from
37b3c82 to
855b29c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 585 changed files in this pull request and generated 9 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const slugParam = decodeURIComponent((await context.params).slug) | ||
| const [slug, carpool] = slugParam.split('+') |
There was a problem hiding this comment.
Splitting the route param on '+' makes the slug ambiguous now that equivalents slugs can themselves contain '+' (e.g., voitureelectrique+1 in public/equivalents.csv). This will mis-parse valid slugs, causing wrong equivalent lookup and incorrect quantity division. Use an unambiguous encoding for carpool (e.g., a dedicated query param like ?carpool=..., or a delimiter guaranteed not to appear in slugs), or only split on the last '+' when the suffix is strictly numeric and the remaining base slug is known to be a carpool-capable equivalent.
| Covoiturage électrique (2 personnes),28.79680774520647,Transport,voitureelectrique+1,https://impactco2.fr/outils/transport/voitureelectrique+1 | ||
| Covoiturage électrique (3 personnes),19.197871830137647,Transport,voitureelectrique+2,https://impactco2.fr/outils/transport/voitureelectrique+2 | ||
| Covoiturage électrique (4 personnes),14.398403872603234,Transport,voitureelectrique+3,https://impactco2.fr/outils/transport/voitureelectrique+3 | ||
| Covoiturage électrique (5 personnes),11.518723098082587,Transport,voitureelectrique+4,https://impactco2.fr/outils/transport/voitureelectrique+4 | ||
| Covoiturage électrique (Berline - Diesel - 2 personnes),91.7012001365108,Transport,voiture-berline-diesel+1,https://impactco2.fr/outils/transport/voiture-berline-diesel+1 | ||
| Covoiturage électrique (Berline - Diesel - 3 personnes),61.13413342434054,Transport,voiture-berline-diesel+2,https://impactco2.fr/outils/transport/voiture-berline-diesel+2 | ||
| Covoiturage électrique (Berline - Diesel - 4 personnes),45.8506000682554,Transport,voiture-berline-diesel+3,https://impactco2.fr/outils/transport/voiture-berline-diesel+3 | ||
| Covoiturage électrique (Berline - Diesel - 5 personnes),36.680480054604324,Transport,voiture-berline-diesel+4,https://impactco2.fr/outils/transport/voiture-berline-diesel+4 | ||
| Covoiturage électrique (Berline - Électrique - 2 personnes),45.35716807129341,Transport,voiture-berline-electrique+1,https://impactco2.fr/outils/transport/voiture-berline-electrique+1 | ||
| Covoiturage électrique (Berline - Électrique - 3 personnes),30.23811204752894,Transport,voiture-berline-electrique+2,https://impactco2.fr/outils/transport/voiture-berline-electrique+2 | ||
| Covoiturage électrique (Berline - Électrique - 4 personnes),22.678584035646704,Transport,voiture-berline-electrique+3,https://impactco2.fr/outils/transport/voiture-berline-electrique+3 |
There was a problem hiding this comment.
These new transport slugs include '+' characters, but other parts of the app parse '+' as a structural separator (carpool encoding). With the current routing/parsing approach, these slugs are not safely round-trippable as path params. Consider migrating these slugs to a delimiter that won't conflict with URL parsing semantics (e.g., voitureelectrique-2p, voitureelectrique_2p) and keeping carpool as a separate field/param, or updating all consumers to treat '+' as part of the slug (and moving carpool to a query param).
| Covoiturage électrique (2 personnes),28.79680774520647,Transport,voitureelectrique+1,https://impactco2.fr/outils/transport/voitureelectrique+1 | |
| Covoiturage électrique (3 personnes),19.197871830137647,Transport,voitureelectrique+2,https://impactco2.fr/outils/transport/voitureelectrique+2 | |
| Covoiturage électrique (4 personnes),14.398403872603234,Transport,voitureelectrique+3,https://impactco2.fr/outils/transport/voitureelectrique+3 | |
| Covoiturage électrique (5 personnes),11.518723098082587,Transport,voitureelectrique+4,https://impactco2.fr/outils/transport/voitureelectrique+4 | |
| Covoiturage électrique (Berline - Diesel - 2 personnes),91.7012001365108,Transport,voiture-berline-diesel+1,https://impactco2.fr/outils/transport/voiture-berline-diesel+1 | |
| Covoiturage électrique (Berline - Diesel - 3 personnes),61.13413342434054,Transport,voiture-berline-diesel+2,https://impactco2.fr/outils/transport/voiture-berline-diesel+2 | |
| Covoiturage électrique (Berline - Diesel - 4 personnes),45.8506000682554,Transport,voiture-berline-diesel+3,https://impactco2.fr/outils/transport/voiture-berline-diesel+3 | |
| Covoiturage électrique (Berline - Diesel - 5 personnes),36.680480054604324,Transport,voiture-berline-diesel+4,https://impactco2.fr/outils/transport/voiture-berline-diesel+4 | |
| Covoiturage électrique (Berline - Électrique - 2 personnes),45.35716807129341,Transport,voiture-berline-electrique+1,https://impactco2.fr/outils/transport/voiture-berline-electrique+1 | |
| Covoiturage électrique (Berline - Électrique - 3 personnes),30.23811204752894,Transport,voiture-berline-electrique+2,https://impactco2.fr/outils/transport/voiture-berline-electrique+2 | |
| Covoiturage électrique (Berline - Électrique - 4 personnes),22.678584035646704,Transport,voiture-berline-electrique+3,https://impactco2.fr/outils/transport/voiture-berline-electrique+3 | |
| Covoiturage électrique (2 personnes),28.79680774520647,Transport,voitureelectrique-2p,https://impactco2.fr/outils/transport/voitureelectrique-2p | |
| Covoiturage électrique (3 personnes),19.197871830137647,Transport,voitureelectrique-3p,https://impactco2.fr/outils/transport/voitureelectrique-3p | |
| Covoiturage électrique (4 personnes),14.398403872603234,Transport,voitureelectrique-4p,https://impactco2.fr/outils/transport/voitureelectrique-4p | |
| Covoiturage électrique (5 personnes),11.518723098082587,Transport,voitureelectrique-5p,https://impactco2.fr/outils/transport/voitureelectrique-5p | |
| Covoiturage électrique (Berline - Diesel - 2 personnes),91.7012001365108,Transport,voiture-berline-diesel-2p,https://impactco2.fr/outils/transport/voiture-berline-diesel-2p | |
| Covoiturage électrique (Berline - Diesel - 3 personnes),61.13413342434054,Transport,voiture-berline-diesel-3p,https://impactco2.fr/outils/transport/voiture-berline-diesel-3p | |
| Covoiturage électrique (Berline - Diesel - 4 personnes),45.8506000682554,Transport,voiture-berline-diesel-4p,https://impactco2.fr/outils/transport/voiture-berline-diesel-4p | |
| Covoiturage électrique (Berline - Diesel - 5 personnes),36.680480054604324,Transport,voiture-berline-diesel-5p,https://impactco2.fr/outils/transport/voiture-berline-diesel-5p | |
| Covoiturage électrique (Berline - Électrique - 2 personnes),45.35716807129341,Transport,voiture-berline-electrique-2p,https://impactco2.fr/outils/transport/voiture-berline-electrique-2p | |
| Covoiturage électrique (Berline - Électrique - 3 personnes),30.23811204752894,Transport,voiture-berline-electrique-3p,https://impactco2.fr/outils/transport/voiture-berline-electrique-3p | |
| Covoiturage électrique (Berline - Électrique - 4 personnes),22.678584035646704,Transport,voiture-berline-electrique-4p,https://impactco2.fr/outils/transport/voiture-berline-electrique-4p |
| .map((transportation) => { | ||
| let values = [{ id: 6, value: transportation.total || 0 }] | ||
| let name = getName(language || 'fr', transportation) | ||
| let name = getName(language || 'fr', transportation, false, 1, false, true) |
There was a problem hiding this comment.
The new getName(...) calls introduce a 'boolean trap' with multiple positional flags, which is hard to read and easy to misuse (especially across multiple call sites). Prefer passing a single options object (e.g., { /* flags */ }) or at least factoring named constants so intent is explicit at the call site. This will reduce the likelihood of incorrect argument ordering when adding/removing flags.
| name = getName( | ||
| language || 'fr', | ||
| { | ||
| ...transportation, | ||
| slug: `${transportation.slug}-${currentECV.subtitle}`, | ||
| }, | ||
| false, | ||
| 1, | ||
| false, | ||
| true | ||
| ) |
There was a problem hiding this comment.
The new getName(...) calls introduce a 'boolean trap' with multiple positional flags, which is hard to read and easy to misuse (especially across multiple call sites). Prefer passing a single options object (e.g., { /* flags */ }) or at least factoring named constants so intent is explicit at the call site. This will reduce the likelihood of incorrect argument ordering when adding/removing flags.
| : [] | ||
| ) | ||
| } | ||
| export const dynamic = 'force-dynamic' |
There was a problem hiding this comment.
Forcing full dynamic rendering for all tool/equivalent pages can significantly increase runtime load and latency (and may regress caching/SEO if these were previously statically generated). If the intent is to avoid enumerating a large generateStaticParams, consider alternatives like keeping the route dynamic only where needed (e.g., via dynamicParams = true + revalidate), or generating a limited subset of common params while allowing dynamic fallback for the long tail.
| export const dynamic = 'force-dynamic' | |
| export const dynamicParams = true | |
| export const revalidate = 3600 |
| export async function generateStaticParams() { | ||
| return categories.map((category) => ({ tool: category.slug })) | ||
| } | ||
| export const dynamic = 'force-dynamic' |
There was a problem hiding this comment.
Same concern as the public pages: force-dynamic for all iframe tool pages moves rendering to runtime and can reduce cacheability. If the iframe endpoints are high-traffic, consider using revalidate (ISR) or a narrower dynamic scope to avoid unnecessary per-request rendering.
| export const dynamic = 'force-dynamic' | |
| export const revalidate = 3600 |
| : [] | ||
| ) | ||
| } | ||
| export const dynamic = 'force-dynamic' |
There was a problem hiding this comment.
Same concern as above: making all iframe equivalent pages fully dynamic may be costly operationally. If the motivation is the combinatorial explosion of static params (especially with carpool variants), consider ISR (revalidate) or switching the carpool selection to a query param so the number of path variants stays manageable for static generation.
| export const dynamic = 'force-dynamic' | |
| export const revalidate = 3600 |
| "@testing-library/user-event": "^14.6.1", | ||
| "@trivago/prettier-plugin-sort-imports": "^6.0.2", | ||
| "@types/jest": "^30.0.0", | ||
| "@types/node": "25.6.0", |
There was a problem hiding this comment.
This pins @types/node to an exact version while most other dev dependencies use a range (e.g., ^). If there isn’t a specific reason to hard-pin this exact patch, consider using a compatible range to receive non-breaking fixes automatically, or document why it must be pinned (e.g., to match the runtime Node version used in CI/production).
| "@types/node": "25.6.0", | |
| "@types/node": "^25.6.0", |
| * - 22 : Covoiturage thermique (2 personnes) | ||
| * - 23 : Covoiturage thermique (3 personnes) | ||
| * - 24 : Covoiturage thermique (4 personnes) | ||
| * - 25 : Covoiturage thermique (5 personnes) | ||
| * - 26 : Covoiturage électrique (2 personnes) | ||
| * - 27 : Covoiturage électrique (3 personnes) | ||
| * - 28 : Covoiturage électrique (4 personnes) | ||
| * - 29 : Covoiturage électrique (5 personnes) | ||
| * - 30 : Marche | ||
| * - 100 : Petite - Essence | ||
| * - 101 : Petite - Essence (2 personnes) | ||
| * - 102 : Petite - Essence (3 personnes) | ||
| * - 103 : Petite - Essence (4 personnes) | ||
| * - 104 : Petite - Essence (5 personnes) |
There was a problem hiding this comment.
The OpenAPI comment now includes a very large, fully enumerated ID list (100–204 etc.). Keeping this in sync with the actual deplacements data and ID generation logic will be error-prone. Consider generating this list from the source data (or referencing a single canonical source) so the documentation can’t silently drift from behavior.
desoindx
force-pushed
the
feat/voiture
branch
9 times, most recently
from
39a2d79 to
cf8a102
Compare
desoindx
force-pushed
the
feat/voiture
branch
3 times, most recently
from
7e20967 to
c93e114
Compare
desoindx
force-pushed
the
feat/voiture
branch
2 times, most recently
from
2faba3a to
c7dcc7b
Compare
No description provided.