fix: fix up change detection with date transformer by gioboa · Pull Request #11963 · typeorm/typeorm

gioboa · 2026-02-06T20:36:28Z

Close #11951

Description of change

Pull-Request Checklist

Code is up-to-date with the master branch
This pull request links relevant issues as Fixes #00000
There are new or updated tests validating the change (tests/**.test.ts)
Documentation has been updated to reflect this change (docs/docs/**.md)

qodo-free-for-open-source-projects · 2026-02-06T20:36:44Z

User description

Close #11951

Description of change

Pull-Request Checklist

Code is up-to-date with the master branch
This pull request links relevant issues as Fixes #00000
There are new or updated tests validating the change (tests/**.test.ts)
Documentation has been updated to reflect this change (docs/docs/**.md)

PR Type

Bug fix

Description

Apply value transformers before normalizing date/time values for change detection
Move transformer logic before type-specific normalization to ensure correct comparison
Add test case for value transformer change detection with custom DateTime class
Improve JSDoc documentation with parameter annotations

Diagram Walkthrough

flowchart LR
  A["Entity Value"] --> B["Apply Transformer"]
  B --> C["Normalize by Type"]
  C --> D["Compare with Database"]
  D --> E["Detect Changes"]

File Walkthrough

Relevant files

Bug fix

SubjectChangedColumnsComputer.ts `Apply transformers before normalizing values` src/persistence/SubjectChangedColumnsComputer.ts Apply column transformers to entity values before type-specific normalization Move transformer logic from after normalization to before it Replace all references to `entityValue` with `normalizedValue` in date/time/json comparisons Add JSDoc parameter annotations for `compute()` and `computeDiffColumns()` methods	+30/-18

Tests

PostWithLuxonDate.ts `Add test entity with custom DateTime transformer` test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts Create custom `DateTime` class wrapping native Date with equality comparison Implement `LuxonTimestampTzTypeT` value transformer for converting between DateTime and Date Define `PostWithLuxonDate` entity with timestamp column using custom transformer	+54/-0
value-transformer-change-detection.test.ts `Add value transformer change detection test` test/functional/columns/value-transformer/value-transformer-change-detection.test.ts Add test suite for value transformer change detection behavior Test that entities with unchanged transformed values don't trigger update events Verify change detection works correctly across postgres, mysql, and sqlite drivers Use dynamic subscriber pattern to track update event invocation	+64/-0

pkg-pr-new · 2026-02-06T20:37:18Z

typeorm-sql-js-example

npm i https://pkg.pr.new/typeorm/typeorm@11963

commit: abefdcf

qodo-free-for-open-source-projects · 2026-02-06T20:38:08Z

PR Code Suggestions ✨

Latest suggestions up to abefdcf

Category	Suggestion	Impact
Possible issue	Always transform the entity value Remove the `shouldTransformDatabaseEntity` condition when applying the transformer to `entityValue` to ensure it also runs for `json` and `jsonb` column types. src/persistence/SubjectChangedColumnsComputer.ts [87-96] -if ( - column.transformer && - shouldTransformDatabaseEntity && - entityValue !== null -) { +if (column.transformer && entityValue !== null) { normalizedValue = ApplyValueTransformers.transformTo( column.transformer, entityValue, ) } Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies that the entity value transformer should be applied even for `json`/`jsonb` columns to ensure accurate change detection, a case the PR's logic currently misses.	Medium
General	Make the update assertion precise Refine the `beforeUpdate` subscriber in the test to verify that the update event corresponds to the specific entity being tested and that there are changed columns. test/functional/columns/value-transformer/value-transformer-change-detection.test.ts [41-49] const subscriber: EntitySubscriberInterface<PostWithLuxonDate> = { listenTo() { return PostWithLuxonDate }, beforeUpdate(event: UpdateEvent<PostWithLuxonDate>) { - updateCalled = true + if (event.entity && (event.entity as any).id === loadedPost.id) { + if (event.updatedColumns.length > 0) updateCalled = true + } }, } Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion improves test robustness by making the assertions in the `beforeUpdate` subscriber more specific, though the risk of flakiness in this particular isolated test is low.	Low
More

Previous suggestions

✅ Suggestions up to commit 590cc0a

Category	Suggestion	Impact
Possible issue	Avoid double-transforming date columns with transformers Avoid double-transforming date-like columns by skipping the built-in date normalization when a custom value transformer is already present. src/persistence/SubjectChangedColumnsComputer.ts [87-104] if (column.transformer && shouldTransformDatabaseEntity) { normalizedValue = ApplyValueTransformers.transformTo( column.transformer, entityValue, ) } // if both values are not null, normalize special values to make proper comparision if (normalizedValue !== null && databaseValue !== null) { - switch (column.type) { - case "date": - normalizedValue = column.isArray - ? normalizedValue.map((date: Date) => - DateUtils.mixedDateToDateString(date), - ) - : DateUtils.mixedDateToDateString( - normalizedValue, - ) + // Skip date normalization if a transformer already handled the conversion + if (!column.transformer \|\| !shouldTransformDatabaseEntity) { + switch (column.type) { + case "date": + normalizedValue = column.isArray + ? normalizedValue.map((date: Date) => + DateUtils.mixedDateToDateString(date), + ) + : DateUtils.mixedDateToDateString( + normalizedValue, + ) Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a design flaw in the PR's logic where a value could be transformed twice (by a custom transformer and then by internal date utils), potentially causing runtime errors. The proposed fix to bypass date normalization when a transformer is present improves the robustness and correctness of the change detection logic.	Medium
General	✅ ~~Ensure subscriber cleanup on test failure~~ Suggestion Impact: The commit wrapped repo.save in a try/finally block and moved subscriber removal into the finally clause to guarantee cleanup. code diff: dataSource.subscribers.push(subscriber) - await repo.save(loadedPost) - - const index = dataSource.subscribers.indexOf(subscriber) - if (index !== -1) { - dataSource.subscribers.splice(index, 1) + try { + await repo.save(loadedPost) + } finally { + const index = dataSource.subscribers.indexOf(subscriber) + if (index !== -1) { + dataSource.subscribers.splice(index, 1) + } } Wrap the test logic in a `try...finally` block to ensure the temporary event subscriber is always removed, even if the test fails. test/functional/columns/value-transformer/value-transformer-change-detection.test.ts [51-60] dataSource.subscribers.push(subscriber) -await repo.save(loadedPost) - -const index = dataSource.subscribers.indexOf(subscriber) -if (index !== -1) { - dataSource.subscribers.splice(index, 1) +try { + await repo.save(loadedPost) +} finally { + const index = dataSource.subscribers.indexOf(subscriber) + if (index !== -1) { + dataSource.subscribers.splice(index, 1) + } } expect(updateCalled).to.be.false Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out a potential for test pollution if an error occurs. Using `try...finally` for resource cleanup in tests is a good practice that improves test suite stability.	Low

Suggestions up to commit 610c9c2

Category Suggestion Impact

Possible issue

Normalize both values with transformer

In SubjectChangedColumnsComputer.ts, apply the column.transformer to
databaseValue in addition to entityValue to ensure a symmetric and correct
comparison.

src/persistence/SubjectChangedColumnsComputer.ts [85-193]

 let normalizedValue = entityValue
+let normalizedDatabaseValue = databaseValue
 
 if (column.transformer && shouldTransformDatabaseEntity) {
     normalizedValue = ApplyValueTransformers.transformTo(
         column.transformer,
         entityValue,
     )
+
+    if (normalizedDatabaseValue !== null && normalizedDatabaseValue !== undefined) {
+        normalizedDatabaseValue = ApplyValueTransformers.transformTo(
+            column.transformer,
+            normalizedDatabaseValue as any,
+        )
+    }
 }
 
 // if both values are not null, normalize special values to make proper comparision
-if (normalizedValue !== null && databaseValue !== null) {
+if (normalizedValue !== null && normalizedDatabaseValue !== null) {
     switch (column.type) {
         case "date":
             normalizedValue = column.isArray
                 ? normalizedValue.map((date: Date) =>
                       DateUtils.mixedDateToDateString(date),
                   )
-                : DateUtils.mixedDateToDateString(
-                      normalizedValue,
-                  )
-            databaseValue = column.isArray
-                ? databaseValue.map((date: Date) =>
+                : DateUtils.mixedDateToDateString(normalizedValue)
+            normalizedDatabaseValue = column.isArray
+                ? (normalizedDatabaseValue as any[]).map((date: Date) =>
                       DateUtils.mixedDateToDateString(date),
                   )
-                : DateUtils.mixedDateToDateString(databaseValue)
+                : DateUtils.mixedDateToDateString(normalizedDatabaseValue)
             break
         ...
     }
 }

Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies a critical flaw in the PR's logic where value transformation is applied asymmetrically, leading to incorrect change detection. Applying the transformer to both values is essential for the feature to work correctly.

High

✅ Suggestions up to commit 4d38b06

Category	Suggestion	Impact
Possible issue	✅ ~~Transform array elements individually~~ Suggestion Impact: The commit modified the transformer guard condition to match part of the suggestion (removed the `entityValue !== null` check), but did not implement the key array-element mapping behavior. code diff: @@ -84,11 +84,7 @@ } let normalizedValue = entityValue - if ( - column.transformer && - shouldTransformDatabaseEntity && - entityValue !== null - ) { + if (column.transformer && shouldTransformDatabaseEntity) { normalizedValue = ApplyValueTransformers.transformTo( column.transformer, entityValue, When a column is an array, apply the value transformer to each element individually instead of to the entire array to prevent runtime errors. src/persistence/SubjectChangedColumnsComputer.ts [85-96] let normalizedValue = entityValue -if ( - column.transformer && - shouldTransformDatabaseEntity && - entityValue !== null -) { - normalizedValue = ApplyValueTransformers.transformTo( - column.transformer, - entityValue, - ) +if (column.transformer && shouldTransformDatabaseEntity) { + if (column.isArray && Array.isArray(entityValue)) { + normalizedValue = entityValue.map((v: any) => + ApplyValueTransformers.transformTo(column.transformer!, v), + ) + } else { + normalizedValue = ApplyValueTransformers.transformTo( + column.transformer, + entityValue, + ) + } } Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a bug where a value transformer for an array column would be applied to the entire array, likely causing a runtime error, and provides a correct fix.	Medium
Possible issue	✅ ~~Prevent test subscriber leakage~~ Suggestion Impact: The test now wraps the repo.save call in a try/finally and removes the subscriber in the finally block, ensuring cleanup even if the save throws. code diff: dataSource.subscribers.push(subscriber) - await repo.save(loadedPost) - - const index = dataSource.subscribers.indexOf(subscriber) - if (index !== -1) { - dataSource.subscribers.splice(index, 1) + try { + await repo.save(loadedPost) + } finally { + const index = dataSource.subscribers.indexOf(subscriber) + if (index !== -1) { + dataSource.subscribers.splice(index, 1) + } } Wrap the test logic in a `try/finally` block to ensure the dynamically added subscriber is always removed, even if an error occurs. test/functional/columns/value-transformer/value-transformer-change-detection.test.ts [51-60] dataSource.subscribers.push(subscriber) -await repo.save(loadedPost) - -const index = dataSource.subscribers.indexOf(subscriber) -if (index !== -1) { - dataSource.subscribers.splice(index, 1) +try { + await repo.save(loadedPost) + expect(updateCalled).to.be.false +} finally { + const index = dataSource.subscribers.indexOf(subscriber) + if (index !== -1) { + dataSource.subscribers.splice(index, 1) + } } -expect(updateCalled).to.be.false - Suggestion importance[1-10]: 6 __ Why: The suggestion improves test reliability by ensuring a dynamically added subscriber is always removed, preventing test pollution, which is a good practice for test hygiene.	Low

Suggestions up to commit f769b7a

Category Suggestion Impact

General

✅ ~~Ensure subscriber cleanup on test failure~~

Suggestion Impact:

The commit wrapped repo.save in a try/finally block and moved subscriber removal into the finally clause to guarantee cleanup.

code diff:

                 dataSource.subscribers.push(subscriber)
 
-                await repo.save(loadedPost)
-
-                const index = dataSource.subscribers.indexOf(subscriber)
-                if (index !== -1) {
-                    dataSource.subscribers.splice(index, 1)
+                try {
+                    await repo.save(loadedPost)
+                } finally {
+                    const index = dataSource.subscribers.indexOf(subscriber)
+                    if (index !== -1) {
+                        dataSource.subscribers.splice(index, 1)
+                    }
                 }

Use a try/finally block to ensure the temporary test subscriber is always
removed, even if the repo.save() call fails.

test/functional/columns/value-transformer/value-transformer-change-detection.test.ts [51-60]

 dataSource.subscribers.push(subscriber)
 
-await repo.save(loadedPost)
-
-const index = dataSource.subscribers.indexOf(subscriber)
-if (index !== -1) {
-    dataSource.subscribers.splice(index, 1)
+try {
+    await repo.save(loadedPost)
+} finally {
+    const index = dataSource.subscribers.indexOf(subscriber)
+    if (index !== -1) {
+        dataSource.subscribers.splice(index, 1)
+    }
 }
 
 expect(updateCalled).to.be.false

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential test pollution issue by ensuring the test subscriber is removed even if an error occurs, which is a good practice for test reliability.

Medium

Suggestions up to commit 988b462

Category	Suggestion	Impact
Possible issue	Always transform the incoming value Always apply the value transformer to the incoming entity value for change detection, regardless of the column type, by removing the `shouldTransformDatabaseEntity` condition. src/persistence/SubjectChangedColumnsComputer.ts [85-92] let normalizedValue = entityValue -if (column.transformer && shouldTransformDatabaseEntity) { +if (column.transformer) { normalizedValue = ApplyValueTransformers.transformTo( column.transformer, entityValue, ) } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a logic flaw in the PR where `json`/`jsonb` columns with transformers would not have their new values transformed before comparison, leading to incorrect change detection.	Medium
	Prevent subscriber leakage in tests Wrap the test logic that adds and removes a subscriber in a `try/finally` block to prevent subscriber leakage and ensure test isolation. test/functional/columns/value-transformer/value-transformer-change-detection.test.ts [51-58] dataSource.subscribers.push(subscriber) -await repo.save(loadedPost) - -const index = dataSource.subscribers.indexOf(subscriber) -if (index !== -1) { - dataSource.subscribers.splice(index, 1) +try { + await repo.save(loadedPost) +} finally { + const index = dataSource.subscribers.indexOf(subscriber) + if (index !== -1) { + dataSource.subscribers.splice(index, 1) + } } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out a potential for test flakiness by ensuring a dynamically added subscriber is always removed, which improves test suite robustness.	Low
	Preserve undefined instead of nulling Modify the `to()` method in the value transformer to handle `undefined` separately from `null`, preventing `undefined` values from being converted to `null`. test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts [26-35] public to( value: DateTime \| null \| undefined \| FindOperator<any>, -): Date \| null \| FindOperator<any> { - if (value === null \|\| value === undefined) { +): Date \| null \| undefined \| FindOperator<any> { + if (value === undefined) { + return undefined + } + if (value === null) { return null - } else if (value instanceof FindOperator) { + } + if (value instanceof FindOperator) { return value } return value.toJSDate() } Suggestion importance[1-10]: 5 __ Why: The suggestion offers a good practice for writing value transformers by distinguishing between `null` and `undefined`, which can prevent unintended `NULL` writes to the database.	Low

src/persistence/SubjectChangedColumnsComputer.ts

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts

qodo-free-for-open-source-projects · 2026-02-18T09:18:52Z

Persistent review updated to latest commit af50fa5

GEMINI.md

ISSUES.md

src/persistence/SubjectChangedColumnsComputer.ts

qodo-free-for-open-source-projects · 2026-02-18T09:20:46Z

Persistent review updated to latest commit 43b7ac8

GEMINI.md

qodo-free-for-open-source-projects · 2026-02-18T09:34:19Z

Code Review by Qodo

🐞 Bugs (22) 📘 Rule violations (11) 📎 Requirement gaps (0)

1. LuxonTimestampTzTypeT non-standard naming ☑ 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence
All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.
Rule 1: Consistent Naming Conventions
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-25]
test/functional/columns/value-transformer/entity/Post.ts[6-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The class `LuxonTimestampTzTypeT` uses a non-standard `T` suffix, breaking the project-wide naming convention where all `ValueTransformer` implementations end with the `Transformer` suffix.
## Issue Context
All existing transformer classes in the codebase consistently use the `Transformer` suffix: `TagTransformer`, `ComplexTransformer`, `PhonesTransformer`, `UuidTransformer`. The new class should follow the same pattern and be renamed accordingly.
## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-45]
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Transformer called with null on nullable columns~~ ☑ 🐞 Bug ✓ Correctness

Description

The transformer's to() method is now invoked unconditionally before the null guard, so saving a
null value into any nullable, transformer-decorated column will throw a runtime TypeError if the
transformer does not explicitly handle null input. The old code applied the transformer only
inside if (entityValue !== null && databaseValue !== null), which prevented this.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-95]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
            // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {

Evidence

The early-exit on line 64 only guards against undefined, so null entity values pass through and
reach the new unconditional transformer call at lines 87–92. ApplyValueTransformers.transformTo
has no internal null guard and directly calls transformer.to(entityValue). The old code nested the
transformer call inside if (entityValue !== null && databaseValue !== null), preventing this. The
new test masks the regression because LuxonTimestampTzTypeT.to() explicitly handles null (lines
29–31 of the test entity file), and the test entity declares date: DateTime without `nullable:
true`, so null is never exercised.

src/persistence/SubjectChangedColumnsComputer.ts[64-64]
src/util/ApplyValueTransformers.ts[19-29]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[26-35]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[52-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The transformer&amp;amp;amp;amp;#x27;s `to()` method is now called unconditionally before the null-check block. For nullable columns, `entityValue` can be `null` (only `undefined` triggers the early return). Any transformer that does not explicitly guard against `null` will throw a runtime `TypeError` when saving a `null` value into a nullable, transformer-decorated column.
## Issue Context
The old code applied the transformer only inside `if (entityValue !== null &amp;amp;amp;amp;amp;&amp;amp;amp;amp;amp; databaseValue !== null)`, guaranteeing `to()` was never called with `null`. The new code hoisted the transformer call outside that guard without preserving the null check.
`ApplyValueTransformers.transformTo` has no internal null guard — it calls `transformer.to(entityValue)` directly.
## Fix Focus Areas
Change the transformer guard condition to also exclude `null`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Transformer may break array .map~~ ☑ 🐞 Bug ⛯ Reliability

Description

Change detection now runs .map() on the transformed value (normalizedValue) for date/time array
columns. If a transformer returns a non-array for an array column, this will throw at runtime during
save() and abort persistence.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-104]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
             // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {
                 switch (column.type) {
                     case "date":
                         normalizedValue = column.isArray
-                                ? entityValue.map((date: Date) =>
+                                ? normalizedValue.map((date: Date) =>
                                   DateUtils.mixedDateToDateString(date),
                               )
-                                : DateUtils.mixedDateToDateString(entityValue)
+                                : DateUtils.mixedDateToDateString(
+                                      normalizedValue,
+                                  )

Evidence

normalizedValue is replaced by the transformer output and then used as the receiver of .map()
when column.isArray is true. Since ApplyValueTransformers.transformTo returns whatever
transformer.to() returns, there is no guarantee the result remains an array, making .map() a
potential runtime crash site.

src/persistence/SubjectChangedColumnsComputer.ts[85-104]
src/util/ApplyValueTransformers.ts[19-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SubjectChangedColumnsComputer` now applies `column.transformer` before date/time normalization and then calls `.map()` on the transformed value for array columns. If a transformer returns a non-array (even accidentally), `.map()` will throw and abort `save()`.
### Issue Context
Transformers can return arbitrary shapes (there’s no enforcement in `ApplyValueTransformers.transformTo`). Change detection should not be able to crash on a mismatched transformer output.
### Fix Focus Areas
- src/persistence/SubjectChangedColumnsComputer.ts[85-158]
- src/util/ApplyValueTransformers.ts[19-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View more (24)

4. LuxonTimestampTzTypeT non-standard naming ☑ 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence
All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.
Rule 1: Consistent Naming Conventions
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-25]
test/functional/columns/value-transformer/entity/Post.ts[6-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The class `LuxonTimestampTzTypeT` uses a non-standard `T` suffix, breaking the project-wide naming convention where all `ValueTransformer` implementations end with the `Transformer` suffix.
## Issue Context
All existing transformer classes in the codebase consistently use the `Transformer` suffix: `TagTransformer`, `ComplexTransformer`, `PhonesTransformer`, `UuidTransformer`. The new class should follow the same pattern and be renamed accordingly.
## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-45]
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Transformer called with null on nullable columns~~ ☑ 🐞 Bug ✓ Correctness

Description

The transformer's to() method is now invoked unconditionally before the null guard, so saving a
null value into any nullable, transformer-decorated column will throw a runtime TypeError if the
transformer does not explicitly handle null input. The old code applied the transformer only
inside if (entityValue !== null && databaseValue !== null), which prevented this.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-95]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
           // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {

Evidence

The early-exit on line 64 only guards against undefined, so null entity values pass through and
reach the new unconditional transformer call at lines 87–92. ApplyValueTransformers.transformTo
has no internal null guard and directly calls transformer.to(entityValue). The old code nested the
transformer call inside if (entityValue !== null && databaseValue !== null), preventing this. The
new test masks the regression because LuxonTimestampTzTypeT.to() explicitly handles null (lines
29–31 of the test entity file), and the test entity declares date: DateTime without `nullable:
true`, so null is never exercised.

src/persistence/SubjectChangedColumnsComputer.ts[64-64]
src/util/ApplyValueTransformers.ts[19-29]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[26-35]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[52-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The transformer&amp;amp;amp;amp;amp;#x27;s `to()` method is now called unconditionally before the null-check block. For nullable columns, `entityValue` can be `null` (only `undefined` triggers the early return). Any transformer that does not explicitly guard against `null` will throw a runtime `TypeError` when saving a `null` value into a nullable, transformer-decorated column.
## Issue Context
The old code applied the transformer only inside `if (entityValue !== null &amp;amp;amp;amp;amp;amp;&amp;amp;amp;amp;amp;amp; databaseValue !== null)`, guaranteeing `to()` was never called with `null`. The new code hoisted the transformer call outside that guard without preserving the null check.
`ApplyValueTransformers.transformTo` has no internal null guard — it calls `transformer.to(entityValue)` directly.
## Fix Focus Areas
Change the transformer guard condition to also exclude `null`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. ~~Transformer may break array .map~~ ☑ 🐞 Bug ⛯ Reliability

Description

Change detection now runs .map() on the transformed value (normalizedValue) for date/time array
columns. If a transformer returns a non-array for an array column, this will throw at runtime during
save() and abort persistence.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-104]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
              // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {
                  switch (column.type) {
                      case "date":
                          normalizedValue = column.isArray
-                                ? entityValue.map((date: Date) =>
+                                ? normalizedValue.map((date: Date) =>
                                    DateUtils.mixedDateToDateString(date),
                                )
-                                : DateUtils.mixedDateToDateString(entityValue)
+                                : DateUtils.mixedDateToDateString(
+                                      normalizedValue,
+                                  )

Evidence

normalizedValue is replaced by the transformer output and then used as the receiver of .map()
when column.isArray is true. Since ApplyValueTransformers.transformTo returns whatever
transformer.to() returns, there is no guarantee the result remains an array, making .map() a
potential runtime crash site.

src/persistence/SubjectChangedColumnsComputer.ts[85-104]
src/util/ApplyValueTransformers.ts[19-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SubjectChangedColumnsComputer` now applies `column.transformer` before date/time normalization and then calls `.map()` on the transformed value for array columns. If a transformer returns a non-array (even accidentally), `.map()` will throw and abort `save()`.
### Issue Context
Transformers can return arbitrary shapes (there’s no enforcement in `ApplyValueTransformers.transformTo`). Change detection should not be able to crash on a mismatched transformer output.
### Fix Focus Areas
- src/persistence/SubjectChangedColumnsComputer.ts[85-158]
- src/util/ApplyValueTransformers.ts[19-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7. LuxonTimestampTzTypeT non-standard naming ☑ 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence
All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.
Rule 1: Consistent Naming Conventions
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-25]
test/functional/columns/value-transformer/entity/Post.ts[6-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The class `LuxonTimestampTzTypeT` uses a non-standard `T` suffix, breaking the project-wide naming convention where all `ValueTransformer` implementations end with the `Transformer` suffix.
## Issue Context
All existing transformer classes in the codebase consistently use the `Transformer` suffix: `TagTransformer`, `ComplexTransformer`, `PhonesTransformer`, `UuidTransformer`. The new class should follow the same pattern and be renamed accordingly.
## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-45]
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

8. ~~Transformer called with null on nullable columns~~ ☑ 🐞 Bug ✓ Correctness

Description

The transformer's to() method is now invoked unconditionally before the null guard, so saving a
null value into any nullable, transformer-decorated column will throw a runtime TypeError if the
transformer does not explicitly handle null input. The old code applied the transformer only
inside if (entityValue !== null && databaseValue !== null), which prevented this.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-95]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
            // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {

Evidence

The early-exit on line 64 only guards against undefined, so null entity values pass through and
reach the new unconditional transformer call at lines 87–92. ApplyValueTransformers.transformTo
has no internal null guard and directly calls transformer.to(entityValue). The old code nested the
transformer call inside if (entityValue !== null && databaseValue !== null), preventing this. The
new test masks the regression because LuxonTimestampTzTypeT.to() explicitly handles null (lines
29–31 of the test entity file), and the test entity declares date: DateTime without `nullable:
true`, so null is never exercised.

src/persistence/SubjectChangedColumnsComputer.ts[64-64]
src/util/ApplyValueTransformers.ts[19-29]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[26-35]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[52-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The transformer&amp;amp;amp;amp;#x27;s `to()` method is now called unconditionally before the null-check block. For nullable columns, `entityValue` can be `null` (only `undefined` triggers the early return). Any transformer that does not explicitly guard against `null` will throw a runtime `TypeError` when saving a `null` value into a nullable, transformer-decorated column.
## Issue Context
The old code applied the transformer only inside `if (entityValue !== null &amp;amp;amp;amp;amp;&amp;amp;amp;amp;amp; databaseValue !== null)`, guaranteeing `to()` was never called with `null`. The new code hoisted the transformer call outside that guard without preserving the null check.
`ApplyValueTransformers.transformTo` has no internal null guard — it calls `transformer.to(entityValue)` directly.
## Fix Focus Areas
Change the transformer guard condition to also exclude `null`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

9. LuxonTimestampTzTypeT non-standard naming ☑ 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence
All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.
Rule 1: Consistent Naming Conventions
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-25]
test/functional/columns/value-transformer/entity/Post.ts[6-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The class `LuxonTimestampTzTypeT` uses a non-standard `T` suffix, breaking the project-wide naming convention where all `ValueTransformer` implementations end with the `Transformer` suffix.
## Issue Context
All existing transformer classes in the codebase consistently use the `Transformer` suffix: `TagTransformer`, `ComplexTransformer`, `PhonesTransformer`, `UuidTransformer`. The new class should follow the same pattern and be renamed accordingly.
## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-45]
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

10. ~~Transformer called with null on nullable columns~~ ☑ 🐞 Bug ✓ Correctness

Description

The transformer's to() method is now invoked unconditionally before the null guard, so saving a
null value into any nullable, transformer-decorated column will throw a runtime TypeError if the
transformer does not explicitly handle null input. The old code applied the transformer only
inside if (entityValue !== null && databaseValue !== null), which prevented this.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-95]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
           // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {

Evidence

The early-exit on line 64 only guards against undefined, so null entity values pass through and
reach the new unconditional transformer call at lines 87–92. ApplyValueTransformers.transformTo
has no internal null guard and directly calls transformer.to(entityValue). The old code nested the
transformer call inside if (entityValue !== null && databaseValue !== null), preventing this. The
new test masks the regression because LuxonTimestampTzTypeT.to() explicitly handles null (lines
29–31 of the test entity file), and the test entity declares date: DateTime without `nullable:
true`, so null is never exercised.

src/persistence/SubjectChangedColumnsComputer.ts[64-64]
src/util/ApplyValueTransformers.ts[19-29]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[26-35]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[52-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The transformer&amp;amp;amp;amp;amp;#x27;s `to()` method is now called unconditionally before the null-check block. For nullable columns, `entityValue` can be `null` (only `undefined` triggers the early return). Any transformer that does not explicitly guard against `null` will throw a runtime `TypeError` when saving a `null` value into a nullable, transformer-decorated column.
## Issue Context
The old code applied the transformer only inside `if (entityValue !== null &amp;amp;amp;amp;amp;amp;&amp;amp;amp;amp;amp;amp; databaseValue !== null)`, guaranteeing `to()` was never called with `null`. The new code hoisted the transformer call outside that guard without preserving the null check.
`ApplyValueTransformers.transformTo` has no internal null guard — it calls `transformer.to(entityValue)` directly.
## Fix Focus Areas
Change the transformer guard condition to also exclude `null`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

11. Transformer may break array .map 🐞 Bug ⛯ Reliability

Description

Change detection now runs .map() on the transformed value (normalizedValue) for date/time array
columns. If a transformer returns a non-array for an array column, this will throw at runtime during
save() and abort persistence.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-104]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
            // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {
                switch (column.type) {
                    case "date":
                        normalizedValue = column.isArray
-                                ? entityValue.map((date: Date) =>
+                                ? normalizedValue.map((date: Date) =>
                                  DateUtils.mixedDateToDateString(date),
                              )
-                                : DateUtils.mixedDateToDateString(entityValue)
+                                : DateUtils.mixedDateToDateString(
+                                      normalizedValue,
+                                  )

Evidence

normalizedValue is replaced by the transformer output and then used as the receiver of .map()
when column.isArray is true. Since ApplyValueTransformers.transformTo returns whatever
transformer.to() returns, there is no guarantee the result remains an array, making .map() a
potential runtime crash site.

src/persistence/SubjectChangedColumnsComputer.ts[85-104]
src/util/ApplyValueTransformers.ts[19-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SubjectChangedColumnsComputer` now applies `column.transformer` before date/time normalization and then calls `.map()` on the transformed value for array columns. If a transformer returns a non-array (even accidentally), `.map()` will throw and abort `save()`.
### Issue Context
Transformers can return arbitrary shapes (there’s no enforcement in `ApplyValueTransformers.transformTo`). Change detection should not be able to crash on a mismatched transformer output.
### Fix Focus Areas
- src/persistence/SubjectChangedColumnsComputer.ts[85-158]
- src/util/ApplyValueTransformers.ts[19-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

12. LuxonTimestampTzTypeT non-standard naming ☑ 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence
All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.
Rule 1: Consistent Naming Conventions
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-25]
test/functional/columns/value-transformer/entity/Post.ts[6-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The class `LuxonTimestampTzTypeT` uses a non-standard `T` suffix, breaking the project-wide naming convention where all `ValueTransformer` implementations end with the `Transformer` suffix.
## Issue Context
All existing transformer classes in the codebase consistently use the `Transformer` suffix: `TagTransformer`, `ComplexTransformer`, `PhonesTransformer`, `UuidTransformer`. The new class should follow the same pattern and be renamed accordingly.
## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-45]
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

13. ~~Transformer called with null on nullable columns~~ ☑ 🐞 Bug ✓ Correctness

Description

The transformer's to() method is now invoked unconditionally before the null guard, so saving a
null value into any nullable, transformer-decorated column will throw a runtime TypeError if the
transformer does not explicitly handle null input. The old code applied the transformer only
inside if (entityValue !== null && databaseValue !== null), which prevented this.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-95]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
          // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {

Evidence

The early-exit on line 64 only guards against undefined, so null entity values pass through and
reach the new unconditional transformer call at lines 87–92. ApplyValueTransformers.transformTo
has no internal null guard and directly calls transformer.to(entityValue). The old code nested the
transformer call inside if (entityValue !== null && databaseValue !== null), preventing this. The
new test masks the regression because LuxonTimestampTzTypeT.to() explicitly handles null (lines
29–31 of the test entity file), and the test entity declares date: DateTime without `nullable:
true`, so null is never exercised.

src/persistence/SubjectChangedColumnsComputer.ts[64-64]
src/util/ApplyValueTransformers.ts[19-29]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[26-35]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[52-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The transformer&amp;amp;amp;amp;amp;amp;#x27;s `to()` method is now called unconditionally before the null-check block. For nullable columns, `entityValue` can be `null` (only `undefined` triggers the early return). Any transformer that does not explicitly guard against `null` will throw a runtime `TypeError` when saving a `null` value into a nullable, transformer-decorated column.
## Issue Context
The old code applied the transformer only inside `if (entityValue !== null &amp;amp;amp;amp;amp;amp;amp;&amp;amp;amp;amp;amp;amp;amp; databaseValue !== null)`, guaranteeing `to()` was never called with `null`. The new code hoisted the transformer call outside that guard without preserving the null check.
`ApplyValueTransformers.transformTo` has no internal null guard — it calls `transformer.to(entityValue)` directly.
## Fix Focus Areas
Change the transformer guard condition to also exclude `null`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

14. Transformer may break array .map 🐞 Bug ⛯ Reliability

Description

Change detection now runs .map() on the transformed value (normalizedValue) for date/time array
columns. If a transformer returns a non-array for an array column, this will throw at runtime during
save() and abort persistence.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-104]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
             // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {
                 switch (column.type) {
                     case "date":
                         normalizedValue = column.isArray
-                                ? entityValue.map((date: Date) =>
+                                ? normalizedValue.map((date: Date) =>
                                   DateUtils.mixedDateToDateString(date),
                               )
-                                : DateUtils.mixedDateToDateString(entityValue)
+                                : DateUtils.mixedDateToDateString(
+                                      normalizedValue,
+                                  )

Evidence

normalizedValue is replaced by the transformer output and then used as the receiver of .map()
when column.isArray is true. Since ApplyValueTransformers.transformTo returns whatever
transformer.to() returns, there is no guarantee the result remains an array, making .map() a
potential runtime crash site.

src/persistence/SubjectChangedColumnsComputer.ts[85-104]
src/util/ApplyValueTransformers.ts[19-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SubjectChangedColumnsComputer` now applies `column.transformer` before date/time normalization and then calls `.map()` on the transformed value for array columns. If a transformer returns a non-array (even accidentally), `.map()` will throw and abort `save()`.
### Issue Context
Transformers can return arbitrary shapes (there’s no enforcement in `ApplyValueTransformers.transformTo`). Change detection should not be able to crash on a mismatched transformer output.
### Fix Focus Areas
- src/persistence/SubjectChangedColumnsComputer.ts[85-158]
- src/util/ApplyValueTransformers.ts[19-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

15. LuxonTimestampTzTypeT non-standard naming 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence
All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.
Rule 1: Consistent Naming Conventions
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-25]
test/functional/columns/value-transformer/entity/Post.ts[6-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The class `LuxonTimestampTzTypeT` uses a non-standard `T` suffix, breaking the project-wide naming convention where all `ValueTransformer` implementations end with the `Transformer` suffix.
## Issue Context
All existing transformer classes in the codebase consistently use the `Transformer` suffix: `TagTransformer`, `ComplexTransformer`, `PhonesTransformer`, `UuidTransformer`. The new class should follow the same pattern and be renamed accordingly.
## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-45]
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

16. ~~Transformer called with null on nullable columns~~ ☑ 🐞 Bug ✓ Correctness

Description

The transformer's to() method is now invoked unconditionally before the null guard, so saving a
null value into any nullable, transformer-decorated column will throw a runtime TypeError if the
transformer does not explicitly handle null input. The old code applied the transformer only
inside if (entityValue !== null && databaseValue !== null), which prevented this.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-95]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
           // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {

Evidence

The early-exit on line 64 only guards against undefined, so null entity values pass through and
reach the new unconditional transformer call at lines 87–92. ApplyValueTransformers.transformTo
has no internal null guard and directly calls transformer.to(entityValue). The old code nested the
transformer call inside if (entityValue !== null && databaseValue !== null), preventing this. The
new test masks the regression because LuxonTimestampTzTypeT.to() explicitly handles null (lines
29–31 of the test entity file), and the test entity declares date: DateTime without `nullable:
true`, so null is never exercised.

src/persistence/SubjectChangedColumnsComputer.ts[64-64]
src/util/ApplyValueTransformers.ts[19-29]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[26-35]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[52-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The transformer&amp;amp;amp;amp;amp;#x27;s `to()` method is now called unconditionally before the null-check block. For nullable columns, `entityValue` can be `null` (only `undefined` triggers the early return). Any transformer that does not explicitly guard against `null` will throw a runtime `TypeError` when saving a `null` value into a nullable, transformer-decorated column.
## Issue Context
The old code applied the transformer only inside `if (entityValue !== null &amp;amp;amp;amp;amp;amp;&amp;amp;amp;amp;amp;amp; databaseValue !== null)`, guaranteeing `to()` was never called with `null`. The new code hoisted the transformer call outside that guard without preserving the null check.
`ApplyValueTransformers.transformTo` has no internal null guard — it calls `transformer.to(entityValue)` directly.
## Fix Focus Areas
Change the transformer guard condition to also exclude `null`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

17. Transformer may break array .map 🐞 Bug ⛯ Reliability

Description

Change detection now runs .map() on the transformed value (normalizedValue) for date/time array
columns. If a transformer returns a non-array for an array column, this will throw at runtime during
save() and abort persistence.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-104]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
               // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {
                   switch (column.type) {
                       case "date":
                           normalizedValue = column.isArray
-                                ? entityValue.map((date: Date) =>
+                                ? normalizedValue.map((date: Date) =>
                                     DateUtils.mixedDateToDateString(date),
                                 )
-                                : DateUtils.mixedDateToDateString(entityValue)
+                                : DateUtils.mixedDateToDateString(
+                                      normalizedValue,
+                                  )

Evidence

normalizedValue is replaced by the transformer output and then used as the receiver of .map()
when column.isArray is true. Since ApplyValueTransformers.transformTo returns whatever
transformer.to() returns, there is no guarantee the result remains an array, making .map() a
potential runtime crash site.

src/persistence/SubjectChangedColumnsComputer.ts[85-104]
src/util/ApplyValueTransformers.ts[19-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SubjectChangedColumnsComputer` now applies `column.transformer` before date/time normalization and then calls `.map()` on the transformed value for array columns. If a transformer returns a non-array (even accidentally), `.map()` will throw and abort `save()`.
### Issue Context
Transformers can return arbitrary shapes (there’s no enforcement in `ApplyValueTransformers.transformTo`). Change detection should not be able to crash on a mismatched transformer output.
### Fix Focus Areas
- src/persistence/SubjectChangedColumnsComputer.ts[85-158]
- src/util/ApplyValueTransformers.ts[19-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

18. LuxonTimestampTzTypeT non-standard naming 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence

All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.

Rule 1: Consistent Naming Conventions
[test/functional/columns/value-tr...

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts

src/persistence/SubjectChangedColumnsComputer.ts

qodo-free-for-open-source-projects · 2026-02-18T09:34:47Z

Persistent review updated to latest commit 988b462

ISSUES.md

src/persistence/SubjectChangedColumnsComputer.ts

gioboa · 2026-02-18T10:08:56Z

/review

qodo-free-for-open-source-projects · 2026-02-18T10:09:20Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Transformer Order The new logic applies `column.transformer` to `entityValue` before the type-specific normalizations (date/time/timestamp, simple-). Please validate this order for all drivers/types, especially where transformers may return strings/numbers instead of `Date`, because the subsequent `DateUtils.mixedDateTo` calls and `.map((date: Date) => ...)` assume `Date`-compatible inputs. let normalizedValue = entityValue if (column.transformer && shouldTransformDatabaseEntity) { normalizedValue = ApplyValueTransformers.transformTo( column.transformer, entityValue, ) } // if both values are not null, normalize special values to make proper comparision if (normalizedValue !== null && databaseValue !== null) { switch (column.type) { case "date": normalizedValue = column.isArray ? normalizedValue.map((date: Date) => DateUtils.mixedDateToDateString(date), ) : DateUtils.mixedDateToDateString( normalizedValue, ) databaseValue = column.isArray ? databaseValue.map((date: Date) => DateUtils.mixedDateToDateString(date), ) : DateUtils.mixedDateToDateString(databaseValue) break case "time": case "time with time zone": case "time without time zone": case "timetz": normalizedValue = column.isArray ? normalizedValue.map((date: Date) => DateUtils.mixedDateToTimeString(date), ) : DateUtils.mixedDateToTimeString( normalizedValue, ) databaseValue = column.isArray Conditional Transform `transformTo` is now gated by `shouldTransformDatabaseEntity`. Double-check that change detection remains correct when this flag is false (e.g., embedded/partial entities, listeners/subscribers scenarios), since `normalizedValue` will remain untransformed while `databaseValue` is compared against normalized DB representations. if (column.transformer && shouldTransformDatabaseEntity) { normalizedValue = ApplyValueTransformers.transformTo( column.transformer, entityValue, ) } // if both values are not null, normalize special values to make proper comparision if (normalizedValue !== null && databaseValue !== null) { switch (column.type) { Test Isolation The test mutates `dataSource.subscribers` by pushing/removing a dynamic subscriber. To avoid leaking the subscriber when an assertion or `repo.save` throws, consider wrapping the push/save/assert in a `try/finally` that always removes the subscriber. let updateCalled = false // Construct a dynamic subscriber const subscriber: EntitySubscriberInterface<PostWithLuxonDate> = { listenTo() { return PostWithLuxonDate }, beforeUpdate(event: UpdateEvent<PostWithLuxonDate>) { updateCalled = true }, } dataSource.subscribers.push(subscriber) await repo.save(loadedPost) const index = dataSource.subscribers.indexOf(subscriber) if (index !== -1) { dataSource.subscribers.splice(index, 1) }

qodo-free-for-open-source-projects · 2026-02-18T10:26:16Z

Persistent review updated to latest commit 4d38b06

src/util/ApplyValueTransformers.ts

qodo-free-for-open-source-projects · 2026-02-23T15:04:22Z

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (1) 📎 Requirement gaps (0)

1. Transformer may break array .map 🐞 Bug ⛯ Reliability ⭐ New

Description

Change detection now runs .map() on the transformed value (normalizedValue) for date/time array
columns. If a transformer returns a non-array for an array column, this will throw at runtime during
save() and abort persistence.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-104]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
                // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {
                    switch (column.type) {
                        case "date":
                            normalizedValue = column.isArray
-                                ? entityValue.map((date: Date) =>
+                                ? normalizedValue.map((date: Date) =>
                                      DateUtils.mixedDateToDateString(date),
                                  )
-                                : DateUtils.mixedDateToDateString(entityValue)
+                                : DateUtils.mixedDateToDateString(
+                                      normalizedValue,
+                                  )

Evidence

normalizedValue is replaced by the transformer output and then used as the receiver of .map()
when column.isArray is true. Since ApplyValueTransformers.transformTo returns whatever
transformer.to() returns, there is no guarantee the result remains an array, making .map() a
potential runtime crash site.

src/persistence/SubjectChangedColumnsComputer.ts[85-104]
src/util/ApplyValueTransformers.ts[19-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SubjectChangedColumnsComputer` now applies `column.transformer` before date/time normalization and then calls `.map()` on the transformed value for array columns. If a transformer returns a non-array (even accidentally), `.map()` will throw and abort `save()`.

### Issue Context
Transformers can return arbitrary shapes (there’s no enforcement in `ApplyValueTransformers.transformTo`). Change detection should not be able to crash on a mismatched transformer output.

### Fix Focus Areas
- src/persistence/SubjectChangedColumnsComputer.ts[85-158]
- src/util/ApplyValueTransformers.ts[19-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. LuxonTimestampTzTypeT non-standard naming 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence
All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.
Rule 1: Consistent Naming Conventions
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-25]
test/functional/columns/value-transformer/entity/Post.ts[6-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The class `LuxonTimestampTzTypeT` uses a non-standard `T` suffix, breaking the project-wide naming convention where all `ValueTransformer` implementations end with the `Transformer` suffix.
## Issue Context
All existing transformer classes in the codebase consistently use the `Transformer` suffix: `TagTransformer`, `ComplexTransformer`, `PhonesTransformer`, `UuidTransformer`. The new class should follow the same pattern and be renamed accordingly.
## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-45]
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. ~~Transformer called with null on nullable columns~~ ☑ 🐞 Bug ✓ Correctness

Description

The transformer's to() method is now invoked unconditionally before the null guard, so saving a
null value into any nullable, transformer-decorated column will throw a runtime TypeError if the
transformer does not explicitly handle null input. The old code applied the transformer only
inside if (entityValue !== null && databaseValue !== null), which prevented this.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-95]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
              // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {

Evidence

The early-exit on line 64 only guards against undefined, so null entity values pass through and
reach the new unconditional transformer call at lines 87–92. ApplyValueTransformers.transformTo
has no internal null guard and directly calls transformer.to(entityValue). The old code nested the
transformer call inside if (entityValue !== null && databaseValue !== null), preventing this. The
new test masks the regression because LuxonTimestampTzTypeT.to() explicitly handles null (lines
29–31 of the test entity file), and the test entity declares date: DateTime without `nullable:
true`, so null is never exercised.

src/persistence/SubjectChangedColumnsComputer.ts[64-64]
src/util/ApplyValueTransformers.ts[19-29]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[26-35]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[52-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The transformer&amp;amp;#x27;s `to()` method is now called unconditionally before the null-check block. For nullable columns, `entityValue` can be `null` (only `undefined` triggers the early return). Any transformer that does not explicitly guard against `null` will throw a runtime `TypeError` when saving a `null` value into a nullable, transformer-decorated column.
## Issue Context
The old code applied the transformer only inside `if (entityValue !== null &amp;amp;amp;&amp;amp;amp; databaseValue !== null)`, guaranteeing `to()` was never called with `null`. The new code hoisted the transformer call outside that guard without preserving the null check.
`ApplyValueTransformers.transformTo` has no internal null guard — it calls `transformer.to(entityValue)` directly.
## Fix Focus Areas
Change the transformer guard condition to also exclude `null`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. Test cleanup not in finally 🐞 Bug ⛯ Reliability ⭐ New

Description

The new functional test registers a dynamic subscriber and removes it afterward, but removal is not
protected by try/finally. If repo.save throws, the subscriber may remain attached for the rest
of the test lifecycle (increasing flake risk).

Code

test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[R40-58]

+                // Construct a dynamic subscriber
+                const subscriber: EntitySubscriberInterface<PostWithLuxonDate> =
+                    {
+                        listenTo() {
+                            return PostWithLuxonDate
+                        },
+                        beforeUpdate(event: UpdateEvent<PostWithLuxonDate>) {
+                            updateCalled = true
+                        },
+                    }
+
+                dataSource.subscribers.push(subscriber)
+
+                await repo.save(loadedPost)
+
+                const index = dataSource.subscribers.indexOf(subscriber)
+                if (index !== -1) {
+                    dataSource.subscribers.splice(index, 1)
+                }

Evidence
The test mutates dataSource.subscribers and only removes the subscriber after repo.save
completes. Without try/finally, failures before cleanup can leave the subscriber installed longer
than intended.
test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[40-58]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The test adds a dynamic subscriber and removes it only after `repo.save` succeeds. If an error occurs before removal, the subscriber can remain registered longer than intended.

### Issue Context
Even though the suite closes connections in `after()`, using `try/finally` makes the test more robust and avoids cross-test interference if the file grows.

### Fix Focus Areas
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[40-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

coveralls · 2026-02-23T15:32:44Z

coverage: 81.446% (-0.001%) from 81.447%
when pulling abefdcf on gioboa:test/issue-11951
into 42819be on typeorm:master.

qodo-free-for-open-source-projects · 2026-02-23T15:43:45Z

Code Review by Qodo

🐞 Bugs (8) 📘 Rule violations (4) 📎 Requirement gaps (0)

1. LuxonTimestampTzTypeT non-standard naming 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence
All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.
Rule 1: Consistent Naming Conventions
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-25]
test/functional/columns/value-transformer/entity/Post.ts[6-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The class `LuxonTimestampTzTypeT` uses a non-standard `T` suffix, breaking the project-wide naming convention where all `ValueTransformer` implementations end with the `Transformer` suffix.
## Issue Context
All existing transformer classes in the codebase consistently use the `Transformer` suffix: `TagTransformer`, `ComplexTransformer`, `PhonesTransformer`, `UuidTransformer`. The new class should follow the same pattern and be renamed accordingly.
## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-45]
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. ~~Transformer called with null on nullable columns ☑~~ ☑ 🐞 Bug ✓ Correctness

Description

The transformer's to() method is now invoked unconditionally before the null guard, so saving a
null value into any nullable, transformer-decorated column will throw a runtime TypeError if the
transformer does not explicitly handle null input. The old code applied the transformer only
inside if (entityValue !== null && databaseValue !== null), which prevented this.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-95]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
             // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {

Evidence

The early-exit on line 64 only guards against undefined, so null entity values pass through and
reach the new unconditional transformer call at lines 87–92. ApplyValueTransformers.transformTo
has no internal null guard and directly calls transformer.to(entityValue). The old code nested the
transformer call inside if (entityValue !== null && databaseValue !== null), preventing this. The
new test masks the regression because LuxonTimestampTzTypeT.to() explicitly handles null (lines
29–31 of the test entity file), and the test entity declares date: DateTime without `nullable:
true`, so null is never exercised.

src/persistence/SubjectChangedColumnsComputer.ts[64-64]
src/util/ApplyValueTransformers.ts[19-29]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[26-35]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[52-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The transformer&amp;amp;amp;#x27;s `to()` method is now called unconditionally before the null-check block. For nullable columns, `entityValue` can be `null` (only `undefined` triggers the early return). Any transformer that does not explicitly guard against `null` will throw a runtime `TypeError` when saving a `null` value into a nullable, transformer-decorated column.
## Issue Context
The old code applied the transformer only inside `if (entityValue !== null &amp;amp;amp;amp;&amp;amp;amp;amp; databaseValue !== null)`, guaranteeing `to()` was never called with `null`. The new code hoisted the transformer call outside that guard without preserving the null check.
`ApplyValueTransformers.transformTo` has no internal null guard — it calls `transformer.to(entityValue)` directly.
## Fix Focus Areas
Change the transformer guard condition to also exclude `null`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Transformer may break array .map 🐞 Bug ⛯ Reliability

Description

Change detection now runs .map() on the transformed value (normalizedValue) for date/time array
columns. If a transformer returns a non-array for an array column, this will throw at runtime during
save() and abort persistence.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-104]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
              // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {
                  switch (column.type) {
                      case "date":
                          normalizedValue = column.isArray
-                                ? entityValue.map((date: Date) =>
+                                ? normalizedValue.map((date: Date) =>
                                    DateUtils.mixedDateToDateString(date),
                                )
-                                : DateUtils.mixedDateToDateString(entityValue)
+                                : DateUtils.mixedDateToDateString(
+                                      normalizedValue,
+                                  )

Evidence

normalizedValue is replaced by the transformer output and then used as the receiver of .map()
when column.isArray is true. Since ApplyValueTransformers.transformTo returns whatever
transformer.to() returns, there is no guarantee the result remains an array, making .map() a
potential runtime crash site.

src/persistence/SubjectChangedColumnsComputer.ts[85-104]
src/util/ApplyValueTransformers.ts[19-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SubjectChangedColumnsComputer` now applies `column.transformer` before date/time normalization and then calls `.map()` on the transformed value for array columns. If a transformer returns a non-array (even accidentally), `.map()` will throw and abort `save()`.
### Issue Context
Transformers can return arbitrary shapes (there’s no enforcement in `ApplyValueTransformers.transformTo`). Change detection should not be able to crash on a mismatched transformer output.
### Fix Focus Areas
- src/persistence/SubjectChangedColumnsComputer.ts[85-158]
- src/util/ApplyValueTransformers.ts[19-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View more (5)

4. LuxonTimestampTzTypeT non-standard naming 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence
All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.
Rule 1: Consistent Naming Conventions
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-25]
test/functional/columns/value-transformer/entity/Post.ts[6-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The class `LuxonTimestampTzTypeT` uses a non-standard `T` suffix, breaking the project-wide naming convention where all `ValueTransformer` implementations end with the `Transformer` suffix.
## Issue Context
All existing transformer classes in the codebase consistently use the `Transformer` suffix: `TagTransformer`, `ComplexTransformer`, `PhonesTransformer`, `UuidTransformer`. The new class should follow the same pattern and be renamed accordingly.
## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-45]
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. ~~Transformer called with null on nullable columns ☑ ☑~~ ☑ 🐞 Bug ✓ Correctness

Description

The transformer's to() method is now invoked unconditionally before the null guard, so saving a
null value into any nullable, transformer-decorated column will throw a runtime TypeError if the
transformer does not explicitly handle null input. The old code applied the transformer only
inside if (entityValue !== null && databaseValue !== null), which prevented this.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-95]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
            // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {

Evidence

The early-exit on line 64 only guards against undefined, so null entity values pass through and
reach the new unconditional transformer call at lines 87–92. ApplyValueTransformers.transformTo
has no internal null guard and directly calls transformer.to(entityValue). The old code nested the
transformer call inside if (entityValue !== null && databaseValue !== null), preventing this. The
new test masks the regression because LuxonTimestampTzTypeT.to() explicitly handles null (lines
29–31 of the test entity file), and the test entity declares date: DateTime without `nullable:
true`, so null is never exercised.

src/persistence/SubjectChangedColumnsComputer.ts[64-64]
src/util/ApplyValueTransformers.ts[19-29]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[26-35]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[52-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The transformer&amp;amp;amp;amp;#x27;s `to()` method is now called unconditionally before the null-check block. For nullable columns, `entityValue` can be `null` (only `undefined` triggers the early return). Any transformer that does not explicitly guard against `null` will throw a runtime `TypeError` when saving a `null` value into a nullable, transformer-decorated column.
## Issue Context
The old code applied the transformer only inside `if (entityValue !== null &amp;amp;amp;amp;amp;&amp;amp;amp;amp;amp; databaseValue !== null)`, guaranteeing `to()` was never called with `null`. The new code hoisted the transformer call outside that guard without preserving the null check.
`ApplyValueTransformers.transformTo` has no internal null guard — it calls `transformer.to(entityValue)` directly.
## Fix Focus Areas
Change the transformer guard condition to also exclude `null`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. Transformer may break array .map 🐞 Bug ⛯ Reliability

Description

Change detection now runs .map() on the transformed value (normalizedValue) for date/time array
columns. If a transformer returns a non-array for an array column, this will throw at runtime during
save() and abort persistence.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-104]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
               // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {
                   switch (column.type) {
                       case "date":
                           normalizedValue = column.isArray
-                                ? entityValue.map((date: Date) =>
+                                ? normalizedValue.map((date: Date) =>
                                     DateUtils.mixedDateToDateString(date),
                                 )
-                                : DateUtils.mixedDateToDateString(entityValue)
+                                : DateUtils.mixedDateToDateString(
+                                      normalizedValue,
+                                  )

Evidence

normalizedValue is replaced by the transformer output and then used as the receiver of .map()
when column.isArray is true. Since ApplyValueTransformers.transformTo returns whatever
transformer.to() returns, there is no guarantee the result remains an array, making .map() a
potential runtime crash site.

src/persistence/SubjectChangedColumnsComputer.ts[85-104]
src/util/ApplyValueTransformers.ts[19-29]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`SubjectChangedColumnsComputer` now applies `column.transformer` before date/time normalization and then calls `.map()` on the transformed value for array columns. If a transformer returns a non-array (even accidentally), `.map()` will throw and abort `save()`.
### Issue Context
Transformers can return arbitrary shapes (there’s no enforcement in `ApplyValueTransformers.transformTo`). Change detection should not be able to crash on a mismatched transformer output.
### Fix Focus Areas
- src/persistence/SubjectChangedColumnsComputer.ts[85-158]
- src/util/ApplyValueTransformers.ts[19-29]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7. LuxonTimestampTzTypeT non-standard naming 📘 Rule violation ✓ Correctness

Description

The new LuxonTimestampTzTypeT class uses a T suffix instead of the project-wide Transformer
suffix used by every other ValueTransformer implementation in the codebase (e.g.,
TagTransformer, ComplexTransformer, PhonesTransformer, UuidTransformer). This inconsistency
breaks the established naming convention.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25]
+export class LuxonTimestampTzTypeT implements ValueTransformer {

Evidence
All ValueTransformer implementations in the project follow the *Transformer suffix pattern. The
new class LuxonTimestampTzTypeT deviates from this established convention by using a non-standard
T suffix instead.
Rule 1: Consistent Naming Conventions
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-25]
test/functional/columns/value-transformer/entity/Post.ts[6-6]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The class `LuxonTimestampTzTypeT` uses a non-standard `T` suffix, breaking the project-wide naming convention where all `ValueTransformer` implementations end with the `Transformer` suffix.
## Issue Context
All existing transformer classes in the codebase consistently use the `Transformer` suffix: `TagTransformer`, `ComplexTransformer`, `PhonesTransformer`, `UuidTransformer`. The new class should follow the same pattern and be renamed accordingly.
## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-45]
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[5-5]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

8. ~~Transformer called with null on nullable columns ☑~~ ☑ 🐞 Bug ✓ Correctness

Description

The transformer's to() method is now invoked unconditionally before the null guard, so saving a
null value into any nullable, transformer-decorated column will throw a runtime TypeError if the
transformer does not explicitly handle null input. The old code applied the transformer only
inside if (entityValue !== null && databaseValue !== null), which prevented this.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R87-95]

+                if (column.transformer && shouldTransformDatabaseEntity) {
+                    normalizedValue = ApplyValueTransformers.transformTo(
+                        column.transformer,
+                        entityValue,
+                    )
+                }
+
             // if both values are not null, normalize special values to make proper comparision
-                if (entityValue !== null && databaseValue !== null) {
+                if (normalizedValue !== null && databaseValue !== null) {

Evidence

The early-exit on line 64 only guards against undefined, so null entity values pass through and
reach the new unconditional transformer call at lines 87–92. ApplyValueTransformers.transformTo
has no internal null guard and directly calls transformer.to(entityValue). The old code nested the
transformer call inside if (entityValue !== null && databaseValue !== null), preventing this. The
new test masks the regression because LuxonTimestampTzTypeT.to() explicitly handles null (lines
29–31 of the test entity file), and the test entity declares date: DateTime without `nullable:
true`, so null is never exercised.

src/persistence/SubjectChangedColumnsComputer.ts[64-64]
src/util/ApplyValueTransformers.ts[19-29]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[26-35]
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[52-53]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The transformer&amp;amp;amp;#x27;s `to()` method is now called unconditionally before the null-check block. For nullable columns, `entityValue` can be `null` (only `undefined` triggers the early return). Any transformer that does not explicitly guard against `null` will throw a runtime `TypeError` when saving a `null` value into a nullable, transformer-decorated column.
## Issue Context
The old code applied the transformer only inside `if (entityValue !== null &amp;amp;amp;amp;&amp;amp;amp;amp; databaseValue !== null)`, guaranteeing `to()` was never called with `null`. The new code hoisted the transformer call outside that guard without preserving the null check.
`ApplyValueTransformers.transformTo` has no internal null guard — it calls `transformer.to(entityValue)` directly.
## Fix Focus Areas
Change the transformer guard condition to also exclude `null`:

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

9. FindOperator<any> in transformer 📘 Rule violation ✓ Correctness ⭐ New

Description

New test code introduces FindOperator<any> in LuxonTimestampTzTransformer, which is an
any-typed escape hatch that weakens type-safety. This violates the requirement to avoid any
usage added to bypass type issues.

Code

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[R25-35]

+export class LuxonTimestampTzTransformer implements ValueTransformer {
+    public to(
+        value: DateTime | null | undefined | FindOperator<any>,
+    ): Date | null | FindOperator<any> {
+        if (value === null || value === undefined) {
+            return null
+        } else if (value instanceof FindOperator) {
+            return value
+        }
+        return value.toJSDate()
+    }

Evidence
PR Compliance ID 4 prohibits introducing any-based typing to bypass type constraints. The newly
added transformer explicitly types FindOperator with any in both the parameter and return type.
Rule 4: Remove AI-generated noise
test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-35]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The newly added `LuxonTimestampTzTransformer` uses `FindOperator&lt;any&gt;` in method signatures, introducing new `any` typing.

## Issue Context
Compliance requires avoiding new `any` usage used to bypass type-safety. Use a concrete generic for `FindOperator` (e.g., `FindOperator&lt;DateTime&gt;` / `FindOperator&lt;Date&gt;` depending on TypeORM expectations) or `FindOperator&lt;unknown&gt;` if the specific type cannot be expressed cleanly.

## Fix Focus Areas
- test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts[25-35]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

10. UTC date compare mismatch 🐞 Bug ✓ Correctness ⭐ New

Description

For type: "date" columns with utc: true, change detection normalizes Date values using local
date components (no { utc: column.utc }), while drivers persist/hydrate using UTC components. This
can cause false positives (unnecessary UPDATEs) when an entity provides a Date but the database
entity holds a date string.

Code

src/persistence/SubjectChangedColumnsComputer.ts[R101-111]

                        case "date":
                            normalizedValue = column.isArray
-                                ? entityValue.map((date: Date) =>
+                                ? normalizedValue.map((date: Date) =>
                                      DateUtils.mixedDateToDateString(date),
                                  )
-                                : DateUtils.mixedDateToDateString(entityValue)
+                                : DateUtils.mixedDateToDateString(
+                                      normalizedValue,
+                                  )
                            databaseValue = column.isArray
                                ? databaseValue.map((date: Date) =>
                                      DateUtils.mixedDateToDateString(date),

Evidence
DateUtils.mixedDateToDateString changes behavior based on options.utc. MySQL/Postgres drivers
both persist and hydrate date columns using { utc: columnMetadata.utc }, meaning databaseEntity
values can be normalized under UTC semantics (often ending up as strings like YYYY-MM-DD). In
change detection, entity-side Date values are normalized without the UTC flag, so an
entity-provided Date can normalize to a different YYYY-MM-DD than the hydrated database string,
even when the persisted value would be identical.
src/persistence/SubjectChangedColumnsComputer.ts[99-114]
src/util/DateUtils.ts[32-54]
src/driver/mysql/MysqlDriver.ts[604-619]
src/driver/mysql/MysqlDriver.ts[653-676]
src/driver/postgres/PostgresDriver.ts[739-742]
src/driver/postgres/PostgresDriver.ts[862-866]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`SubjectChangedColumnsComputer` normalizes `type: &quot;date&quot;` values with `DateUtils.mixedDateToDateString(...)` but does not pass `{ utc: column.utc }`. Drivers do pass this flag on both persist and hydrate paths, so change detection can disagree with what would actually be persisted, causing spurious UPDATEs.

### Issue Context
This is most visible when the entity value is a `Date` (normalized using local Y/M/D) but the database entity value is a string `YYYY-MM-DD` produced under UTC semantics (or vice versa).

### Fix Focus Areas
- src/persistence/SubjectChangedColumnsComputer.ts[99-114]

### Suggested fix directions
- In the `case &quot;date&quot;` branch, call:
 - `DateUtils.mixedDateToDateString(value, { utc: column.utc })`
 for both `normalizedValue` and `databaseValue` (and inside the array `.map()` paths).
- Add a regression test for `@Column({ type: &quot;date&quot;, utc: true, transformer: ... })` where the entity supplies a `Date` and the re-save should not trigger an update.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

11. Test cleanup not in finally 🐞 Bug ⛯ Reliability

Description

The new functional test registers a dynamic subscriber and removes it afterward, but removal is not
protected by try/finally. If repo.save throws, the subscriber may remain attached for the rest
of the test lifecycle (increasing flake risk).

Code

test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[R40-58]

+                // Construct a dynamic subscriber
+                const subscriber: EntitySubscriberInterface<PostWithLuxonDate> =
+                    {
+                        listenTo() {
+                            return PostWithLuxonDate
+                        },
+                        beforeUpdate(event: UpdateEvent<PostWithLuxonDate>) {
+                            updateCalled = true
+                        },
+                    }
+
+                dataSource.subscribers.push(subscriber)
+
+                await repo.save(loadedPost)
+
+                const index = dataSource.subscribers.indexOf(subscriber)
+                if (index !== -1) {
+                    dataSource.subscribers.splice(index, 1)
+                }

Evidence
The test mutates dataSource.subscribers and only removes the subscriber after repo.save
completes. Without try/finally, failures before cleanup can leave the subscriber installed longer
than intended.
test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[40-58]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test adds a dynamic subscriber and removes it only after `repo.save` succeeds. If an error occurs before removal, the subscriber can remain registered longer than intended.
### Issue Context
Even though the suite closes connections in `after()`, using `try/finally` makes the test more robust and avoids cross-test interference if the file grows.
### Fix Focus Areas
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[40-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View more (1)

12. Test cleanup not in finally 🐞 Bug ⛯ Reliability

Description

The new functional test registers a dynamic subscriber and removes it afterward, but removal is not
protected by try/finally. If repo.save throws, the subscriber may remain attached for the rest
of the test lifecycle (increasing flake risk).

Code

test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[R40-58]

+                // Construct a dynamic subscriber
+                const subscriber: EntitySubscriberInterface<PostWithLuxonDate> =
+                    {
+                        listenTo() {
+                            return PostWithLuxonDate
+                        },
+                        beforeUpdate(event: UpdateEvent<PostWithLuxonDate>) {
+                            updateCalled = true
+                        },
+                    }
+
+                dataSource.subscribers.push(subscriber)
+
+                await repo.save(loadedPost)
+
+                const index = dataSource.subscribers.indexOf(subscriber)
+                if (index !== -1) {
+                    dataSource.subscribers.splice(index, 1)
+                }

Evidence
The test mutates dataSource.subscribers and only removes the subscriber after repo.save
completes. Without try/finally, failures before cleanup can leave the subscriber installed longer
than intended.
test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[40-58]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test adds a dynamic subscriber and removes it only after `repo.save` succeeds. If an error occurs before removal, the subscriber can remain registered longer than intended.
### Issue Context
Even though the suite closes connections in `after()`, using `try/finally` makes the test more robust and avoids cross-test interference if the file grows.
### Fix Focus Areas
- test/functional/columns/value-transformer/value-transformer-change-detection.test.ts[40-60]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ The new review experience is currently in Beta. Learn more

gioboa mentioned this pull request Feb 6, 2026

luxon.DateTime columns triggers update every time #11951

Closed

18 tasks

qodo-free-for-open-source-projects bot reviewed Feb 6, 2026

View reviewed changes

src/persistence/SubjectChangedColumnsComputer.ts Show resolved Hide resolved

qodo-free-for-open-source-projects bot reviewed Feb 17, 2026

View reviewed changes

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts Outdated Show resolved Hide resolved

fix: fix up change detection with date transformer

43b7ac8

gioboa force-pushed the test/issue-11951 branch from 58c560a to 43b7ac8 Compare February 18, 2026 09:09

Merge branch 'master' into test/issue-11951

af50fa5

qodo-free-for-open-source-projects bot reviewed Feb 18, 2026

View reviewed changes

GEMINI.md Outdated Show resolved Hide resolved

ISSUES.md Outdated Show resolved Hide resolved

src/persistence/SubjectChangedColumnsComputer.ts Outdated Show resolved Hide resolved

qodo-free-for-open-source-projects bot reviewed Feb 18, 2026

View reviewed changes

GEMINI.md Outdated Show resolved Hide resolved

fix: fix up change detection with date transformer

f769b7a

gioboa force-pushed the test/issue-11951 branch from 988b462 to f769b7a Compare February 18, 2026 09:27

typeorm deleted a comment from qodo-free-for-open-source-projects bot Feb 18, 2026

qodo-free-for-open-source-projects bot reviewed Feb 18, 2026

View reviewed changes

test/functional/columns/value-transformer/entity/PostWithLuxonDate.ts Outdated Show resolved Hide resolved

src/persistence/SubjectChangedColumnsComputer.ts Outdated Show resolved Hide resolved

qodo-free-for-open-source-projects bot reviewed Feb 18, 2026

View reviewed changes

ISSUES.md Outdated Show resolved Hide resolved

src/persistence/SubjectChangedColumnsComputer.ts Outdated Show resolved Hide resolved

qodo-free-for-open-source-projects bot added the Review effort 3/5 label Feb 18, 2026

fix: fix up change detection with date transformer

4d38b06

alumni mentioned this pull request Feb 22, 2026

fix-luxon-datetime-update #12003

Closed

4 tasks

alumni reviewed Feb 22, 2026

View reviewed changes

src/util/ApplyValueTransformers.ts Outdated Show resolved Hide resolved

gioboa and others added 2 commits February 23, 2026 15:54

chore: address feedback

610c9c2

Merge branch 'master' into test/issue-11951

590cc0a

gioboa requested review from G0maa, alumni and naorpeled February 23, 2026 14:55

alumni approved these changes Feb 23, 2026

View reviewed changes

typeorm deleted a comment from qodo-free-for-open-source-projects bot Feb 23, 2026

chore: address feedback

abefdcf

G0maa approved these changes Feb 23, 2026

View reviewed changes

gioboa merged commit e3e3c97 into typeorm:master Feb 23, 2026
65 checks passed

gioboa deleted the test/issue-11951 branch February 23, 2026 17:20

pkuczynski added this to the 1.0 milestone Mar 8, 2026

Uh oh!

Conversation

gioboa commented Feb 6, 2026 • edited by alumni Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of change

Pull-Request Checklist

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 6, 2026

User description

Description of change

Pull-Request Checklist

PR Type

Description

Diagram Walkthrough

File Walkthrough

Uh oh!

pkg-pr-new bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 18, 2026

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 18, 2026

Uh oh!

Uh oh!

Uh oh!

gioboa commented Feb 18, 2026

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 18, 2026

PR Reviewer Guide 🔍

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 18, 2026

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 23, 2026

Code Review by Qodo

Uh oh!

coveralls commented Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qodo-free-for-open-source-projects bot commented Feb 23, 2026

Code Review by Qodo

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

gioboa commented Feb 6, 2026 •

edited by alumni

Loading

pkg-pr-new bot commented Feb 6, 2026 •

edited

Loading

qodo-free-for-open-source-projects bot commented Feb 6, 2026 •

edited

Loading

qodo-free-for-open-source-projects bot commented Feb 18, 2026 •

edited

Loading

coveralls commented Feb 23, 2026 •

edited

Loading