feat(db): add property registry table by kaligrafy · Pull Request #1779 · chairemobilite/transition

kaligrafy · 2026-02-12T14:45:59Z

Add two migrations to chaire-lib-backend:

Add 'propertyRegistry' to tr_data_source_types enum
Create tr_property_registry table with schema based on the Property Registry wiki documentation

The table supports international property/cadastral data with:

Integer PK with optional UUID
MultiPolygon geography for building/parcel footprints
Point geography for centroids and main entrance
Building characteristics (flats, floors, area, year built)
Assessment values and land use codes
Location metadata (country, region, municipality, borough)
FK to tr_data_sources for import tracking

Updates to capnp schema for data sources have also been updated. However, we should not need any capnp schema for data sources and zones, so they will be removed in the future. See #1778

See: https://github.com/chairemobilite/transition/wiki/Documentation-%E2%80%90-Property-Registry
Also see: #1691 (closes step 1)

Summary by CodeRabbit

New Features
- Added Property Registry data source: DB table, full CRUD with PostGIS geo support, GeoJSON point/feature outputs, client model/types, and geometry conversion helper.
Tests
- Extensive tests covering Property Registry DB flows, geo handling, batch operations, and geometry utility.
Chores
- Dependency adjustments and serialization/schema mapping updates to support the new data source.

cubic-dev-ai

1 issue found across 6 files

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/transition-backend/src/models/capnpDataModel/capnpFiles/dataSource.capnp">

<violation number="1" location="packages/transition-backend/src/models/capnpDataModel/capnpFiles/dataSource.capnp:31">
P1: Changing the numeric tag for the existing `unknown` enum value breaks backwards compatibility with previously serialized data. Keep `unknown` at @14 and assign the new value a new tag.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

packages/transition-backend/src/models/capnpDataModel/capnpFiles/dataSource.capnp

coderabbitai · 2026-02-12T14:49:07Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

Walkthrough

Adds a new "propertyRegistry" data source across the codebase: a DB enum migration and a migration that creates tr_property_registry with PostGIS geography columns and indexes; a geometry helper and geometry unit tests; a property registry TypeScript model and tests; a new DB queries module implementing CRUD, geo-queries, and GeoJSON output; updates to default query generics (introducing Idable); Cap'n Proto and Rust enum updates for the new data source; multiple tests exercising DB and model logic; and minor package dependency edits.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested reviewers

greenscientist
tahini

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'feat(db): add property registry table' clearly and specifically summarizes the main change—introducing a new property registry database table with schema.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

tahini

Bon, j'avais des commentaires pending...

Aussi, tel que discuté hors ligne, ce serait bien d'avoir les requêtes qui seront associées à cette table, afin de pouvoir mieux juger de l'utilisation qui en sera faite et, donc, de la validité des index/champs proposés.

...ages/chaire-lib-backend/src/models/db/migrations/20260212090800_createPropertyRegistryTbl.ts

kaligrafy · 2026-02-23T23:39:54Z

J'ai ajouté les requêtes et les tests associés.

greenscientist · 2026-02-24T00:11:30Z

@kaligrafy Can you compare with the work that Paul did? See how close you are in the table structure? (If we can reconcile both that could be interesting in the future)

kaligrafy · 2026-02-24T00:26:27Z

@kaligrafy Can you compare with the work that Paul did? See how close you are in the table structure? (If we can reconcile both that could be interesting in the future)

The work of Paul is specified for Quebec only. Mine is for any property registry databse and should be generic to most countries.

All the columns he used in his projects are indeed included in the table schema implemented here.

tahini

qq petits commentaires et réponses, je ferai un review plus complet demain.

...ages/chaire-lib-backend/src/models/db/migrations/20260212090800_createPropertyRegistryTbl.ts

packages/chaire-lib-backend/src/models/db/default.db.queries.ts

The util function does convert geojson input into a postgis raw query string. If the geojson input is undefined, it will return undefined. This conversion operation is repeated in every db insert query where at least one column is of type geom or geog. Now, we will be able to use this util instead.

Data source type did not include property registry The capnp enum must also be updated.

Accept Idable instead of GenericObject so we can input custom objects with at least an id (number of string) Right now, the default queries only accept GenericObject, but new classes will not have some of the attributes in GenericObject. To make any object with at least an id to be able to use these query templates, we need to change the input type to a simple Idable.

Includes: - db queries - associated types - tests completes step 1 of chairemobilite#1691

tahini · 2026-02-26T13:12:42Z

services/json2capnp/transition_capnp_data/src/capnp/dataSource_capnp.rs

      ::capnp::word(5, 0, 7, 0, 0, 0, 0, 0),
      ::capnp::word(0, 0, 0, 0, 0, 0, 0, 0),
-      ::capnp::word(92, 0, 0, 0, 80, 4, 0, 0),
+      ::capnp::word(92, 0, 0, 0, 113, 4, 0, 0),


Les modifs dans ce fichier, tu ne les as pas faites à la main j'imagine? Pourrais-tu ajouter dans le commit message ce que tu as roulé pour re-générer ça? Ou pointer vers les instructions, mais à tout le moins dire que tu as roulé qqc.

C'est documenté ici: services/json2capnp/transition_capnp_data/src/capnp/_generating_classes.txt

tahini · 2026-02-26T13:24:28Z

packages/chaire-lib-backend/src/models/db/default.db.queries.ts

        transaction?: Knex.Transaction;
    } = {}
