Skip to content

refactor(audits): use W prefix for warning-level audit error codes #1733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kaligrafy merged 1 commit into from
Jul 31, 2026
Merged

refactor(audits): use W prefix for warning-level audit error codes #1733

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions locales/en/audits.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"HH_M_Size": "Household size is missing",
"HH_M_CarNumber": "Car number is missing",
"HH_I_CarNumber": "Car number is out of range (should be an integer between 0 and 13)",
"HH_L_CarNumberPerPotentialDrivingLicenseTooHigh": "Car number per potential driving license holder > 3",
"HH_W_CarNumberPerPotentialDrivingLicenseTooHigh": "Car number per potential driving license holder > 3",
"HH_L_SizeMembersCountMismatch": "Size and members count mismatch",
"HH_L_InvalidPersonSequences": "At least one person sequence is invalid or duplicated",
"HH_W_PersonSequenceGaps": "Person sequences are not contiguous",
Expand All @@ -23,7 +23,7 @@
"HH_M_AtLeastOnePersonWithDisability": "At least one person with disability is missing",

"HM_I_preGeographyAndHomeGeographyTooFarApartError": "Pre-filled and declared home geographies are far apart",
"HM_I_preGeographyAndHomeGeographyTooFarApartWarning": "Pre-filled and declared home geographies are a bit far apart",
"HM_W_preGeographyAndHomeGeographyTooFarApart": "Pre-filled and declared home geographies are a bit far apart",
"HM_M_Geography": "Home geography is missing",
"HM_I_Geography": "Home geography is invalid",
"HM_I_geographyNotInSurveyTerritory": "Home geography is outside the survey territory",
Expand Down
4 changes: 2 additions & 2 deletions locales/fr/audits.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"HH_M_Size": "La taille du ménage est manquante",
"HH_M_CarNumber": "Le nombre de véhicules automobiles est manquant",
"HH_I_CarNumber": "Le nombre de véhicules automobiles est en dehors de la plage autorisée (devrait être un entier entre 0 et 13)",
"HH_L_CarNumberPerPotentialDrivingLicenseTooHigh": "Le nombre de véhicules automobiles par titulaire potentiel d'un permis de conduire > 3",
"HH_W_CarNumberPerPotentialDrivingLicenseTooHigh": "Le nombre de véhicules automobiles par titulaire potentiel d'un permis de conduire > 3",
"HH_L_SizeMembersCountMismatch": "La taille du ménage et le nombre de personnes ne correspondent pas",
"HH_L_InvalidPersonSequences": "Le numéro de séquence d'au moins une personne est invalide ou en double",
"HH_W_PersonSequenceGaps": "Les numéros de séquence des personnes ne sont pas consécutifs",
Expand All @@ -23,7 +23,7 @@
"HH_M_AtLeastOnePersonWithDisability": "La présence d'au moins une personne avec un handicap est manquante",

"HM_I_preGeographyAndHomeGeographyTooFarApartError": "Les géographies pré-remplie et déclarée du domicile sont éloignées",
"HM_I_preGeographyAndHomeGeographyTooFarApartWarning": "Les géographies pré-remplie et déclarée du domicile sont un peu éloignées",
"HM_W_preGeographyAndHomeGeographyTooFarApart": "Les géographies pré-remplie et déclarée du domicile sont un peu éloignées",
"HM_M_Geography": "La géographie du domicile est manquante",
"HM_I_Geography": "La géographie du domicile est invalide",
"HM_I_geographyNotInSurveyTerritory": "La géographie du domicile est en dehors du territoire d'enquête",
Expand Down
19 changes: 13 additions & 6 deletions packages/evolution-backend/src/services/audits/auditChecks/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,22 +106,29 @@ All contexts are defined in [`./AuditCheckContexts.ts`](./AuditCheckContexts.ts)

### 3.2 What kind of problem is it?

