Skip to content

fix(db): remove hardcoded Postgres epoch expressions from query SQL - #542

Merged
emoss08 merged 1 commit into
masterfrom
claude/trenova-sqlite-support-4oc8h8
Aug 12, 2026
Merged

fix(db): remove hardcoded Postgres epoch expressions from query SQL#542
emoss08 merged 1 commit into
masterfrom
claude/trenova-sqlite-support-4oc8h8

Conversation

@emoss08

@emoss08 emoss08 commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Description

Login failed on SQLite. The RBAC role lookup runs:

ura.expires_at > extract(epoch from current_timestamp)::bigint

which SQLite rejects with near "from": syntax error.

Migrations and seeding both passed beforehand, because this dialect leak lives in query code rather than in the schema. It only surfaces when that code path executes, which is a different failure class from everything fixed so far and the reason it appeared at login rather than at startup.

Related Issue or Discussion

Follow-up to #540. Same SQLite development-support effort.

Type of Change

  • Bug fix
  • Feature
  • Documentation
  • Refactor
  • Tests
  • Build, CI, or infrastructure

Scope

pkg/dbdialect — adds Kind.NowEpoch(), returning extract(epoch from current_timestamp)::bigint on PostgreSQL and unixepoch() on SQLite, plus NowEpochFromBun() for call sites holding only a bun.IDB or a transaction. (*postgres.Connection).NowEpoch() is the convenience for repositories.

20 live call sites across 16 repository, service and seed files now use those helpers instead of the literal expression.

The roughly 398 default: tags in domain models are deliberately not touched. bun only emits a default for a zero value, and BeforeAppendModel sets created_at/updated_at before every insert, so those never reach the database.

TestNoHardcodedEpochExpression fails the build if the expression reappears in query SQL. Worth noting that writing it immediately found six occurrences a grep had already missed — different casing and multi-line forms — which is the argument for having it rather than trusting a search.

Three files are genuinely Postgres-only rather than careless, and opt out with a dialect:postgres-only marker near the package clause:

  • databasesessionrepository reads pg_stat_activity and pg_blocking_pids. It now gates on a new CapSessionDiagnostic capability and returns 501 on SQLite.
  • The reporting compiler and A/R analytics bucket dates with date_trunc. Left as-is and recorded as an outstanding gap; a strftime equivalent is real work and did not belong in a login fix.

docs/databases.md gains a section on dialect leaks in query code, and a table of the remaining runtime surface — ILIKE in 17 files, roughly 109 numeric and jsonb casts, and the date_trunc bucketing — so the next contributor is not left discovering it one failing request at a time. Two stale figures are corrected while there: the converter now emits 1518 statements rather than 1471, and numeric maps to REAL, not NUMERIC.

Validation

  • cd services/tms && task test — passes
  • cd services/tms && task lintnot run. golangci-lint does not start here: built against Go 1.25 while the module targets 1.26. Changed files were formatted with github.com/golangci/golines v0.15.0, the fork golangci-lint v2.12.2 embeds, which is what CI checks.
  • cd client && pnpm build — no client changes
  • cd client && pnpm lint — no client changes
  • Other: go build ./... — clean
  • Other: go test ./pkg/dbdialect/ ./pkg/dberror/ ./internal/infrastructure/database/seeder/ — passes, including the full SQLite seed run

Deployment Notes

No migrations, config or env changes.

PostgreSQL generates identical SQL: Kind.NowEpoch() returns exactly the expression that was previously hardcoded. The only behavioural change on PostgreSQL is that database session diagnostics now pass through a capability check, which PostgreSQL always satisfies.

Checklist

  • I kept the change focused and reviewable.
  • I followed AGENTS.md, CLAUDE.md, and existing repository patterns.
  • I added or updated tests for behavior changes, or explained why tests are not applicable.
  • I updated relevant documentation, examples, migrations, or configuration.
  • I did not include secrets, credentials, private customer data, unrelated refactors, or placeholder code.

Generated by Claude Code

