Skip to content

refactor(backend): collapse duplicate PR row types into one domain definition#60

Merged
AgentWrapper merged 2 commits into
mainfrom
refactor/collapse-pr-row-types
May 31, 2026
Merged

refactor(backend): collapse duplicate PR row types into one domain definition#60
AgentWrapper merged 2 commits into
mainfrom
refactor/collapse-pr-row-types

Conversation

@AgentWrapper

Copy link
Copy Markdown
Contributor

What

Collapses the duplicate PR row types. Each PR-child table (pr / pr_checks / pr_comment) had three near-identical structs — gen.* (sqlc-generated), sqlite.*Row, and ports.* — with wiring.Adapter copying field-by-field between them. This keeps one shared definition per table in domain, used by both the PRWriter port and the sqlite store. The generated gen.* stays sealed inside the storage layer (it's the one layer that legitimately can't be the canonical model — driver-typed, flat, generated).

Changes

  • Deleted the entire wiring.Adapter package. *sqlite.Store now satisfies ports.SessionStore + ports.PRWriter directly, so the composition root is just lifecycle.New(store, store, …) — no bridge.
  • Added domain.PRRow / domain.PRCheckRow / domain.PRComment; removed the sqlite.*Row and ports.PR* copies.
  • The real translation (bool PR state ↔ single state column, intint64, enum defaults) now lives in exactly one place — pr_store.go's gen ↔ domain boundary.
  • WritePRObservationWritePR to match the port. The integration test and wiring_test.go drop their adapter copies.

Impact

13 files changed, 186 insertions(+), 466 deletions(-)

From 3 structs per table → 2 (the generated one + one shared domain type), behaviour unchanged. go build, go vet, and go test -race ./... all green.

Note for review

PRFactsForSession's "which PR drives display status" rule was already a *Store method (pr_facts.go); deleting the adapter leaves it there. It's a read-query but mild product logic in the storage layer — happy to pull it into a thin helper if reviewers prefer storage stay rule-free.

🤖 Generated with Claude Code

@greptile-apps

greptile-apps Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR collapses three layers of near-identical PR-row structs (gen.*, sqlite.*Row, ports.*) down to two by introducing shared domain.PRRow, domain.PRCheckRow, and domain.PRComment types, and deleting the wiring.Adapter package entirely. *sqlite.Store now directly implements both ports.SessionStore and ports.PRWriter, so the composition root wires store straight into lifecycle.New.

  • wiring/adapter.go deleted: all field-by-field copying between ports.* and sqlite.*Row shapes is gone; the only remaining translation boundary is pr_store.go's gen ↔ domain helper functions (genPRParams, prRowFromGen, genCheckParams, checkRowFromGen, genCommentParams, commentFromGen).
  • WritePRObservationWritePR: method renamed to match the port signature; callers in the lifecycle manager, integration tests, and wiring tests are updated consistently.
  • PRFactsForSession moved to *Store: previously lived in the adapter as Adapter.PRFactsForSession; now a first-class method on *Store with the equivalent but cleaner !r.Merged && !r.Closed guard replacing the old string-based r.State == "draft" || r.State == "open" predicate.

Confidence Score: 5/5

Safe to merge — purely structural, no logic added or removed.

Every behavioural path is preserved exactly. The state-column encoding round-trips correctly through the new helper functions, the PRFactsForSession predicate change is semantically equivalent, default enum handling is faithfully replicated, and compile-time interface guards are present.

No files require special attention.

Important Files Changed

