Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughReplaces many per-table entities with a single TimescaleDB hypertable entity Changes
Sequence Diagram(s)sequenceDiagram
participant Backend as BackendController
participant DBSvc as DatabaseService (singleton)
participant Repo as TypeORM Repository<TelemetryPacket>
participant DB as Postgres (TimescaleDB)
Backend->>DBSvc: getInstance() / initialize()
Backend->>DBSvc: insertPacketData(message)
DBSvc->>DBSvc: flattenTelemetryData(message)
DBSvc->>Repo: save(Partial<TelemetryPacket>)
Repo->>DB: INSERT telemetry_packet row
DB-->>Repo: INSERT result
Repo-->>DBSvc: saved entity
DBSvc-->>Backend: resolve Promise
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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. Comment |
skand088
force-pushed
the
move-to-timescale
branch
from
1d7040f to
83a9f30
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
packages/db/src/entities/MBMS.entity.ts (1)
8-25: Column name mismatch between Hypertable configuration and property names.The Hypertable decorator references
"timestamp"and"rfid"(lowercase), but the property names areTimestampandRfid(PascalCase). Without explicitnameoptions in the column decorators, TypeORM will create database columns using the PascalCase property names, causing the TimescaleDB configuration to fail.Either add explicit column names to preserve lowercase database column names, or update the Hypertable/Index configuration to match the PascalCase property names.
Option 1: Add explicit column names to decorators
@Entity("mbms") @Hypertable({ timeColumnName: "timestamp", chunkTimeInterval: "1 month", compression: { compress: true, compress_segmentby: "rfid", compress_orderby: "timestamp DESC", }, }) @Index(["rfid", "timestamp"]) export class MBMS { @TimeColumn() - @PrimaryColumn({ type: "timestamptz" }) + @PrimaryColumn({ type: "timestamptz", name: "timestamp" }) Timestamp!: Date; - @PrimaryColumn({ type: "text" }) + @PrimaryColumn({ type: "text", name: "rfid" }) Rfid!: string;Option 2: Update Hypertable/Index config to use PascalCase
@Entity("mbms") @Hypertable({ - timeColumnName: "timestamp", + timeColumnName: "Timestamp", chunkTimeInterval: "1 month", compression: { compress: true, - compress_segmentby: "rfid", - compress_orderby: "timestamp DESC", + compress_segmentby: "Rfid", + compress_orderby: "Timestamp DESC", }, }) -@Index(["rfid", "timestamp"]) +@Index(["Rfid", "Timestamp"]) export class MBMS {packages/db/src/repositories/BaseRepository.ts (1)
18-18: Critical: primaryKey default value doesn't match BaseEntity interface.The default value
"timestamp"(lowercase) no longer matches theBaseEntityinterface which now definesTimestamp(PascalCase). This will cause runtime errors infindById,update, and other methods that rely onthis.primaryKeyas a key lookup.🐛 Proposed fix
constructor( protected repository: Repository<T>, - protected primaryKey: keyof T = "timestamp" as keyof T, + protected primaryKey: keyof T = "Timestamp" as keyof T, ) {}packages/server/src/controllers/BackendController/BackendController.ts (1)
62-72: Missingawaiton database insert calls could cause silent data loss.Both
dynamoDB.insertPacketDataanddatabaseService.insertPacketDataare async operations but are called withoutawait. If either fails, the error will be unhandled and data loss will go unnoticed.Proposed fix
public async handlePacketReceive(message: ITelemetryData) { // Insert the packet into the database - this.dynamoDB.insertPacketData(message); - - this.databaseService.insertPacketData(message); + try { + await Promise.all([ + this.dynamoDB.insertPacketData(message), + this.databaseService.insertPacketData(message), + ]); + } catch (error) { + logger.error("Failed to insert packet data:", error); + } // Broadcast the packet to the frontend this.socketIO.broadcastPacket(message);packages/db/src/entities/Telemetry.entity.ts (2)
18-26: Fix@Indexfield names to match entity property names.TypeORM's
@Indexdecorator requires entity property names (class field names), not database column names. The decorator references["rfid", "timestamp"]but the properties areRfidandTimestamp(PascalCase). This mismatch will cause TypeORM's metadata build to fail since the properties with the referenced names do not exist.Proposed fix
-@Index(["rfid", "timestamp"]) +@Index(["Rfid", "Timestamp"]) export class Telemetry {
9-17: Fix critical column naming mismatch between TypeORM properties and TimeScaleDB decorator references.With no naming strategy configured, TypeORM will create columns named
Timestamp,Rfid,GpsLatitude, etc. (matching the PascalCase property names), but@Hypertable({ timeColumnName: "timestamp" })and@Index(["rfid", "timestamp"])reference lowercase names that won't exist. This will cause runtime failures when the hypertable initialization and indexing are applied.Either:
- Add explicit
{ name: "..." }mappings to all@Column()and@PrimaryColumn()decorators to match the lowercase names expected by@Hypertableand@Index, or- Configure a
SnakeNamingStrategyin the DataSource to automatically convert PascalCase properties to snake_case columns.This affects all columns in this entity (lines 20–81).
packages/db/src/entities/BatteryFaults.entity.ts (1)
18-26: Fix@Indexfield names to match property names.TypeORM's
@Indexdecorator requires entity property names (class field names), not database column names. The properties areRfidandTimestamp(capitalized), so the decorator should reference them with matching capitalization:-@Index(["rfid", "timestamp"]) +@Index(["Rfid", "Timestamp"]) export class BatteryFaults {Without this fix, TypeORM will fail to create the index during schema synchronization.
packages/db/src/entities/B3.entity.ts (1)
18-26: Update@Indexand related decorator field names to match property names.
@Index(["rfid", "timestamp"])should use the actual property namesRfidandTimestamp. This is inconsistent with the pattern used in active entities (TelemetryPacket, Lap), which use@Index(["Rfid", "Timestamp"]). Additionally, the@Hypertabledecorator'stimeColumnName,compress_segmentby, andcompress_orderbyalso use lowercase names, creating further inconsistency.Note: B3 is not currently registered in the data source, but should be corrected before it's activated.
Proposed fix
@Hypertable({ timeColumnName: "timestamp", chunkTimeInterval: "1 month", compression: { compress: true, - compress_segmentby: "rfid", - compress_orderby: "timestamp DESC", + compress_segmentby: "Rfid", + compress_orderby: "Timestamp DESC", }, }) -@Index(["rfid", "timestamp"]) +@Index(["Rfid", "Timestamp"]) export class B3 {packages/db/src/entities/MotorDetails.entity.ts (2)
19-21: Fix @Index decorators to use entity property names instead of DB column names.The @Index decorators reference
rfid,timestamp, andmotor_id, but the entity defines properties asRfid,Timestamp, andMotorId. TypeORM requires @Index to use entity property names. With incorrect names, indices won't be created in the database.Proposed fix
-@Index(["rfid", "timestamp"]) -@Index(["motor_id", "timestamp"]) +@Index(["Rfid", "Timestamp"]) +@Index(["MotorId", "Timestamp"]) export class MotorDetails {
10-20: Fix @Index property references to use PascalCase property names—@Index(["rfid", "timestamp"]) will fail at runtime because properties are Rfid and Timestamp.All entities declare PascalCase properties (Timestamp, Rfid, MotorId, etc.) with no explicit
namemappings. Without explicit column name mappings, TypeORM defaults DB columns to the property name exactly. The @Index decorators and @hypertable configuration reference lowercase names ("rfid", "timestamp", "motor_id") that won't exist in the schema—indices will target non-existent columns or the entity metadata will fail to build.Either:
- Add explicit
name: "<snake_case>"to all @Column/@PrimaryColumn decorators, or- Change property names to lowercase (rfid, timestamp, motor_id, race_name).
This pattern affects all hypertable entities (MotorDetails, MPPT, Battery, Telemetry, TelemetryMetadata, etc.).
packages/db/src/entities/Contactor.entity.ts (1)
18-18: Update index definition to match property names:@Index(["Rfid", "Timestamp"])TypeORM's @Index decorator expects property names (not database column names). The current index references
["rfid", "timestamp"](lowercase), but the actual properties areRfidandTimestamp(PascalCase on lines 22 and 25). This mismatch will cause TypeORM to fail resolving the index.🔧 Proposed fix
-@Index(["rfid", "timestamp"]) +@Index(["Rfid", "Timestamp"])Note: This same issue exists in multiple other entities (Telemetry, TelemetryMetadata, ProximitySensors, MotorDetails, MBMS, MPPT, KeyMotor, BatteryFaults, Battery, B3). Consider applying the same fix across the codebase for consistency.
🤖 Fix all issues with AI agents
In @packages/db/src/data-source.ts:
- Line 18: The file exposes hardcoded DB credentials in the DataSource config
(the url property); remove the literal connection string and instead read the
connection string from an environment variable (e.g. process.env.DATABASE_URL)
when constructing the DataSource/connection, fail fast with a clear error if the
env var is missing, move DATABASE_URL into your .db.env (ensure .db.env is added
to .gitignore), and rotate the compromised Timescale credentials immediately
since they were committed.
In @packages/db/src/entities/Battery.entity.ts:
- Around line 22-127: The Battery entity uses PascalCase property names (e.g.,
Timestamp, Rfid, AlwaysOnSignalStatus) but its @Hypertable timeColumnName
("timestamp") and @Index(["rfid","timestamp"]) use snake_case; add explicit
column name mappings on every @Column in the Battery class (for example set
name: "timestamp", "rfid", "always_on_signal_status", etc.) so TypeORM queries
the correct DB columns; update the @Column decorators for all properties
(Timestamp, Rfid, AlwaysOnSignalStatus, AverageCellVoltage, AverageTemperature,
BmuAlive, ChargeRelayEnabled, ChargerSafetyEnabled, DischargeRelayEnabled,
FanSpeed, FanVoltage, HighCellVoltage, HighCellVoltageId, HighTemperature,
HighThermistorId, Input12v, InternalTemperature, IsChargingSignalStatus,
IsReadySignalStatus, LowCellVoltage, LowCellVoltageId, LowTemperature,
LowThermistorId, MalfunctionIndicatorActive, MaximumCellVoltage,
MaximumPackVoltage, MinimumCellVoltage, MinimumPackVoltage,
MultiPurposeInputSignalStatus, PackAmphours, PackCurrent, PackDepthOfDischarge,
PackStateOfCharge, PackVoltage, PopulatedCells, RequestedFanSpeed) to include
the correct snake_case name and appropriate DB type, or alternatively configure
a global namingStrategy in your DataSource to auto-convert PascalCase to
snake_case.
In @packages/db/src/entities/Contactor.entity.ts:
- Around line 29-180: The entity property names for BPS errors (ArrayBpsError,
ChargeBpsError, MotorBpsError) do not match the shared type IContactorType
(ArrayBPSError, ChargeBPSError, MotorBPSError), causing data loss when
DatabaseService.flattenTelemetryData() spreads packet.Contactor into
TelemetryPacket; rename the properties in Contactor.entity.ts to match the
shared type casing (ArrayBPSError, ChargeBPSError, MotorBPSError), update any
TypeORM @Column-backed property references and all usages (e.g., where
DatabaseService.flattenTelemetryData or TelemetryPacket expects the uppercase
"BPS" names), and run a repository-wide refactor/search-replace to ensure
consistency across IContactorType, Contactor entity, TelemetryPacket, and any
code that accesses these properties.
In @packages/db/src/entities/Lap.entity.ts:
- Around line 14-15: The Timestamp property in the Lap entity is decorated with
@TimeColumn() but lacks an explicit column type, which can lead to incorrect
type inference; update the Lap entity by adding an explicit column type (e.g., {
type: "timestamptz" }) to the Timestamp declaration so it matches other entities
(like TelemetryPacket and KeyMotor) that use @PrimaryColumn({ type:
"timestamptz" }) alongside their time column decorators, ensuring the Timestamp
field is persisted with the correct DB timezone-aware timestamp type.
In @packages/db/src/entities/ProximitySensors.entity.ts:
- Line 34: The entity has a typo in the property name "RroximitySensor3" which
should be "ProximitySensor3"; rename the class property in
ProximitySensors.entity (replace RroximitySensor3 with ProximitySensor3) and
update any references/usages (queries, DTOs, serializers, tests) that access the
misspelled symbol so they use ProximitySensor3 to restore naming consistency and
avoid runtime errors.
In @packages/db/src/entities/TelemetryPacket.entity.ts:
- Around line 957-958: In TelemetryPacket.entity update the misspelled property
and column decorator from RroximitySensor3 to ProximitySensor3: rename the class
property and ensure the @Column metadata/DB column name (if explicitly set
elsewhere) matches the new ProximitySensor3 name, then search the repo for any
references/usages (queries, DTOs, serializers, tests, migrations) and update
them accordingly to avoid runtime mapping errors.
In @packages/db/src/services/DatabaseService.ts:
- Around line 11-13: The constructor of DatabaseService calls
AppDataSource.getRepository(TelemetryPacket) before the DataSource is
initialized; move repository acquisition out of the constructor and into the
service's initialize() (or a lazy getter) so that you call
AppDataSource.getRepository(TelemetryPacket) only after awaiting
AppDataSource.initialize() or verifying AppDataSource.isInitialized; update
DatabaseService to set this.telemetryPacketRepo inside initialize() (or
throw/await if not initialized) to avoid TypeORM errors.
- Around line 46-67: The Contactor property names in IContactorType
(ArrayBPSError, ChargeBPSError, MotorBPSError) don't match TelemetryPacket
columns (ArrayBpsError, ChargeBpsError, MotorBpsError), causing silent drops
when spreading packet.Contactor; update the type definitions in
packages/shared/src/types.ts to rename those properties to ArrayBpsError,
ChargeBpsError, and MotorBpsError (and update any references/usages of
IContactorType accordingly) so the spread in DatabaseService (packet.Contactor)
maps cleanly to the TelemetryPacket entity columns.
🧹 Nitpick comments (6)
packages/db/src/repositories/TelemetryPacketRepository.ts (1)
6-8: Class name inconsistency with file and interface.The class is named
TelemetryRepositorybut the file isTelemetryPacketRepository.tsand the interface isITelemetryPacketRepository. This inconsistency could cause confusion.Proposed fix
-export class TelemetryRepository +export class TelemetryPacketRepository extends BaseRepository<TelemetryPacket> implements ITelemetryPacketRepositorypackages/db/src/entities/TelemetryPacket.entity.ts (1)
8-17: Consider specifyingchunkTimeIntervalfor the hypertable.Other entities (e.g.,
KeyMotor) specifychunkTimeInterval: "1 month". Explicitly setting this ensures predictable chunk sizing for query and compression performance.Example
@Hypertable({ + chunkTimeInterval: "1 month", compression: { compress: true, compress_segmentby: "Rfid", compress_orderby: "Timestamp", policy: { schedule_interval: "7 days", }, }, })packages/db/src/services/DatabaseService.ts (2)
1-1: Unused import:DataSource.
DataSourceis imported but not used; onlyRepositoryis needed.Proposed fix
-import { DataSource, Repository } from "typeorm"; +import { Repository } from "typeorm";
136-141: Replaceconsole.logwith structured logging.The
BackendControlleruses a properloggerinstance. This service should follow the same pattern for consistency and to enable log level control.packages/db/src/entities/B3.entity.ts (1)
9-17: Same column-name mapping risk here (PascalCaseprops vs existing snake_case columns).
If the project relies on a naming strategy to translateB3Heartbeat→b3_heartbeat, confirm it’s configured everywhere you run migrations / initialize the DataSource.Also applies to: 20-92
packages/db/src/entities/BatteryFaults.entity.ts (1)
27-133: VerifyError*/Warning*fields still map to the intended DB columns.
These renames are especially sensitive because many columns were previously snake_case; ensure the naming strategy (or explicit{ name: "..." }) keeps compatibility with existing hypertables and any historical data.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
packages/db/src/data-source.tspackages/db/src/entities/B3.entity.tspackages/db/src/entities/Battery.entity.tspackages/db/src/entities/BatteryFaults.entity.tspackages/db/src/entities/Contactor.entity.tspackages/db/src/entities/Driver.entity.tspackages/db/src/entities/KeyMotor.entity.tspackages/db/src/entities/Lap.entity.tspackages/db/src/entities/MBMS.entity.tspackages/db/src/entities/MPPT.entity.tspackages/db/src/entities/MotorDetails.entity.tspackages/db/src/entities/ProximitySensors.entity.tspackages/db/src/entities/Telemetry.entity.tspackages/db/src/entities/TelemetryMetadata.entity.tspackages/db/src/entities/TelemetryPacket.entity.tspackages/db/src/index.tspackages/db/src/interfaces/repositories.interface.tspackages/db/src/repositories/BaseRepository.tspackages/db/src/repositories/TelemetryPacketRepository.tspackages/db/src/services/DatabaseService.tspackages/server/src/controllers/BackendController/BackendController.tspackages/server/src/database/DatabaseManager.ts
💤 Files with no reviewable changes (2)
- packages/server/src/database/DatabaseManager.ts
- packages/db/src/index.ts
🧰 Additional context used
🧬 Code graph analysis (6)
packages/db/src/repositories/TelemetryPacketRepository.ts (2)
packages/db/src/index.ts (1)
BaseRepository(2-2)packages/db/src/interfaces/repositories.interface.ts (1)
ITelemetryPacketRepository(57-58)
packages/server/src/controllers/BackendController/BackendController.ts (2)
packages/db/src/services/DatabaseService.ts (1)
DatabaseService(6-150)packages/db/src/index.ts (1)
DatabaseService(8-8)
packages/db/src/data-source.ts (1)
packages/db/src/index.ts (1)
AppDataSource(1-1)
packages/db/src/entities/Lap.entity.ts (3)
packages/db/src/entities/TelemetryPacket.entity.ts (1)
Entity(7-1019)packages/db/src/entities/KeyMotor.entity.ts (1)
Entity(8-35)packages/db/src/entities/Telemetry.entity.ts (1)
Entity(8-82)
packages/db/src/entities/Driver.entity.ts (4)
packages/db/src/entities/TelemetryPacket.entity.ts (1)
Entity(7-1019)packages/db/src/entities/KeyMotor.entity.ts (1)
Entity(8-35)packages/db/src/entities/ProximitySensors.entity.ts (1)
Entity(8-38)packages/db/src/entities/TelemetryMetadata.entity.ts (1)
Entity(9-34)
packages/db/src/services/DatabaseService.ts (2)
packages/db/src/data-source.ts (1)
AppDataSource(12-19)packages/shared/src/types.ts (1)
ITelemetryData(399-399)
🔇 Additional comments (13)
packages/db/src/entities/MBMS.entity.ts (1)
27-197: Property declarations follow consistent naming convention.The remaining property declarations are consistent with the project-wide PascalCase renaming. Types are appropriate for the telemetry data (boolean for status flags, float for numeric values like
AuxiliaryBatteryVoltage,StartupState,SystemState).Consider adding explicit
nameoptions to all columns if you choose Option 1 above to maintain lowercase database column names, ensuring consistency across the entity.packages/db/src/entities/TelemetryMetadata.entity.ts (1)
24-33: LGTM! Property naming is consistent.The PascalCase property renames are applied consistently across all fields. The decorators correctly maintain the database column mappings independently of the property names.
packages/db/src/entities/KeyMotor.entity.ts (1)
22-34: LGTM! Consistent PascalCase naming applied.All properties have been renamed to PascalCase appropriately, maintaining consistency with the broader refactor across entities.
packages/db/src/entities/Driver.entity.ts (1)
16-25: LGTM! Clean refactor with unused import removed.The PascalCase property renames are consistent, and the removal of the unused
Indeximport improves code cleanliness.packages/db/src/entities/MPPT.entity.ts (1)
22-124: LGTM! Comprehensive and consistent PascalCase naming.All 34 properties (Timestamp, Rfid, and 32 MPPT channel measurements) follow a consistent PascalCase naming pattern:
Mppt{X}Ch{Y}{Measurement}. The refactor maintains proper structure across all four MPPTs and their channels.packages/db/src/entities/Lap.entity.ts (1)
9-12: LGTM!The index and entity property naming have been consistently updated to PascalCase, aligning with the broader refactoring pattern in this PR.
packages/db/src/interfaces/repositories.interface.ts (1)
15-15: LGTM!The new
ITelemetryPacketRepositoryinterface follows the established pattern and integrates cleanly with the existing repository interface hierarchy.Also applies to: 57-58
packages/db/src/entities/TelemetryPacket.entity.ts (1)
20-26: LGTM!The composite primary key design with
TimestampandRfidis appropriate for time-series data, and the wide-table consolidation aligns with the PR objectives.packages/db/src/entities/MotorDetails.entity.ts (1)
22-114: The column name mappings are already handled correctly by TypeORM's default naming strategy (v0.3.20). PascalCase properties likeTimestamp,Rfid, andMotorIdare automatically converted to snake_case columns (timestamp,rfid,motor_id) without explicitnamemappings, which aligns with the@Entity("motor_details")table name and the@Hypertableconfiguration. Other entities in the same package (e.g., Battery) follow this same pattern without issues. Explicitnamemappings are unnecessary here.Likely an incorrect or invalid review comment.
packages/db/src/entities/Battery.entity.ts (2)
1-19: Entity configuration looks good.The TimescaleDB hypertable configuration is properly set up with appropriate chunking, compression, and indexing.
22-127: Address critical BaseRepository compatibility issue with PascalCase properties.Consuming code in
PIS.battery.tsxis correctly updated with PascalCase property access. However, a critical issue exists:BatteryRepositoryinherits aprimaryKey: keyof T = "timestamp"default fromBaseRepository(line in BaseRepository.ts), but the Battery entity defines the property asTimestamp(PascalCase). This mismatch will cause query failures.Required fixes:
- Override the primaryKey in
BatteryRepositoryconstructor to correctly map to"Timestamp"and"Rfid"(composite primary key for Battery)- Verify all
@Columndecorators have explicit name mappings (e.g.,@Column({ type: "boolean", name: "always_on_signal_status" })) to match the database schema, or ensure the database schema uses PascalCase column names- Test repository queries to confirm
find(),findOneBy(), and other methods work with the new primary keypackages/db/src/entities/Contactor.entity.ts (2)
22-180: Confirm this PascalCase property naming is an intentional architectural choice and document it.All entity properties use PascalCase (e.g.,
Timestamp,Rfid,ArrayChargeCurrent), which diverges from TypeScript conventions where camelCase is standard for properties. This pattern is systematic across all 14 entity files in the repository, indicating it is deliberate. While this standardization may simplify consistency with database schemas or external systems, it differs from TypeScript best practices and may surprise developers. Consider documenting this architectural decision and its rationale.
10-10: Verify whether @timescaledb/typeorm configurations should reference database column names or property names.The
timeColumnName: "timestamp"andcompress_segmentby: "rfid"reference lowercase strings, while properties are defined asTimestampandRfid(PascalCase). However, TypeORM 0.3.x uses a default snakeCase naming strategy that automatically converts property names to lowercase database column names without explicit@Column({ name: "..." })mappings. This means the lowercase configuration strings likely reference the resulting database column names, not the property names.However,
TelemetryPacket.entity.tsuses the opposite pattern—PascalCase in its compression and index configurations—suggesting inconsistency across the codebase about which format @timescaledb/typeorm expects. Without access to the @timescaledb/typeorm documentation or the ability to verify which pattern works at runtime, the correct approach cannot be definitively determined.
| Timestamp!: Date; | ||
|
|
||
| @PrimaryColumn({ type: "text" }) | ||
| rfid!: string; | ||
| Rfid!: string; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| always_on_signal_status!: boolean; | ||
| AlwaysOnSignalStatus!: boolean; | ||
|
|
||
| @Column({ type: "float" }) | ||
| average_cell_voltage!: number; | ||
| AverageCellVoltage!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| average_temperature!: number; | ||
| AverageTemperature!: number; | ||
|
|
||
| @Column({ type: "integer" }) | ||
| bmu_alive!: number; | ||
| BmuAlive!: number; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| charge_relay_enabled!: boolean; | ||
| ChargeRelayEnabled!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| charger_safety_enabled!: boolean; | ||
| ChargerSafetyEnabled!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| discharge_relay_enabled!: boolean; | ||
| DischargeRelayEnabled!: boolean; | ||
|
|
||
| @Column({ type: "float" }) | ||
| fan_speed!: number; | ||
| FanSpeed!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| fan_voltage!: number; | ||
| FanVoltage!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| high_cell_voltage!: number; | ||
| HighCellVoltage!: number; | ||
|
|
||
| @Column({ type: "integer" }) | ||
| high_cell_voltage_id!: number; | ||
| HighCellVoltageId!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| high_temperature!: number; | ||
| HighTemperature!: number; | ||
|
|
||
| @Column({ type: "integer" }) | ||
| high_thermistor_id!: number; | ||
| HighThermistorId!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| input_12v!: number; | ||
| Input12v!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| internal_temperature!: number; | ||
| InternalTemperature!: number; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| is_charging_signal_status!: boolean; | ||
| IsChargingSignalStatus!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| is_ready_signal_status!: boolean; | ||
| IsReadySignalStatus!: boolean; | ||
|
|
||
| @Column({ type: "float" }) | ||
| low_cell_voltage!: number; | ||
| LowCellVoltage!: number; | ||
|
|
||
| @Column({ type: "integer" }) | ||
| low_cell_voltage_id!: number; | ||
| LowCellVoltageId!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| low_temperature!: number; | ||
| LowTemperature!: number; | ||
|
|
||
| @Column({ type: "integer" }) | ||
| low_thermistor_id!: number; | ||
| LowThermistorId!: number; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| malfunction_indicator_active!: boolean; | ||
| MalfunctionIndicatorActive!: boolean; | ||
|
|
||
| @Column({ type: "float" }) | ||
| maximum_cell_voltage!: number; | ||
| MaximumCellVoltage!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| maximum_pack_voltage!: number; | ||
| MaximumPackVoltage!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| minimum_cell_voltage!: number; | ||
| MinimumCellVoltage!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| minimum_pack_voltage!: number; | ||
| MinimumPackVoltage!: number; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| multi_purpose_input_signal_status!: boolean; | ||
| MultiPurposeInputSignalStatus!: boolean; | ||
|
|
||
| @Column({ type: "float" }) | ||
| pack_amphours!: number; | ||
| PackAmphours!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| pack_current!: number; | ||
| PackCurrent!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| pack_depth_of_discharge!: number; | ||
| PackDepthOfDischarge!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| pack_state_of_charge!: number; | ||
| PackStateOfCharge!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| pack_voltage!: number; | ||
| PackVoltage!: number; | ||
|
|
||
| @Column({ type: "integer" }) | ||
| populated_cells!: number; | ||
| PopulatedCells!: number; | ||
|
|
||
| @Column({ type: "float" }) | ||
| requested_fan_speed!: number; | ||
| RequestedFanSpeed!: number; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find migration files related to battery table renaming
find . -type f \( -name "*migration*" -o -name "*migrate*" \) -path "*/db/*" | head -20Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Check Battery entity for explicit column name mappings
rg '@Column' packages/db/src/entities/Battery.entity.ts -A 1 -B 0 | head -40Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 936
🏁 Script executed:
#!/bin/bash
# Get full Battery entity file to see complete structure
wc -l packages/db/src/entities/Battery.entity.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 121
🏁 Script executed:
#!/bin/bash
# Read full Battery.entity.ts file
cat -n packages/db/src/entities/Battery.entity.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 3778
🏁 Script executed:
#!/bin/bash
# Search for migration files more broadly
find . -type f \( -name "*.ts" -o -name "*.js" \) | grep -i migrat | head -20Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Check for TypeORM migrations directory
fd -t d migrationRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Search for TypeORM DataSource or Connection configuration
rg -l "DataSource|createConnection|getConnection" packages/db/src --type ts | head -10Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 174
🏁 Script executed:
#!/bin/bash
# Search for naming strategy configuration
rg "namingStrategy|NamingStrategy" packages/db/src --type ts -B 2 -A 2Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Check package.json for TypeORM version and configuration
cat packages/db/package.json 2>/dev/null || cat package.json | grep -A 20 '"typeorm"'Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 1367
🏁 Script executed:
#!/bin/bash
# Search for any database configuration files
find . -name "*.env*" -o -name "*config*" -o -name "*ormconfig*" | grep -v node_modules | head -20Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 615
🏁 Script executed:
#!/bin/bash
# Read the TypeORM data source configuration
cat packages/db/src/data-source.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 787
🏁 Script executed:
#!/bin/bash
# Check for migrations directory and any migration files
find packages/db -type d -name migrations -o -name migrationRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Look for any naming strategy usage or imports
rg "snake|pascal|kebab|naming" packages/db/src --type ts -iRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 57
🏁 Script executed:
#!/bin/bash
# Check if Battery entity is imported or used anywhere else
rg "Battery" packages/db/src --type ts -lRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 539
🏁 Script executed:
#!/bin/bash
# Check if Battery entity is referenced in any migration or services
rg "Battery" packages/ --type ts | head -20Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 1487
🏁 Script executed:
#!/bin/bash
# List all entity files in the entities directory
ls -la packages/db/src/entities/Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 1167
🏁 Script executed:
#!/bin/bash
# Check if there's a migrations directory
ls -la packages/db/src/migrations 2>/dev/null || echo "No migrations directory found"Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 104
🏁 Script executed:
#!/bin/bash
# Check if Battery is used in TelemetryPacket or other entities
rg "Battery" packages/db/src/entities --type ts -B 2 -A 2Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 10165
🏁 Script executed:
#!/bin/bash
# Check if Battery is used in repositories or services
cat packages/db/src/repositories/BatteryRepository.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 470
🏁 Script executed:
#!/bin/bash
# Check if Battery is imported or exported from index
cat packages/db/src/index.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 363
🏁 Script executed:
#!/bin/bash
# Check TypeORM version and how it handles naming strategy by default
npm list typeorm 2>/dev/null || rg '"typeorm"' packages/db/package.jsonRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 137
🏁 Script executed:
#!/bin/bash
# Check if other entities also use PascalCase properties
head -50 packages/db/src/entities/MBMS.entity.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 1294
🏁 Script executed:
#!/bin/bash
# Check the BatteryFaults entity for comparison
head -50 packages/db/src/entities/BatteryFaults.entity.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 1380
🏁 Script executed:
#!/bin/bash
# Verify TypeORM's default naming strategy behavior
web_search "TypeORM 0.3 default naming strategy column names"Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 125
🌐 Web query:
TypeORM 0.3.20 default naming strategy PascalCase to snake_case
💡 Result:
Yes — in TypeORM 0.3.x you set a namingStrategy on the DataSource. Two common approaches to convert PascalCase / camelCase → snake_case:
- Use the community package (quick, drop-in)
- Install: npm i typeorm-naming-strategies
- DataSource example:
const AppDataSource = new DataSource({
...,
namingStrategy: new SnakeNamingStrategy(),
});
(usage documented in the package README). [1]
- Implement a small custom SnakeNamingStrategy (works with 0.3.x DataSource API)
- Example (uses TypeORM's snakeCase util and DefaultNamingStrategy):
export class SnakeNamingStrategy extends DefaultNamingStrategy implements NamingStrategyInterface {
tableName(className: string, customName: string) {
return customName ? customName : snakeCase(className);
}
columnName(propertyName: string, customName: string, embeddedPrefixes: string[]) {
const prefix = embeddedPrefixes.length ? embeddedPrefixes.join('') + '' : '';
return snakeCase(prefix) + (customName ? customName : snakeCase(propertyName));
}
relationName(propertyName: string) { return snakeCase(propertyName); }
joinColumnName(relationName: string, referencedColumnName: string) {
return snakeCase(relationName + '_' + referencedColumnName);
}
/* implement joinTableName, joinTableColumnName, classTableInheritanceParentColumnName similarly */
} - Then pass namingStrategy: new SnakeNamingStrategy() to your DataSource. (Example implementations available online.) [3]
Where to configure: DataSourceOptions includes namingStrategy — pass the instance when creating new DataSource. [2]
References:
- typeorm-naming-strategies (SnakeNamingStrategy). [1]
- TypeORM DataSource / DataSourceOptions (namingStrategy option). [2]
- Example custom SnakeNamingStrategy gist. [3]
🏁 Script executed:
#!/bin/bash
# Verify that data-source.ts doesn't have a namingStrategy
grep -i "namingstrategy" packages/db/src/data-source.ts || echo "No namingStrategy found in data-source.ts"Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 116
🏁 Script executed:
#!/bin/bash
# Check if any other entity files have explicit name mappings in @Column
rg '@Column.*name:' packages/db/src/entities --type ts -cRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 57
Add explicit column name mappings to align properties with database schema.
TypeORM 0.3.20 does not automatically convert property names to snake_case—it uses property names as column names by default. Your Battery entity uses PascalCase properties without explicit name parameters in @Column decorators, meaning TypeORM will query for columns named Timestamp, Rfid, AlwaysOnSignalStatus, etc.
However, your @Hypertable configuration specifies timeColumnName: "timestamp" (lowercase), and your @Index(["rfid", "timestamp"]) decorator also uses lowercase. This mismatch will cause "column does not exist" errors at runtime.
Add explicit column name mappings to preserve compatibility with your database schema:
Example fix for first few columns
@Column({ name: "timestamp", type: "timestamptz" })
Timestamp!: Date;
@Column({ name: "rfid", type: "text" })
Rfid!: string;
@Column({ name: "always_on_signal_status", type: "boolean" })
AlwaysOnSignalStatus!: boolean;
@Column({ name: "average_cell_voltage", type: "float" })
AverageCellVoltage!: number;Apply this pattern to all 37 properties in the Battery entity, or configure a global namingStrategy in your DataSource to automatically handle the conversion.
🤖 Prompt for AI Agents
In @packages/db/src/entities/Battery.entity.ts around lines 22 - 127, The
Battery entity uses PascalCase property names (e.g., Timestamp, Rfid,
AlwaysOnSignalStatus) but its @Hypertable timeColumnName ("timestamp") and
@Index(["rfid","timestamp"]) use snake_case; add explicit column name mappings
on every @Column in the Battery class (for example set name: "timestamp",
"rfid", "always_on_signal_status", etc.) so TypeORM queries the correct DB
columns; update the @Column decorators for all properties (Timestamp, Rfid,
AlwaysOnSignalStatus, AverageCellVoltage, AverageTemperature, BmuAlive,
ChargeRelayEnabled, ChargerSafetyEnabled, DischargeRelayEnabled, FanSpeed,
FanVoltage, HighCellVoltage, HighCellVoltageId, HighTemperature,
HighThermistorId, Input12v, InternalTemperature, IsChargingSignalStatus,
IsReadySignalStatus, LowCellVoltage, LowCellVoltageId, LowTemperature,
LowThermistorId, MalfunctionIndicatorActive, MaximumCellVoltage,
MaximumPackVoltage, MinimumCellVoltage, MinimumPackVoltage,
MultiPurposeInputSignalStatus, PackAmphours, PackCurrent, PackDepthOfDischarge,
PackStateOfCharge, PackVoltage, PopulatedCells, RequestedFanSpeed) to include
the correct snake_case name and appropriate DB type, or alternatively configure
a global namingStrategy in your DataSource to auto-convert PascalCase to
snake_case.
| @TimeColumn() | ||
| timestamp!: Date; | ||
| Timestamp!: Date; |
There was a problem hiding this comment.
Missing column type specification for Timestamp.
Other entities (e.g., TelemetryPacket, KeyMotor) use @PrimaryColumn({ type: "timestamptz" }) alongside @TimeColumn(). Here, Timestamp only has @TimeColumn() without a type specification, which may cause TypeORM to infer an incorrect default type.
Proposed fix
@TimeColumn()
+ @Column({ type: "timestamptz" })
Timestamp!: Date;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @TimeColumn() | |
| timestamp!: Date; | |
| Timestamp!: Date; | |
| @TimeColumn() | |
| @Column({ type: "timestamptz" }) | |
| Timestamp!: Date; |
🤖 Prompt for AI Agents
In @packages/db/src/entities/Lap.entity.ts around lines 14 - 15, The Timestamp
property in the Lap entity is decorated with @TimeColumn() but lacks an explicit
column type, which can lead to incorrect type inference; update the Lap entity
by adding an explicit column type (e.g., { type: "timestamptz" }) to the
Timestamp declaration so it matches other entities (like TelemetryPacket and
KeyMotor) that use @PrimaryColumn({ type: "timestamptz" }) alongside their time
column decorators, ensuring the Timestamp field is persisted with the correct DB
timezone-aware timestamp type.
| // B3 data | ||
| ...packet.B3, | ||
|
|
||
| // Battery data | ||
| ...packet.Battery, | ||
|
|
||
| // Battery Faults (flatten Errors and Warnings) | ||
| ...packet.BatteryFaults.Errors, | ||
| ...packet.BatteryFaults.Warnings, | ||
|
|
||
| // Contactor data | ||
| ...packet.Contactor, | ||
|
|
||
| // KeyMotor data | ||
| ...packet.KeyMotor, | ||
|
|
||
| // MBMS data | ||
| ...packet.MBMS, | ||
|
|
||
| // MPPT data | ||
| ...packet.MPPT, | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the ITelemetryData and related type definitions
rg -nP --type=ts 'interface.*Telemetry|type.*Telemetry|B3Type|BatteryType' -A 5 packages/Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 7825
🏁 Script executed:
# Find the TelemetryPacket definition to see expected column names
rg -nP --type=ts 'interface TelemetryPacket|type TelemetryPacket' -A 30 packages/Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 57
🏁 Script executed:
# Look for the type definitions referenced in the spread operators
rg -nP --type=ts 'interface.*B3|type.*B3|I.*B3' -A 20 packages/Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 8841
🏁 Script executed:
# Find TelemetryPacket entity definition
rg -nP --type=ts 'class TelemetryPacket|export.*TelemetryPacket' -A 40 packages/db/Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 3676
🏁 Script executed:
# Search for entity files in db package
fd --type=f -e ts 'TelemetryPacket|telemetry' packages/db/src/ | head -20Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 183
🏁 Script executed:
# Look for TypeORM entity definitions
rg -nP --type=ts '@Entity|@Column' packages/db/src/ -B 2 -A 2 | grep -A 5 -B 5 TelemetryPacket | head -100Repository: UCSolarCarTeam/Helios-Telemetry
Length of output: 7895
🏁 Script executed:
# Get entire TelemetryPacket entity to see all columns
wc -l packages/db/src/entities/TelemetryPacket.entity.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 130
🏁 Script executed:
# Get all lines to see complete entity
cat -n packages/db/src/entities/TelemetryPacket.entity.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 36142
🏁 Script executed:
# Get all IBatteryFaults, IContactor, IKeyMotor, IMBMS, IMPPT type definitions
rg -nP --type=ts 'const IBatteryFaultsType|const IContactorType|const IKeyMotorType|const IMBMSType|const IMPPTType' -A 30 packages/shared/src/types.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 4885
🏁 Script executed:
# Verify the exact property names in IContactorType for BPS fields
rg -nP --type=ts 'BPS|Bps' packages/shared/src/types.ts packages/db/src/entities/TelemetryPacket.entity.tsRepository: UCSolarCarTeam/Helios-Telemetry
Length of output: 650
Fix property name case mismatches between IContactorType and TelemetryPacket entity columns.
The spread operators assume property names in IContactorType match TelemetryPacket column names, but case sensitivity mismatches cause data loss:
ArrayBPSError(type) vsArrayBpsError(entity)ChargeBPSError(type) vsChargeBpsError(entity)MotorBPSError(type) vsMotorBpsError(entity)
When ...packet.Contactor is spread, these properties will not match database columns and will be silently dropped. Align the type definitions in packages/shared/src/types.ts to use lowercase Bps to match the entity columns.
🤖 Prompt for AI Agents
In @packages/db/src/services/DatabaseService.ts around lines 46 - 67, The
Contactor property names in IContactorType (ArrayBPSError, ChargeBPSError,
MotorBPSError) don't match TelemetryPacket columns (ArrayBpsError,
ChargeBpsError, MotorBpsError), causing silent drops when spreading
packet.Contactor; update the type definitions in packages/shared/src/types.ts to
rename those properties to ArrayBpsError, ChargeBpsError, and MotorBpsError (and
update any references/usages of IContactorType accordingly) so the spread in
DatabaseService (packet.Contactor) maps cleanly to the TelemetryPacket entity
columns.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/db/src/entities/Contactor.entity.ts (1)
1-181: Register the Contactor entity in AppDataSource before this change can function.The
Contactorentity is not registered inpackages/db/src/data-source.ts. TheAppDataSource.entitiesarray currently only includes[TelemetryPacket, Driver, Lap], which means TypeORM won't recognizeContactoras a managed entity and any queries using it will fail. AddContactorto the entities array and import it at the top of the file.Additionally, the PascalCase naming convention (
Timestamp,Rfid,ArrayBpsError, etc.) is intentional and consistent with other new hypertable entities (B3,Battery,BatteryFaults), but be aware this requires database columns to match these exact names since no explicit@Column({ name: "..." })mappings are provided.
🤖 Fix all issues with AI agents
In @packages/db/src/data-source.ts:
- Line 18: The DataSource configuration currently hardcodes sensitive DB
credentials in the "url" property; remove the literal connection string and read
the connection URL from an environment variable (e.g., process.env.DATABASE_URL)
when initializing the DataSource (the object/variable that contains the url
field in data-source.ts), validate that DATABASE_URL is present and throw a
clear error if not, and ensure local secrets are stored in a .db.env (or
similar) that is gitignored and documented rather than committed to version
control.
In @packages/db/src/entities/BatteryFaults.entity.ts:
- Around line 22-132: The entity properties in BatteryFaults (e.g., Rfid,
ErrorAlwaysOnSupplyFault, WarningDclReducedDueToLowSoc, etc.) were renamed to
PascalCase but the @PrimaryColumn/@Column decorators do not set the database
column names, so TypeORM will try to use the PascalCase names at runtime; either
add explicit name mappings to each decorator (e.g., @Column({ name:
"error_always_on_supply_fault", type: "boolean" })) for every field in the
BatteryFaults entity (including primary columns like Rfid/Timestamp), or instead
create a migration that renames the DB columns to the new PascalCase names—pick
one approach and apply it consistently across all listed properties.
In @packages/db/src/entities/MotorDetails.entity.ts:
- Around line 21-115: MotorDetails is dead/incorrectly mapped: either remove its
entity/repository or fix mappings and types and register it; to fix, update the
MotorDetails class to add explicit column name mappings (e.g., @PrimaryColumn({
name: "timestamp", type: "timestamptz" }) for Timestamp, @PrimaryColumn({ name:
"rfid", type: "text" }) for Rfid, @PrimaryColumn({ name: "motor_id", type: "int"
}) for MotorId, and similar @Column({ name: "field_name", type: "int" }) or
type: "float" for each property) and change ErrorFlags, LimitFlags,
RxErrorCount, TxErrorCount, SerialNumber, and MotorId to integer types; finally
register MotorDetails with AppDataSource and ensure its repository is used where
intended, or remove the entity and any unused repository/interface if you do not
intend to use it.
In @packages/db/src/entities/ProximitySensors.entity.ts:
- Line 34: The property name RroximitySensor3 in the ProximitySensors entity has
a typo; rename the class property from RroximitySensor3 to ProximitySensor3
(preserving its type and any non-null assertion) and update all
references/usages (queries, DTOs, serializers, tests, and any migrations/seeds)
to use ProximitySensor3 so naming matches the other sensor fields.
In @packages/db/src/entities/TelemetryPacket.entity.ts:
- Line 958: Typo in the TelemetryPacket entity: rename the property
RroximitySensor3 to ProximitySensor3; update the property declaration in
TelemetryPacket.entity (replace RroximitySensor3 with ProximitySensor3) and then
search for and update all references/usages (constructor, serializers, mappers,
interfaces, tests) that reference RroximitySensor3 to the corrected
ProximitySensor3 name to keep types and runtime behavior consistent.
- Around line 8-17: The Hypertable configuration on the TelemetryPacket entity
is missing a chunkTimeInterval setting; update the @Hypertable decorator for
TelemetryPacket (TelemetryPacket.entity) to include chunkTimeInterval: "1 month"
alongside the existing compression config so TimescaleDB uses the same monthly
chunking as other entities (e.g., KeyMotor, MBMS).
In @packages/db/src/repositories/TelemetryPacketRepository.ts:
- Around line 6-13: Rename the exported class TelemetryRepository to
TelemetryPacketRepository so the class name matches the file
TelemetryPacketRepository.ts and the implemented interface
ITelemetryPacketRepository; update the class declaration (export class
TelemetryPacketRepository extends BaseRepository<TelemetryPacket> implements
ITelemetryPacketRepository) and adjust any imports/usages across the codebase
that reference TelemetryRepository to the new TelemetryPacketRepository
identifier.
In @packages/db/src/services/DatabaseService.ts:
- Around line 11-13: The constructor currently calls
AppDataSource.getRepository(TelemetryPacket) before the DataSource is
initialized, causing failures; remove repository initialization from the
DatabaseService constructor and instead initialize telemetryPacketRepo inside
the DatabaseService.initialize method (after calling
AppDataSource.initialize()), e.g., call AppDataSource.initialize() first then
assign this.telemetryPacketRepo = AppDataSource.getRepository(TelemetryPacket)
within initialize so repositories are acquired only after the DataSource is
ready.
In @packages/server/src/controllers/BackendController/BackendController.ts:
- Around line 66-67: The call to this.databaseService.insertPacketData(message)
in BackendController lacks error handling and has an extra blank line; wrap the
insertPacketData invocation in a try-catch (await it if it returns a Promise),
catch any error and log it via the controller logger (e.g., this.logger.error or
processLogger) including contextual info like the message/id and error.stack,
and decide whether to retry, emit a metric, or continue gracefully instead of
swallowing the error; also remove the unnecessary blank line after the call.
🧹 Nitpick comments (5)
packages/db/src/entities/Contactor.entity.ts (1)
22-180: Verify the PascalCase naming convention is intentional and documented.All entity properties have been renamed to PascalCase (e.g.,
Timestamp,Rfid,ArrayBpsError), which deviates from standard TypeScript/JavaScript conventions where properties typically use camelCase. This naming choice will affect:
- JSON serialization in API responses (keys will be PascalCase)
- Frontend code consuming these entities
- Type definitions and interfaces
- Developer expectations
Ensure this convention is documented in project guidelines and consistently applied across the codebase.
</content_end>
packages/server/src/controllers/BackendController/BackendController.ts (1)
62-73: Consider operational implications of dual-write pattern.Writing telemetry data to both DynamoDB and the new database service introduces a dual-write pattern. This can lead to data inconsistency if one write succeeds and the other fails. Consider whether:
- This is a transitional approach before fully migrating to TimescaleDB
- Both data stores need to be kept in sync long-term
- A more robust pattern (e.g., event sourcing, CDC, or transactional outbox) is warranted
packages/db/src/entities/TelemetryPacket.entity.ts (1)
1-1019: Consider the implications of a wide table design.This entity consolidates all telemetry data into a single wide table with ~200+ nullable columns. While this simplifies insertions, be aware of potential trade-offs:
- Sparse data may lead to storage inefficiency despite compression
- Query performance may degrade when selecting specific subsets
- Schema evolution becomes more complex with a monolithic entity
Ensure this design aligns with your query patterns and consider monitoring query performance after deployment.
packages/db/src/services/DatabaseService.ts (1)
138-138: Replace console.log with proper logging.Using
console.login production code makes it difficult to control log levels and format. Consider using a proper logging library.📝 Suggested improvement
- console.log("Inserting packet data into database"); + // Use a logging library like winston or pino + logger.info("Inserting packet data into database");packages/db/src/entities/MotorDetails.entity.ts (1)
54-54: Consider using integer types for counts, IDs, and flags.Properties like
ErrorFlags,LimitFlags,RxErrorCount,TxErrorCount,SerialNumber, andTritiumIdare currently typed asfloat, but they represent integer quantities. Using integer types would be more semantically correct and avoid potential precision issues.♻️ Proposed refactor: Use integer types
- @Column({ type: "float" }) + @Column({ type: "int" }) ErrorFlags!: number; - @Column({ type: "float" }) + @Column({ type: "int" }) LimitFlags!: number; - @Column({ type: "float" }) + @Column({ type: "int" }) RxErrorCount!: number; - @Column({ type: "float" }) + @Column({ type: "int" }) SerialNumber!: number; - @Column({ type: "float" }) + @Column({ type: "int" }) TritiumId!: number; - @Column({ type: "float" }) + @Column({ type: "int" }) TxErrorCount!: number;Note: Verify existing data is compatible with integer types before applying this change.
Also applies to: 66-66, 84-84, 87-87, 102-102, 105-105
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
packages/db/src/data-source.tspackages/db/src/entities/B3.entity.tspackages/db/src/entities/Battery.entity.tspackages/db/src/entities/BatteryFaults.entity.tspackages/db/src/entities/Contactor.entity.tspackages/db/src/entities/Driver.entity.tspackages/db/src/entities/KeyMotor.entity.tspackages/db/src/entities/Lap.entity.tspackages/db/src/entities/MBMS.entity.tspackages/db/src/entities/MPPT.entity.tspackages/db/src/entities/MotorDetails.entity.tspackages/db/src/entities/ProximitySensors.entity.tspackages/db/src/entities/Telemetry.entity.tspackages/db/src/entities/TelemetryMetadata.entity.tspackages/db/src/entities/TelemetryPacket.entity.tspackages/db/src/index.tspackages/db/src/interfaces/repositories.interface.tspackages/db/src/repositories/BaseRepository.tspackages/db/src/repositories/TelemetryPacketRepository.tspackages/db/src/services/DatabaseService.tspackages/server/src/controllers/BackendController/BackendController.tspackages/server/src/database/DatabaseManager.ts
💤 Files with no reviewable changes (2)
- packages/db/src/index.ts
- packages/server/src/database/DatabaseManager.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/db/src/entities/TelemetryMetadata.entity.ts
- packages/db/src/repositories/BaseRepository.ts
- packages/db/src/entities/Lap.entity.ts
- packages/db/src/entities/Telemetry.entity.ts
- packages/db/src/entities/Battery.entity.ts
- packages/db/src/entities/MPPT.entity.ts
- packages/db/src/interfaces/repositories.interface.ts
🧰 Additional context used
🧬 Code graph analysis (3)
packages/db/src/repositories/TelemetryPacketRepository.ts (2)
packages/db/src/index.ts (1)
BaseRepository(2-2)packages/db/src/interfaces/repositories.interface.ts (1)
ITelemetryPacketRepository(57-58)
packages/db/src/entities/Driver.entity.ts (13)
packages/db/src/entities/Lap.entity.ts (1)
Entity(8-49)packages/db/src/entities/TelemetryPacket.entity.ts (1)
Entity(7-1019)packages/db/src/entities/B3.entity.ts (1)
Entity(8-92)packages/db/src/entities/Battery.entity.ts (1)
Entity(8-128)packages/db/src/entities/BatteryFaults.entity.ts (1)
Entity(8-133)packages/db/src/entities/Contactor.entity.ts (1)
Entity(8-181)packages/db/src/entities/KeyMotor.entity.ts (1)
Entity(8-35)packages/db/src/entities/MBMS.entity.ts (1)
Entity(8-197)packages/db/src/entities/MPPT.entity.ts (1)
Entity(8-125)packages/db/src/entities/MotorDetails.entity.ts (1)
Entity(9-115)packages/db/src/entities/ProximitySensors.entity.ts (1)
Entity(8-38)packages/db/src/entities/Telemetry.entity.ts (1)
Entity(8-82)packages/db/src/entities/TelemetryMetadata.entity.ts (1)
Entity(9-34)
packages/db/src/entities/TelemetryPacket.entity.ts (2)
packages/db/src/entities/Driver.entity.ts (1)
Entity(13-26)packages/db/src/entities/Lap.entity.ts (1)
Entity(8-49)
🔇 Additional comments (8)
packages/db/src/entities/BatteryFaults.entity.ts (1)
8-18: TimescaleDB configuration looks appropriate.The hypertable configuration with compression, chunking (1 month intervals), and indexing on
[rfid, timestamp]is well-suited for time-series telemetry data.packages/db/src/entities/Contactor.entity.ts (1)
1-181: Contactor is a data structure type, not a registered database entity, and doesn't require database column name mappings.The Contactor entity is not registered in AppDataSource (only TelemetryPacket, Driver, and Lap are). Its properties are flattened into TelemetryPacket via the spread operator in DatabaseService. The PascalCase naming convention is consistent across all registered entities in the codebase (Driver, Lap, TelemetryPacket) and is intentional; no explicit column name mappings are needed since TypeORM defaults to using property names as column names when no naming strategy is configured.
Likely an incorrect or invalid review comment.
packages/db/src/entities/B3.entity.ts (1)
22-92: LGTM!The field renaming from camelCase to PascalCase is consistent and correct across all properties.
packages/db/src/entities/Driver.entity.ts (1)
16-25: LGTM!The field renaming from snake_case to PascalCase is consistent and aligns with the broader entity naming convention updates across the PR.
packages/db/src/data-source.ts (1)
16-16: Verify synchronize behavior in production.Setting
synchronize: truein development automatically syncs schema changes with the database. Ensure this is never enabled in production as it can lead to data loss.packages/db/src/entities/MBMS.entity.ts (1)
22-196: Confirm all MBMS property references have been updated to PascalCase.Verification confirms that all MBMS properties have been successfully renamed to PascalCase (AbattDisable, ArrayContactorCommand, etc.). No remaining references to old naming patterns (snake_case/camelCase) were found across the codebase. All consumers—including shared types, helper functions, client components, and repositories—use the new PascalCase property names.
packages/db/src/services/DatabaseService.ts (1)
40-40: The RaceName field cannot be mapped to packet.RaceName because ITelemetryData does not have a RaceName field. ITelemetryData only contains: B3, Battery, BatteryFaults, Contactor, KeyMotor, MBMS, MPPT, MotorDetails0, MotorDetails1, Pi, ProximitySensors, Telemetry, TimeStamp, and Title. The current mapping of RaceName to packet.Title is the only available option from the input data.Likely an incorrect or invalid review comment.
packages/db/src/entities/KeyMotor.entity.ts (1)
22-34: All references to the renamed properties have been properly updated.Verification confirms that:
- No references to old property names (busCurrentOut, keyMotorVelocity, motorCurrent, bus_current_out, key_motor_velocity) exist in the codebase
- DatabaseService and all consuming code correctly use the new PascalCase property names (BusCurrentOut, KeyMotorVelocity, MotorCurrent)
- TypeORM decorators properly handle column mapping from PascalCase properties to snake_case database columns
- The @hypertable and @Index configurations reference the correct database column names
The breaking change to the public API is complete and consistent throughout the codebase.
| Timestamp!: Date; | ||
|
|
||
| @PrimaryColumn({ type: "text" }) | ||
| rfid!: string; | ||
| Rfid!: string; | ||
|
|
||
| // Errors | ||
| @Column({ type: "boolean" }) | ||
| error_always_on_supply_fault!: boolean; | ||
| ErrorAlwaysOnSupplyFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_canbus_communication_fault!: boolean; | ||
| ErrorCanbusCommunicationFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_charge_limit_enforcement_fault!: boolean; | ||
| ErrorChargeLimitEnforcementFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_charger_safety_relay_fault!: boolean; | ||
| ErrorChargerSafetyRelayFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_current_sensor_fault!: boolean; | ||
| ErrorCurrentSensorFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_discharge_limit_enforcement_fault!: boolean; | ||
| ErrorDischargeLimitEnforcementFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_fan_monitor_fault!: boolean; | ||
| ErrorFanMonitorFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_high_voltage_isolation_fault!: boolean; | ||
| ErrorHighVoltageIsolationFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_internal_communication_fault!: boolean; | ||
| ErrorInternalCommunicationFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_internal_conversion_fault!: boolean; | ||
| ErrorInternalConversionFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_internal_logic_fault!: boolean; | ||
| ErrorInternalLogicFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_internal_memory_fault!: boolean; | ||
| ErrorInternalMemoryFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_internal_thermistor_fault!: boolean; | ||
| ErrorInternalThermistorFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_low_cell_voltage_fault!: boolean; | ||
| ErrorLowCellVoltageFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_open_wiring_fault!: boolean; | ||
| ErrorOpenWiringFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_pack_voltage_sensor_fault!: boolean; | ||
| ErrorPackVoltageSensorFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_power_supply_12v_fault!: boolean; | ||
| ErrorPowerSupply12vFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_thermistor_fault!: boolean; | ||
| ErrorThermistorFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_voltage_redundancy_fault!: boolean; | ||
| ErrorVoltageRedundancyFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_weak_cell_fault!: boolean; | ||
| ErrorWeakCellFault!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| error_weak_pack_fault!: boolean; | ||
| ErrorWeakPackFault!: boolean; | ||
|
|
||
| // Warnings | ||
| @Column({ type: "boolean" }) | ||
| warning_ccl_reduced_due_to_alternate_current_limit!: boolean; | ||
| WarningCclReducedDueToAlternateCurrentLimit!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_ccl_reduced_due_to_charger_latch!: boolean; | ||
| WarningCclReducedDueToChargerLatch!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_ccl_reduced_due_to_high_cell_resistance!: boolean; | ||
| WarningCclReducedDueToHighCellResistance!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_ccl_reduced_due_to_high_cell_voltage!: boolean; | ||
| WarningCclReducedDueToHighCellVoltage!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_ccl_reduced_due_to_high_pack_voltage!: boolean; | ||
| WarningCclReducedDueToHighPackVoltage!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_ccl_reduced_due_to_high_soc!: boolean; | ||
| WarningCclReducedDueToHighSoc!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_ccl_reduced_due_to_temperature!: boolean; | ||
| WarningCclReducedDueToTemperature!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_dcl_and_ccl_reduced_due_to_communication_failsafe!: boolean; | ||
| WarningDclAndCclReducedDueToCommunicationFailsafe!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_dcl_and_ccl_reduced_due_to_voltage_failsafe!: boolean; | ||
| WarningDclAndCclReducedDueToVoltageFailsafe!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_dcl_reduced_due_to_high_cell_resistance!: boolean; | ||
| WarningDclReducedDueToHighCellResistance!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_dcl_reduced_due_to_low_cell_voltage!: boolean; | ||
| WarningDclReducedDueToLowCellVoltage!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_dcl_reduced_due_to_low_pack_voltage!: boolean; | ||
| WarningDclReducedDueToLowPackVoltage!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_dcl_reduced_due_to_low_soc!: boolean; | ||
| WarningDclReducedDueToLowSoc!: boolean; | ||
|
|
||
| @Column({ type: "boolean" }) | ||
| warning_dcl_reduced_due_to_temperature!: boolean; | ||
| WarningDclReducedDueToTemperature!: boolean; |
There was a problem hiding this comment.
Verify database column names match the renamed properties.
All entity properties have been renamed from snake_case to PascalCase, but the @Column decorators don't specify explicit column name mappings. TypeORM defaults to using the property name as the database column name, so this change will fail at runtime unless:
- Database columns were migrated to PascalCase naming, OR
- Explicit
@Column({ name: "old_snake_case_name" })mappings should be added
Run the following script to check for database migrations that renamed these columns:
#!/bin/bash
# Description: Search for migrations that rename battery_faults columns
# Look for migration files that mention battery_faults column renaming
rg -i "battery_faults|BatteryFaults" --type=ts -A 10 -B 2 -g "*migration*"
# Look for any @Column decorators with name mappings in entity files
rg "@Column.*name:" packages/db/src/entities/ -A 1 -B 1If no migration exists and column mappings aren't present, either:
- Add a migration to rename database columns to PascalCase, OR
- Add explicit column name mappings to preserve snake_case in the database
Example: Adding explicit column name mappings
@TimeColumn()
@PrimaryColumn({ type: "timestamptz" })
-Timestamp!: Date;
+@Column({ name: "timestamp" })
+Timestamp!: Date;
@PrimaryColumn({ type: "text" })
+@Column({ name: "rfid" })
Rfid!: string;
@Column({ type: "boolean" })
+@Column({ name: "error_always_on_supply_fault" })
ErrorAlwaysOnSupplyFault!: boolean;Note: Apply similar mappings to all fields if database columns remain in snake_case.
🤖 Prompt for AI Agents
In @packages/db/src/entities/BatteryFaults.entity.ts around lines 22 - 132, The
entity properties in BatteryFaults (e.g., Rfid, ErrorAlwaysOnSupplyFault,
WarningDclReducedDueToLowSoc, etc.) were renamed to PascalCase but the
@PrimaryColumn/@Column decorators do not set the database column names, so
TypeORM will try to use the PascalCase names at runtime; either add explicit
name mappings to each decorator (e.g., @Column({ name:
"error_always_on_supply_fault", type: "boolean" })) for every field in the
BatteryFaults entity (including primary columns like Rfid/Timestamp), or instead
create a migration that renames the DB columns to the new PascalCase names—pick
one approach and apply it consistently across all listed properties.
| @Hypertable({ | ||
| compression: { | ||
| compress: true, | ||
| compress_segmentby: "Rfid", | ||
| compress_orderby: "Timestamp", | ||
| policy: { | ||
| schedule_interval: "7 days", | ||
| }, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add chunkTimeInterval to Hypertable configuration.
Other entity hypertables (e.g., KeyMotor, MBMS) specify chunkTimeInterval: "1 month", but this configuration is missing here. Without it, TimescaleDB will use its default chunk interval, which may not be optimal for your data access patterns.
⚙️ Proposed fix
@Hypertable({
+ timeColumnName: "Timestamp",
+ chunkTimeInterval: "1 month",
compression: {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Hypertable({ | |
| compression: { | |
| compress: true, | |
| compress_segmentby: "Rfid", | |
| compress_orderby: "Timestamp", | |
| policy: { | |
| schedule_interval: "7 days", | |
| }, | |
| }, | |
| }) | |
| @Hypertable({ | |
| timeColumnName: "Timestamp", | |
| chunkTimeInterval: "1 month", | |
| compression: { | |
| compress: true, | |
| compress_segmentby: "Rfid", | |
| compress_orderby: "Timestamp", | |
| policy: { | |
| schedule_interval: "7 days", | |
| }, | |
| }, | |
| }) |
🤖 Prompt for AI Agents
In @packages/db/src/entities/TelemetryPacket.entity.ts around lines 8 - 17, The
Hypertable configuration on the TelemetryPacket entity is missing a
chunkTimeInterval setting; update the @Hypertable decorator for TelemetryPacket
(TelemetryPacket.entity) to include chunkTimeInterval: "1 month" alongside the
existing compression config so TimescaleDB uses the same monthly chunking as
other entities (e.g., KeyMotor, MBMS).
| export class TelemetryRepository | ||
| extends BaseRepository<TelemetryPacket> | ||
| implements ITelemetryPacketRepository | ||
| { | ||
| constructor(repository: Repository<TelemetryPacket>) { | ||
| super(repository); | ||
| } | ||
| } |
There was a problem hiding this comment.
Rename class to match file name and interface.
The class is named TelemetryRepository, but the file is named TelemetryPacketRepository.ts and it implements ITelemetryPacketRepository. This naming inconsistency can cause confusion and maintainability issues.
🔧 Proposed fix
-export class TelemetryRepository
+export class TelemetryPacketRepository
extends BaseRepository<TelemetryPacket>
implements ITelemetryPacketRepository
{📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export class TelemetryRepository | |
| extends BaseRepository<TelemetryPacket> | |
| implements ITelemetryPacketRepository | |
| { | |
| constructor(repository: Repository<TelemetryPacket>) { | |
| super(repository); | |
| } | |
| } | |
| export class TelemetryPacketRepository | |
| extends BaseRepository<TelemetryPacket> | |
| implements ITelemetryPacketRepository | |
| { | |
| constructor(repository: Repository<TelemetryPacket>) { | |
| super(repository); | |
| } | |
| } |
🤖 Prompt for AI Agents
In @packages/db/src/repositories/TelemetryPacketRepository.ts around lines 6 -
13, Rename the exported class TelemetryRepository to TelemetryPacketRepository
so the class name matches the file TelemetryPacketRepository.ts and the
implemented interface ITelemetryPacketRepository; update the class declaration
(export class TelemetryPacketRepository extends BaseRepository<TelemetryPacket>
implements ITelemetryPacketRepository) and adjust any imports/usages across the
codebase that reference TelemetryRepository to the new TelemetryPacketRepository
identifier.
| constructor() { | ||
| this.dataSource = AppDataSource; | ||
| this.telemetryPacketRepo = AppDataSource.getRepository(TelemetryPacket); | ||
| } |
There was a problem hiding this comment.
Fix repository initialization order.
The constructor calls AppDataSource.getRepository(TelemetryPacket) before AppDataSource.initialize() is called. This will fail because the DataSource must be initialized before repositories can be obtained.
🔧 Proposed fix
Move repository initialization to the initialize method:
+ private telemetryPacketRepo!: Repository<TelemetryPacket>;
constructor() {
- this.telemetryPacketRepo = AppDataSource.getRepository(TelemetryPacket);
}
async initialize() {
if (!this.isConnected) {
await AppDataSource.initialize();
+ this.telemetryPacketRepo = AppDataSource.getRepository(TelemetryPacket);
this.isConnected = true;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| constructor() { | |
| this.dataSource = AppDataSource; | |
| this.telemetryPacketRepo = AppDataSource.getRepository(TelemetryPacket); | |
| } | |
| private telemetryPacketRepo!: Repository<TelemetryPacket>; | |
| constructor() { | |
| } | |
| async initialize() { | |
| if (!this.isConnected) { | |
| await AppDataSource.initialize(); | |
| this.telemetryPacketRepo = AppDataSource.getRepository(TelemetryPacket); | |
| this.isConnected = true; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @packages/db/src/services/DatabaseService.ts around lines 11 - 13, The
constructor currently calls AppDataSource.getRepository(TelemetryPacket) before
the DataSource is initialized, causing failures; remove repository
initialization from the DatabaseService constructor and instead initialize
telemetryPacketRepo inside the DatabaseService.initialize method (after calling
AppDataSource.initialize()), e.g., call AppDataSource.initialize() first then
assign this.telemetryPacketRepo = AppDataSource.getRepository(TelemetryPacket)
within initialize so repositories are acquired only after the DataSource is
ready.
| this.databaseService.insertPacketData(message); | ||
|
|
There was a problem hiding this comment.
Add error handling for database insertion.
The call to databaseService.insertPacketData(message) lacks error handling. If the insertion fails, it could silently drop telemetry data. Consider wrapping this in a try-catch block or handling failures appropriately.
Additionally, Line 67 appears to be an unnecessary blank line.
🛡️ Proposed fix with error handling
this.dynamoDB.insertPacketData(message);
- this.databaseService.insertPacketData(message);
-
+ try {
+ await this.databaseService.insertPacketData(message);
+ } catch (error) {
+ logger.error("Failed to insert packet data to database:", error);
+ // Consider whether to throw or continue based on criticality
+ }
+
// Broadcast the packet to the frontend🤖 Prompt for AI Agents
In @packages/server/src/controllers/BackendController/BackendController.ts
around lines 66 - 67, The call to this.databaseService.insertPacketData(message)
in BackendController lacks error handling and has an extra blank line; wrap the
insertPacketData invocation in a try-catch (await it if it returns a Promise),
catch any error and log it via the controller logger (e.g., this.logger.error or
processLogger) including contextual info like the message/id and error.stack,
and decide whether to retry, emit a metric, or continue gracefully instead of
swallowing the error; also remove the unnecessary blank line after the call.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/db/src/data-source.ts:
- Line 18: Validate that process.env.DATABASE_URL is defined before using it to
construct the TypeORM DataSource (where new DataSource or the exported
dataSource config is created). If DATABASE_URL is missing, throw or log a clear
error (including guidance to set .db.env or the env var) and stop initialization
instead of passing undefined into the DataSource config. Add this check at the
top of the module (before the DataSource instantiation) so functions/classes
like the DataSource constructor or exported dataSource see a guaranteed
non-empty URL.
🧹 Nitpick comments (1)
packages/db/src/data-source.ts (1)
13-13: Remove orphaned entity files and repository classes — the consolidation is implemented correctly but obsolete files remain.The schema consolidation is working as designed through
DatabaseService.flattenTelemetryData(), which transforms the old message structure into the flattenedTelemetryPacketschema. No migrations are needed for this fresh schema redesign.However, the old entity files (Battery, B3, MotorDetails, KeyMotor, MPPT, Contactor, MBMS, Telemetry, TelemetryMetadata, ProximitySensors, BatteryFaults) and their corresponding repositories are now dead code. They should be deleted along with their unused interface definitions to avoid confusion and reduce maintenance burden.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/db/src/data-source.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/db/src/data-source.ts (1)
packages/db/src/index.ts (1)
AppDataSource(1-1)
🔇 Additional comments (2)
packages/db/src/data-source.ts (2)
8-8: LGTM!The import follows the established pattern and correctly includes the new
TelemetryPacketentity.
16-16: synchronize: true does not drop tables for removed entities.TypeORM's
synchronizeoption creates missing tables and updates existing ones to match current entity metadata, but it does not automatically drop tables for entities that are no longer in the entities array. Removed tables would remain in the database, which could be desirable for maintaining legacy data during the transition.If you specifically need to remove orphaned tables from development databases, use explicit migrations rather than relying on synchronize behavior.
Likely an incorrect or invalid review comment.
…Telemetry into move-to-timescale
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/db/src/entities/ProximitySensors.entity.ts (1)
9-18: Fix @Index property names and @column name mappings to match PascalCase properties, and correct the typo in ProximitySensor3.The @Index decorator requires entity property names (case-sensitive), so
@Index(["rfid", "timestamp"])does not match propertiesRfidandTimestamp— this will cause TypeORM to fail to create the index correctly. Additionally, without explicitnamemappings in the column decorators, your PascalCase properties will not map to the lowercase database column names referenced in@Hypertableand@Index. The typoRroximitySensor3must also be fixed.Proposed fix
-@Index(["rfid", "timestamp"]) +@Index(["Rfid", "Timestamp"]) export class ProximitySensors { @TimeColumn() - @PrimaryColumn({ type: "timestamptz" }) + @PrimaryColumn({ type: "timestamptz", name: "timestamp" }) Timestamp!: Date; - @PrimaryColumn({ type: "text" }) + @PrimaryColumn({ type: "text", name: "rfid" }) Rfid!: string; - @Column({ type: "float" }) + @Column({ type: "float", name: "proximity_sensor_1" }) ProximitySensor1!: number; - @Column({ type: "float" }) + @Column({ type: "float", name: "proximity_sensor_2" }) ProximitySensor2!: number; - @Column({ type: "float" }) - RroximitySensor3!: number; + @Column({ type: "float", name: "proximity_sensor_3" }) + ProximitySensor3!: number; - @Column({ type: "float" }) + @Column({ type: "float", name: "proximity_sensor_4" }) ProximitySensor4!: number; }Also applies to: lines 20-37 and other similar entities with the same pattern (Telemetry, TelemetryMetadata, Battery, etc.).
🤖 Fix all issues with AI agents
In @packages/db/src/data-source.ts:
- Line 20: synchronize is enabled in the TypeORM DataSource config which can
break TimescaleDB hypertables (e.g., TelemetryPacket); change the DataSource
config to set synchronize: false (remove the development-only conditional) and
rely on migrations for schema changes, ensuring any code that references the
DataSource options (the DataSource construction in data-source.ts and any
TelemetryPacket entity setup) no longer allows synchronize to be true.
In @packages/db/src/entities/ProximitySensors.entity.ts:
- Around line 33-35: The entity property name RroximitySensor3 is misspelled and
should be ProximitySensor3; rename the class field RroximitySensor3 to
ProximitySensor3 in ProximitySensors.entity (and adjust its decorator @Column
unchanged), then update all usages/references (queries, DTOs, mappers, tests,
serializers/deserializers) that read or write RroximitySensor3 to use
ProximitySensor3 so the ORM mapping and API remain consistent.
🧹 Nitpick comments (1)
packages/db/src/data-source.ts (1)
12-14: Good validation for required environment variable.The runtime check ensures DATABASE_URL is present before attempting to connect, which provides a clear error message if misconfigured.
Optional: Add URL format validation
If you want stricter validation, consider checking the URL format:
if (!process.env.DATABASE_URL) { throw new Error("DATABASE_URL is required in .db.env file"); } + +// Optional: validate URL format +try { + new URL(process.env.DATABASE_URL); +} catch { + throw new Error("DATABASE_URL must be a valid PostgreSQL connection URL"); +}
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/db/src/data-source.tspackages/db/src/entities/ProximitySensors.entity.ts
🧰 Additional context used
🧬 Code graph analysis (1)
packages/db/src/data-source.ts (1)
packages/db/src/index.ts (1)
AppDataSource(1-1)
🔇 Additional comments (1)
packages/db/src/data-source.ts (1)
22-22: Verify connection options for production usage.The URL-based connection is a good approach. However, ensure the DATABASE_URL includes appropriate query parameters for production, such as:
- Connection pooling settings (
poolSize,max,min)- SSL configuration (
ssl=trueorsslmode=require)- Connection timeouts
Example production DATABASE_URL format:
postgresql://user:password@host:port/database?sslmode=require&pool_size=20
| password: process.env.POSTGRES_PASSWORD || "postgres", | ||
| port: parseInt(process.env.DB_PORT || "5432"), | ||
| synchronize: process.env.NODE_ENV === "development", // Only in development | ||
| synchronize: process.env.NODE_ENV === "development", |
There was a problem hiding this comment.
Disable synchronize when using TimescaleDB hypertables.
TypeORM's synchronize: true can interfere with TimescaleDB hypertables because it doesn't understand TimescaleDB-specific features like create_hypertable(). When synchronize runs, it may recreate the TelemetryPacket table without the hypertable configuration, breaking time-series functionality even in development.
🔧 Recommended fix
- synchronize: process.env.NODE_ENV === "development",
+ synchronize: false,Always use migrations for schema changes with TimescaleDB to ensure hypertable configuration is properly applied.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| synchronize: process.env.NODE_ENV === "development", | |
| synchronize: false, |
🤖 Prompt for AI Agents
In @packages/db/src/data-source.ts at line 20, synchronize is enabled in the
TypeORM DataSource config which can break TimescaleDB hypertables (e.g.,
TelemetryPacket); change the DataSource config to set synchronize: false (remove
the development-only conditional) and rely on migrations for schema changes,
ensuring any code that references the DataSource options (the DataSource
construction in data-source.ts and any TelemetryPacket entity setup) no longer
allows synchronize to be true.
| @Column({ type: "float" }) | ||
| proximity_sensor_3!: number; | ||
| RroximitySensor3!: number; | ||
|
|
There was a problem hiding this comment.
Fix typo: RroximitySensor3 should be ProximitySensor3.
This is a breaking API/mapping mistake and will likely cause runtime/compile-time mismatches wherever sensor 3 is used.
🤖 Prompt for AI Agents
In @packages/db/src/entities/ProximitySensors.entity.ts around lines 33 - 35,
The entity property name RroximitySensor3 is misspelled and should be
ProximitySensor3; rename the class field RroximitySensor3 to ProximitySensor3 in
ProximitySensors.entity (and adjust its decorator @Column unchanged), then
update all usages/references (queries, DTOs, mappers, tests,
serializers/deserializers) that read or write RroximitySensor3 to use
ProximitySensor3 so the ORM mapping and API remain consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/server/.env.example:
- Line 17: The .env.example is missing a trailing newline after the final line;
add a single newline character at the end of the file so the last entry
(DATABASE_URL='') is followed by a newline to satisfy POSIX/linter expectations.
🧹 Nitpick comments (1)
packages/server/.env.example (1)
17-17: Consider documenting the expected DATABASE_URL format.To help developers understand what connection string format is expected (e.g.,
postgres://user:password@host:port/database), consider adding an inline comment above the variable.💡 Proposed enhancement
+# PostgreSQL connection string (used by TimescaleDB) DATABASE_URL=''
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/server/.env.example
🧰 Additional context used
🪛 dotenv-linter (4.0.0)
packages/server/.env.example
[warning] 15-15: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 17-17: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 17-17: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
packages/server/.env.example
Outdated
| GPS_TABLE_NAME='' No newline at end of file | ||
| GPS_TABLE_NAME='' | ||
|
|
||
| DATABASE_URL='' No newline at end of file |
There was a problem hiding this comment.
Add a trailing newline to the file.
Environment configuration files should end with a newline character to comply with POSIX standards and avoid linter warnings.
✏️ Proposed fix
-DATABASE_URL=''
+DATABASE_URL=''
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DATABASE_URL='' | |
| DATABASE_URL='' | |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 17-17: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 17-17: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🤖 Prompt for AI Agents
In @packages/server/.env.example at line 17, The .env.example is missing a
trailing newline after the final line; add a single newline character at the end
of the file so the last entry (DATABASE_URL='') is followed by a newline to
satisfy POSIX/linter expectations.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/server/src/controllers/BackendController/BackendController.ts (1)
12-12: Remove unused import causing pipeline failure.The
DatabaseManagerimport is no longer used (replaced byDatabaseService) and the module has been removed, causing the build to fail. Remove this line to fix the CI error.🔧 Proposed fix
-import { DatabaseManager } from "@/database/DatabaseManager";
♻️ Duplicate comments (1)
packages/server/src/controllers/BackendController/BackendController.ts (1)
67-68: Add error handling for database insertion.The call to
databaseService.insertPacketData(message)lacks error handling, which is inconsistent with the error handling patterns used ininitializeDatabase()andcleanup(). If the insertion fails, telemetry data could be silently dropped.🛡️ Proposed fix with error handling
this.dynamoDB.insertPacketData(message); - this.databaseService.insertPacketData(message); - + try { + await this.databaseService.insertPacketData(message); + } catch (error) { + logger.error("Failed to insert packet data to database:", error); + } + // Broadcast the packet to the frontend
🧹 Nitpick comments (2)
packages/server/.env.example (2)
20-22: Clarify the section name to distinguish it from the DynamoDB section.Having two sections labeled "Database" (lines 5-12 for DynamoDB, lines 20-24 for the new configuration) creates ambiguity. Consider renaming this section header to "# Database (TimescaleDB) #" or "# Database Connection #" to clarify its purpose and distinguish it from the legacy DynamoDB configuration.
📝 Suggested improvement
######################## -# Database # +# Database (TimescaleDB) # ######################## DATABASE_URL=''
23-23: Provide an example URL format instead of empty quotes.The empty string
DATABASE_URL=''provides no guidance on the expected PostgreSQL connection string format. Consider showing an example placeholder to help developers understand the required format.📝 Suggested improvement
-DATABASE_URL='' +DATABASE_URL="postgresql://username:password@localhost:5432/database_name"
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/db/src/entities/ProximitySensors.entity.ts (1)
8-19: Fix@Indexto reference entity property names.
@Index(["rfid", "timestamp"])does not match the class properties (Rfid,Timestamp). TypeORM@Indexexpects entity property names, not database column names. This will cause a metadata build error at runtime.🔧 Proposed fix
-@Index(["rfid", "timestamp"]) +@Index(["Rfid", "Timestamp"])
🤖 Fix all issues with AI agents
In `@packages/db/src/services/DatabaseService.ts`:
- Around line 136-138: Remove the console.log calls in insertPacketData (the one
after the this.isConnected check and the later one around line 147) to satisfy
no-console lint rules, or replace them with the project logger instance used
elsewhere (e.g., this.logger.info/debug or processLogger.info). Specifically
update the method insertPacketData to use the existing logger instead of
console.log (preserve the message content but call this.logger.info("Inserting
packet data into database") or remove the log entirely if redundant).
- Around line 144-148: The close() method currently destroys AppDataSource but
never flips the DatabaseService.isConnected flag, allowing insertPacketData() to
attempt writes on a destroyed connection; update close() (in DatabaseService) to
set this.isConnected = false after await AppDataSource.destroy() and ensure any
cleanup order is safe (destroy then set flag) so subsequent calls see the closed
state.
♻️ Duplicate comments (3)
packages/db/src/entities/TelemetryPacket.entity.ts (1)
8-19: Confirm Hypertable time column + casing, and add chunk interval.
@Hypertabledoes not specifytimeColumnNameorchunkTimeInterval. Please verify that@timescaledb/typeormcorrectly infersTimestampand preserves quoted PascalCase identifiers in its SQL; if not, hypertable creation/compression will target the wrong columns. Also consider adding a chunk interval for consistency with other hypertables (previously requested).🔧 Suggested baseline
`@Hypertable`({ + timeColumnName: "Timestamp", + chunkTimeInterval: "1 month", compression: { compress: true, compress_segmentby: "Rfid", compress_orderby: "Timestamp",packages/db/src/entities/ProximitySensors.entity.ts (1)
20-37: Align column names with Hypertable config after PascalCase renames.The hypertable config uses lowercase identifiers (
timeColumnName: "timestamp",compress_segmentby: "rfid"), but the columns will be created as"Timestamp"/"Rfid"unless you map them explicitly. Please either (a) add explicitnamemappings for each column to keep snake_case, or (b) update the Hypertable config to PascalCase names. This was previously flagged and still applies.🔧 Example mapping (apply consistently)
-@PrimaryColumn({ type: "timestamptz" }) +@PrimaryColumn({ type: "timestamptz", name: "timestamp" }) Timestamp!: Date; -@PrimaryColumn({ type: "text" }) +@PrimaryColumn({ type: "text", name: "rfid" }) Rfid!: string; -@Column({ type: "float" }) +@Column({ type: "float", name: "proximity_sensor_1" }) ProximitySensor1!: number;packages/db/src/services/DatabaseService.ts (1)
11-18: Initialize repository only afterAppDataSource.initialize().
AppDataSource.getRepository()is invoked in the constructor before the DataSource is initialized (Line 12). This can throw at runtime. Move repository initialization intoinitialize()afterAppDataSource.initialize()completes.🔧 Proposed fix
private telemetryPacketRepo: Repository<TelemetryPacket>; constructor() { - this.telemetryPacketRepo = AppDataSource.getRepository(TelemetryPacket); } async initialize() { if (!this.isConnected) { await AppDataSource.initialize(); + this.telemetryPacketRepo = AppDataSource.getRepository(TelemetryPacket); this.isConnected = true; } }
| async insertPacketData(message: ITelemetryData): Promise<void> { | ||
| if (this.isConnected) { | ||
| console.log("Inserting packet data into database"); |
There was a problem hiding this comment.
Remove console.log or replace with the project logger (ESLint no-console).
Lines 138 and 147 will fail linting. Please swap for the project logger or remove.
🔧 Proposed fix (remove logs)
- console.log("Inserting packet data into database");
const flattenedData = this.flattenTelemetryData(message);
await this.telemetryPacketRepo.save(flattenedData);
@@
- console.log("Database connection closed");Also applies to: 147-147
🧰 Tools
🪛 ESLint
[error] 138-138: Unexpected console statement.
(no-console)
🤖 Prompt for AI Agents
In `@packages/db/src/services/DatabaseService.ts` around lines 136 - 138, Remove
the console.log calls in insertPacketData (the one after the this.isConnected
check and the later one around line 147) to satisfy no-console lint rules, or
replace them with the project logger instance used elsewhere (e.g.,
this.logger.info/debug or processLogger.info). Specifically update the method
insertPacketData to use the existing logger instead of console.log (preserve the
message content but call this.logger.info("Inserting packet data into database")
or remove the log entirely if redundant).
| async close(): Promise<void> { | ||
| if (this.isConnected) { | ||
| await AppDataSource.destroy(); | ||
| console.log("Database connection closed"); | ||
| } |
There was a problem hiding this comment.
Reset isConnected after closing the data source.
close() destroys the DataSource but leaves isConnected set to true, so insertPacketData() can still attempt writes against a destroyed connection.
🔧 Proposed fix
async close(): Promise<void> {
if (this.isConnected) {
await AppDataSource.destroy();
+ this.isConnected = false;
console.log("Database connection closed");
}
}🧰 Tools
🪛 ESLint
[error] 147-147: Unexpected console statement.
(no-console)
🤖 Prompt for AI Agents
In `@packages/db/src/services/DatabaseService.ts` around lines 144 - 148, The
close() method currently destroys AppDataSource but never flips the
DatabaseService.isConnected flag, allowing insertPacketData() to attempt writes
on a destroyed connection; update close() (in DatabaseService) to set
this.isConnected = false after await AppDataSource.destroy() and ensure any
cleanup order is safe (destroy then set flag) so subsequent calls see the closed
state.
2 participants
Summary by CodeRabbit
Refactor
New Features
Chores
Bug Fix
✏️ Tip: You can customize this high-level summary in your review settings.