Summary by CodeRabbit

  • New Features

    • Improved SQLite compatibility, including support for more column conversions and DROP COLUMN operations.
    • Added database-aware timestamp handling across supported database systems.
    • Added capability checks for database session diagnostics.
  • Bug Fixes

    • Corrected numeric type conversion to use SQLite REAL values where appropriate.
    • Improved reliability of timestamp updates across database operations.
  • Documentation

    • Documented database-specific query behavior, timestamp helpers, compatibility markers, and remaining SQLite limitations.

Login failed on SQLite when the RBAC role lookup ran
"ura.expires_at > extract(epoch from current_timestamp)::bigint". Migrations and
seeding both passed first, because this dialect leak lives in query code rather
than in the schema, so it only surfaced when that path executed.

Adds Kind.NowEpoch, plus dbdialect.NowEpochFromBun and a NowEpoch method on the
connection for call sites that hold one, and routes all 20 live occurrences
through them. Model default: tags are left alone: bun only emits those for zero
values and BeforeAppendModel sets the fields first, so they never reach the
database.

TestNoHardcodedEpochExpression now fails if the expression reappears in query
SQL. Writing it immediately found six occurrences a grep had missed, which is
the point of having it.

Three files are genuinely Postgres-only rather than sloppy, and opt out with a
dialect:postgres-only marker: the session diagnostics repository reads
pg_stat_activity and pg_blocking_pids, and the reporting compiler and A/R
analytics bucket dates with date_trunc. The diagnostics repository now gates on
a new CapSessionDiagnostic capability and returns 501 on SQLite; the two date
bucketing sites are recorded in docs/databases.md as an outstanding gap.

Also documents the rest of the known runtime surface, ILIKE and the numeric
casts, so the next contributor does not have to find it one failing request at
a time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FXYnxBszfwkNUeeAcVSSNf
@cloudflare-workers-and-pages

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
trenova b054248 Aug 12 2026, 04:34 PM

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change centralizes dialect-specific epoch SQL, adds PostgreSQL session-diagnostic capability checks, replaces hardcoded epoch expressions, documents SQLite conversion details, and applies formatting-only updates.

Changes

Database dialect portability