> **TODO:** The prefixes below mix audit *type* (missing, invalid, logical) with *severity* (info, warning). This is historical (`M`/`I`/`L` from 2023; `W` and `F` were added later). A cleaner split is tracked in [#1771](https://github.com/chairemobilite/evolution/issues/1771).

Follow the existing prefix convention (see the checks folder README):

- `_M_` — **Missing** required data (`HH_M_Size`).
- `_I_` — **Invalid** data (`HH_I_Size` — out of range, malformed, wrong type).
- `_L_` — **Logical** inconsistency between fields or objects (`HH_L_SizeMembersCountMismatch`).
- `_F_` — **Info** — informational flag, not an error or warning (`HH_F_AtLeastOneTransitSegmentInHousehold`).
- `_W_` — **Warning** — suspicious data that may be fine but needs reviewer attention (`HH_W_CarNumberPerPotentialDrivingLicenseTooHigh`, `HM_W_preGeographyAndHomeGeographyTooFarApart`).

@greenscientist greenscientist Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should we have a type specific for NEED_REVIEW? This feel different than a warning, where a warning is just a notice that can be investigate at some point but will not affect results??

@kaligrafy kaligrafy Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True, I will propose something more stable and better. Back in few minutes (hours?)

@kaligrafy kaligrafy Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See #1771


Pick `_M_` only if the field should always be present. If "required" depends on survey configuration, use `fieldIsRequired(...)` (see §5.1) and still prefix with `_M_`.

The type prefix should match the audit `level`: use `_W_` with `level: 'warning'`, `_F_` with `level: 'info'`, and `_M_` / `_I_` / `_L_` with `level: 'error'`.

### 3.3 Which severity?

> **TODO:** Same conflation as §3.2 — this table maps severity to prefixes, but `error` still spans three type letters (`M`, `I`, `L`) instead of one. See [#1771](https://github.com/chairemobilite/evolution/issues/1771).

| Level | Meaning | Reviewer behavior expected |
| --------- | -------------------------------------------------------------------------- | -------------------------------------- |
| `error` | The data is wrong or unusable and must be corrected or explicitly ignored. | Must look at it. |
| `warning` | Something suspicious that often turns out fine. | Should look at it when they have time. |
| `info` | Informational only, typically for stats or routing. | Can ignore. |
| Level | Prefix | Meaning | Reviewer behavior expected |
| --------- | ------ | -------------------------------------------------------------------------- | -------------------------------------- |

@greenscientist greenscientist Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe also put a similar comment here mentionning the same.

@kaligrafy kaligrafy Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

added

| `error` | `M`, `I`, or `L` | The data is wrong or unusable and must be corrected or explicitly ignored. | Must look at it. |
| `warning` | `W` | Something suspicious that often turns out fine. | Should look at it when they have time. |
| `info` | `F` | Informational only, typically for stats or routing. | Can ignore. |


When in doubt, start at `warning`. Escalating later is cheap; downgrading creates review backlog.
Expand Down Expand Up @@ -293,7 +300,7 @@ Grep before adding to avoid duplicates. Checks live in `<Object>AuditChecks.ts`
and `<Object>ExtendedAuditChecks.ts` under [`./checks/`](./checks/), one file per survey object type (see the table in §3.1). To list all current checks:

```bash
grep -rEn "^[[:space:]]+[IHMPJVTS]{1,2}_[MIL]_[A-Za-z]+:" \
grep -rEn "^[[:space:]]+[IHMPJVTS]{1,2}_[MILFW]_[A-Za-z]+:" \
packages/evolution-backend/src/services/audits/auditChecks/checks
```

Expand Down
6 changes: 2 additions & 4 deletions packages/evolution-backend/src/services/audits/auditChecks/checks/HomeAuditChecks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,7 @@ export const homeAuditChecks: { [errorCode: string]: HomeAuditCheckFunction } =
* @param context - HomeAuditCheckContext
* @returns AuditForObject
*/
HM_I_preGeographyAndHomeGeographyTooFarApartWarning: (
context: HomeAuditCheckContext
): AuditForObject | undefined => {
HM_W_preGeographyAndHomeGeographyTooFarApart: (context: HomeAuditCheckContext): AuditForObject | undefined => {
const home = context.home;
const preGeography = home.preGeography;
const geography = home.geography;
Expand All @@ -124,7 +122,7 @@ export const homeAuditChecks: { [errorCode: string]: HomeAuditCheckFunction } =
return {
objectType: 'home',
objectUuid: home._uuid!,
errorCode: 'HM_I_preGeographyAndHomeGeographyTooFarApartWarning',
errorCode: 'HM_W_preGeographyAndHomeGeographyTooFarApart',
version: 1,
level: 'warning',
message: 'Pre-filled and declared home geography are a bit far apart',
Expand Down
4 changes: 2 additions & 2 deletions packages/evolution-backend/src/services/audits/auditChecks/checks/HouseholdAuditChecks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ export const householdAuditChecks: { [errorCode: string]: HouseholdAuditCheckFun
* @param context - HouseholdAuditCheckContext
* @returns AuditForObject
*/
HH_L_CarNumberPerPotentialDrivingLicenseTooHigh: (
HH_W_CarNumberPerPotentialDrivingLicenseTooHigh: (
context: HouseholdAuditCheckContext
): AuditForObject | undefined => {
const { household } = context;
Expand All @@ -280,7 +280,7 @@ export const householdAuditChecks: { [errorCode: string]: HouseholdAuditCheckFun
return {
objectType: 'household',
objectUuid: household._uuid!,
errorCode: 'HH_L_CarNumberPerPotentialDrivingLicenseTooHigh',
errorCode: 'HH_W_CarNumberPerPotentialDrivingLicenseTooHigh',
version: 1,
level: 'warning',
message: 'Car number per potential driving license holder > 3',
Expand Down
3 changes: 2 additions & 1 deletion packages/evolution-backend/src/services/audits/auditChecks/checks/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ All audit check functions follow the pattern: `[OBJECTPREFIX]_[AUDITCHECKTYPE]_[
- `M` - **Missing** data (e.g., `HH_M_Size` - household size is missing)
- `I` - **Invalid** data (e.g., `HH_I_Size` - household size is out of valid range)
- `L` - **Logical** error (e.g., `J_L_EndBeforeStart` - journey end time before start time)
- `W` - **Warning**: suspicious data that is still usable, worth a reviewer's attention (e.g., `HH_W_PersonSequenceGaps`). Use with `level: 'warning'`.
- `F` - **Info** — informational check, not an error or warning (e.g., `HH_F_AtLeastOneTransitSegmentInHousehold`). Use with `level: 'info'`.
- `W` - **Warning** — suspicious data that may indicate a possible error; needs review. Use with `level: 'warning'`.

### Examples
```typescript
Expand Down
10 changes: 5 additions & 5 deletions ...phyTooFarApartWarning.integration.test.ts → ...eGeographyTooFarApart.integration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { v4 as uuidV4 } from 'uuid';
import { homeAuditChecks } from '../../HomeAuditChecks';
import { createContextWithHome } from './testHelper';

describe('HM_I_preGeographyAndHomeGeographyTooFarApartWarning audit check - Integration tests with real turfDistance', () => {
describe('HM_W_preGeographyAndHomeGeographyTooFarApart audit check - Integration tests with real turfDistance', () => {
const validUuid = uuidV4();

it('should warn when coordinates are moderately far apart (~93.5 meters)', () => {
Expand All @@ -29,11 +29,11 @@ describe('HM_I_preGeographyAndHomeGeographyTooFarApartWarning audit check - Inte
validUuid
);

const result = homeAuditChecks.HM_I_preGeographyAndHomeGeographyTooFarApartWarning(context);
const result = homeAuditChecks.HM_W_preGeographyAndHomeGeographyTooFarApart(context);

expect(result).toBeDefined();
expect(result).toMatchObject({
errorCode: 'HM_I_preGeographyAndHomeGeographyTooFarApartWarning',
errorCode: 'HM_W_preGeographyAndHomeGeographyTooFarApart',
level: 'warning',
objectType: 'home',
objectUuid: validUuid
Expand All @@ -57,7 +57,7 @@ describe('HM_I_preGeographyAndHomeGeographyTooFarApartWarning audit check - Inte
validUuid
);

const result = homeAuditChecks.HM_I_preGeographyAndHomeGeographyTooFarApartWarning(context);
const result = homeAuditChecks.HM_W_preGeographyAndHomeGeographyTooFarApart(context);

expect(result).toBeUndefined();
});
Expand All @@ -79,7 +79,7 @@ describe('HM_I_preGeographyAndHomeGeographyTooFarApartWarning audit check - Inte
validUuid
);

const result = homeAuditChecks.HM_I_preGeographyAndHomeGeographyTooFarApartWarning(context);
const result = homeAuditChecks.HM_W_preGeographyAndHomeGeographyTooFarApart(context);

expect(result).toBeUndefined();
});
Expand Down
10 changes: 5 additions & 5 deletions ...ndHomeGeographyTooFarApartWarning.test.ts → ...graphyAndHomeGeographyTooFarApart.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jest.mock('@turf/distance', () => ({

import { distance as turfDistance } from '@turf/distance';

describe('HM_I_preGeographyAndHomeGeographyTooFarApartWarning audit check', () => {
describe('HM_W_preGeographyAndHomeGeographyTooFarApart audit check', () => {
const validUuid = uuidV4();

beforeEach(() => {
Expand Down Expand Up @@ -56,7 +56,7 @@ describe('HM_I_preGeographyAndHomeGeographyTooFarApartWarning audit check', () =
validUuid
);

const result = homeAuditChecks.HM_I_preGeographyAndHomeGeographyTooFarApartWarning(context);
const result = homeAuditChecks.HM_W_preGeographyAndHomeGeographyTooFarApart(context);

expect(result).toBeUndefined();
});
Expand Down Expand Up @@ -91,7 +91,7 @@ describe('HM_I_preGeographyAndHomeGeographyTooFarApartWarning audit check', () =
validUuid
);

const result = homeAuditChecks.HM_I_preGeographyAndHomeGeographyTooFarApartWarning(context);
const result = homeAuditChecks.HM_W_preGeographyAndHomeGeographyTooFarApart(context);

expect(result).toBeUndefined();
});
Expand Down Expand Up @@ -126,12 +126,12 @@ describe('HM_I_preGeographyAndHomeGeographyTooFarApartWarning audit check', () =
validUuid
);

const result = homeAuditChecks.HM_I_preGeographyAndHomeGeographyTooFarApartWarning(context);
const result = homeAuditChecks.HM_W_preGeographyAndHomeGeographyTooFarApart(context);

expect(result).toMatchObject({
objectType: 'home',
objectUuid: validUuid,
errorCode: 'HM_I_preGeographyAndHomeGeographyTooFarApartWarning',
errorCode: 'HM_W_preGeographyAndHomeGeographyTooFarApart',
level: 'warning',
message: 'Pre-filled and declared home geography are a bit far apart',
ignore: false
Expand Down
6 changes: 3 additions & 3 deletions ...PerPotentialDrivingLicenseTooHigh.test.ts → ...PerPotentialDrivingLicenseTooHigh.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { householdAuditChecks, MAX_CAR_NUMBER_PER_POTENTIAL_DRIVING_LICENSE } fr
import { createContextWithHouseholdAndHome } from './testHelper';
import { SurveyObjectsRegistry } from 'evolution-common/lib/services/baseObjects/SurveyObjectsRegistry';

describe('HH_L_CarNumberPerPotentialDrivingLicenseTooHigh audit check', () => {
describe('HH_W_CarNumberPerPotentialDrivingLicenseTooHigh audit check', () => {
const validHouseholdUuid = uuidV4();
const validHomeUuid = uuidV4();
const surveyObjectsRegistry = new SurveyObjectsRegistry();
Expand Down Expand Up @@ -74,13 +74,13 @@ describe('HH_L_CarNumberPerPotentialDrivingLicenseTooHigh audit check', () => {
validHomeUuid
);

const result = householdAuditChecks.HH_L_CarNumberPerPotentialDrivingLicenseTooHigh(context);
const result = householdAuditChecks.HH_W_CarNumberPerPotentialDrivingLicenseTooHigh(context);

if (shouldWarn) {
expect(result).toMatchObject({
objectType: 'household',
objectUuid: validHouseholdUuid,
errorCode: 'HH_L_CarNumberPerPotentialDrivingLicenseTooHigh',
errorCode: 'HH_W_CarNumberPerPotentialDrivingLicenseTooHigh',
version: 1,
level: 'warning',
message: 'Car number per potential driving license holder > 3',
Expand Down
Loading