Filename Overview
backend/internal/domain/pr.go New file: introduces the three canonical domain types (PRRow, PRCheckRow, PRComment) that replace the parallel ports.* and sqlite.*Row definitions.
backend/internal/storage/sqlite/pr_store.go Core of the refactor: removes the three sqlite.*Row struct definitions, adds compile-time interface guards, moves all gen↔domain translation into focused helper functions, and renames WritePRObservation to WritePR.
backend/internal/storage/sqlite/pr_facts.go PRFactsForSession moved from the deleted wiring.Adapter; logic is semantically equivalent — string state comparison replaced with boolean field checks on the now-decoded domain.PRRow.
backend/internal/storage/sqlite/wiring/adapter.go Deleted entirely. All bridging logic (PRFactsForSession, WritePR, prState) now lives directly on *sqlite.Store or in pr_store.go helpers.
backend/internal/ports/facts.go Removes the three ports.PR* DTO types (PRRow, PRCheckRow, PRComment) and updates PRObservation.Checks/Comments to use the new domain types.
backend/internal/ports/outbound.go PRWriter.WritePR signature updated to use domain.* types; no logic changes.
backend/internal/lifecycle/manager.go writePR updated to construct domain.PRRow/PRCheckRow/PRComment instead of ports.*; behavior unchanged.
backend/internal/daemon/lifecycle_wiring.go Removes wiring.Adapter instantiation; lifecycle.New now receives store directly; lifecycleStack.Adapter field replaced with Store.
backend/internal/integration/lifecycle_sqlite_test.go Drops the ~90-line in-test storeAdapter duplicate; integration tests now wire store directly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    subgraph Before
        LCM_OLD["lifecycle.Manager"] -->|"ports.PRRow/PRCheckRow/PRComment"| WA["wiring.Adapter"]
        WA -->|"sqlite.PRRow/PRCheckRow/PRCommentRow"| STORE_OLD["*sqlite.Store"]
        STORE_OLD -->|"gen.* params"| DB[("SQLite DB")]
        WA -.->|"3 struct copies"| STRUCTS["ports.* + sqlite.* + gen.*"]
    end
    subgraph After
        LCM_NEW["lifecycle.Manager"] -->|"domain.PRRow/PRCheckRow/PRComment"| STORE_NEW["*sqlite.Store"]
        STORE_NEW -->|"genPRParams / genCheckParams / genCommentParams"| DB2[("SQLite DB")]
        STORE_NEW -.->|"2 struct copies"| STRUCTS2["domain.* + gen.*"]
    end
    style WA fill:#f99,stroke:#900
    style STRUCTS fill:#f99,stroke:#900
    style STORE_NEW fill:#9f9,stroke:#090
    style STRUCTS2 fill:#9f9,stroke:#090
Loading

Reviews (2): Last reviewed commit: "refactor(storage): add compile-time port..." | Re-trigger Greptile

Comment thread backend/internal/storage/sqlite/pr_store.go
AgentWrapper added a commit that referenced this pull request May 31, 2026
Re-add the blank-identifier interface assertions lost when wiring.Adapter was
collapsed: *Store now directly satisfies ports.SessionStore and ports.PRWriter,
so prove it at the point of definition. Drift between either port and the
implementation now fails here instead of at the call sites in lifecycle_wiring
or tests.

Addresses greptile review comment on #60.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AgentWrapper and others added 2 commits June 1, 2026 03:40
…finition

Each PR-child table (pr / pr_checks / pr_comment) had three near-identical
structs — gen.* (generated), sqlite.*Row, and ports.* — with wiring.Adapter
copying field-by-field between them. Collapse to one shared definition per
table in domain (PRRow / PRCheckRow / PRComment), used by both the PRWriter
port and the sqlite store; gen.* stays sealed inside the storage layer.

- *sqlite.Store now satisfies ports.SessionStore + ports.PRWriter directly,
  so the entire wiring.Adapter package is deleted (lifecycle.New(store, store)).
- The bool PR state <-> single state column, int<->int64, and enum-default
  translation now lives only at the gen<->domain boundary in pr_store.go.
- WritePRObservation renamed WritePR to match the port; the integration test
  and composition root drop their adapter copies.

Net -280 lines, behaviour unchanged. go test -race ./... green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Re-add the blank-identifier interface assertions lost when wiring.Adapter was
collapsed: *Store now directly satisfies ports.SessionStore and ports.PRWriter,
so prove it at the point of definition. Drift between either port and the
implementation now fails here instead of at the call sites in lifecycle_wiring
or tests.

Addresses greptile review comment on #60.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@AgentWrapper AgentWrapper force-pushed the refactor/collapse-pr-row-types branch from 0baa4f0 to 42eab57 Compare May 31, 2026 22:11
@AgentWrapper AgentWrapper merged commit 70718f8 into main May 31, 2026
6 checks passed
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.

1 participant