Layer / File(s) Summary
Dialect contracts and validation
services/tms/pkg/dbdialect/*
Adds Kind.NowEpoch, NowEpochFromBun, CapSessionDiagnostic, dialect capability classification, and tests for generated SQL and hardcoded epoch expressions.
Timestamp helper integration
services/tms/internal/core/services/ediservice/service.go, services/tms/internal/infrastructure/database/seeds/development/12_driverpay_ledger.go, services/tms/internal/infrastructure/postgres/connection.go, services/tms/internal/infrastructure/postgres/repositories/*
Replaces inline PostgreSQL epoch expressions with database-specific helpers in updates, upserts, filters, and balance operations.
PostgreSQL capability gating
services/tms/internal/infrastructure/postgres/repositories/databasesessionrepository/databasesession.go, services/tms/internal/core/services/reporting/compiler/outputs.go, services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go
Gates session diagnostics by capability and marks date_trunc query code as PostgreSQL-only.
Conversion documentation and formatting
docs/databases.md, services/tms/internal/core/services/hosprojection/project_test.go, services/tms/internal/infrastructure/pdfrender/gotenberg/client_live_test.go, services/tms/internal/infrastructure/reporting/render/render_test.go
Updates SQLite conversion documentation and applies formatting-only changes to tests and imports.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Repository
  participant DatabaseConnection
  participant Dialect
  Repository->>DatabaseConnection: request NowEpoch()
  DatabaseConnection->>Dialect: resolve configured database kind
  Dialect-->>DatabaseConnection: return dialect-specific epoch SQL
  DatabaseConnection-->>Repository: return timestamp expression
  Repository->>DatabaseConnection: execute query with epoch expression
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: replacing hardcoded PostgreSQL epoch expressions with dialect-aware SQL.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/trenova-sqlite-support-4oc8h8

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

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@emoss08
emoss08 merged commit 08b27ec into master Aug 12, 2026
18 of 20 checks passed
@emoss08
emoss08 deleted the claude/trenova-sqlite-support-4oc8h8 branch August 12, 2026 16:36

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (3)
services/tms/pkg/dbdialect/dbdialect.go (1)

62-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover CapSessionDiagnostic in the dialect tests.

The shown TestNowEpoch test checks only SQL output. Add assertions that dbdialect.Postgres.Supports(db...CapSessionDiagnostic) is true and dbdialect.SQLite.Supports(db...CapSessionDiagnostic) is false. This protects the new SQLite session-diagnostic gate.

Also applies to: 77-77

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@services/tms/pkg/dbdialect/dbdialect.go` at line 62, Extend the existing
TestNowEpoch dialect test to assert capability support for CapSessionDiagnostic:
Postgres.Supports must be true and SQLite.Supports must be false. Keep the
existing SQL-output assertions unchanged and use the current dbdialect
capability-testing conventions.
services/tms/internal/infrastructure/postgres/connection.go (1)

404-404: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the added Go comments in infrastructure/postgres code.

Both changes violate the repository path rule that forbids comments in Go code.

  • services/tms/internal/infrastructure/postgres/connection.go#L404-L404: remove the new NowEpoch comment.
  • services/tms/internal/infrastructure/postgres/repositories/databasesessionrepository/databasesession.go#L1-L2: remove the PostgreSQL-only comment; it also uses the incorrect identifier CapSessionDiagnostics.

As per coding guidelines, services/tms/internal/infrastructure/postgres/**/*.go must not add comments to Go code.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@services/tms/internal/infrastructure/postgres/connection.go` at line 404,
Remove the added NowEpoch comment in
services/tms/internal/infrastructure/postgres/connection.go at lines 404-404.
Also remove the PostgreSQL-only comment referencing CapSessionDiagnostics in
services/tms/internal/infrastructure/postgres/repositories/databasesessionrepository/databasesession.go
at lines 1-2; no replacement comments are needed.

Source: Coding guidelines

services/tms/internal/infrastructure/postgres/repositories/edimessagerepository/edimessage.go (1)

293-293: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use generated Bun column helpers for all changed repository column references.

These changed repository statements continue to embed column names as string literals. Replace each field reference with its generated helper while retaining the dialect-aware epoch expression.

  • services/tms/internal/infrastructure/postgres/repositories/edimessagerepository/edimessage.go#L293-L293: use the generated EDI message UpdatedAt helper.
  • services/tms/internal/infrastructure/postgres/repositories/edimessagerepository/edimessage.go#L630-L630: use the generated EDI message UpdatedAt helper.
  • services/tms/internal/infrastructure/postgres/repositories/emailrepository/email.go#L295-L295: use the generated profile-assignment UpdatedAt helper.
  • services/tms/internal/infrastructure/postgres/repositories/invoiceadjustmentrepository/invoiceadjustment.go#L594-L594: use the generated batch-item UpdatedAt helper.
  • services/tms/internal/infrastructure/postgres/repositories/journalpostingrepository/journalposting.go#L349-L349: use the generated GL-account UpdatedAt helper.
  • services/tms/internal/infrastructure/postgres/repositories/rbacrepository/rbac.go#L170-L170: use the generated user-role-assignment expiration helper and preserve the ura alias.
  • services/tms/internal/infrastructure/postgres/repositories/shipmentimportchatrepository/shipmentimportchat.go#L158-L158: use the generated conversation UpdatedAt helper.
  • services/tms/internal/infrastructure/postgres/repositories/tableconfigurationrepository/tableconfiguration.go#L412-L412: use the generated table-configuration UpdatedAt helper.
  • services/tms/internal/infrastructure/postgres/repositories/tableconfigurationrepository/tableconfiguration.go#L444-L444: use the generated table-configuration UpdatedAt helper.

As per coding guidelines, repository code must use generated column helpers from services/tms/pkg/buncolgen/ and must not hand-write column references. Based on learnings, this rule also applies to these repository paths.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@services/tms/internal/infrastructure/postgres/repositories/edimessagerepository/edimessage.go`
at line 293, Replace every hand-written changed repository column reference with
the corresponding generated helper from services/tms/pkg/buncolgen/, while
preserving each dialect-aware r.db.NowEpoch() expression. Update EDI message
UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/edimessagerepository/edimessage.go:293-293
and :630-630; profile-assignment UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/emailrepository/email.go:295-295;
batch-item UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/invoiceadjustmentrepository/invoiceadjustment.go:594-594;
GL-account UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/journalpostingrepository/journalposting.go:349-349;
user-role-assignment expiration at
services/tms/internal/infrastructure/postgres/repositories/rbacrepository/rbac.go:170-170
while preserving the ura alias; conversation UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/shipmentimportchatrepository/shipmentimportchat.go:158-158;
and table-configuration UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/tableconfigurationrepository/tableconfiguration.go:412-412
and :444-444.