-): Promise<string[][]> => {
+): Promise<(string | number)[][]> => {


Je vais laisser passer pour tout de suite, car j'ai une branche assez avancée qui réécrit les fonctions de BD pour des IDs numériques sous forme d'objet pour simplifier le typage. Mais le problème de l'approche ici est que l'ID ne peut pas être string ou numérique pour un même type d'objet, ce que laisse entendre le type Idable. Un type d'objet donné a soit un type numérique ou string. Il aurait fallu rendre ça générique (j'ai essayé, on a déjà un générique T, ça rend le code super complexe, c'est pourquoi j'y suis allée de l'approche d'un autre set de requête pour les ID, en faisant du ménage en même temps, bref, ne perds pas de temps avec ce commentaire, c'est juste informatif sur les limites de ID: string | number. Aussi la différence des number et string, c'est que nos uuids sont généralement générés par le code, donc ils sont là initialement et insérer l'objet ne changera pas l'ID, alors que les IDs numériques doivent venir de la BD et l'objet doit être mis à jour après insertion, il faut donc absolument avoir un return du id.)

tahini · 2026-02-26T13:25:51Z

packages/chaire-lib-backend/src/models/db/__tests__/propertyRegistry.db.test.ts

+ * License text available at https://opensource.org/licenses/MIT
+ */
+import { v4 as uuidV4 } from 'uuid';
+import { Iso3166Alpha2Code } from 'iso-3166-ts';


Le commit ajoute une dépendance à iso-3166-ts il faudrait le mentionner et expliquer pourquoi dans le commit message.

tahini · 2026-02-26T13:32:58Z

packages/chaire-lib-backend/src/models/db/__tests__/propertyRegistry.db.test.ts

+            expect(error).toBeDefined();
+
+            // Verify no records were created (testProp1.id should be undefined since transaction rolled back)
+            expect(testProp1.id).toBeUndefined();


Je ne crois pas que createMultiple va mettre à jour le id de testProp1, donc ceci sera toujours undefined et passera. Peut-être plutôt compter le nomber de rows dans la table avant et après et s'assurer que le nombre est identique.

tahini · 2026-02-26T13:38:59Z

packages/chaire-lib-backend/src/models/db/__tests__/propertyRegistry.db.test.ts

+                numFlats: 21
+            }));
+
+            const response = await dbQueries.updateMultiple(updates);


Y a-t-il un use case pour avoir la fonction updateMultiple de la façon actuelle (ie où chaque objet à mettre à jour a des propriétés potentiellement différentes à mettre à jour et donc où chaque objet est individuellement sauvegardé, via des requêtes séparées). Je demande car ce n'est pas parce que la fonction est disponible dans default.db qu'il faut la rendre disponible dans le fichier de propertyRegistry.db. Une fonction comme updateMultiple pourrait plutôt être utilisée pour des batch updates, par exemple, changer le landUse à blablabla pour plusieurs éléments en même temps, ce que l'actuelle updateMultiple ne supporte pas bien.

tahini · 2026-02-26T14:09:16Z

packages/chaire-lib-backend/src/models/db/propertyRegistry.db.queries.ts

+ * @param id - The ID of the property to fetch.
+ * @returns The property registry point.
+ */
+const readPoint = async (id: string | number) => {


Serait-il possible de mettre la valeur de retour ici. C'est un geojson Feature | undefined?

tahini · 2026-02-26T14:17:45Z

packages/chaire-lib-common/src/services/propertyRegistry/__tests__/PropertyRegistry.test.ts

+            'should return $attributeName when present for $getterName',
+            ({ attributeName, getterName, sampleValue }) => {
+                const attributes = { id: 1, [attributeName]: sampleValue } as PropertyRegistryAttributes;
+                const property = new PropertyRegistry(attributes);


I know this is a property registry, but for later uses of this class, naming a variable property can be confusing, with so many property concepts in programmings, especially as synonym of attributes which is being compared here...

tahini · 2026-02-26T14:21:13Z

packages/chaire-lib-common/src/services/propertyRegistry/PropertyRegistry.ts

+
+/**
+ * Property registry attributes type
+ * Based on the db table schema defined in the migrations.


Please remove this line of comments. 1- this is in common and should not have knowledge of the backend, 2- The object is for the functionality use cases involving the property registry, there can be a converter from/to db, they do not have to remain perfectly mirror or each other.

tahini · 2026-02-26T14:22:45Z

packages/chaire-lib-common/src/services/propertyRegistry/PropertyRegistry.ts

+    totalFloorAreaM2?: number;
+    levels?: number;
+    yearBuilt?: number;
+    buildingType?: string;


Est-ce que ce champ pourra éventuellement être un enum ou c'est trop divergent d'un lieu à l'autre? Peut-être documenter un peu les types possibles.

tahini · 2026-02-26T14:23:31Z

packages/chaire-lib-common/src/services/propertyRegistry/PropertyRegistry.ts

+    assessedValueLand?: number;
+    assessedValueBuilding?: number;
+    parcelAreaM2?: number;
+    landUseCode?: string;


Même chose ici, j'imagine que le landUseCode est très changeant d'un pays à l'autre, donc qu'on ne peut pas uniformiser, mais peut-être documenter le genre d'information attendu ici.

kaligrafy requested review from greenscientist and tahini February 12, 2026 14:45

cubic-dev-ai bot reviewed Feb 12, 2026

View reviewed changes

packages/transition-backend/src/models/capnpDataModel/capnpFiles/dataSource.capnp Show resolved Hide resolved