Sources: Coding guidelines, Learnings

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/databases.md`:
- Line 86: Correct the `ALTER TABLE ... DROP COLUMN` description to state that
the converter emits the statement after removing blocking indexes when no index
or constraint still references the column; when a reference remains, it should
omit the statement.

In `@services/tms/pkg/dbdialect/dbdialect.go`:
- Around line 121-123: Remove the added NowEpoch documentation comment in
services/tms/pkg/dbdialect/dbdialect.go (lines 121-123), and remove the
dialect:postgres-only annotations in
services/tms/internal/core/services/reporting/compiler/outputs.go (line 1) and
services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go
(line 1); no other changes are needed.

In `@services/tms/pkg/dbdialect/nowepoch_test.go`:
- Around line 50-53: Update the marker handling in the nowepoch test so
PostgreSQL-only files are exempted only when they are explicitly approved or
contain a capability guard such as dbdialect.RequireFromBun(...,
dbdialect.CapSessionDiagnostic); otherwise continue validating the file instead
of returning nil. Preserve the existing marker behavior for approved guarded
files.

---

Nitpick comments:
In `@services/tms/internal/infrastructure/postgres/connection.go`:
- Line 404: Remove the added NowEpoch comment in
services/tms/internal/infrastructure/postgres/connection.go at lines 404-404.
Also remove the PostgreSQL-only comment referencing CapSessionDiagnostics in
services/tms/internal/infrastructure/postgres/repositories/databasesessionrepository/databasesession.go
at lines 1-2; no replacement comments are needed.

In
`@services/tms/internal/infrastructure/postgres/repositories/edimessagerepository/edimessage.go`:
- Line 293: Replace every hand-written changed repository column reference with
the corresponding generated helper from services/tms/pkg/buncolgen/, while
preserving each dialect-aware r.db.NowEpoch() expression. Update EDI message
UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/edimessagerepository/edimessage.go:293-293
and :630-630; profile-assignment UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/emailrepository/email.go:295-295;
batch-item UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/invoiceadjustmentrepository/invoiceadjustment.go:594-594;
GL-account UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/journalpostingrepository/journalposting.go:349-349;
user-role-assignment expiration at
services/tms/internal/infrastructure/postgres/repositories/rbacrepository/rbac.go:170-170
while preserving the ura alias; conversation UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/shipmentimportchatrepository/shipmentimportchat.go:158-158;
and table-configuration UpdatedAt at
services/tms/internal/infrastructure/postgres/repositories/tableconfigurationrepository/tableconfiguration.go:412-412
and :444-444.

In `@services/tms/pkg/dbdialect/dbdialect.go`:
- Line 62: Extend the existing TestNowEpoch dialect test to assert capability
support for CapSessionDiagnostic: Postgres.Supports must be true and
SQLite.Supports must be false. Keep the existing SQL-output assertions unchanged
and use the current dbdialect capability-testing conventions.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: e26f5824-e655-4a77-a5e6-0b89bf99ac50

📥 Commits

Reviewing files that changed from the base of the PR and between 214eab0 and b054248.

📒 Files selected for processing (28)
  • docs/databases.md
  • services/tms/internal/core/services/ediservice/service.go
  • services/tms/internal/core/services/hosprojection/project_test.go
  • services/tms/internal/core/services/reporting/compiler/outputs.go
  • services/tms/internal/infrastructure/database/seeds/development/12_driverpay_ledger.go
  • services/tms/internal/infrastructure/pdfrender/gotenberg/client_live_test.go
  • services/tms/internal/infrastructure/postgres/connection.go
  • services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go
  • services/tms/internal/infrastructure/postgres/repositories/databasesessionrepository/databasesession.go
  • services/tms/internal/infrastructure/postgres/repositories/documentparsingrulerepository/documentparsingrule.go
  • services/tms/internal/infrastructure/postgres/repositories/driverpayrepository/assignment.go
  • services/tms/internal/infrastructure/postgres/repositories/driverpayrepository/payprofile.go
  • services/tms/internal/infrastructure/postgres/repositories/driversettlementrepository/batch.go
  • services/tms/internal/infrastructure/postgres/repositories/edimappingprofilerepository/edimappingprofiler.go
  • services/tms/internal/infrastructure/postgres/repositories/edimessagerepository/edimessage.go
  • services/tms/internal/infrastructure/postgres/repositories/editenderchangerepository/editenderchange.go
  • services/tms/internal/infrastructure/postgres/repositories/editenderrecipientrepository/editenderrecipient.go
  • services/tms/internal/infrastructure/postgres/repositories/editransferrepository/editransfer.go
  • services/tms/internal/infrastructure/postgres/repositories/emailrepository/email.go
  • services/tms/internal/infrastructure/postgres/repositories/invoiceadjustmentrepository/invoiceadjustment.go
  • services/tms/internal/infrastructure/postgres/repositories/journalpostingrepository/journalposting.go
  • services/tms/internal/infrastructure/postgres/repositories/rbacrepository/rbac.go
  • services/tms/internal/infrastructure/postgres/repositories/shipmentimportchatrepository/shipmentimportchat.go
  • services/tms/internal/infrastructure/postgres/repositories/tableconfigurationrepository/tableconfiguration.go
  • services/tms/internal/infrastructure/reporting/render/render_test.go
  • services/tms/pkg/dbdialect/bun.go
  • services/tms/pkg/dbdialect/dbdialect.go
  • services/tms/pkg/dbdialect/nowepoch_test.go
💤 Files with no reviewable changes (1)
  • services/tms/internal/infrastructure/reporting/render/render_test.go

Comment thread docs/databases.md
| `ALTER TABLE ... ADD CONSTRAINT` | SQLite cannot add constraints after creation |
| `ALTER TABLE ... ALTER COLUMN` | SQLite cannot alter column definitions |
| `ALTER TABLE ... DROP COLUMN` | rejected whenever an index or `CHECK` still references the column |
| `ALTER TABLE ... DROP COLUMN` | only when an index or constraint still pins the column; otherwise it is emitted, dropping blocking indexes first |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Correct the DROP COLUMN condition.

Line 86 says the statement is emitted only when an index or constraint still references the column, then says it is emitted otherwise. This reverses the documented behavior. State that the converter emits the statement after removing blocking indexes when no index or constraint still references the column; otherwise it drops the statement.

Proposed wording
-| `ALTER TABLE ... DROP COLUMN` | only when an index or constraint still pins the column; otherwise it is emitted, dropping blocking indexes first |
+| `ALTER TABLE ... DROP COLUMN` | emitted after dropping blocking indexes when no index or constraint still references the column; otherwise the statement is dropped |

This follows the stated converter behavior in the line-range change details.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
| `ALTER TABLE ... DROP COLUMN` | only when an index or constraint still pins the column; otherwise it is emitted, dropping blocking indexes first |
| `ALTER TABLE ... DROP COLUMN` | emitted after dropping blocking indexes when no index or constraint still references the column; otherwise the statement is dropped |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/databases.md` at line 86, Correct the `ALTER TABLE ... DROP COLUMN`
description to state that the converter emits the statement after removing
blocking indexes when no index or constraint still references the column; when a
reference remains, it should omit the statement.

Comment on lines +121 to +123
// NowEpoch returns the SQL expression for the current Unix timestamp as an
// integer. PostgreSQL has no portable spelling of this and SQLite has no
// extract(), so query builders must ask for it rather than hardcoding either.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- repository guidance ---'
for f in CLAUDE.md AGENTS.md; do
  if [ -f "$f" ]; then
    echo "### $f"
    cat "$f"
  fi
done

printf '%s\n' '--- target files and line ranges ---'
for f in \
  services/tms/pkg/dbdialect/dbdialect.go \
  services/tms/internal/core/services/reporting/compiler/outputs.go \
  services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go
do
  echo "### $f"
  wc -l "$f"
  case "$f" in
    */dbdialect.go) sed -n '108,132p' "$f" ;;
    */outputs.go) sed -n '1,20p' "$f" ;;
    */analytics.go) sed -n '1,20p' "$f" ;;
  esac
done

printf '%s\n' '--- targeted diff ---'
git diff --unified=8 -- \
  services/tms/pkg/dbdialect/dbdialect.go \
  services/tms/internal/core/services/reporting/compiler/outputs.go \
  services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go

Repository: emoss08/Trenova

Length of output: 19164


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- repository state ---'
git status --short
git rev-parse --show-toplevel
git log -2 --oneline --decorate

printf '%s\n' '--- target-file change in HEAD versus its parent ---'
git diff --stat HEAD^ HEAD -- \
  services/tms/pkg/dbdialect/dbdialect.go \
  services/tms/internal/core/services/reporting/compiler/outputs.go \
  services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go

git diff --unified=12 HEAD^ HEAD -- \
  services/tms/pkg/dbdialect/dbdialect.go \
  services/tms/internal/core/services/reporting/compiler/outputs.go \
  services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go

printf '%s\n' '--- commits that introduced the exact comments ---'
git log --all --oneline -S'NowEpoch returns the SQL expression for the current Unix timestamp' -- services/tms/pkg/dbdialect/dbdialect.go
git log --all --oneline -S'dialect:postgres-only — report date bucketing compiles to date_trunc.' -- services/tms/internal/core/services/reporting/compiler/outputs.go
git log --all --oneline -S'dialect:postgres-only — collections analytics bucket dates with date_trunc.' -- services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go

Repository: emoss08/Trenova

Length of output: 4977


Remove or relocate the three added Go comments.

  • services/tms/pkg/dbdialect/dbdialect.go: remove the NowEpoch documentation comment.
  • services/tms/internal/core/services/reporting/compiler/outputs.go: remove the dialect:postgres-only annotation.
  • services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go: remove the dialect:postgres-only annotation.
📍 Affects 3 files
  • services/tms/pkg/dbdialect/dbdialect.go#L121-L123 (this comment)
  • services/tms/internal/core/services/reporting/compiler/outputs.go#L1-L1
  • services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go#L1-L1
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@services/tms/pkg/dbdialect/dbdialect.go` around lines 121 - 123, Remove the
added NowEpoch documentation comment in services/tms/pkg/dbdialect/dbdialect.go
(lines 121-123), and remove the dialect:postgres-only annotations in
services/tms/internal/core/services/reporting/compiler/outputs.go (line 1) and
services/tms/internal/infrastructure/postgres/repositories/accountsreceivablerepository/analytics.go
(line 1); no other changes are needed.

Source: Coding guidelines

Comment on lines +50 to +53
// Files that are deliberately Postgres-only opt out with a marker.
// They must gate themselves on a capability instead.
if strings.Contains(string(contents), postgresOnlyMarker) {
return nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Make the PostgreSQL-only exemption enforce its capability check.

The marker check skips the entire file. It does not verify dbdialect.RequireFromBun(..., dbdialect.CapSessionDiagnostic) or another capability guard. A future file can add the marker and hardcode PostgreSQL SQL while this test still passes. Restrict the exemption to approved files or validate the guard before returning.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@services/tms/pkg/dbdialect/nowepoch_test.go` around lines 50 - 53, Update the
marker handling in the nowepoch test so PostgreSQL-only files are exempted only
when they are explicitly approved or contain a capability guard such as
dbdialect.RequireFromBun(..., dbdialect.CapSessionDiagnostic); otherwise
continue validating the file instead of returning nil. Preserve the existing
marker behavior for approved guarded files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants