feat: fee rules per field with predicate-based matching by fredcamaral · Pull Request #54 · LerianStudio/matcher

fredcamaral · 2026-03-21T14:56:28Z

Summary

Implements fee rules per field with enhanced predicate-based matching for transaction reconciliation.

Changes

feat(configuration): add fee rule tests and improve swagger docs
feat(fee): improve predicate value type coercion
docs(migrations): add comprehensive cutover documentation
refactor(tests): split dashboard stresser into focused test files
refactor(matching): split rule execution into focused modules

Testing

Unit tests added for fee rule functionality
Integration tests for fee calculation flows
Dashboard stress tests reorganized

Migration Notes

Includes comprehensive cutover documentation for database migrations
Fee predicate value handling improved for type coercion

…solution Introduces the shared kernel types for metadata-driven fee schedule selection: FeeRule entity with priority-ordered evaluation, FieldPredicate value object supporting EQUALS/IN/EXISTS operators, MatchingSide enum (LEFT/RIGHT/ANY) for directional rule targeting, and ResolveFeeSchedule function that evaluates transaction metadata against ordered predicates to find the first matching schedule. Includes database migration for the fee_rules and fee_rule_predicates tables. X-Lerian-Ref: 0x1

Implements the full configuration-side lifecycle for fee rules: repository interface with FindByContextID/FindByID/Create/Update/Delete, PostgreSQL adapter with tenant-isolated JSONB predicate storage, command and query services, HTTP handlers with Swagger annotations, request/response DTOs with validation, and route registration under /v1/config/contexts/:contextId/fee-rules. Adds fee-rule:create/read/update/delete auth action constants. X-Lerian-Ref: 0x1

…d resolution Removes FeeScheduleID from ReconciliationSource entity, SourceInfo port, DTOs, clone logic, and E2E types -- eliminating the static source-to-schedule binding. Fee schedule selection is now driven by FeeRule predicates evaluated against transaction metadata at match time: loadFeeRulesAndSchedules fetches context-scoped rules, splits them by side (LEFT/RIGHT/ANY), and ResolveFeeSchedule picks the first matching schedule per transaction. Wires FeeRuleProvider into the matching UseCase via a cross-context adapter backed by the configuration repository, and updates bootstrap, integration tests, and all matching unit tests. X-Lerian-Ref: 0x1

X-Lerian-Ref: 0x1

Changes .air.toml and docker-compose.yml build flags from -mod=vendor to -mod=mod, allowing direct module resolution without a vendored directory. X-Lerian-Ref: 0x1

Introduces a required Side field (LEFT/RIGHT) on reconciliation sources, replacing the runtime primarySourceId approach with a declarative configuration model. Sources now carry their matching role at creation time, enabling deterministic left/right classification during match execution. Key changes: - Domain entity validates Side is exclusive (LEFT/RIGHT only, not ANY) - Postgres adapter persists and scans the new side column - Cross-context adapters propagate Side through shared kernel types - Migration 000017 adds the side column with LEFT default X-Lerian-Ref: 0x1

Hardens fee rule lifecycle with input sanitization, proper error propagation, and referential integrity enforcement. Validation: trims whitespace on name and predicate fields before checking invariants. Error handling: maps sql.ErrNoRows to domain ErrFeeRuleNotFound in postgres adapter; FK constraint 23503 violations surface as ErrFeeScheduleNotFound for missing schedules. Ownership: GetFeeRule, UpdateFeeRule, and DeleteFeeRule now verify context belongs to the requesting tenant before proceeding. Referential integrity: deleting a fee schedule referenced by fee rules returns 409 Conflict. Migration: 000016 uses named FK constraint fk_fee_rules_fee_schedule; removes legacy fee_schedule_id column drop (kept during rollout). X-Lerian-Ref: 0x1

Reworks context cloning to deep-copy fee rules while reusing referenced fee schedules. Previously, cloning duplicated fee schedules; now it duplicates the rules that point to them, preserving the schedule-as-shared-reference model. Removes includeFeeSchedules option from clone request. Adds FeeRulesCloned counter to clone result. Clone copies source Side into cloned sources. X-Lerian-Ref: 0x1

Removes the runtime primarySourceId parameter from RunMatchRequest. Left/right classification is now determined entirely by the source's configured Side field, making match topology declarative and deterministic. Adds validateSourceCountForContextType enforcing 1:1, 1:N, and M:N topology constraints. loadFeeRulesAndSchedules now returns an error instead of silently falling back, and validates that every fee rule references a loaded schedule. Fee normalization is skipped when fee mode is NONE. X-Lerian-Ref: 0x1

Injects configFeeRule repository into both command and query use cases via WithFeeRuleRepository option, enabling fee rule CRUD and clone operations at runtime. X-Lerian-Ref: 0x1

Reflects all API surface changes: source Side field (required), fee rule priority uniqueness description, predicates maxItems:50, clone response feeRulesCloned counter, and removal of primarySourceId from RunMatchRequest. X-Lerian-Ref: 0x1

Propagates the required Side field into all source creation inputs across integration tests. Removes PrimarySourceID from RunMatch inputs. Adds fee rule repository integration test and updates clone tests to verify FeeRulesCloned counts. X-Lerian-Ref: 0x1

…ource side Adds FeeRule client methods, factory with auto-side-assignment (LEFT for first source, RIGHT for subsequent), and full fee rule lifecycle types. Updates fee schedule journeys to create explicit fee rules per side, replacing the former per-source schedule model. Factory cleanup handles 404 gracefully for cascade-deleted schedules. X-Lerian-Ref: 0x1

Enforce field name (255), value (1024), and IN-clause count (100) limits on field predicates to prevent oversized payloads and storage bloat. Remove DEFAULT on priority column to force explicit rule ordering at insert time. Add boundary and no-op update tests for fee rules, and IsExclusive coverage for matching side semantics. X-Lerian-Ref: 0x1

…mprove persistence Add max-length validation tags to FieldPredicate DTO (field=255, value=1024, values=100). Add NotFound handler test with FindByID mock override. Trace all repository errors unconditionally instead of skipping sql.ErrNoRows. Make source matching side nullable via sql.NullString to support optional side assignment. X-Lerian-Ref: 0x1

…te error mapping Replace 8 individual bad-request switch branches with a data-driven error map for maintainability. Add 422 response for fee rules referencing missing schedules. Remove nil-guard on feeRuleProvider (now a required dependency) and delete its obsolete test. X-Lerian-Ref: 0x1

Simplify RunMatch, RunMatchCommit, and RunMatchDryRun signatures to use symmetric mode by default. Update all 30 journey tests to drop the now- removed third argument. X-Lerian-Ref: 0x1

…ction Replace single action parameter with variadic actions slice in ProtectedGroupWithMiddleware. Enables endpoints to require multiple authorization checks (e.g., fee rule write + fee schedule read). Updates all bounded context route registrations to use new signature. X-Lerian-Ref: 0x1

…ints Introduce MaxFeeRulesPerContext constant (50 rules per context limit). Add ErrFeeRuleCountLimitExceeded error for enforcement. Add predicate validation errors for forbidden value combinations. Update MatchingSide validation to accept LEFT/RIGHT/ANY case-insensitively. X-Lerian-Ref: 0x1

…operations Add count limit check before creating new fee rules (max 50 per context). Require context ID for update and delete operations to ensure proper authorization. Update SQL queries to filter by context_id for security. Add ErrFeeRuleCountLimitExceeded to matching error mapping for proper HTTP response. X-Lerian-Ref: 0x1

Replace single-page query with cursor-based pagination to fetch all records. Add loop detection to prevent infinite pagination on stuck cursors. Add pagination error types: ErrMatchRulePaginationCursorDidNotAdvance, ErrSourcePaginationCursorDidNotAdvance. Use constants.MaximumPaginationLimit (1000) as page size. X-Lerian-Ref: 0x1

Update fee_rules migration to add predicates JSONB column with size constraint. Add NOT NULL constraint to source side column. Create migration to drop legacy source_fee_schedule table and foreign keys. Update all E2E and integration tests for fee rules, source side, and context cloning. X-Lerian-Ref: 0x1

Consolidate monolithic handlers.go into focused feature files: - handlers_context.go: reconciliation context CRUD operations - handlers_field_map.go: field mapping endpoints - handlers_match_rule.go: match rule management - handlers_source.go: source configuration Improves code organization and maintainability by separating concerns. X-Lerian-Ref: 0x1

Decompose source.postgresql.go into cohesive modules: - source_lookup.go: lookup operations by ID and code - source_pagination.go: cursor-based pagination logic - source_queries.go: complex query building with filters Each module has a single responsibility, improving testability. X-Lerian-Ref: 0x1

Split clone_commands.go into focused modules: - clone_context_creation.go: context cloning orchestration - clone_rules.go: match rule and fee rule cloning - clone_sources.go: source cloning with field maps Reduces cognitive load and improves testability of clone operations. X-Lerian-Ref: 0x1

Split 2667-line match_group_commands.go into 11 focused modules: - match_group_run_commands.go: primary RunMatch orchestration - match_group_run_support.go: helper functions for match run - match_group_proposal_processing.go: match proposal evaluation - match_group_persistence.go: database persistence operations - match_group_execution.go: match execution logic - match_group_source_classification.go: left/right source classification - match_group_lock_commands.go: transaction locking - match_group_manual_commands.go: manual match operations - match_group_unmatch_commands.go: unmatch operations - match_group_outbox_events.go: event publishing Each module under 500 lines, dramatically improving maintainability. X-Lerian-Ref: 0x1

Decompose rule_execution_commands.go into: - rule_execution_fee_normalization.go: fee schedule normalization logic - rule_execution_support.go: helper functions and utilities Separates fee normalization from rule evaluation for clarity. X-Lerian-Ref: 0x1

Decompose 2331-line dashboard_stresser_test.go into 6 focused tests: - dashboard_stresser_full_test.go: complete flow journey - dashboard_stresser_quick_test.go: fast validation journey - dashboard_stresser_shared_test.go: shared utilities and config - dashboard_stresser_volume_test.go: volume testing - dashboard_stresser_volume_enrichment_test.go: enrichment volume tests - dashboard_stresser_volume_summary_test.go: summary volume tests Enables running specific stress scenarios independently. X-Lerian-Ref: 0x1

Add detailed documentation to pre-launch cutover migrations: - Explain hard cutover strategy for fee rules and source sides - Document strict migration blocking behavior - Add PRODUCTION_MIGRATIONS.md section on cutover procedures - Include SQL queries for inspecting legacy state - Provide rollback guidance and recommended paths Ensures operators understand migration requirements before applying. X-Lerian-Ref: 0x1

Enhance field predicate matching with comprehensive type support: - Add stringifyPredicateValue with scalar type handling - Support bool, float, int, uint, []byte, and fmt.Stringer - Use strconv for precise numeric conversions - Add nil schedules map defense in fee rule resolver - Expand tests for boolean and float coercion scenarios Ensures accurate matching regardless of underlying metadata types. X-Lerian-Ref: 0x1

Comprehensive testing and documentation improvements: - Add fee_rule_test.go E2E journey covering full CRUD lifecycle - Improve swagger descriptions for fee rule endpoints - Add clone operation documentation details - Add routes comments on fee-schedule:read permission requirements - Add fee rule provider interface documentation - Add fee rule validation tests (invalid predicates, too many predicates) - Add deep copy test for nested structures - Add nil schedules map test in rule execution - Add concurrent duplicate priority integration test - Improve test harness rollback logging - Clarify source factory side tracking variable naming Strengthens test coverage and API documentation. X-Lerian-Ref: 0x1

coderabbitai · 2026-03-21T14:56:48Z

Walkthrough

This PR introduces fee rules: predicate-based mappings from transaction fields to fee schedules, adds persistent fee-rule storage and HTTP CRUD endpoints, changes sources to record a matching side (LEFT/RIGHT) instead of a fee schedule, updates cloning to copy fee rules (not schedules), integrates fee-rule access into matching via a FeeRuleProvider, and adapts repos, services, migrations, authorization actions, and tests to support these changes.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Handler as HTTP Handler
    participant Command as Command Service
    participant Repo as Fee Rule Repo
    participant DB as Database
    participant Audit as Audit Event

    Client->>Handler: POST /v1/config/contexts/{contextId}/fee-rules
    Handler->>Handler: Parse tenant/context, validate body
    Handler->>Command: CreateFeeRule(contextID, side, feeScheduleID, ...)
    Command->>Repo: FindByContextID(contextID)
    Repo->>DB: SELECT * FROM fee_rules WHERE context_id=?
    DB-->>Repo: existing rules
    Repo-->>Command: []*FeeRule
    Command->>Command: Validate limits, construct entity
    Command->>Repo: Create(newRule)
    Repo->>DB: INSERT INTO fee_rules (...)
    DB-->>Repo: success
    Repo-->>Command: nil
    Command->>Audit: Publish audit event
    Audit-->>Command: queued
    Command-->>Handler: created FeeRule
    Handler-->>Client: 201 Created + FeeRuleResponse

sequenceDiagram
    participant Client
    participant MatchHandler as Match HTTP Handler
    participant MatchCommand as Match Command UseCase
    participant SourceProv as Source Provider
    participant FeeRuleProv as FeeRule Provider
    participant FeeScheduleRepo as FeeSchedule Repo
    participant MatchEngine as Match Engine

    Client->>MatchHandler: POST /v1/match/run (RunMatchRequest)
    MatchHandler->>MatchCommand: RunMatch(ctx, tenantID, contextID, mode)
    MatchCommand->>SourceProv: FindByContextID(contextID)
    SourceProv-->>MatchCommand: []*SourceInfo (with Side LEFT/RIGHT)
    MatchCommand->>FeeRuleProv: FindByContextID(contextID)
    FeeRuleProv-->>MatchCommand: []*FeeRule (ordered by priority)
    MatchCommand->>FeeScheduleRepo: GetByIDs(scheduleIDs from rules)
    FeeScheduleRepo-->>MatchCommand: map[id]*FeeSchedule
    MatchCommand->>MatchEngine: Execute matching with sources, rules, schedules
    MatchEngine-->>MatchCommand: match proposals with selected fee schedules
    MatchCommand->>MatchHandler: results
    MatchHandler-->>Client: 200 OK + results

lerian-studio · 2026-03-21T14:56:57Z

Consider updating CHANGELOG.md to document this change. If this change doesn't need a changelog entry, add the skip-changelog label.

lerian-studio · 2026-03-21T14:57:02Z

This PR is very large (211 files, 29467 lines changed). Consider breaking it into smaller PRs for easier review.

lerian-studio · 2026-03-21T14:59:42Z

📊 Unit Test Coverage Report: `matcher`

Metric	Value
Overall Coverage	`80.2%` ✅ PASS
Threshold	`70%`

Coverage by Package

Package	Coverage
`github.com/LerianStudio/matcher/cmd/health-probe`	50.0%
`github.com/LerianStudio/matcher/cmd/matcher`	47.0%
`github.com/LerianStudio/matcher/internal/auth`	84.0%
`github.com/LerianStudio/matcher/internal/bootstrap`	81.9%
`github.com/LerianStudio/matcher/internal/configuration/adapters/audit`	94.8%
`github.com/LerianStudio/matcher/internal/configuration/adapters/http/dto`	95.1%
`github.com/LerianStudio/matcher/internal/configuration/adapters/http`	86.7%
`github.com/LerianStudio/matcher/internal/configuration/adapters/postgres/common`	50.0%
`github.com/LerianStudio/matcher/internal/configuration/adapters/postgres/context`	96.5%
`github.com/LerianStudio/matcher/internal/configuration/adapters/postgres/fee_rule`	83.0%
`github.com/LerianStudio/matcher/internal/configuration/adapters/postgres/field_map`	93.3%
`github.com/LerianStudio/matcher/internal/configuration/adapters/postgres/match_rule`	94.1%
`github.com/LerianStudio/matcher/internal/configuration/adapters/postgres/schedule`	77.7%
`github.com/LerianStudio/matcher/internal/configuration/adapters/postgres/source`	88.4%
`github.com/LerianStudio/matcher/internal/configuration/domain/entities`	91.1%
`github.com/LerianStudio/matcher/internal/configuration/domain/repositories/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/configuration/domain/value_objects`	100.0%
`github.com/LerianStudio/matcher/internal/configuration/ports/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/configuration/services/command`	80.4%
`github.com/LerianStudio/matcher/internal/configuration/services/query`	100.0%
`github.com/LerianStudio/matcher/internal/configuration/services/worker`	91.2%
`github.com/LerianStudio/matcher/internal/discovery/adapters/fetcher`	91.3%
`github.com/LerianStudio/matcher/internal/discovery/adapters/http/dto`	67.8%
`github.com/LerianStudio/matcher/internal/discovery/adapters/http`	86.3%
`github.com/LerianStudio/matcher/internal/discovery/adapters/postgres/connection`	89.3%
`github.com/LerianStudio/matcher/internal/discovery/adapters/postgres/extraction`	86.3%
`github.com/LerianStudio/matcher/internal/discovery/adapters/postgres/schema`	87.8%
`github.com/LerianStudio/matcher/internal/discovery/adapters/redis`	85.5%
`github.com/LerianStudio/matcher/internal/discovery/domain/entities`	86.8%
`github.com/LerianStudio/matcher/internal/discovery/domain/value_objects`	100.0%
`github.com/LerianStudio/matcher/internal/discovery/services/command`	68.4%
`github.com/LerianStudio/matcher/internal/discovery/services/query`	93.1%
`github.com/LerianStudio/matcher/internal/discovery/services/syncer`	78.3%
`github.com/LerianStudio/matcher/internal/discovery/services/worker`	68.1%
`github.com/LerianStudio/matcher/internal/exception/adapters/audit`	84.5%
`github.com/LerianStudio/matcher/internal/exception/adapters/http/connectors`	89.0%
`github.com/LerianStudio/matcher/internal/exception/adapters/http/dto`	100.0%
`github.com/LerianStudio/matcher/internal/exception/adapters/http`	90.4%
`github.com/LerianStudio/matcher/internal/exception/adapters/postgres/comment`	81.8%
`github.com/LerianStudio/matcher/internal/exception/adapters/postgres/dispute`	94.0%
`github.com/LerianStudio/matcher/internal/exception/adapters/postgres/exception`	93.2%
`github.com/LerianStudio/matcher/internal/exception/adapters/redis`	83.5%
`github.com/LerianStudio/matcher/internal/exception/adapters/resolution`	91.7%
`github.com/LerianStudio/matcher/internal/exception/adapters`	100.0%
`github.com/LerianStudio/matcher/internal/exception/domain/dispute`	99.1%
`github.com/LerianStudio/matcher/internal/exception/domain/entities`	100.0%
`github.com/LerianStudio/matcher/internal/exception/domain/repositories/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/exception/domain/services`	89.5%
`github.com/LerianStudio/matcher/internal/exception/domain/value_objects`	94.7%
`github.com/LerianStudio/matcher/internal/exception/ports/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/exception/ports`	50.0%
`github.com/LerianStudio/matcher/internal/exception/services/command`	89.0%
`github.com/LerianStudio/matcher/internal/exception/services/query`	99.0%
`github.com/LerianStudio/matcher/internal/governance/adapters/audit`	91.8%
`github.com/LerianStudio/matcher/internal/governance/adapters/http/dto`	100.0%
`github.com/LerianStudio/matcher/internal/governance/adapters/http`	92.5%
`github.com/LerianStudio/matcher/internal/governance/adapters/postgres/actor_mapping`	97.5%
`github.com/LerianStudio/matcher/internal/governance/adapters/postgres/archive_metadata`	83.3%
`github.com/LerianStudio/matcher/internal/governance/adapters/postgres`	95.7%
`github.com/LerianStudio/matcher/internal/governance/domain/entities`	97.6%
`github.com/LerianStudio/matcher/internal/governance/domain/hashchain`	88.5%
`github.com/LerianStudio/matcher/internal/governance/domain/repositories/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/governance/services/command`	91.1%
`github.com/LerianStudio/matcher/internal/governance/services/query`	100.0%
`github.com/LerianStudio/matcher/internal/governance/services/worker`	84.6%
`github.com/LerianStudio/matcher/internal/ingestion/adapters/http/dto`	80.0%
`github.com/LerianStudio/matcher/internal/ingestion/adapters/http`	94.2%
`github.com/LerianStudio/matcher/internal/ingestion/adapters/parsers`	96.5%
`github.com/LerianStudio/matcher/internal/ingestion/adapters/postgres/common`	59.5%
`github.com/LerianStudio/matcher/internal/ingestion/adapters/postgres/job`	95.9%
`github.com/LerianStudio/matcher/internal/ingestion/adapters/postgres/transaction`	95.7%
`github.com/LerianStudio/matcher/internal/ingestion/adapters/rabbitmq`	77.3%
`github.com/LerianStudio/matcher/internal/ingestion/adapters/redis`	87.7%
`github.com/LerianStudio/matcher/internal/ingestion/domain/entities`	96.8%
`github.com/LerianStudio/matcher/internal/ingestion/domain/repositories/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/ingestion/domain/value_objects`	97.1%
`github.com/LerianStudio/matcher/internal/ingestion/services/command`	87.7%
`github.com/LerianStudio/matcher/internal/ingestion/services/query`	89.1%
`github.com/LerianStudio/matcher/internal/matching/adapters/http/dto`	95.0%
`github.com/LerianStudio/matcher/internal/matching/adapters/http`	91.1%
`github.com/LerianStudio/matcher/internal/matching/adapters/postgres/adjustment`	95.1%
`github.com/LerianStudio/matcher/internal/matching/adapters/postgres/exception_creator`	95.8%
`github.com/LerianStudio/matcher/internal/matching/adapters/postgres/fee_schedule`	92.2%
`github.com/LerianStudio/matcher/internal/matching/adapters/postgres/fee_variance`	96.8%
`github.com/LerianStudio/matcher/internal/matching/adapters/postgres/match_group`	90.8%
`github.com/LerianStudio/matcher/internal/matching/adapters/postgres/match_item`	96.2%
`github.com/LerianStudio/matcher/internal/matching/adapters/postgres/match_run`	96.5%
`github.com/LerianStudio/matcher/internal/matching/adapters/postgres/rate`	99.2%
`github.com/LerianStudio/matcher/internal/matching/adapters/rabbitmq`	69.9%
`github.com/LerianStudio/matcher/internal/matching/adapters/redis`	92.8%
`github.com/LerianStudio/matcher/internal/matching/domain/entities`	98.6%
`github.com/LerianStudio/matcher/internal/matching/domain/enums`	100.0%
`github.com/LerianStudio/matcher/internal/matching/domain/repositories/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/matching/domain/services`	88.0%
`github.com/LerianStudio/matcher/internal/matching/domain/value_objects`	100.0%
`github.com/LerianStudio/matcher/internal/matching/ports/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/matching/services/command`	81.9%
`github.com/LerianStudio/matcher/internal/matching/services/query`	94.1%
`github.com/LerianStudio/matcher/internal/outbox/adapters/postgres`	100.0%
`github.com/LerianStudio/matcher/internal/outbox/domain/entities`	100.0%
`github.com/LerianStudio/matcher/internal/outbox/domain/repositories/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/outbox/services`	87.6%
`github.com/LerianStudio/matcher/internal/reporting/adapters/http/dto`	82.3%
`github.com/LerianStudio/matcher/internal/reporting/adapters/http`	91.1%
`github.com/LerianStudio/matcher/internal/reporting/adapters/postgres/dashboard`	92.3%
`github.com/LerianStudio/matcher/internal/reporting/adapters/postgres/export_job`	94.5%
`github.com/LerianStudio/matcher/internal/reporting/adapters/postgres/report`	88.4%
`github.com/LerianStudio/matcher/internal/reporting/adapters/redis`	94.9%
`github.com/LerianStudio/matcher/internal/reporting/adapters/storage`	86.4%
`github.com/LerianStudio/matcher/internal/reporting/domain/entities`	98.9%
`github.com/LerianStudio/matcher/internal/reporting/domain/repositories/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/reporting/ports/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/reporting/services/command`	98.3%
`github.com/LerianStudio/matcher/internal/reporting/services/query/exports`	83.0%
`github.com/LerianStudio/matcher/internal/reporting/services/query`	87.4%
`github.com/LerianStudio/matcher/internal/reporting/services/worker`	88.7%
`github.com/LerianStudio/matcher/internal/shared/adapters/cross`	92.6%
`github.com/LerianStudio/matcher/internal/shared/adapters/http`	86.2%
`github.com/LerianStudio/matcher/internal/shared/adapters/postgres/common`	93.7%
`github.com/LerianStudio/matcher/internal/shared/adapters/postgres/outbox`	76.7%
`github.com/LerianStudio/matcher/internal/shared/adapters/rabbitmq`	92.0%
`github.com/LerianStudio/matcher/internal/shared/domain/exception`	95.4%
`github.com/LerianStudio/matcher/internal/shared/domain/fee`	93.5%
`github.com/LerianStudio/matcher/internal/shared/domain`	98.8%
`github.com/LerianStudio/matcher/internal/shared/infrastructure/tenant/adapters`	89.9%
`github.com/LerianStudio/matcher/internal/shared/infrastructure/tenant`	86.3%
`github.com/LerianStudio/matcher/internal/shared/infrastructure/testutil`	44.8%
`github.com/LerianStudio/matcher/internal/shared/ports/mocks`	0.0%
`github.com/LerianStudio/matcher/internal/shared/ports`	11.3%
`github.com/LerianStudio/matcher/internal/shared/sanitize`	96.0%
`github.com/LerianStudio/matcher/internal/shared/testutil`	98.6%
`github.com/LerianStudio/matcher/internal/shared/utils`	100.0%
`github.com/LerianStudio/matcher/internal/testutil`	95.4%
`github.com/LerianStudio/matcher/pkg/chanutil`	100.0%
`github.com/LerianStudio/matcher/pkg/storageopt`	100.0%
`github.com/LerianStudio/matcher/tests/chaos`	84.1%

Generated by Go PR Analysis workflow

coderabbitai

Actionable comments posted: 32

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)

internal/configuration/adapters/http/handlers_test.go (1)

1011-1030: ⚠️ Potential issue | 🟡 Minor

Assert the new Side field in the create-source test.

This test now sends Side: LEFT, but it only checks Name and ID. If the handler drops the field or defaults it incorrectly, the new source-side behavior will regress unnoticed.

💡 Suggested assertion

 			require.NoError(t, json.NewDecoder(response.Body).Decode(&payload))
 			require.Equal(t, "Source A", payload.Name)
+			require.Equal(t, fee.MatchingSideLeft, payload.Side)
 			require.NotEqual(t, uuid.Nil, payload.ID)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/configuration/adapters/http/handlers_test.go` around lines 1011 -
1030, The test for CreateSource currently posts CreateReconciliationSourceInput
with Side set but only asserts Name and ID; update the assertion block in
handlers_test.go (the assertResponse closure that decodes into
entities.ReconciliationSource) to also verify the Side field is preserved—e.g.,
require.Equal(t, fee.MatchingSideLeft, payload.Side)—so the handler.CreateSource
path, CreateReconciliationSourceInput.Side and the returned
entities.ReconciliationSource.Side are validated.

internal/matching/services/command/commands_test.go (1)

893-915: ⚠️ Potential issue | 🟡 Minor

Assert the new feeRuleProvider in TestUseCaseFieldsInitialized.

This test now injects FeeRuleProvider, but it never checks uc.feeRuleProvider. If New stops wiring that dependency, this regression test will still pass.

💡 Suggested assertion

 	assert.NotNil(t, uc.feeVarianceRepo)
 	assert.NotNil(t, uc.adjustmentRepo)
 	assert.NotNil(t, uc.feeScheduleRepo)
+	assert.NotNil(t, uc.feeRuleProvider)
 	assert.NotNil(t, uc.executeRules)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/matching/services/command/commands_test.go` around lines 893 - 915,
TestUseCaseFieldsInitialized currently injects FeeRuleProvider but doesn't
assert it's wired; add an assertion verifying uc.feeRuleProvider is set. In the
test function (TestUseCaseFieldsInitialized) add a line like assert.NotNil(t,
uc.feeRuleProvider) (or require.NotNil if you prefer consistency with other
checks) immediately with the other uc.* assertions so the constructor New will
fail the test if it stops wiring feeRuleProvider.

internal/configuration/adapters/postgres/source/source_sqlmock_test.go (1)

620-639: ⚠️ Potential issue | 🟡 Minor

Missing Side field in source type test case.

The entity constructed in the loop at line 622 does not set the Side field, unlike other test cases in this file. This may cause inconsistencies if the model conversion logic requires a valid Side value.

🔧 Proposed fix

 		for _, srcType := range sourceTypes {
 			now := time.Now().UTC()
 			entity := &entities.ReconciliationSource{
 				ID:        uuid.New(),
 				ContextID: uuid.New(),
 				Name:      "Test " + srcType.String(),
 				Type:      srcType,
+				Side:      sharedfee.MatchingSideLeft,
 				Config:    map[string]any{},
 				CreatedAt: now,
 				UpdatedAt: now,
 			}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/configuration/adapters/postgres/source/source_sqlmock_test.go`
around lines 620 - 639, The test loop builds entities.ReconciliationSource
without setting the Side field, which can cause inconsistent behavior when
converting via NewSourcePostgreSQLModel and ToEntity; update the entity
construction in the loop to include a valid Side value (matching other tests in
this file) — e.g., set Side: entities.SourceSideWhatever (or the same default
used elsewhere) so the model conversion assertions remain consistent.

internal/configuration/services/command/coverage_boost_test.go (1)

436-475: 🧹 Nitpick | 🔵 Trivial

Assert the cloned Side in the replacement test.

This renamed test is now the regression guard for the fee-schedule→side migration, but it only re-checks Name. Seed a non-zero Side on the fixture and assert that the cloned source preserves it, otherwise the new required field can regress without any test failing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/configuration/services/command/coverage_boost_test.go` around lines
436 - 475, The test
TestCloneSourcesAndFieldMaps_SourceClonedWithoutLegacyFeeScheduleField only
asserts Name and can miss regressions on the new required Side field; update the
fixture in the sources slice to set a non-zero Side on the
entities.ReconciliationSource, then after uc.cloneSourcesAndFieldMaps completes
assert that capturedSource.Side equals the original Side value (use the existing
capturedSource variable), ensuring cloneSourcesAndFieldMaps preserves Side.

docs/swagger/swagger.json (2)

10581-10618: ⚠️ Potential issue | 🟠 Major

Align source type enums across request and response schemas.

CreateSourceRequest still advertises FETCHER, but both source response models omit it. That internal contract mismatch can break generated clients on perfectly valid source payloads.

Proposed fix

                 "type": {
                     "description": "Type of the source",
                     "type": "string",
                     "enum": [
                         "LEDGER",
                         "BANK",
                         "GATEWAY",
-                        "CUSTOM"
+                        "CUSTOM",
+                        "FETCHER"
                     ],

Apply the same enum update to both github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.ReconciliationSourceResponse and github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.SourceWithFieldMapStatusResponse.

Also applies to: 11030-11049, 11199-11218

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/swagger.json` around lines 10581 - 10618, The
CreateSourceRequest enum includes "FETCHER" but the response schemas do not;
update the response models to match the request by adding "FETCHER" to the
"type" enum in
github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.ReconciliationSourceResponse
and
github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.SourceWithFieldMapStatusResponse
(and any other duplicate response schema enums) so all three schemas share the
identical enum set: ["LEDGER","BANK","GATEWAY","CUSTOM","FETCHER"].

10727-10735: ⚠️ Potential issue | 🟠 Major

Don't restrict predicate values to strings only.

value and values[] are documented as strings, but fee-rule predicates are stored/coerced as JSON values. This prevents numeric/boolean predicates from being expressed correctly in generated clients and misdocuments response payloads.

Proposed fix

-                "value": {
-                    "type": "string",
-                    "example": "Banco do Brasil"
-                },
+                "value": {
+                    "description": "Scalar predicate value; may be string, number, or boolean",
+                    "example": "Banco do Brasil"
+                },
                 "values": {
                     "type": "array",
                     "items": {
-                        "type": "string"
+                        "description": "Predicate value; may be string, number, or boolean"
                     }
                 }

Apply the same change to github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FieldPredicateResponse.

Also applies to: 10855-10863

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/swagger.json` around lines 10727 - 10735, The schema currently
forces predicate fields to type "string" so numeric/boolean JSON predicates are
misdocumented; update the FeeScheduleItemResponse schema so the predicate
properties (the single "value" and the "values" array item type) accept
arbitrary JSON values instead of only strings (for example replace
"type":"string" with a JSON-value-friendly schema such as a oneOf/anyOf allowing
string, number, boolean, object, array, null or remove the strict "type" and use
a schema that represents any JSON value), and apply the exact same change to
FieldPredicateResponse so both definitions allow non-string predicate values.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/migrations/PRODUCTION_MIGRATIONS.md`:
- Around line 64-65: The runbook currently demands pre-cutover rows have an
explicit source side but names migration
000017_add_source_side_to_reconciliation_sources.up.sql as the change that
introduces/enforces that side, which is impossible; update the docs to provide
an executable sequence: 1) clarify that 000017 must be implemented in two phases
(add a nullable side column without constraint, backfill existing rows to
'LEFT'/'RIGHT', then add the NOT NULL/constraint in a follow-up step) or
alternatively state that the cutover only supports empty/preserved environments
and must be run on fresh DBs; explicitly reference the migration filenames
000017_add_source_side_to_reconciliation_sources.up.sql and
000018_drop_legacy_source_fee_schedule.up.sql and modify the text in the
affected blocks (lines around the existing notes and also the section covering
lines 75-92) to show the two-step approach or the unsupported-environment
statement so operators have an actionable sequence.

In `@docs/swagger/docs.go`:
- Around line 10757-10788: The FieldPredicateRequest schema currently allows
EQUALS/IN without operands and permits empty arrays; update the generated
contract for
"github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FieldPredicateRequest"
to express operator-driven invariants: replace the flat schema with a
discriminating oneOf/if-then structure (or equivalent OpenAPI conditional) that
defines three cases—EQUALS must include "value" (required string), IN must
include "values" as a non-empty array (required with minItems: 1), and EXISTS
must not require operands (no value/values); ensure the "operator" enum values
(EQUALS, IN, EXISTS) are used to select the correct subschema, then regenerate
the docs so codegen and clients see the tightened contract.
- Line 476: The Swagger description currently implies unconditional cloning;
update the upstream swag annotation for the CloneContextRequest handler to state
that cloning of sources, field maps, and match rules is conditional based on the
CloneContextRequest flags includeSources and includeRules, and separately note
that fee rules are cloned while referenced fee schedules are reused; then
regenerate docs.go so the updated annotation replaces the existing description.
- Around line 10347-10375: The CreateFeeRuleRequest DTO currently omits priority
from the required set in the generated spec; update the upstream Go DTO
(CreateFeeRuleRequest) to mark the Priority field as required (e.g., add the
appropriate struct tag used by your generator such as
`binding:"required"`/`validate:"required"` or the swagger annotation/tag your
project uses) and then re-run the swagger/spec generation so
docs/swagger/docs.go includes "priority" in the required array; ensure the field
name is exactly Priority (CreateFeeRuleRequest.Priority) so the generator picks
it up.

In `@docs/swagger/swagger.json`:
- Around line 644-649: The response array schema that references
"github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FeeRuleResponse"
must be bounded; update the schema where the "items" ref to FeeRuleResponse
appears to include "maxItems": 50 (and optionally "minItems": 0) so the OpenAPI
contract enforces the feature cap of 50 for the fee-rule list.
- Around line 10397-10425: The CreateFeeRuleRequest schema currently lists
"feeScheduleId", "name", and "side" in its required array but omits "priority",
allowing invalid payloads; update the schema for CreateFeeRuleRequest by adding
"priority" to the required array (next to "feeScheduleId", "name", "side") so
the "priority" property becomes mandatory in the published OpenAPI/Swagger
contract.

In `@docs/swagger/swagger.yaml`:
- Around line 374-377: The response schemas need the same 50-item upper bound as
the request schemas: add maxItems: 50 to the predicates array definition (the
array whose items reference
github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FieldPredicateResponse)
and likewise add maxItems: 50 to the list-fee-rules response array schema(s)
used for fee-rule listings so the OpenAPI contract matches runtime limits and
resolves the Checkov finding.
- Around line 200-217: The CreateFeeRuleRequest schema currently marks priority
as optional; update the OpenAPI schema for CreateFeeRuleRequest by adding
"priority" to the required array so the field is required, leaving the existing
priority property (type: integer, minimum: 0, example: 0) intact; ensure the
schema's required list includes "priority" alongside "feeScheduleId", "name",
and "side" so generated clients and Swagger UI enforce the required priority.

In `@internal/auth/routes.go`:
- Around line 29-45: The route-protection helper currently only rejects
nil/zero-length action slices but accepts entries like "" or "  "; update the
check in the block that builds handlers (before making handlers and before
appending Authorize) to iterate over actions and fail-fast if any action is
empty or whitespace-only (use strings.TrimSpace) by returning the same
router.Group error used for len(actions)==0; ensure you import the strings
package and keep existing behavior for extractor.validateTenantClaims() and
Authorize(authClient, resource, action).

In `@internal/bootstrap/routes.go`:
- Around line 111-120: The current Protected helper wraps routes via
auth.ProtectedGroupWithActionsWithMiddleware(resource, actions, ...) which
causes auth.Authorize middleware to be applied once per action so every route in
the group requires all listed actions; update routing to avoid composing
multiple action checks on a shared group by either (A) split grouped routes by
single action (e.g., create separate Protected(...) calls per action such as
Protected(resource, "read") and Protected(resource, "write") and attach
respective handlers), or (B) stop passing multiple actions into
auth.ProtectedGroupWithActionsWithMiddleware and instead apply auth.Authorize at
the individual route level; locate usages of Protected(...) in
internal/bootstrap/routes.go and calls to
auth.ProtectedGroupWithActionsWithMiddleware and refactor accordingly while
keeping tenantExtractor, idempotencyMiddleware and rateLimiter attached as
before.

In `@internal/configuration/adapters/http/handlers_context.go`:
- Around line 283-288: The OpenAPI annotations for the DeleteContext handler are
missing the 409 conflict response; update the Swagger block(s) associated with
the DeleteContext handler (the annotation blocks near the current
`@Success/`@Failure entries) to include a `@Failure` 409 {object} ErrorResponse
"Context has child entities (has_children)" so the contract advertises the
conflict path—add the same `@Failure` 409 line to the other similar annotation
block referenced in the file as well.
- Around line 383-396: The CloneContext error handling currently only checks raw
Postgres unique-violation (pgconn.PgError code "23505") and can return 500 if
the domain error ErrContextNameAlreadyExists is wrapped; update the CloneContext
handler to also check for errors.Is(err, command.ErrContextNameAlreadyExists)
(similar to CreateContext) and call libHTTP.RespondError(fiberCtx,
fiber.StatusConflict, "duplicate_name", "a context with this name already
exists") before falling back to the pgErr check and writeServiceError; ensure
the new check appears alongside the existing sql.ErrNoRows and pgErr branches so
wrapped domain errors map to 409.

In `@internal/configuration/adapters/http/handlers_fee_rule_test.go`:
- Around line 675-680: The subtests in the table-driven loop use shared state
and never call t.Parallel(), so add parallel safety by copying the loop variable
(do tt := tt at start of the subtest), construct any shared state per-subtest
(create a fresh app/fixture inside the t.Run closure instead of using the shared
app), then call t.Parallel() inside the subtest before making requests; finally
use performRequest with the per-subtest app and keep the defer resp.Body.Close()
as you do now. This ensures t.Run, tests, performRequest, app/fixture are safe
for parallel execution and satisfies the tparallel check.

In `@internal/configuration/adapters/http/handlers_fee_rule.go`:
- Around line 31-36: Update the Swagger `@Failure` annotations in
handlers_fee_rule.go to match actual responses: add 404 descriptions for
create/update to document the "fee schedule not found" path returned by
mapFeeRuleError(), and replace or remove 403 annotations on get/update/delete
endpoints because handleOwnershipVerificationError() surfaces ownership failures
as 404; ensure the `@Failure` entries for the blocks around the existing ones (the
other two similar blocks in the file) are changed the same way so OpenAPI
reflects 404 for not-found/ownership failures and retains 409/400/401/500 as
appropriate.

In `@internal/configuration/adapters/http/handlers_fee_schedule.go`:
- Around line 285-292: Update the Swagger/OpenAPI comment block for the
DeleteFeeSchedule handler to include a 409 Conflict response annotation: add a
line like `@Failure 409 {object} libHTTP.ErrorResponse "fee_schedule_in_use"`
(or the project's ErrorResponse type and message string) in the comment above
the DeleteFeeSchedule function so the generated docs reflect the conflict case
returned when errors.Is(err, command.ErrFeeScheduleReferencedByFeeRule).

In `@internal/configuration/adapters/http/handlers_field_map.go`:
- Around line 182-183: Replace the inconsistent 404 payload text: in
handlers_field_map.go update every call to writeNotFound(fiberCtx, "resource not
found") used by this handler so it returns writeNotFound(fiberCtx, "field map
not found") instead (apply the same change to the other occurrences in this file
that produce the 404 for this resource). Ensure all branches in this handler
that currently log/send "resource not found" are updated so the 404 message
matches the Swagger contract and sibling responses ("field map not found").

In `@internal/configuration/adapters/http/handlers_match_rule.go`:
- Around line 349-352: The ReorderRequest struct is an HTTP DTO and should be
moved from handlers_match_rule.go into the adapters/http/dto package; create a
new file under internal/configuration/adapters/http/dto (e.g.,
reorder_request.go) defining ReorderRequest with the same json/validate tags,
then update the handler that currently references ReorderRequest to import the
dto package and consume dto.ReorderRequest via sharedhttp.ParseBodyAndValidate()
(also move any other request structs from this file that belong to the DTO layer
and update their handler usages similarly).

In `@internal/configuration/adapters/http/handlers_source.go`:
- Around line 313-318: The OpenAPI comments for the delete-source handler in
handlers_source.go are missing the 409 case returned when
command.ErrSourceHasFieldMap occurs; update the Swagger/@Failure annotations for
the affected handler(s) (the block around lines 313–318 and the similar block at
352–354) to include a 409 entry (e.g. "@Failure 409 {object} ErrorResponse
\"Source has field map\"") so generated docs/clients reflect the actual runtime
response from the delete handler that checks command.ErrSourceHasFieldMap.
- Around line 147-160: The code calls handler.query.CheckFieldMapsExistence even
when result is empty, causing an unnecessary query and potential errors; modify
the handler so that after building sourceIDs from result you check if
len(result) == 0 (or len(sourceIDs) == 0) and in that case skip calling
handler.query.CheckFieldMapsExistence — instead set fieldMapsExist to an
empty/zero-value result the later code expects (or return the empty page
immediately) so you avoid invoking CheckFieldMapsExistence with an empty slice;
update references around result, sourceIDs and
handler.query.CheckFieldMapsExistence (and preserve existing error handling via
logSpanError/writeServiceError where the call is still made).

In `@internal/configuration/adapters/http/test_helpers_test.go`:
- Around line 260-275: The WithTx helpers drop the caller context by calling
context.Background(); change feeRuleRepository.UpdateWithTx and
feeRuleRepository.DeleteWithTx to propagate the incoming ctx into the underlying
methods (call repo.Update(ctx, rule) and repo.Delete(ctx, id) respectively) so
tenant/tracing values are preserved while keeping the same signatures and tx
parameters.

In `@internal/configuration/adapters/postgres/fee_rule/fee_rule.go`:
- Around line 27-52: NewPostgreSQLModel currently only validates ID; add checks
to reject zero-value ContextID and FeeScheduleID (uuid.Nil) and an empty/invalid
Side before marshalling Predicates — return appropriate errors (e.g.,
ErrFeeRuleEntityContextIDNil, ErrFeeRuleEntityFeeScheduleIDNil,
ErrFeeRuleEntitySideInvalid). Mirror these checks in ToEntity(): after parsing
stored ID/ContextID/FeeScheduleID strings back to uuid and converting Side to
the domain enum, validate they are non-nil/valid and return corresponding errors
so malformed persisted rows are rejected on read.

In `@internal/configuration/adapters/postgres/fee_rule/fee_rule.postgresql.go`:
- Around line 302-313: In UpdateWithTx and DeleteWithTx, detect sql.ErrNoRows
and return fee.ErrFeeRuleNotFound instead of returning or wrapping
sql.ErrNoRows; specifically, inside the error handling blocks where you
currently build wrappedErr and call libOpentelemetry.HandleSpanError and logger
(the branches that check errors.Is(err, sql.ErrNoRows)), replace the path that
returns fmt.Errorf("update/delete fee rule with tx: %w", err) with a return of
fee.ErrFeeRuleNotFound (optionally wrap for context only if needed) so
transactional variants mirror Update() and Delete() behavior; update both the
block around the wrappedErr variable and the analogous block later (the 407-418
spot) to return fee.ErrFeeRuleNotFound when err is sql.ErrNoRows.

In `@internal/configuration/adapters/postgres/source/source.go`:
- Around line 63-69: The mapper currently writes empty Side values into
SourcePostgreSQLModel and silently accepts NULL/invalid sides when rehydrating,
allowing invalid ReconciliationSource objects to escape; update the mapping in
the SourcePostgreSQLModel<->domain conversion so that when creating the model
from an entity (where entity.Side is available) you only set Side Valid=true if
entity.Side is non-empty and passes the domain-side validation, otherwise
return/propagate an error; likewise when converting from SourcePostgreSQLModel
back to the domain entity, treat sql.NullString{Valid:false} or unrecognized
side strings as a validation error instead of silently mapping them—apply this
logic in the functions handling SourcePostgreSQLModel creation and entity
reconstruction (refer to SourcePostgreSQLModel and
entity.Side/ReconciliationSource mapping code) so invalid sides cannot be
persisted or rehydrated.

In `@internal/configuration/domain/repositories/fee_rule_repository_test.go`:
- Around line 42-93: The mock methods in mockFeeRuleRepository currently hide
missing-rule cases; change FindByID to return a not-found error (e.g.,
sql.ErrNoRows or the same domain not-found error used by the real repository)
instead of (nil, nil) when the id is absent, and make Update, UpdateWithTx,
Delete and DeleteWithTx return an error when the target rule does not exist (or
return the same not-found error) rather than silently succeeding; update the
methods FindByID, Update, UpdateWithTx, Delete and DeleteWithTx to check
presence in m.rules and return the appropriate not-found error so tests exercise
real repository error paths.

In `@internal/configuration/services/command/clone_commands.go`:
- Around line 149-156: The audit payload was changed to use fee_rules_cloned in
publishCloneAudit (calling publishAudit) but external consumers may still expect
the old fee_schedules_cloned key; either restore compatibility by including the
legacy key alongside the new one (e.g., add "fee_schedules_cloned":
result.FeeRulesCloned to the map in publishCloneAudit) or confirm and coordinate
the rename with external teams and remove the legacy key; update
publishCloneAudit and any callers accordingly and add a short comment noting the
compatibility decision.

In `@internal/configuration/services/command/context_commands.go`:
- Around line 530-540: The code returns the sentinel ErrContextHasChildEntities
when fee rules exist but the sentinel's message doesn't mention fee rules;
update the error so callers see fee rules listed. Either change the sentinel
error declaration (ErrContextHasChildEntities) to include "fee rules" in its
message, or wrap it here with context like return fmt.Errorf("%w: fee rules
exist", ErrContextHasChildEntities); modify the symbol
ErrContextHasChildEntities (or this return statement) so the error text
explicitly mentions "fee rules" alongside sources/rules/schedules.

In `@internal/configuration/services/command/fee_rule_commands_test.go`:
- Around line 50-52: The test double feeRuleMockRepo.CreateWithTx is dropping
the caller's context by calling context.Background(); change it to forward the
received ctx (i.e., return m.Create(ctx, rule)) so cancellation/values
propagate; do the same for the other test-stub CreateWithTx methods in this file
that currently call context.Background() so all transactional test doubles
preserve and propagate the incoming context.

In `@internal/configuration/services/command/fee_rule_commands.go`:
- Line 67: The error messages in fee_rule_commands.go use inconsistent prefixes
("create fee rule:" vs "creating fee rule:"); update all fmt.Errorf calls that
build fee rule errors (the fmt.Errorf calls in the create/creation flow found in
functions handling fee rule creation) to use a single consistent prefix (pick
one form, e.g., "creating fee rule:") and replace the other occurrences so all
error returns (including the ones wrapping loading existing fee rules and
subsequent errors) share that exact prefix for consistent logging and
searchability.
- Around line 64-75: The current pre-check using uc.feeRuleRepo.FindByContextID
and fee.MaxFeeRulesPerContext (checking existingRules length) can race with
concurrent Create calls; make the limit check atomic by moving the count
enforcement into the persistence layer or using a DB transaction/row-level lock
or unique constraint: update the Create flow in the fee rule repository (the
method invoked by the use-case's Create path) to perform the count+insert inside
a single transaction (e.g., SELECT ... FOR UPDATE or a conditional INSERT that
fails when count >= fee.MaxFeeRulesPerContext) and return a clear error that the
use-case can propagate (instead of relying only on the in-memory pre-check).
- Around line 169-171: The audit for updates only logs the name; modify the
uc.publishAudit call in fee_rule_commands.go (the uc.publishAudit(...)
invocation for "update") to include the same richer attributes used on create
(e.g., "context_id", "side", "priority") and/or a "changed_fields" map that
lists which fields were modified (compare the incoming update payload vs
existing entity to build the changed_fields). Ensure the payload keys match the
create event for consistency and include entity.ID and entity.Name as now.
- Around line 230-251: findFeeRuleInContext currently returns whatever
feeRuleRepo.FindByID returns when contextID == uuid.Nil, which can be (nil, nil)
and causes inconsistent behavior compared to the explicit ErrFeeRuleNotFound
returned for context lookups; update the UseCase.findFeeRuleInContext path for
contextID == uuid.Nil to call uc.feeRuleRepo.FindByID(ctx, feeRuleID), check if
the returned rule is nil and if so return (nil, fee.ErrFeeRuleNotFound),
otherwise return the rule and nil error (preserve existing error wrapping when
repo returns an error); reference functions: findFeeRuleInContext,
feeRuleRepo.FindByID, and fee.ErrFeeRuleNotFound.

In `@internal/governance/adapters/http/handlers_test.go`:
- Around line 88-95: Replace the raw action string in the protected test helper
with the constant to match project conventions: inside the protected function
(the helper that sets protectedCalled and returns app) update the assertion that
currently expects "audit:read" to use auth.ActionAuditRead instead, i.e., change
the require.Equal call that compares actions to use auth.ActionAuditRead for
consistency with other tests like handlers_archive_test.go.

---

Outside diff comments:
In `@docs/swagger/swagger.json`:
- Around line 10581-10618: The CreateSourceRequest enum includes "FETCHER" but
the response schemas do not; update the response models to match the request by
adding "FETCHER" to the "type" enum in
github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.ReconciliationSourceResponse
and
github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.SourceWithFieldMapStatusResponse
(and any other duplicate response schema enums) so all three schemas share the
identical enum set: ["LEDGER","BANK","GATEWAY","CUSTOM","FETCHER"].
- Around line 10727-10735: The schema currently forces predicate fields to type
"string" so numeric/boolean JSON predicates are misdocumented; update the
FeeScheduleItemResponse schema so the predicate properties (the single "value"
and the "values" array item type) accept arbitrary JSON values instead of only
strings (for example replace "type":"string" with a JSON-value-friendly schema
such as a oneOf/anyOf allowing string, number, boolean, object, array, null or
remove the strict "type" and use a schema that represents any JSON value), and
apply the exact same change to FieldPredicateResponse so both definitions allow
non-string predicate values.

In `@internal/configuration/adapters/http/handlers_test.go`:
- Around line 1011-1030: The test for CreateSource currently posts
CreateReconciliationSourceInput with Side set but only asserts Name and ID;
update the assertion block in handlers_test.go (the assertResponse closure that
decodes into entities.ReconciliationSource) to also verify the Side field is
preserved—e.g., require.Equal(t, fee.MatchingSideLeft, payload.Side)—so the
handler.CreateSource path, CreateReconciliationSourceInput.Side and the returned
entities.ReconciliationSource.Side are validated.

In `@internal/configuration/adapters/postgres/source/source_sqlmock_test.go`:
- Around line 620-639: The test loop builds entities.ReconciliationSource
without setting the Side field, which can cause inconsistent behavior when
converting via NewSourcePostgreSQLModel and ToEntity; update the entity
construction in the loop to include a valid Side value (matching other tests in
this file) — e.g., set Side: entities.SourceSideWhatever (or the same default
used elsewhere) so the model conversion assertions remain consistent.

In `@internal/configuration/services/command/coverage_boost_test.go`:
- Around line 436-475: The test
TestCloneSourcesAndFieldMaps_SourceClonedWithoutLegacyFeeScheduleField only
asserts Name and can miss regressions on the new required Side field; update the
fixture in the sources slice to set a non-zero Side on the
entities.ReconciliationSource, then after uc.cloneSourcesAndFieldMaps completes
assert that capturedSource.Side equals the original Side value (use the existing
capturedSource variable), ensuring cloneSourcesAndFieldMaps preserves Side.

In `@internal/matching/services/command/commands_test.go`:
- Around line 893-915: TestUseCaseFieldsInitialized currently injects
FeeRuleProvider but doesn't assert it's wired; add an assertion verifying
uc.feeRuleProvider is set. In the test function (TestUseCaseFieldsInitialized)
add a line like assert.NotNil(t, uc.feeRuleProvider) (or require.NotNil if you
prefer consistency with other checks) immediately with the other uc.* assertions
so the constructor New will fail the test if it stops wiring feeRuleProvider.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9fe1dd8c-d7ae-4610-b08a-37dd367e9f24

📥 Commits

Reviewing files that changed from the base of the PR and between 06b236b and 4346474.

📒 Files selected for processing (211)

.air.toml
docker-compose.yml
docs/migrations/PRODUCTION_MIGRATIONS.md
docs/swagger/docs.go
docs/swagger/swagger.json
docs/swagger/swagger.yaml
internal/auth/resources.go
internal/auth/resources_test.go
internal/auth/routes.go
internal/bootstrap/config_api_routes.go
internal/bootstrap/config_api_routes_test.go
internal/bootstrap/config_api_test.go
internal/bootstrap/init.go
internal/bootstrap/routes.go
internal/configuration/adapters/http/dto/clone.go
internal/configuration/adapters/http/dto/clone_test.go
internal/configuration/adapters/http/dto/converters.go
internal/configuration/adapters/http/dto/fee_rule.go
internal/configuration/adapters/http/dto/fee_rule_test.go
internal/configuration/adapters/http/dto/requests.go
internal/configuration/adapters/http/dto/requests_test.go
internal/configuration/adapters/http/dto/responses.go
internal/configuration/adapters/http/handlers.go
internal/configuration/adapters/http/handlers_auth_test.go
internal/configuration/adapters/http/handlers_context.go
internal/configuration/adapters/http/handlers_coverage_test.go
internal/configuration/adapters/http/handlers_fee_rule.go
internal/configuration/adapters/http/handlers_fee_rule_test.go
internal/configuration/adapters/http/handlers_fee_schedule.go
internal/configuration/adapters/http/handlers_fee_schedule_test.go
internal/configuration/adapters/http/handlers_field_map.go
internal/configuration/adapters/http/handlers_match_rule.go
internal/configuration/adapters/http/handlers_source.go
internal/configuration/adapters/http/handlers_test.go
internal/configuration/adapters/http/routes.go
internal/configuration/adapters/http/routes_test.go
internal/configuration/adapters/http/test_helpers_test.go
internal/configuration/adapters/postgres/fee_rule/errors.go
internal/configuration/adapters/postgres/fee_rule/errors_test.go
internal/configuration/adapters/postgres/fee_rule/fee_rule.go
internal/configuration/adapters/postgres/fee_rule/fee_rule.postgresql.go
internal/configuration/adapters/postgres/fee_rule/fee_rule_sqlmock_test.go
internal/configuration/adapters/postgres/fee_rule/fee_rule_test.go
internal/configuration/adapters/postgres/source/errors.go
internal/configuration/adapters/postgres/source/source.go
internal/configuration/adapters/postgres/source/source.postgresql.go
internal/configuration/adapters/postgres/source/source_lookup.go
internal/configuration/adapters/postgres/source/source_pagination.go
internal/configuration/adapters/postgres/source/source_pagination_test.go
internal/configuration/adapters/postgres/source/source_queries.go
internal/configuration/adapters/postgres/source/source_sqlmock_test.go
internal/configuration/domain/entities/clone_result.go
internal/configuration/domain/entities/clone_result_test.go
internal/configuration/domain/entities/reconciliation_context_test.go
internal/configuration/domain/entities/reconciliation_source.go
internal/configuration/domain/entities/reconciliation_source_test.go
internal/configuration/domain/repositories/doc.go
internal/configuration/domain/repositories/fee_rule_repository.go
internal/configuration/domain/repositories/fee_rule_repository_test.go
internal/configuration/services/command/clone_commands.go
internal/configuration/services/command/clone_commands_test.go
internal/configuration/services/command/clone_context_creation.go
internal/configuration/services/command/clone_rules.go
internal/configuration/services/command/clone_sources.go
internal/configuration/services/command/commands.go
internal/configuration/services/command/context_commands.go
internal/configuration/services/command/context_commands_test.go
internal/configuration/services/command/coverage_boost_test.go
internal/configuration/services/command/fee_rule_commands.go
internal/configuration/services/command/fee_rule_commands_test.go
internal/configuration/services/command/fee_schedule_commands.go
internal/configuration/services/command/fee_schedule_commands_test.go
internal/configuration/services/command/source_commands.go
internal/configuration/services/command/source_commands_test.go
internal/configuration/services/query/fee_rule_queries.go
internal/configuration/services/query/fee_rule_queries_test.go
internal/configuration/services/query/queries.go
internal/discovery/adapters/http/routes.go
internal/discovery/adapters/http/routes_test.go
internal/exception/adapters/http/routes.go
internal/exception/adapters/http/routes_test.go
internal/governance/adapters/http/handlers_actor_mapping_test.go
internal/governance/adapters/http/handlers_archive_test.go
internal/governance/adapters/http/handlers_test.go
internal/governance/adapters/http/routes.go
internal/governance/adapters/http/routes_test.go
internal/ingestion/adapters/http/routes.go
internal/ingestion/adapters/http/routes_test.go
internal/matching/adapters/http/handlers.go
internal/matching/adapters/http/handlers_adjustment_test.go
internal/matching/adapters/http/handlers_run.go
internal/matching/adapters/http/handlers_run_test.go
internal/matching/adapters/http/handlers_test_stubs_test.go
internal/matching/adapters/http/routes.go
internal/matching/adapters/http/routes_test.go
internal/matching/ports/fee_rule_provider.go
internal/matching/ports/fee_rule_provider_test.go
internal/matching/ports/source_provider.go
internal/matching/services/command/commands.go
internal/matching/services/command/commands_test.go
internal/matching/services/command/match_group_commands.go
internal/matching/services/command/match_group_commands_dry_run_test.go
internal/matching/services/command/match_group_commands_helpers_test.go
internal/matching/services/command/match_group_commands_run_test.go
internal/matching/services/command/match_group_execution.go
internal/matching/services/command/match_group_lock_commands.go
internal/matching/services/command/match_group_manual_commands.go
internal/matching/services/command/match_group_outbox_events.go
internal/matching/services/command/match_group_persistence.go
internal/matching/services/command/match_group_proposal_processing.go
internal/matching/services/command/match_group_run_commands.go
internal/matching/services/command/match_group_run_support.go
internal/matching/services/command/match_group_source_classification.go
internal/matching/services/command/match_group_unmatch_commands.go
internal/matching/services/command/rule_execution_commands.go
internal/matching/services/command/rule_execution_commands_fee_test.go
internal/matching/services/command/rule_execution_commands_test.go
internal/matching/services/command/rule_execution_fee_normalization.go
internal/matching/services/command/rule_execution_support.go
internal/reporting/adapters/http/handlers_export_job_test.go
internal/reporting/adapters/http/routes.go
internal/reporting/adapters/http/routes_test.go
internal/shared/adapters/cross/configuration_adapters.go
internal/shared/adapters/cross/matching_adapters.go
internal/shared/adapters/cross/matching_adapters_test.go
internal/shared/domain/fee/errors.go
internal/shared/domain/fee/fee_rule.go
internal/shared/domain/fee/fee_rule_resolver.go
internal/shared/domain/fee/fee_rule_resolver_test.go
internal/shared/domain/fee/fee_rule_test.go
internal/shared/domain/fee/field_predicate.go
internal/shared/domain/fee/field_predicate_test.go
internal/shared/domain/fee/matching_side.go
internal/shared/domain/fee/matching_side_test.go
internal/shared/domain/field_map.go
migrations/000016_fee_rules.down.sql
migrations/000016_fee_rules.up.sql
migrations/000017_add_source_side_to_reconciliation_sources.down.sql
migrations/000017_add_source_side_to_reconciliation_sources.up.sql
migrations/000018_drop_legacy_source_fee_schedule.down.sql
migrations/000018_drop_legacy_source_fee_schedule.up.sql
tests/e2e/client/configuration.go
tests/e2e/client/matching.go
tests/e2e/client/matching_test.go
tests/e2e/client/types.go
tests/e2e/factories/factories.go
tests/e2e/factories/fee_rule.go
tests/e2e/factories/fee_schedule.go
tests/e2e/factories/source.go
tests/e2e/journeys/adjust_entry_test.go
tests/e2e/journeys/concurrent_operations_test.go
tests/e2e/journeys/context_lifecycle_test.go
tests/e2e/journeys/dashboard_stresser_full_test.go
tests/e2e/journeys/dashboard_stresser_quick_test.go
tests/e2e/journeys/dashboard_stresser_shared_test.go
tests/e2e/journeys/dashboard_stresser_test.go
tests/e2e/journeys/dashboard_stresser_volume_enrichment_test.go
tests/e2e/journeys/dashboard_stresser_volume_summary_test.go
tests/e2e/journeys/dashboard_stresser_volume_test.go
tests/e2e/journeys/dispatch_test.go
tests/e2e/journeys/dispute_lifecycle_test.go
tests/e2e/journeys/error_handling_test.go
tests/e2e/journeys/error_recovery_test.go
tests/e2e/journeys/exception_handling_test.go
tests/e2e/journeys/export_jobs_test.go
tests/e2e/journeys/export_test.go
tests/e2e/journeys/fee_rule_test.go
tests/e2e/journeys/fee_schedule_test.go
tests/e2e/journeys/field_map_variations_test.go
tests/e2e/journeys/force_match_test.go
tests/e2e/journeys/full_reconciliation_test.go
tests/e2e/journeys/idempotency_test.go
tests/e2e/journeys/ignore_transaction_test.go
tests/e2e/journeys/json_ingestion_test.go
tests/e2e/journeys/large_volume_test.go
tests/e2e/journeys/manual_match_test.go
tests/e2e/journeys/match_group_composition_test.go
tests/e2e/journeys/match_run_history_test.go
tests/e2e/journeys/matching_adjustments_test.go
tests/e2e/journeys/matching_modes_test.go
tests/e2e/journeys/multi_tenant_test.go
tests/e2e/journeys/negative_matching_test.go
tests/e2e/journeys/pagination_test.go
tests/e2e/journeys/reporting_extended_test.go
tests/e2e/journeys/rule_priority_test.go
tests/e2e/journeys/transaction_queries_test.go
tests/integration/configuration/clone_context_test.go
tests/integration/configuration/edge_cases_test.go
tests/integration/configuration/fee_rule_repository_test.go
tests/integration/configuration/field_map_repository_test.go
tests/integration/configuration/http_crud_test.go
tests/integration/configuration/source_repository_test.go
tests/integration/configuration_flow_test.go
tests/integration/cross_domain_flow_test.go
tests/integration/exception/exception_creation_test.go
tests/integration/exception/helpers_test.go
tests/integration/exception/idempotency_middleware_test.go
tests/integration/flow/helpers_test.go
tests/integration/governance/dashboard_aggregates_test.go
tests/integration/harness.go
tests/integration/matching/adjustment_repository_test.go
tests/integration/matching/auto_match_test.go
tests/integration/matching/dry_run_isolation_test.go
tests/integration/matching/helpers_test.go
tests/integration/matching/ignore_transaction_test.go
tests/integration/matching/ingestion_to_matching_test.go
tests/integration/matching/match_event_publication_test.go
tests/integration/reporting/dashboard_source_cash_impact_test.go
tests/integration/reporting/report_list_matched_test.go
tests/integration/reporting/variance_report_test.go
tests/integration/shared_harness.go

💤 Files with no reviewable changes (1)

internal/matching/adapters/http/handlers.go

docs/migrations/PRODUCTION_MIGRATIONS.md

coderabbitai · 2026-03-21T15:09:45Z

docs/swagger/docs.go

                    }
                ],
-                "description": "Creates a deep copy of a reconciliation context including its sources, field maps, match rules, and optionally fee schedules.",
+                "description": "Creates a deep copy of a reconciliation context including its sources, field maps, match rules, and fee rules. Referenced fee schedules are reused by cloned fee rules.",


⚠️ Potential issue | 🟡 Minor

Clarify that cloning is still flag-driven.

CloneContextRequest still has includeSources and includeRules, but this description now reads as if sources, field maps, and match rules are always cloned. Please make the conditional behavior explicit and separate it from the fee-rule/schedule note. Update the upstream swag annotation and regenerate this file.

✏️ Suggested wording

- "description": "Creates a deep copy of a reconciliation context including its sources, field maps, match rules, and fee rules. Referenced fee schedules are reused by cloned fee rules.", + "description": "Creates a deep copy of a reconciliation context. Sources/field maps and match rules are cloned when `includeSources`/`includeRules` are enabled; fee rules are cloned and continue to reference the existing fee schedules.",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@docs/swagger/docs.go` at line 476, The Swagger description currently implies unconditional cloning; update the upstream swag annotation for the CloneContextRequest handler to state that cloning of sources, field maps, and match rules is conditional based on the CloneContextRequest flags includeSources and includeRules, and separately note that fee rules are cloned while referenced fee schedules are reused; then regenerate docs.go so the updated annotation replaces the existing description.

coderabbitai · 2026-03-21T15:09:45Z

docs/swagger/docs.go

+            "required": [
+                "feeScheduleId",
+                "name",
+                "side"
+            ],
+            "properties": {
+                "feeScheduleId": {
+                    "type": "string",
+                    "example": "550e8400-e29b-41d4-a716-446655440000"
+                },
+                "name": {
+                    "type": "string",
+                    "maxLength": 100,
+                    "minLength": 1,
+                    "example": "BB Right-Side Rule"
+                },
+                "predicates": {
+                    "type": "array",
+                    "maxItems": 50,
+                    "items": {
+                        "$ref": "#/definitions/github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FieldPredicateRequest"
+                    }
+                },
+                "priority": {
+                    "description": "Unique within context; LEFT, RIGHT, and ANY share the same priority space",
+                    "type": "integer",
+                    "minimum": 0,
+                    "example": 0
+                },


⚠️ Potential issue | 🟠 Major

Make priority required in CreateFeeRuleRequest.

The fee-rule flow is documented as priority-driven, but the create schema currently allows payloads without priority. That will let generated clients build requests the server should reject. Fix the upstream annotation/DTO tags and regenerate the spec.

🔧 Minimal fix

"required": [ "feeScheduleId", "name", + "priority", "side" ],

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@docs/swagger/docs.go` around lines 10347 - 10375, The CreateFeeRuleRequest DTO currently omits priority from the required set in the generated spec; update the upstream Go DTO (CreateFeeRuleRequest) to mark the Priority field as required (e.g., add the appropriate struct tag used by your generator such as `binding:"required"`/`validate:"required"` or the swagger annotation/tag your project uses) and then re-run the swagger/spec generation so docs/swagger/docs.go includes "priority" in the required array; ensure the field name is exactly Priority (CreateFeeRuleRequest.Priority) so the generator picks it up.

coderabbitai · 2026-03-21T15:09:45Z

docs/swagger/docs.go

+        "github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FieldPredicateRequest": {
+            "type": "object",
+            "required": [
+                "field",
+                "operator"
+            ],
+            "properties": {
+                "field": {
+                    "type": "string",
+                    "example": "institution"
+                },
+                "operator": {
+                    "type": "string",
+                    "enum": [
+                        "EQUALS",
+                        "IN",
+                        "EXISTS"
+                    ],
+                    "example": "EQUALS"
+                },
+                "value": {
+                    "type": "string",
+                    "example": "Banco do Brasil"
+                },
+                "values": {
+                    "type": "array",
+                    "items": {
+                        "type": "string"
+                    }
+                }
+            }
+        },


⚠️ Potential issue | 🟠 Major

Document the operator/operand invariant for predicates.

As published, EQUALS and IN are schema-valid without value/values, and IN also permits an empty array. That makes malformed predicates look valid to clients and codegen. Please encode as much of the rule as possible in the generated contract and regenerate.

🔧 Minimum contract hardening

"operator": { + "description": "EQUALS requires `value`; IN requires non-empty `values`; EXISTS omits both.", "type": "string", "enum": [ "EQUALS", "IN", "EXISTS" ], "example": "EQUALS" }, "value": { + "description": "Used when operator = EQUALS.", "type": "string", "example": "Banco do Brasil" }, "values": { + "description": "Used when operator = IN.", "type": "array", + "minItems": 1, "items": { "type": "string" } }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@docs/swagger/docs.go` around lines 10757 - 10788, The FieldPredicateRequest schema currently allows EQUALS/IN without operands and permits empty arrays; update the generated contract for "github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FieldPredicateRequest" to express operator-driven invariants: replace the flat schema with a discriminating oneOf/if-then structure (or equivalent OpenAPI conditional) that defines three cases—EQUALS must include "value" (required string), IN must include "values" as a non-empty array (required with minItems: 1), and EXISTS must not require operands (no value/values); ensure the "operator" enum values (EQUALS, IN, EXISTS) are used to select the correct subschema, then regenerate the docs so codegen and clients see the tightened contract.

coderabbitai · 2026-03-21T15:09:45Z

docs/swagger/swagger.json

+                        "schema": {
+                            "type": "array",
+                            "items": {
+                                "$ref": "#/definitions/github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FeeRuleResponse"
+                            }
+                        }


⚠️ Potential issue | 🟡 Minor

Publish the fee-rule list bound in the response schema.

This endpoint returns the full rule set for a context, and the feature caps that set at 50. Leaving the array unbounded weakens the contract and keeps the current OpenAPI check failing.

Proposed fix

"schema": { "type": "array", + "maxItems": 50, "items": { "$ref": "#/definitions/github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FeeRuleResponse" } }

📝 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

"schema": {

"type": "array",

"items": {

"$ref": "#/definitions/github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FeeRuleResponse"

}

}

"schema": {

"type": "array",

"maxItems": 50,

"items": {

"$ref": "#/definitions/github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FeeRuleResponse"

}

}

🧰 Tools

🪛 Checkov (3.2.508)

[medium] 644-649: Ensure that arrays have a maximum number of items

(CKV_OPENAPI_21)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@docs/swagger/swagger.json` around lines 644 - 649, The response array schema that references "github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FeeRuleResponse" must be bounded; update the schema where the "items" ref to FeeRuleResponse appears to include "maxItems": 50 (and optionally "minItems": 0) so the OpenAPI contract enforces the feature cap of 50 for the fee-rule list.

coderabbitai · 2026-03-21T15:09:46Z

internal/configuration/services/command/fee_rule_commands.go

+	existingRules, err := uc.feeRuleRepo.FindByContextID(ctx, contextID)
+	if err != nil {
+		libOpentelemetry.HandleSpanError(span, "failed to load fee rules for limit check", err)
+		return nil, fmt.Errorf("create fee rule: loading existing fee rules: %w", err)
+	}
+
+	if len(existingRules) >= fee.MaxFeeRulesPerContext {
+		limitErr := fmt.Errorf("create fee rule: %w", fee.ErrFeeRuleCountLimitExceeded)
+		libOpentelemetry.HandleSpanError(span, "fee rule count limit exceeded", limitErr)
+
+		return nil, limitErr
+	}


⚠️ Potential issue | 🟡 Minor

Potential TOCTOU race on fee rule count limit.

The limit check (fetching existing rules and comparing the count) and the subsequent Create call are not atomic. Under concurrent requests, multiple callers could pass the limit check before any persist, allowing the limit to be exceeded by the number of concurrent calls.

If strict enforcement is required, consider using a transaction that locks rows for the context or relying on a database-level constraint. For a soft limit of 50 rules, this may be acceptable depending on requirements.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/configuration/services/command/fee_rule_commands.go` around lines 64 - 75, The current pre-check using uc.feeRuleRepo.FindByContextID and fee.MaxFeeRulesPerContext (checking existingRules length) can race with concurrent Create calls; make the limit check atomic by moving the count enforcement into the persistence layer or using a DB transaction/row-level lock or unique constraint: update the Create flow in the fee rule repository (the method invoked by the use-case's Create path) to perform the count+insert inside a single transaction (e.g., SELECT ... FOR UPDATE or a conditional INSERT that fails when count >= fee.MaxFeeRulesPerContext) and return a clear error that the use-case can propagate (instead of relying only on the in-memory pre-check).

coderabbitai · 2026-03-21T15:09:47Z

internal/configuration/services/command/fee_rule_commands.go

+	existingRules, err := uc.feeRuleRepo.FindByContextID(ctx, contextID)
+	if err != nil {
+		libOpentelemetry.HandleSpanError(span, "failed to load fee rules for limit check", err)
+		return nil, fmt.Errorf("create fee rule: loading existing fee rules: %w", err)


🧹 Nitpick | 🔵 Trivial

Minor inconsistency in error message prefixes.

Error messages alternate between "create fee rule:" (lines 67, 80) and "creating fee rule:" (line 92). Consistent verb tense improves log searchability.

Suggested fix for consistency

- return nil, fmt.Errorf("creating fee rule: %w", err) + return nil, fmt.Errorf("create fee rule: %w", err)

Also applies to: 80-80, 92-92

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/configuration/services/command/fee_rule_commands.go` at line 67, The error messages in fee_rule_commands.go use inconsistent prefixes ("create fee rule:" vs "creating fee rule:"); update all fmt.Errorf calls that build fee rule errors (the fmt.Errorf calls in the create/creation flow found in functions handling fee rule creation) to use a single consistent prefix (pick one form, e.g., "creating fee rule:") and replace the other occurrences so all error returns (including the ones wrapping loading existing fee rules and subsequent errors) share that exact prefix for consistent logging and searchability.

coderabbitai · 2026-03-21T15:09:47Z

internal/configuration/services/command/fee_rule_commands.go

+	uc.publishAudit(ctx, "fee_rule", entity.ID, "update", map[string]any{
+		"name": entity.Name,
+	})


🧹 Nitpick | 🔵 Trivial

Consider enriching the audit event payload for updates.

The update audit event only includes name, whereas the create audit includes context_id, side, and priority. For traceability, consider logging which fields were updated or including a consistent set of attributes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/configuration/services/command/fee_rule_commands.go` around lines 169 - 171, The audit for updates only logs the name; modify the uc.publishAudit call in fee_rule_commands.go (the uc.publishAudit(...) invocation for "update") to include the same richer attributes used on create (e.g., "context_id", "side", "priority") and/or a "changed_fields" map that lists which fields were modified (compare the incoming update payload vs existing entity to build the changed_fields). Ensure the payload keys match the create event for consistency and include entity.ID and entity.Name as now.

coderabbitai · 2026-03-21T15:09:47Z

internal/configuration/services/command/fee_rule_commands.go

+func (uc *UseCase) findFeeRuleInContext(
+	ctx context.Context,
+	contextID uuid.UUID,
+	feeRuleID uuid.UUID,
+) (*fee.FeeRule, error) {
+	if contextID == uuid.Nil {
+		return uc.feeRuleRepo.FindByID(ctx, feeRuleID)
+	}
+
+	rules, err := uc.feeRuleRepo.FindByContextID(ctx, contextID)
+	if err != nil {
+		return nil, fmt.Errorf("find fee rules by context: %w", err)
+	}
+
+	for _, rule := range rules {
+		if rule != nil && rule.ID == feeRuleID {
+			return rule, nil
+		}
+	}
+
+	return nil, fee.ErrFeeRuleNotFound
+}


🛠️ Refactor suggestion | 🟠 Major

Inconsistent return behavior when fee rule is not found.

When contextID == uuid.Nil, this function returns whatever FindByID returns, which may be (nil, nil) if the repository signals "not found" without an error. When contextID is specified, it explicitly returns ErrFeeRuleNotFound. This inconsistency forces callers to add redundant nil checks (lines 138–140 and 209–211).

Normalize behavior by checking for nil after FindByID:

Proposed fix

func (uc *UseCase) findFeeRuleInContext( ctx context.Context, contextID uuid.UUID, feeRuleID uuid.UUID, ) (*fee.FeeRule, error) { if contextID == uuid.Nil { - return uc.feeRuleRepo.FindByID(ctx, feeRuleID) + rule, err := uc.feeRuleRepo.FindByID(ctx, feeRuleID) + if err != nil { + return nil, err + } + if rule == nil { + return nil, fee.ErrFeeRuleNotFound + } + return rule, nil } rules, err := uc.feeRuleRepo.FindByContextID(ctx, contextID)

After this change, the redundant nil checks in UpdateFeeRule (lines 138–140) and deleteFeeRule (lines 209–211) can be removed.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/configuration/services/command/fee_rule_commands.go` around lines 230 - 251, findFeeRuleInContext currently returns whatever feeRuleRepo.FindByID returns when contextID == uuid.Nil, which can be (nil, nil) and causes inconsistent behavior compared to the explicit ErrFeeRuleNotFound returned for context lookups; update the UseCase.findFeeRuleInContext path for contextID == uuid.Nil to call uc.feeRuleRepo.FindByID(ctx, feeRuleID), check if the returned rule is nil and if so return (nil, fee.ErrFeeRuleNotFound), otherwise return the rule and nil error (preserve existing error wrapping when repo returns an error); reference functions: findFeeRuleInContext, feeRuleRepo.FindByID, and fee.ErrFeeRuleNotFound.

coderabbitai · 2026-03-21T15:09:47Z

internal/governance/adapters/http/handlers_test.go

+		protected := func(resource string, actions ...string) fiber.Router {
 			protectedCalled = true

 			require.Equal(t, "governance", resource)
-			require.Equal(t, "audit:read", action)
+			require.Equal(t, []string{"audit:read"}, actions)

 			return app
 		}


🧹 Nitpick | 🔵 Trivial

LGTM with a minor suggestion.

The protected helper correctly uses the variadic signature. Consider using the constant auth.ActionAuditRead instead of the raw string "audit:read" on Line 92 for consistency with other assertions in this PR (e.g., handlers_archive_test.go uses auth.ActionArchiveRead).

♻️ Optional consistency improvement

require.Equal(t, "governance", resource) - require.Equal(t, []string{"audit:read"}, actions) + require.Equal(t, []string{auth.ActionAuditRead}, actions)

📝 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

protected := func(resource string, actions ...string) fiber.Router {

protectedCalled = true

require.Equal(t, "governance", resource)

require.Equal(t, "audit:read", action)

require.Equal(t, []string{"audit:read"}, actions)

return app

}

protected := func(resource string, actions ...string) fiber.Router {

protectedCalled = true

require.Equal(t, "governance", resource)

require.Equal(t, []string{auth.ActionAuditRead}, actions)

return app

}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/governance/adapters/http/handlers_test.go` around lines 88 - 95, Replace the raw action string in the protected test helper with the constant to match project conventions: inside the protected function (the helper that sets protectedCalled and returns app) update the assertion that currently expects "audit:read" to use auth.ActionAuditRead instead, i.e., change the require.Equal call that compares actions to use auth.ActionAuditRead for consistency with other tests like handlers_archive_test.go.

…-phase path Split the source-side migration from a 2-phase approach (add+enforce in one step, then drop legacy) into a 3-phase additive path: 017: Add nullable side column (existing data preserved) 018: Enforce NOT NULL + CHECK after backfill (fails if NULLs remain) 019: Drop legacy fee_schedule_id column (renumbered from old 018) This prevents migration failures on existing environments with data and provides a clear backfill window between 017 and 018. Updated the production cutover documentation with the phased migration instructions. X-Lerian-Ref: 0x1

The matching engine silently skipped fee normalization when fee rules were missing even if the context had fee normalization enabled. Now it returns ErrFeeRulesRequiredForNormalization (422) to surface the misconfiguration immediately. Also tracks external transaction references in a by-ID map for accurate fee verification across match groups, and adds doc comments clarifying the SourceType abstraction boundary and fee normalization scope. X-Lerian-Ref: 0x1

…n-depth The Update query already included AND context_id in the WHERE clause but Delete used only WHERE id, creating an asymmetry. Now Delete and DeleteWithTx require contextID and the SQL enforces context_id scoping at the database level. This adds a defense-in-depth layer matching the pattern already used by Source and MatchRule repositories. Updated the interface, implementation, all 8 mock stubs, and the integration test. X-Lerian-Ref: 0x1

…tion Clone reads (sources, field maps, rules, fee rules) were non-atomic with the write transaction, risking inconsistent snapshots under concurrent edits. Added a FOR SHARE row lock on the source context within the clone transaction to serialize concurrent modifications during the copy. Also trims clone name input whitespace and updates the DTO comment to reflect that fee rules are now included in the clone operation. X-Lerian-Ref: 0x1

…th empty-action guard Three defensive improvements across shared, configuration, and auth layers: - Predicate stringifyPredicateValue now guards against typed-nil fmt.Stringer interface values that would panic on String() (e.g. (*time.Time)(nil)). - Fee rule HTTP error mapping now includes predicate boundary errors (field too long, value too long, values too many) as 400 client errors. - Auth ProtectedGroupWithActionsWithMiddleware now returns an error handler when any action string in the slice is empty/whitespace. X-Lerian-Ref: 0x1

…ger coverage Review follow-up items M8, H15, H16, H22, M21, M23: - Predicate boundary tests: exact-at-limit and one-over-limit for field (255), value (1024), and values count (100) constants. - Auth helper tests: ProtectedGroupWithActionsWithMiddleware behavior for nil extractor, empty actions, whitespace action, and valid input. - Migration regression: TestMigrations_017 verifies 3-phase cutover lifecycle (nullable add, NULL-blocker on 018, enforce NOT NULL after backfill). - Swagger content test: verifies all 5 fee-rule endpoints and BearerAuth. - Fixed side column in chaos test raw-SQL inserts (post migration 018 compat). - Added TODO comments to schedule_repository_test mock-behavior tests. X-Lerian-Ref: 0x1

…nd cross-tenant tests Review follow-up items H23, M10, H21, H17: - Clone side preservation: asserts LEFT/RIGHT survive clone via name-to-side map. - Fee-rule CRUD flow: adds steps 9-14 to configuration_flow_test (schedule prereq, create, get, update, list, delete, verify 404). - Cross-tenant isolation: documented skip with explanation of multi-schema harness requirement for fee-rule tenant boundary tests. - Fee-rule normalization: documented skip with full prerequisite chain (context, sources, schedule, rules, transactions, match run). X-Lerian-Ref: 0x1

…ound-trip assertions Review follow-up items H19, H20, M12, M13, M22: - Side provenance: matching_modes_test verifies LEFT/RIGHT survive create and GET round-trip for both explicit (.Left/.Right) and auto-assigned sources. - Dashboard stresser: replaced bare > 0 assertions with seed-derived bounds (quick: 30-40 groups, full: 220-300 groups) and fixed the no-op reorder to actually reverse then restore rule order. - Predicate round-trip: TestFeeRule_PredicateRoundTrip creates EQUALS+IN predicates, verifies GET/list consistency, then updates and re-verifies. - Client wrapper: 8 httptest-based tests for fee-rule CRUD + error handling. X-Lerian-Ref: 0x1

…ions Regenerated from updated handler annotations: predicate field constraints (maxLength, maxItems), clone description with fee-rule inclusion, and 409 conflict error on fee schedule delete when referenced by fee rules. X-Lerian-Ref: 0x1

This commit introduces a mock for the FeeRuleRepository interface, generated using the 'mockgen' tool. Having this mock is crucial for writing isolated unit tests for the application's services and use cases. It allows simulating database operations related to fee rules, improving test reliability and execution speed by removing the dependency on a real database.

lerian-studio · 2026-03-21T19:36:16Z

This PR is very large (224 files, 31034 lines changed). Consider breaking it into smaller PRs for easier review.

lerian-studio · 2026-03-21T19:36:17Z

Consider updating CHANGELOG.md to document this change. If this change doesn't need a changelog entry, add the skip-changelog label.

coderabbitai

Actionable comments posted: 16

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)

internal/configuration/ports/schedule_repository_test.go (1)
122-150: 🛠️ Refactor suggestion | 🟠 Major

Remove test file from ports directory; ports should contain only interface definitions.

The mockScheduleRepository is not actively used by other tests—services and commands tests define their own separate mocks. More critically, this test file violates the hexagonal architecture guideline: internal/*/ports/**/* must contain only interface definitions as port contracts; test implementations and mocks belong in adapters/ or alongside service tests. Both test functions explicitly acknowledge they test mock behavior, not adapter behavior. Delete this entire test file. If integration tests are needed, implement them in internal/configuration/adapters/postgres/ using sqlmock to verify actual repository implementations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/configuration/ports/schedule_repository_test.go` around lines 122 -
150, Delete the entire test file that defines
TestScheduleRepositoryMock_CustomBehavior and the mockScheduleRepository mock
implementation: this file belongs to ports (which must only contain interface
definitions) and is testing mock behavior rather than an adapter; remove
TestScheduleRepositoryMock_CustomBehavior, mockScheduleRepository and any
related test helpers from internal/configuration/ports. If you need to verify
repository behavior instead, add adapter-level tests for the actual repository
(e.g., using sqlmock) under internal/configuration/adapters/postgres or move
mock definitions to the service/command tests that consume the port.
internal/configuration/services/command/clone_commands.go (1)
87-104: ⚠️ Potential issue | 🟠 Major

Don't silently downgrade clone atomicity when infraProvider is missing.

This fallback can persist the cloned context and some children before a later step fails, leaving partial data even though the caller receives an error. For a multi-repository write like this, CloneContext should fail fast when transactional support is unavailable and return a dedicated sentinel instead of warning and continuing.

As per coding guidelines, "Provide constructor functions that validate all dependencies are not nil, returning sentinel errors if missing".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/configuration/services/command/clone_commands.go` around lines 87 -
104, When uc.infraProvider is nil, do not fall back to
cloneContextNonTransactional; instead fail fast and return a dedicated sentinel
error (e.g., ErrMissingInfrastructureProvider or
ErrTransactionalSupportUnavailable) from the public clone entrypoint (the method
wrapping cloneContextTransactional/cloneContextNonTransactional), remove the
warning+non-transactional call path, and ensure callers can detect this
sentinel. Add that sentinel error to the package and update any
constructor/initializer for the unit that builds 'uc' (or provide a New*
constructor) to validate infraProvider is not nil and return the sentinel early
if missing. Keep publishCloneAudit/cloneContextTransactional usage unchanged for
the happy path.
docs/swagger/docs.go (1)
10989-11007: ⚠️ Potential issue | 🟠 Major

Add FETCHER to the source response enums.

CreateContextSourceRequest, CreateSourceRequest, and UpdateSourceRequest now admit FETCHER, but both source response schemas here still publish only LEDGER|BANK|GATEWAY|CUSTOM. A successful create/get/list of a FETCHER source will violate the advertised contract and can break generated clients.
🔧 Minimal contract fix
 "enum": [
     "LEDGER",
     "BANK",
     "GATEWAY",
-    "CUSTOM"
+    "CUSTOM",
+    "FETCHER"
 ]
Apply the same change to both source response schemas via the upstream DTO/annotation and regenerate docs/swagger/docs.go.
Also applies to: 11158-11176
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/docs.go` around lines 10989 - 11007, The source response schemas
in the generated swagger still enumerate only "LEDGER","BANK","GATEWAY","CUSTOM"
but the DTOs (CreateContextSourceRequest, CreateSourceRequest,
UpdateSourceRequest) now accept "FETCHER"; add "FETCHER" to the corresponding
response enum(s) in the source response definitions (the generated schema block
for "side"/"type") by updating the upstream DTO/annotation to include FETCHER
and then regenerate docs/swagger/docs.go so the response schemas include
"FETCHER" (ensure both occurrences referenced in the comment are updated).
docs/swagger/swagger.json (1)
11039-11058: ⚠️ Potential issue | 🟠 Major

Publish FETCHER in the source response enums.

The request DTOs in this file still allow FETCHER, but both source response schemas exclude it. A valid FETCHER source will therefore violate the published response contract and break generated-client validation.
📘 Proposed fix
                 "type": {
                     "description": "Type of the source",
                     "type": "string",
                     "enum": [
                         "LEDGER",
                         "BANK",
                         "GATEWAY",
-                        "CUSTOM"
+                        "CUSTOM",
+                        "FETCHER"
                     ],
                     "example": "BANK"
                 },
                 "type": {
                     "description": "Type of the source",
                     "type": "string",
                     "enum": [
                         "LEDGER",
                         "BANK",
                         "GATEWAY",
-                        "CUSTOM"
+                        "CUSTOM",
+                        "FETCHER"
                     ],
                     "example": "BANK"
                 },
Also applies to: 11208-11227
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/swagger.json` around lines 11039 - 11058, The source response
schema's "type" enum is missing the "FETCHER" member causing a mismatch with
request DTOs; update the "type" enum in the source response schemas (the "type"
property definitions where enum currently lists
"LEDGER","BANK","GATEWAY","CUSTOM") to include "FETCHER" in both occurrences
(the block around the shown diff and the other occurrence referenced at
11208-11227) so responses and published contracts match the request DTOs and
generated-client validation succeeds.
docs/swagger/swagger.yaml (1)
676-691: ⚠️ Potential issue | 🟠 Major

Add FETCHER to the source response enums.

The request-side source schemas in this file accept FETCHER, but both response DTOs still exclude it. Generated clients can reject valid GET /sources and GET /sources/{sourceId} payloads for FETCHER sources.
🛠️ Suggested contract fix
   github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.ReconciliationSourceResponse:
     properties:
@@
       type:
         description: Type of the source
         enum:
         - LEDGER
         - BANK
         - GATEWAY
         - CUSTOM
+        - FETCHER
         example: BANK
         type: string
@@
   github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.SourceWithFieldMapStatusResponse:
     properties:
@@
       type:
         description: Type of the source
         enum:
         - LEDGER
         - BANK
         - GATEWAY
         - CUSTOM
+        - FETCHER
         example: BANK
         type: string
Also applies to: 804-819
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/swagger.yaml` around lines 676 - 691, The response schemas for
sources currently list the "type" enum values (LEDGER, BANK, GATEWAY, CUSTOM)
but omit FETCHER; add "FETCHER" to the "type" enum in the source response DTOs
so that GET /sources and GET /sources/{sourceId} responses accept FETCHER.
Locate the source response schema(s) that define the "type" property (the blocks
containing "type:" with description "Type of the source") and add "- FETCHER" to
their enum arrays to match the request-side schemas.

♻️ Duplicate comments (10)

docs/swagger/docs.go (3)

476-476: ⚠️ Potential issue | 🟡 Minor

Make the clone description match the request flags.

The endpoint text still reads as if sources, field maps, and rules are always cloned, but CloneContextRequest now makes that conditional via includeSources and includeRules. Update the upstream swag annotation to describe the flag-driven behavior and regenerate this file.

Also applies to: 10178-10185
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/docs.go` at line 476, Update the swagger description for the
clone endpoint to reflect that cloning is conditional based on the
CloneContextRequest flags (includeSources and includeRules) rather than always
copying sources, field maps, and rules; locate the description string in
docs/swagger/docs.go (the annotation associated with the clone endpoint) and
change the text to explain that sources and rules are copied only when
includeSources/includeRules are set, then regenerate the swagger docs so the
compiled docs reflect the new annotation.
639-639: ⚠️ Potential issue | 🟠 Major

CreateFeeRuleRequest still leaves priority optional.

The create contract now documents caller-controlled priority and a 409 duplicate priority path, but the schema’s required set still omits priority. That leaves generated clients free to omit a field the contract otherwise treats as essential. Mark it required in the upstream DTO/annotation, or explicitly document a server-assigned default, then regenerate.

Also applies to: 10353-10380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/docs.go` at line 639, The OpenAPI schema for
CreateFeeRuleRequest currently omits priority from the required set while the
operation and responses treat caller-controlled priority as mandatory (and may
return 409 on duplicates); update the upstream DTO/annotation that defines
CreateFeeRuleRequest to make the priority field required (or explicitly document
a server-assigned default in the DTO), then regenerate the swagger docs so the
schema in CreateFeeRuleRequest includes priority in required. Locate the
CreateFeeRuleRequest definition and its annotations used to generate docs (the
DTO/annotation referenced by docs/swagger/docs.go) and adjust the required
fields accordingly before re-running the generator.
10775-10795: ⚠️ Potential issue | 🟠 Major

Tighten predicate operands by operator.

This schema still makes malformed predicates codegen-valid: EQUALS can omit value, IN can omit values or send [], and EXISTS is not distinguished as the only operand-free case. Harden the upstream contract as far as Swagger 2.0 allows, then regenerate.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/docs.go` around lines 10775 - 10795, The predicate schema allows
malformed combos; require the operator explicitly and tighten operands: mark
"operator" as required, add a minLength:1 (or non-empty constraint) for "value"
and minItems:1 for "values" so EQUALS cannot be an empty value and IN cannot be
an empty array, and add clear descriptions (or a vendor extension like
x-oneOf/x-discriminator) documenting that EQUALS requires "value", IN requires
non-empty "values", and EXISTS requires neither; update the "operator" enum
usage and regenerate the swagger so codegen reflects these stronger constraints
(refer to the operator, value, and values properties).

internal/configuration/services/command/fee_rule_commands.go (1)

64-79: ⚠️ Potential issue | 🟠 Major

Make the per-context fee-rule cap atomic.

Line 68 and Line 87 split the limit check and insert across separate repository calls. Concurrent creates with different priorities can all observe the same count and push a context well past fee.MaxFeeRulesPerContext, so this is not actually bounded to “by 1”. The cap needs to be enforced inside a single repository transaction / DB lock, with CreateFeeRule only mapping the resulting sentinel error.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/configuration/services/command/fee_rule_commands.go` around lines 64
- 79, The count check against fee.MaxFeeRulesPerContext is racy because
uc.feeRuleRepo.FindByContextID and the insert are separate calls; enforce the
cap inside the repository within a single DB transaction or row-level lock so
the check+insert is atomic (e.g. add a repository method like
CreateFeeRuleWithLimit/CreateWithContextCap that checks current count and
inserts or returns a sentinel error if the cap is reached), update the use-case
CreateFeeRule to call that new repo method and only map the repository's
sentinel error to fee.ErrFeeRuleCountLimitExceeded (and log via
libOpentelemetry.HandleSpanError), leaving no application-level separate
FindByContextID/count check for this limit.

internal/configuration/services/command/fee_rule_commands_test.go (1)

50-52: ⚠️ Potential issue | 🟡 Minor

Forward the transactional stub inputs instead of replacing them.

Line 50 and Line 93 drop the caller’s ctx by switching to context.Background(), and Line 107 also drops the caller’s contextID by hardcoding uuid.Nil. That makes these tx doubles behave differently from the real repository and can hide context-propagation or context-scoping bugs in transactional paths.

Suggested fix

-func (m *feeRuleMockRepo) CreateWithTx(_ context.Context, _ *sql.Tx, rule *fee.FeeRule) error {
-	return m.Create(context.Background(), rule)
+func (m *feeRuleMockRepo) CreateWithTx(ctx context.Context, _ *sql.Tx, rule *fee.FeeRule) error {
+	return m.Create(ctx, rule)
 }
@@
-func (m *feeRuleMockRepo) UpdateWithTx(_ context.Context, _ *sql.Tx, rule *fee.FeeRule) error {
-	return m.Update(context.Background(), rule)
+func (m *feeRuleMockRepo) UpdateWithTx(ctx context.Context, _ *sql.Tx, rule *fee.FeeRule) error {
+	return m.Update(ctx, rule)
 }
@@
-func (m *feeRuleMockRepo) DeleteWithTx(_ context.Context, _ *sql.Tx, _ uuid.UUID, id uuid.UUID) error {
-	return m.Delete(context.Background(), uuid.Nil, id)
+func (m *feeRuleMockRepo) DeleteWithTx(ctx context.Context, _ *sql.Tx, contextID uuid.UUID, id uuid.UUID) error {
+	return m.Delete(ctx, contextID, id)
 }

As per coding guidelines, "Always pass context as first parameter in function signatures and propagate it through call chains."

Also applies to: 93-95, 107-109

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/configuration/services/command/fee_rule_commands_test.go` around
lines 50 - 52, The mock repository methods (e.g., feeRuleMockRepo.CreateWithTx)
currently replace the caller's context and contextID by calling
Create(context.Background(), ...) and passing uuid.Nil; change these stubs to
forward the original parameters instead of hardcoding new ones—i.e., pass the
received ctx through to the underlying Create/CreateWithTx calls and forward any
contextID/tx arguments unchanged so the transactional doubles behave like the
real repository and preserve context propagation.

internal/configuration/adapters/postgres/fee_rule/fee_rule.postgresql.go (1)

302-313: ⚠️ Potential issue | 🟠 Major

Translate sql.ErrNoRows in the tx variants too.

Line 302 and Line 407 still wrap sql.ErrNoRows, while Update() and Delete() already normalize that case to fee.ErrFeeRuleNotFound. Transactional callers will miss the sentinel and can surface a normal not-found as a generic repository failure.

Suggested fix

 	if err != nil {
-		if !errors.Is(err, sql.ErrNoRows) {
-			wrappedErr := fmt.Errorf("update fee rule with tx: %w", err)
-			libOpentelemetry.HandleSpanError(span, "failed to update fee rule", wrappedErr)
-
-			logger.With(libLog.Any("error", wrappedErr.Error())).Log(ctx, libLog.LevelError, "failed to update fee rule")
-
-			return wrappedErr
-		}
-
-		return fmt.Errorf("update fee rule with tx: %w", err)
+		if errors.Is(err, sql.ErrNoRows) {
+			return fee.ErrFeeRuleNotFound
+		}
+
+		wrappedErr := fmt.Errorf("update fee rule with tx: %w", err)
+		libOpentelemetry.HandleSpanError(span, "failed to update fee rule", wrappedErr)
+		logger.With(libLog.Any("error", wrappedErr.Error())).Log(ctx, libLog.LevelError, "failed to update fee rule")
+		return wrappedErr
 	}
@@
 	if err != nil {
-		if !errors.Is(err, sql.ErrNoRows) {
-			wrappedErr := fmt.Errorf("delete fee rule with tx: %w", err)
-			libOpentelemetry.HandleSpanError(span, "failed to delete fee rule", wrappedErr)
-
-			logger.With(libLog.Any("error", wrappedErr.Error())).Log(ctx, libLog.LevelError, "failed to delete fee rule")
-
-			return wrappedErr
-		}
-
-		return fmt.Errorf("delete fee rule with tx: %w", err)
+		if errors.Is(err, sql.ErrNoRows) {
+			return fee.ErrFeeRuleNotFound
+		}
+
+		wrappedErr := fmt.Errorf("delete fee rule with tx: %w", err)
+		libOpentelemetry.HandleSpanError(span, "failed to delete fee rule", wrappedErr)
+		logger.With(libLog.Any("error", wrappedErr.Error())).Log(ctx, libLog.LevelError, "failed to delete fee rule")
+		return wrappedErr
 	}

Also applies to: 407-418

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@internal/configuration/adapters/postgres/fee_rule/fee_rule.postgresql.go`
around lines 302 - 313, The tx variants still return a wrapped sql.ErrNoRows
instead of the repository sentinel; update the error handling in the
transactional functions (the block that checks errors.Is(err, sql.ErrNoRows) and
currently does return fmt.Errorf("update fee rule with tx: %w", err)) to
translate sql.ErrNoRows into fee.ErrFeeRuleNotFound (i.e., when errors.Is(err,
sql.ErrNoRows) return fee.ErrFeeRuleNotFound or wrap that sentinel if context is
needed), and apply the same change to the corresponding delete transaction error
block (the similar handler around lines 407-418) so transactional callers
receive the normalized fee.ErrFeeRuleNotFound sentinel.

docs/swagger/swagger.json (2)

642-649: ⚠️ Potential issue | 🟡 Minor

Add the missing maxItems bound to listFeeRules.

The feature caps a context at 50 fee rules, but this response is still unbounded in the published contract. That keeps the spec looser than the API and leaves CKV_OPENAPI_21 failing.

📏 Proposed fix

                         "schema": {
                             "type": "array",
+                            "maxItems": 50,
                             "items": {
                                 "$ref": "#/definitions/github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FeeRuleResponse"
                             }
                         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/swagger.json` around lines 642 - 649, The response schema for
the listFeeRules endpoint currently defines an unbounded array of
FeeRuleResponse items; add a maxItems constraint of 50 to that response schema
so the OpenAPI contract matches the runtime limit. Locate the response under the
"200" entry that references
github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FeeRuleResponse
and add "maxItems": 50 alongside "type": "array" to enforce the bound.

10403-10407: ⚠️ Potential issue | 🟠 Major

Make priority mandatory in CreateFeeRuleRequest.

The schema defines priority, but the required list still omits it. That lets generated clients build invalid create payloads for a priority-ordered resource.

🧩 Proposed fix

             "required": [
                 "feeScheduleId",
                 "name",
+                "priority",
                 "side"
             ],

Also applies to: 10426-10430

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/swagger.json` around lines 10403 - 10407, The
CreateFeeRuleRequest schema's required array is missing "priority", allowing
invalid payloads; update the CreateFeeRuleRequest schema(s) by adding "priority"
to the "required" array(s) so that the property is enforced (ensure you modify
both occurrences of the CreateFeeRuleRequest schema found in the file).

docs/swagger/swagger.yaml (2)

379-382: ⚠️ Potential issue | 🟡 Minor

Mirror the 50-item limits on the fee-rule response schemas.

FeeRuleResponse.predicates and the list-fee-rules response are still unbounded, so the OpenAPI contract is looser than the documented/runtime limits and leaves the Checkov finding unresolved.

📏 Suggested schema fix

       predicates:
         items:
           $ref: '#/definitions/github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FieldPredicateResponse'
+        maxItems: 50
         type: array
@@
         "200":
           description: List of fee rules
           schema:
             items:
               $ref: '#/definitions/github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FeeRuleResponse'
+            maxItems: 50
             type: array

Also applies to: 3858-3863

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/swagger.yaml` around lines 379 - 382, The OpenAPI schemas allow
unbounded lists for FeeRuleResponse.predicates and the list-fee-rules response,
so add maxItems: 50 to the predicates array schema and to the list response
array schema(s) to enforce the documented/runtime 50-item limit; locate the
array schema referencing
github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FieldPredicateResponse
(used by FeeRuleResponse.predicates) and the list-fee-rules response definition
and add maxItems: 50 to each array definition to satisfy the Checkov rule.

205-221: ⚠️ Potential issue | 🟠 Major

Make priority required in CreateFeeRuleRequest.

The schema still publishes priority as optional even though the API documents it as a unique key within the context. Swagger UI and generated SDKs can still construct requests that fail server-side.

🛠️ Suggested contract fix

     required:
     - feeScheduleId
     - name
+    - priority
     - side
     type: object

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/swagger/swagger.yaml` around lines 205 - 221, The OpenAPI schema for
CreateFeeRuleRequest currently defines priority but doesn't mark it required;
update the CreateFeeRuleRequest schema by adding "priority" to the required
array (alongside feeScheduleId, name, side) so priority is validated as
mandatory; keep the existing priority attributes (type: integer, minimum: 0,
example) unchanged to preserve contract details.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/migrations/PRODUCTION_MIGRATIONS.md`:
- Around line 76-95: The runbook currently clears
reconciliation_sources.fee_schedule_id and later drops that column without
creating equivalent fee_rules, risking data loss; update the phased cutover to
include a per-context snapshot-and-migrate step that, for each context_id found
in reconciliation_sources with a non-null fee_schedule_id, inserts corresponding
rows into fee_rules (preserving context_id, source_id, fee_schedule_id and any
applicable side) before running the global UPDATE that clears fee_schedule_id
and before running migrations 000018/000019, or explicitly state preserved
environments are unsupported; reference reconciliation_sources, fee_schedule_id,
fee_rules, side and migrations 000016–000019 in the instructions so operators
perform the context-scoped migration instead of the global UPDATE shown now.

In `@docs/swagger/swagger_test.go`:
- Around line 70-84: TestSwagger_FeeRuleSecurityAnnotations currently only
checks for the presence of "BearerAuth" anywhere in swagger.yaml; change it to
parse the YAML (e.g., via yaml.Unmarshal into a map[string]interface{}) and
assert that each fee-rule operation path under the Swagger "paths" object
contains a security entry referencing "BearerAuth". Locate the test function
TestSwagger_FeeRuleSecurityAnnotations and replace the strings.Contains check
with code that unmarshals content, iterates the keys under paths (matching the
fee-rule endpoints or path prefixes used by your API), and asserts that for
every operation object (get/post/put/delete) there is a non-empty "security"
array that includes an entry with "BearerAuth".

In `@docs/swagger/swagger.json`:
- Around line 709-713: The X-Idempotency-Key header parameter for fee-rule
create/update is currently optional; update the parameter object(s) with "name":
"X-Idempotency-Key" to include "required": true so generated clients are forced
to send it for fee-rule writes (apply this change in the parameter block shown
and the other occurrence around lines 2477-2480 that defines the same header);
ensure both the create and update operations reference the now-required header
parameter by name.

In `@internal/auth/routes_test.go`:
- Around line 360-375: The test currently only checks that
ProtectedGroupWithActionsWithMiddleware(...) returns a non-nil group but doesn't
exercise the middleware chain; update
TestProtectedGroupWithActionsWithMiddleware_ValidInputCreatesGroup to register a
simple handler on the returned group (e.g., GET or POST) that writes a 200
response, then create a test HTTP request (using app.Test or
httptest.NewRequest) that includes the expected tenant info/headers to satisfy
NewTenantExtractor and hits the group's route, call app.Test(req) and assert the
response status is 200 (and optionally expected body) to validate the middleware
and handler execute correctly.

In `@internal/auth/routes.go`:
- Around line 40-58: The loop validates actions using strings.TrimSpace but
later passes the original untrimmed values to Authorize, causing
whitespace-mismatch denials; fix by normalizing (trim) each action before
building handlers—either replace the original actions slice entries with
strings.TrimSpace(action) in the initial validation loop or create a
trimmedActions slice and iterate that when appending handlers to call
Authorize(authClient, resource, trimmedAction); ensure the same trimmed values
are used consistently where actions are validated and where handlers are
constructed (e.g., where handlers are appended and where Authorize is invoked).

In `@internal/configuration/adapters/http/test_helpers_test.go`:
- Around line 264-275: The test double currently ignores the context-scoped
delete by calling Delete with uuid.Nil and always allowing deletion; update
feeRuleRepository.Delete to enforce that the stored fee rule's context ID
matches the passed contextID parameter before deleting (return
fee.ErrFeeRuleNotFound if mismatch), and change DeleteWithTx to call repo.Delete
with the provided contextID (not context.Background()/uuid.Nil) so tests can no
longer delete items from the wrong context; reference functions
feeRuleRepository.Delete, feeRuleRepository.DeleteWithTx and the repo.items
lookup when adding the context check.

In `@internal/configuration/adapters/postgres/fee_rule/fee_rule_sqlmock_test.go`:
- Around line 90-97: The constructor NewRepository must validate its dependency
injection and reject a nil provider instead of allowing construction that later
yields ErrRepoNotInitialized; change NewRepository to return (*Repository,
error) (or return an error sentinel such as ErrNilProvider) and check if
provider == nil then return the sentinel error, update the test case in
fee_rule_sqlmock_test.go to expect an error (and not a non-nil repo) when
calling NewRepository(nil), and update any callers of NewRepository to handle
the new error return; keep existing method-level ErrRepoNotInitialized for
nil-receiver checks but ensure construction fails fast in NewRepository.

In `@internal/configuration/domain/repositories/fee_rule_repository_test.go`:
- Around line 85-93: The mock implementations of mockFeeRuleRepository.Delete
and DeleteWithTx currently ignore the passed contextID parameter and delete
solely by id, so update both methods to look up the rule by id in m.rules,
verify the rule's ContextID equals the provided contextID parameter, and only
delete when they match; if the rule is missing or the ContextID does not match,
return an appropriate error (e.g., not found / context mismatch) instead of
deleting. Ensure both Delete(context.Context, uuid.UUID, uuid.UUID) and
DeleteWithTx(context.Context, *sql.Tx, uuid.UUID, uuid.UUID) perform the same
check against the stored rule's ContextID before calling delete(m.rules, id).

In `@internal/configuration/ports/schedule_repository_test.go`:
- Around line 79-83: Remove the behavior-verification tests
TestScheduleRepositoryMock_CustomBehavior and
TestScheduleRepositoryInterfaceSatisfaction (the ones that call
mockScheduleRepository methods and assert canned responses); keep only the
compile-time interface check var _ ScheduleRepository =
(*mockScheduleRepository)(nil) to prevent interface drift. In practice delete
the test functions that exercise mockScheduleRepository and ensure the single
interface-satisfaction line remains in the file so the mock still satisfies
ScheduleRepository at compile time.

In `@internal/configuration/services/command/clone_context_creation.go`:
- Around line 16-31: The FOR SHARE lock only protects reads done inside the same
transaction, but cloneContextTransactional currently locks the parent row then
calls helpers that use non-transactional repo methods (fetchAllSources,
fetchAllRules, feeRuleRepo.FindByContextID) causing race conditions; fix by
reloading the source context inside the begun transaction (after
lockSourceContextForShare) and add transactional variants to the repository
interfaces (e.g. FetchAllSourcesWithTx(ctx, tx, contextID),
FetchAllRulesWithTx(ctx, tx, contextID), FeeRuleRepo.FindByContextIDWithTx(ctx,
tx, contextID)) then update the helpers cloneSourcesAndFieldMaps,
cloneMatchRulesInternal, cloneFeeRulesInternal and any callers in
cloneContextTransactional to use these WithTx methods so all child reads occur
within the same tx returned by beginTenantTx.

In `@internal/configuration/services/command/context_commands_test.go`:
- Around line 2353-2354: The test only asserts ErrorIs for uc.DeleteContext in
context_commands_test.go but misses verifying the sentinel error string in
ErrContextHasChildEntities (defined in
internal/configuration/services/command/commands.go); update the sentinel text
in ErrContextHasChildEntities to include the new "fee-rule" child dependency (or
change it to a generic phrase like "child entities") and then modify the test at
uc.DeleteContext(...) to assert the returned error message equals or contains
the updated sentinel string so the regression is covered.
- Around line 2338-2341: The feeRuleRepo stub's findByContextIDFn currently
ignores its contextID parameter; update the closure in feeRuleRepoStub (the
findByContextIDFn used in the test) to assert that the passed uuid equals
existing.ID (e.g., compare the incoming uuid to existing.ID and fail the test if
not equal) so the test ensures DeleteContext calls FindByContextID with the
correct ID, and apply the same assertion change in the
TestDeleteContext_FeeRuleCheckError test's feeRuleRepoStub.

In `@internal/configuration/services/command/fee_rule_commands.go`:
- Around line 229-236: findFeeRuleInContext currently treats contextID ==
uuid.Nil as an unscoped lookup by calling feeRuleRepo.FindByID, allowing callers
to bypass tenant scoping; change findFeeRuleInContext to require a non-nil
contextID and return an error if uuid.Nil is passed, and extract the unscoped
behavior into a new helper (e.g., findFeeRuleByID) that calls
feeRuleRepo.FindByID; update callers such as UpdateFeeRule to explicitly choose
the scoped findFeeRuleInContext or the new unscoped findFeeRuleByID so tenant
scoping is never silently bypassed.

In `@internal/configuration/services/command/tx_helpers.go`:
- Around line 39-46: The lockSourceContextForShare function currently uses
tx.ExecContext which doesn't detect a missing row; change it to use
tx.QueryRowContext(ctx, query, sourceContextID) and Scan into a dummy variable,
returning an explicit error when Scan returns sql.ErrNoRows (or wrap that error)
so the zero-row case fails; update the test suite by adding a case in
internal/configuration/services/command/tx_helpers_test.go that calls
lockSourceContextForShare with a non-existent UUID and asserts an error is
returned.

In `@internal/matching/adapters/http/handlers_run.go`:
- Around line 444-464: Update the Swagger/OpenAPI annotation block above the
RunMatch handler to document the new 422 responses introduced for error cases
ErrFeeRulesReferenceMissingSchedules, ErrFeeRulesRequiredForNormalization, and
sharedfee.ErrFeeRuleCountLimitExceeded; add `@Failure` entries (422) with
appropriate response keys ("fee_rules_misconfigured", "fee_rules_missing", etc.)
and brief descriptions so the generated OpenAPI reflects these Unprocessable
Entity paths for the RunMatch function and matches the fiber response logic.

In `@internal/matching/ports/source_provider.go`:
- Around line 30-32: The Side field on the Source provider struct is using the
string alias sharedfee.MatchingSide and can silently be empty; update the
mapping/validation where instances of this struct are created or converted
(e.g., the provider mapping function that constructs the Source with ID, Type,
Side) to explicitly validate Side is non-empty and one of the known values (or
change the type to an enum/explicit Unknown variant), and return an error or set
a deterministic default when validation fails; reference the Source struct's
Side, the SourceType mapping code and sharedfee.MatchingSide when adding the
check so uninitialized/"" values cannot propagate.

---

Outside diff comments:
In `@docs/swagger/docs.go`:
- Around line 10989-11007: The source response schemas in the generated swagger
still enumerate only "LEDGER","BANK","GATEWAY","CUSTOM" but the DTOs
(CreateContextSourceRequest, CreateSourceRequest, UpdateSourceRequest) now
accept "FETCHER"; add "FETCHER" to the corresponding response enum(s) in the
source response definitions (the generated schema block for "side"/"type") by
updating the upstream DTO/annotation to include FETCHER and then regenerate
docs/swagger/docs.go so the response schemas include "FETCHER" (ensure both
occurrences referenced in the comment are updated).

In `@docs/swagger/swagger.json`:
- Around line 11039-11058: The source response schema's "type" enum is missing
the "FETCHER" member causing a mismatch with request DTOs; update the "type"
enum in the source response schemas (the "type" property definitions where enum
currently lists "LEDGER","BANK","GATEWAY","CUSTOM") to include "FETCHER" in both
occurrences (the block around the shown diff and the other occurrence referenced
at 11208-11227) so responses and published contracts match the request DTOs and
generated-client validation succeeds.

In `@docs/swagger/swagger.yaml`:
- Around line 676-691: The response schemas for sources currently list the
"type" enum values (LEDGER, BANK, GATEWAY, CUSTOM) but omit FETCHER; add
"FETCHER" to the "type" enum in the source response DTOs so that GET /sources
and GET /sources/{sourceId} responses accept FETCHER. Locate the source response
schema(s) that define the "type" property (the blocks containing "type:" with
description "Type of the source") and add "- FETCHER" to their enum arrays to
match the request-side schemas.

In `@internal/configuration/ports/schedule_repository_test.go`:
- Around line 122-150: Delete the entire test file that defines
TestScheduleRepositoryMock_CustomBehavior and the mockScheduleRepository mock
implementation: this file belongs to ports (which must only contain interface
definitions) and is testing mock behavior rather than an adapter; remove
TestScheduleRepositoryMock_CustomBehavior, mockScheduleRepository and any
related test helpers from internal/configuration/ports. If you need to verify
repository behavior instead, add adapter-level tests for the actual repository
(e.g., using sqlmock) under internal/configuration/adapters/postgres or move
mock definitions to the service/command tests that consume the port.

In `@internal/configuration/services/command/clone_commands.go`:
- Around line 87-104: When uc.infraProvider is nil, do not fall back to
cloneContextNonTransactional; instead fail fast and return a dedicated sentinel
error (e.g., ErrMissingInfrastructureProvider or
ErrTransactionalSupportUnavailable) from the public clone entrypoint (the method
wrapping cloneContextTransactional/cloneContextNonTransactional), remove the
warning+non-transactional call path, and ensure callers can detect this
sentinel. Add that sentinel error to the package and update any
constructor/initializer for the unit that builds 'uc' (or provide a New*
constructor) to validate infraProvider is not nil and return the sentinel early
if missing. Keep publishCloneAudit/cloneContextTransactional usage unchanged for
the happy path.

---

Duplicate comments:
In `@docs/swagger/docs.go`:
- Line 476: Update the swagger description for the clone endpoint to reflect
that cloning is conditional based on the CloneContextRequest flags
(includeSources and includeRules) rather than always copying sources, field
maps, and rules; locate the description string in docs/swagger/docs.go (the
annotation associated with the clone endpoint) and change the text to explain
that sources and rules are copied only when includeSources/includeRules are set,
then regenerate the swagger docs so the compiled docs reflect the new
annotation.
- Line 639: The OpenAPI schema for CreateFeeRuleRequest currently omits priority
from the required set while the operation and responses treat caller-controlled
priority as mandatory (and may return 409 on duplicates); update the upstream
DTO/annotation that defines CreateFeeRuleRequest to make the priority field
required (or explicitly document a server-assigned default in the DTO), then
regenerate the swagger docs so the schema in CreateFeeRuleRequest includes
priority in required. Locate the CreateFeeRuleRequest definition and its
annotations used to generate docs (the DTO/annotation referenced by
docs/swagger/docs.go) and adjust the required fields accordingly before
re-running the generator.
- Around line 10775-10795: The predicate schema allows malformed combos; require
the operator explicitly and tighten operands: mark "operator" as required, add a
minLength:1 (or non-empty constraint) for "value" and minItems:1 for "values" so
EQUALS cannot be an empty value and IN cannot be an empty array, and add clear
descriptions (or a vendor extension like x-oneOf/x-discriminator) documenting
that EQUALS requires "value", IN requires non-empty "values", and EXISTS
requires neither; update the "operator" enum usage and regenerate the swagger so
codegen reflects these stronger constraints (refer to the operator, value, and
values properties).

In `@docs/swagger/swagger.json`:
- Around line 642-649: The response schema for the listFeeRules endpoint
currently defines an unbounded array of FeeRuleResponse items; add a maxItems
constraint of 50 to that response schema so the OpenAPI contract matches the
runtime limit. Locate the response under the "200" entry that references
github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FeeRuleResponse
and add "maxItems": 50 alongside "type": "array" to enforce the bound.
- Around line 10403-10407: The CreateFeeRuleRequest schema's required array is
missing "priority", allowing invalid payloads; update the CreateFeeRuleRequest
schema(s) by adding "priority" to the "required" array(s) so that the property
is enforced (ensure you modify both occurrences of the CreateFeeRuleRequest
schema found in the file).

In `@docs/swagger/swagger.yaml`:
- Around line 379-382: The OpenAPI schemas allow unbounded lists for
FeeRuleResponse.predicates and the list-fee-rules response, so add maxItems: 50
to the predicates array schema and to the list response array schema(s) to
enforce the documented/runtime 50-item limit; locate the array schema
referencing
github_com_LerianStudio_matcher_internal_configuration_adapters_http_dto.FieldPredicateResponse
(used by FeeRuleResponse.predicates) and the list-fee-rules response definition
and add maxItems: 50 to each array definition to satisfy the Checkov rule.
- Around line 205-221: The OpenAPI schema for CreateFeeRuleRequest currently
defines priority but doesn't mark it required; update the CreateFeeRuleRequest
schema by adding "priority" to the required array (alongside feeScheduleId,
name, side) so priority is validated as mandatory; keep the existing priority
attributes (type: integer, minimum: 0, example) unchanged to preserve contract
details.

In `@internal/configuration/adapters/postgres/fee_rule/fee_rule.postgresql.go`:
- Around line 302-313: The tx variants still return a wrapped sql.ErrNoRows
instead of the repository sentinel; update the error handling in the
transactional functions (the block that checks errors.Is(err, sql.ErrNoRows) and
currently does return fmt.Errorf("update fee rule with tx: %w", err)) to
translate sql.ErrNoRows into fee.ErrFeeRuleNotFound (i.e., when errors.Is(err,
sql.ErrNoRows) return fee.ErrFeeRuleNotFound or wrap that sentinel if context is
needed), and apply the same change to the corresponding delete transaction error
block (the similar handler around lines 407-418) so transactional callers
receive the normalized fee.ErrFeeRuleNotFound sentinel.

In `@internal/configuration/services/command/fee_rule_commands_test.go`:
- Around line 50-52: The mock repository methods (e.g.,
feeRuleMockRepo.CreateWithTx) currently replace the caller's context and
contextID by calling Create(context.Background(), ...) and passing uuid.Nil;
change these stubs to forward the original parameters instead of hardcoding new
ones—i.e., pass the received ctx through to the underlying Create/CreateWithTx
calls and forward any contextID/tx arguments unchanged so the transactional
doubles behave like the real repository and preserve context propagation.

In `@internal/configuration/services/command/fee_rule_commands.go`:
- Around line 64-79: The count check against fee.MaxFeeRulesPerContext is racy
because uc.feeRuleRepo.FindByContextID and the insert are separate calls;
enforce the cap inside the repository within a single DB transaction or
row-level lock so the check+insert is atomic (e.g. add a repository method like
CreateFeeRuleWithLimit/CreateWithContextCap that checks current count and
inserts or returns a sentinel error if the cap is reached), update the use-case
CreateFeeRule to call that new repo method and only map the repository's
sentinel error to fee.ErrFeeRuleCountLimitExceeded (and log via
libOpentelemetry.HandleSpanError), leaving no application-level separate
FindByContextID/count check for this limit.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1aab508f-aaaa-4463-95f3-c69bdfb51510

📥 Commits

Reviewing files that changed from the base of the PR and between 4346474 and 55125c0.

📒 Files selected for processing (55)

docs/migrations/PRODUCTION_MIGRATIONS.md
docs/swagger/docs.go
docs/swagger/swagger.json
docs/swagger/swagger.yaml
docs/swagger/swagger_test.go
internal/auth/routes.go
internal/auth/routes_test.go
internal/bootstrap/migrations_integration_test.go
internal/configuration/adapters/http/dto/clone.go
internal/configuration/adapters/http/handlers_fee_rule.go
internal/configuration/adapters/http/handlers_fee_schedule.go
internal/configuration/adapters/http/test_helpers_test.go
internal/configuration/adapters/postgres/fee_rule/fee_rule.postgresql.go
internal/configuration/adapters/postgres/fee_rule/fee_rule_sqlmock_test.go
internal/configuration/domain/repositories/fee_rule_repository.go
internal/configuration/domain/repositories/fee_rule_repository_test.go
internal/configuration/domain/repositories/mocks/fee_rule_repository_mock.go
internal/configuration/ports/schedule_repository_test.go
internal/configuration/services/command/clone_commands.go
internal/configuration/services/command/clone_context_creation.go
internal/configuration/services/command/context_commands_test.go
internal/configuration/services/command/fee_rule_commands.go
internal/configuration/services/command/fee_rule_commands_test.go
internal/configuration/services/command/tx_helpers.go
internal/configuration/services/command/tx_helpers_test.go
internal/configuration/services/query/fee_rule_queries_test.go
internal/matching/adapters/http/handlers_run.go
internal/matching/ports/source_provider.go
internal/matching/services/command/match_group_commands.go
internal/matching/services/command/match_group_execution.go
internal/matching/services/command/match_group_run_commands.go
internal/matching/services/command/match_group_run_support.go
internal/matching/services/command/rule_execution_fee_normalization.go
internal/shared/adapters/cross/matching_adapters_test.go
internal/shared/domain/fee/field_predicate.go
internal/shared/domain/fee/field_predicate_test.go
migrations/000017_add_source_side_to_reconciliation_sources.down.sql
migrations/000017_add_source_side_to_reconciliation_sources.up.sql
migrations/000018_enforce_source_side_not_null.down.sql
migrations/000018_enforce_source_side_not_null.up.sql
migrations/000019_drop_legacy_source_fee_schedule.down.sql
migrations/000019_drop_legacy_source_fee_schedule.up.sql
tests/chaos/business_chaos_test.go
tests/chaos/exhaustion_chaos_test.go
tests/e2e/client/configuration_fee_rule_test.go
tests/e2e/journeys/dashboard_stresser_full_test.go
tests/e2e/journeys/dashboard_stresser_quick_test.go
tests/e2e/journeys/dashboard_stresser_shared_test.go
tests/e2e/journeys/dashboard_stresser_volume_enrichment_test.go
tests/e2e/journeys/fee_rule_test.go
tests/e2e/journeys/matching_modes_test.go
tests/integration/configuration/clone_context_test.go
tests/integration/configuration/fee_rule_repository_test.go
tests/integration/configuration_flow_test.go
tests/integration/matching/fee_rule_normalization_test.go

coderabbitai · 2026-03-21T19:51:24Z

docs/migrations/PRODUCTION_MIGRATIONS.md

+If the environment must be preserved, follow the phased cutover:
+
+```sql
+-- Step 1: Clear legacy fee schedule bindings (before 000016).
+UPDATE reconciliation_sources SET fee_schedule_id = NULL WHERE fee_schedule_id IS NOT NULL;
+
+-- Step 2: Run 000016 (creates fee_rules table) and 000017 (adds nullable side column).
+
+-- Step 3: Backfill explicit side assignments (between 000017 and 000018).
+-- Inspect current sources:
+SELECT id, context_id, name FROM reconciliation_sources WHERE side IS NULL;
+
+-- Assign sides according to your intended matching topology:
+UPDATE reconciliation_sources SET side = 'LEFT' WHERE name LIKE '%bank%';
+UPDATE reconciliation_sources SET side = 'RIGHT' WHERE name LIKE '%gateway%';
+
+-- Verify no NULL sides remain:
+SELECT COUNT(*) FROM reconciliation_sources WHERE side IS NULL;  -- must be 0
+
+-- Step 4: Run 000018 (enforces NOT NULL) and 000019 (drops legacy column).


⚠️ Potential issue | 🟠 Major

Preserved-environment cutover currently drops legacy fee behavior.

Step 1 clears reconciliation_sources.fee_schedule_id, and Step 4 drops that column, but the runbook never tells operators how to materialize equivalent fee_rules first. The sample UPDATE ... WHERE name LIKE ... statements are also global, so following this literally can both erase existing fee bindings and misassign sides in unrelated contexts. Please add an explicit per-context migration step that snapshots legacy bindings and inserts replacement fee rules before clearing the column, or state that preserved environments are unsupported.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@docs/migrations/PRODUCTION_MIGRATIONS.md` around lines 76 - 95, The runbook currently clears reconciliation_sources.fee_schedule_id and later drops that column without creating equivalent fee_rules, risking data loss; update the phased cutover to include a per-context snapshot-and-migrate step that, for each context_id found in reconciliation_sources with a non-null fee_schedule_id, inserts corresponding rows into fee_rules (preserving context_id, source_id, fee_schedule_id and any applicable side) before running the global UPDATE that clears fee_schedule_id and before running migrations 000018/000019, or explicitly state preserved environments are unsupported; reference reconciliation_sources, fee_schedule_id, fee_rules, side and migrations 000016–000019 in the instructions so operators perform the context-scoped migration instead of the global UPDATE shown now.

coderabbitai · 2026-03-21T19:51:24Z

docs/swagger/swagger_test.go

+func TestSwagger_FeeRuleSecurityAnnotations(t *testing.T) {
+	t.Parallel()
+
+	data, err := os.ReadFile("swagger.yaml")
+	require.NoError(t, err)
+
+	content := string(data)
+
+	// All fee-rule operations should be behind BearerAuth.
+	// We verify this by checking that "BearerAuth" appears in the spec
+	// (the handler annotations all include @Security BearerAuth).
+	assert.True(t,
+		strings.Contains(content, "BearerAuth"),
+		"swagger.yaml should reference BearerAuth security scheme",
+	)


⚠️ Potential issue | 🟠 Major

Assert fee-rule auth on the operations, not just the whole file.

Lines 81-84 only check that BearerAuth exists somewhere in swagger.yaml. This still passes if every fee-rule endpoint loses its security block while the scheme definition or another route keeps the same string in the spec.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@docs/swagger/swagger_test.go` around lines 70 - 84, TestSwagger_FeeRuleSecurityAnnotations currently only checks for the presence of "BearerAuth" anywhere in swagger.yaml; change it to parse the YAML (e.g., via yaml.Unmarshal into a map[string]interface{}) and assert that each fee-rule operation path under the Swagger "paths" object contains a security entry referencing "BearerAuth". Locate the test function TestSwagger_FeeRuleSecurityAnnotations and replace the strings.Contains check with code that unmarshals content, iterates the keys under paths (matching the fee-rule endpoints or path prefixes used by your API), and asserts that for every operation object (get/post/put/delete) there is a non-empty "security" array that includes an entry with "BearerAuth".

coderabbitai · 2026-03-21T19:51:24Z

docs/swagger/swagger.json

+                        "type": "string",
+                        "description": "Idempotency key for safe retries",
+                        "name": "X-Idempotency-Key",
+                        "in": "header"
+                    },


⚠️ Potential issue | 🟠 Major

Require X-Idempotency-Key on fee-rule writes.

The PR contract says both create and update require this header, but both parameter objects still leave it optional. Generated clients can omit it and send requests the API rejects.

🧾 Proposed fix

{ "type": "string", "description": "Idempotency key for safe retries", "name": "X-Idempotency-Key", - "in": "header" + "in": "header", + "required": true },

{ "type": "string", "description": "Idempotency key for safe retries", "name": "X-Idempotency-Key", - "in": "header" + "in": "header", + "required": true },

Also applies to: 2477-2480

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@docs/swagger/swagger.json` around lines 709 - 713, The X-Idempotency-Key header parameter for fee-rule create/update is currently optional; update the parameter object(s) with "name": "X-Idempotency-Key" to include "required": true so generated clients are forced to send it for fee-rule writes (apply this change in the parameter block shown and the other occurrence around lines 2477-2480 that defines the same header); ensure both the create and update operations reference the now-required header parameter by name.

coderabbitai · 2026-03-21T19:51:24Z

internal/auth/routes_test.go

+func TestProtectedGroupWithActionsWithMiddleware_ValidInputCreatesGroup(t *testing.T) {
+	t.Parallel()
+
+	app := fiber.New()
+	router := app.Group("/api")
+
+	extractor, err := NewTenantExtractor(
+		false, DefaultTenantID, DefaultTenantSlug, "", "development",
+	)
+	require.NoError(t, err)
+
+	group := ProtectedGroupWithActionsWithMiddleware(
+		router, nil, extractor, "resource", []string{"read", "write"},
+	)
+	require.NotNil(t, group)
+}


⚠️ Potential issue | 🟡 Minor

ValidInputCreatesGroup does not validate runtime behavior.

This test only asserts group != nil, which does not prove the middleware chain is valid. It can pass while requests still fail. Please drive one request through the group and assert the expected status/result for the “valid input” scenario.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/auth/routes_test.go` around lines 360 - 375, The test currently only checks that ProtectedGroupWithActionsWithMiddleware(...) returns a non-nil group but doesn't exercise the middleware chain; update TestProtectedGroupWithActionsWithMiddleware_ValidInputCreatesGroup to register a simple handler on the returned group (e.g., GET or POST) that writes a 200 response, then create a test HTTP request (using app.Test or httptest.NewRequest) that includes the expected tenant info/headers to satisfy NewTenantExtractor and hits the group's route, call app.Test(req) and assert the response status is 200 (and optionally expected body) to validate the middleware and handler execute correctly.

coderabbitai · 2026-03-21T19:51:24Z

internal/auth/routes.go

+	for _, action := range actions {
+		if strings.TrimSpace(action) == "" {
+			return router.Group("/", func(c *fiber.Ctx) error {
+				return fiber.NewError(
+					fiber.StatusInternalServerError,
+					"authorization actions contain empty entry",
+				)
+			})
+		}
+	}

-	handlers := make([]fiber.Handler, 0, baseHandlerCount+len(additionalMiddleware))
+	handlers := make([]fiber.Handler, 0, len(actions)+2+len(additionalMiddleware))
 	if authClient != nil && extractor.authEnabled {
 		handlers = append(handlers, extractor.validateTenantClaims())
 	}

-	handlers = append(handlers, Authorize(authClient, resource, action), extractor.ExtractTenant())
+	for _, action := range actions {
+		handlers = append(handlers, Authorize(authClient, resource, action))
+	}


⚠️ Potential issue | 🟡 Minor

Normalize action values before Authorize to avoid whitespace-mismatch denials.

You validate TrimSpace(action) for emptiness, but still pass the original untrimmed string to Authorize. Inputs like " read " will be treated as a different action and can fail authorization unexpectedly.

💡 Suggested fix

- for _, action := range actions { - if strings.TrimSpace(action) == "" { + normalizedActions := make([]string, 0, len(actions)) + for _, action := range actions { + normalized := strings.TrimSpace(action) + if normalized == "" { return router.Group("/", func(c *fiber.Ctx) error { return fiber.NewError( fiber.StatusInternalServerError, "authorization actions contain empty entry", ) }) } + normalizedActions = append(normalizedActions, normalized) } - handlers := make([]fiber.Handler, 0, len(actions)+2+len(additionalMiddleware)) + handlers := make([]fiber.Handler, 0, len(normalizedActions)+2+len(additionalMiddleware)) if authClient != nil && extractor.authEnabled { handlers = append(handlers, extractor.validateTenantClaims()) } - for _, action := range actions { + for _, action := range normalizedActions { handlers = append(handlers, Authorize(authClient, resource, action)) }

📝 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

for _, action := range actions {

if strings.TrimSpace(action) == "" {

return router.Group("/", func(c *fiber.Ctx) error {

return fiber.NewError(

fiber.StatusInternalServerError,

"authorization actions contain empty entry",

)

})

}

}

handlers := make([]fiber.Handler, 0, baseHandlerCount+len(additionalMiddleware))

handlers := make([]fiber.Handler, 0, len(actions)+2+len(additionalMiddleware))

if authClient != nil && extractor.authEnabled {

handlers = append(handlers, extractor.validateTenantClaims())

}

handlers = append(handlers, Authorize(authClient, resource, action), extractor.ExtractTenant())

for _, action := range actions {

handlers = append(handlers, Authorize(authClient, resource, action))

}

normalizedActions := make([]string, 0, len(actions))

for _, action := range actions {

normalized := strings.TrimSpace(action)

if normalized == "" {

return router.Group("/", func(c *fiber.Ctx) error {

return fiber.NewError(

fiber.StatusInternalServerError,

"authorization actions contain empty entry",

)

})

}

normalizedActions = append(normalizedActions, normalized)

}

handlers := make([]fiber.Handler, 0, len(normalizedActions)+2+len(additionalMiddleware))

if authClient != nil && extractor.authEnabled {

handlers = append(handlers, extractor.validateTenantClaims())

}

for _, action := range normalizedActions {

handlers = append(handlers, Authorize(authClient, resource, action))

}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/auth/routes.go` around lines 40 - 58, The loop validates actions using strings.TrimSpace but later passes the original untrimmed values to Authorize, causing whitespace-mismatch denials; fix by normalizing (trim) each action before building handlers—either replace the original actions slice entries with strings.TrimSpace(action) in the initial validation loop or create a trimmedActions slice and iterate that when appending handlers to call Authorize(authClient, resource, trimmedAction); ensure the same trimmed values are used consistently where actions are validated and where handlers are constructed (e.g., where handlers are appended and where Authorize is invoked).

coderabbitai · 2026-03-21T19:51:24Z

internal/configuration/services/command/context_commands_test.go

+	err = uc.DeleteContext(context.Background(), existing.ID)
+	require.ErrorIs(t, err, ErrContextHasChildEntities)


⚠️ Potential issue | 🟡 Minor

Assert the updated blocked-delete message.

This only checks ErrorIs, so it misses that ErrContextHasChildEntities in internal/configuration/services/command/commands.go, Line 22, still says sources, rules, or schedules and omits the new fee-rule child dependency. Please update that sentinel text (or make it generic) and assert the returned message here so this regression is covered.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/configuration/services/command/context_commands_test.go` around lines 2353 - 2354, The test only asserts ErrorIs for uc.DeleteContext in context_commands_test.go but misses verifying the sentinel error string in ErrContextHasChildEntities (defined in internal/configuration/services/command/commands.go); update the sentinel text in ErrContextHasChildEntities to include the new "fee-rule" child dependency (or change it to a generic phrase like "child entities") and then modify the test at uc.DeleteContext(...) to assert the returned error message equals or contains the updated sentinel string so the regression is covered.

coderabbitai · 2026-03-21T19:51:24Z

internal/configuration/services/command/fee_rule_commands.go

+func (uc *UseCase) findFeeRuleInContext(
+	ctx context.Context,
+	contextID uuid.UUID,
+	feeRuleID uuid.UUID,
+) (*fee.FeeRule, error) {
+	if contextID == uuid.Nil {
+		return uc.feeRuleRepo.FindByID(ctx, feeRuleID)
+	}


⚠️ Potential issue | 🟠 Major

Don’t let uuid.Nil disable context scoping.

Line 234 turns findFeeRuleInContext into an unscoped lookup. UpdateFeeRule then persists the loaded entity using entity.ContextID, so a caller that accidentally passes uuid.Nil can still update a fee rule by ID alone within the tenant. If you need unscoped lookups, split that into a separate helper instead of overloading the context-bound one.

Suggested fix

func (uc *UseCase) findFeeRuleInContext( ctx context.Context, contextID uuid.UUID, feeRuleID uuid.UUID, ) (*fee.FeeRule, error) { if contextID == uuid.Nil { - return uc.feeRuleRepo.FindByID(ctx, feeRuleID) + return nil, fee.ErrFeeRuleNotFound } rules, err := uc.feeRuleRepo.FindByContextID(ctx, contextID)

📝 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

func (uc *UseCase) findFeeRuleInContext(

ctx context.Context,

contextID uuid.UUID,

feeRuleID uuid.UUID,

) (*fee.FeeRule, error) {

if contextID == uuid.Nil {

return uc.feeRuleRepo.FindByID(ctx, feeRuleID)

}

func (uc *UseCase) findFeeRuleInContext(

ctx context.Context,

contextID uuid.UUID,

feeRuleID uuid.UUID,

) (*fee.FeeRule, error) {

if contextID == uuid.Nil {

return nil, fee.ErrFeeRuleNotFound

}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/configuration/services/command/fee_rule_commands.go` around lines 229 - 236, findFeeRuleInContext currently treats contextID == uuid.Nil as an unscoped lookup by calling feeRuleRepo.FindByID, allowing callers to bypass tenant scoping; change findFeeRuleInContext to require a non-nil contextID and return an error if uuid.Nil is passed, and extract the unscoped behavior into a new helper (e.g., findFeeRuleByID) that calls feeRuleRepo.FindByID; update callers such as UpdateFeeRule to explicitly choose the scoped findFeeRuleInContext or the new unscoped findFeeRuleByID so tenant scoping is never silently bypassed.

coderabbitai · 2026-03-21T19:51:24Z

internal/configuration/services/command/tx_helpers.go

+func lockSourceContextForShare(ctx context.Context, tx *sql.Tx, sourceContextID uuid.UUID) error {
+	const query = `SELECT 1 FROM reconciliation_contexts WHERE id = $1 FOR SHARE`
+
+	if _, err := tx.ExecContext(ctx, query, sourceContextID); err != nil {
+		return fmt.Errorf("lock source context for share: %w", err)
+	}
+
+	return nil


⚠️ Potential issue | 🟠 Major

Fail when no source row was actually locked.

Lines 42-43 only catch SQL execution failures. If sourceContextID no longer exists, this SELECT ... FOR SHARE path can still return success, so the clone flow proceeds without ever locking the source row.

Suggested fix

func lockSourceContextForShare(ctx context.Context, tx *sql.Tx, sourceContextID uuid.UUID) error { const query = `SELECT 1 FROM reconciliation_contexts WHERE id = $1 FOR SHARE` - if _, err := tx.ExecContext(ctx, query, sourceContextID); err != nil { + var locked int + if err := tx.QueryRowContext(ctx, query, sourceContextID).Scan(&locked); err != nil { return fmt.Errorf("lock source context for share: %w", err) } return nil }

Please add the zero-row case to internal/configuration/services/command/tx_helpers_test.go as well.

📝 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

func lockSourceContextForShare(ctx context.Context, tx *sql.Tx, sourceContextID uuid.UUID) error {

const query = `SELECT 1 FROM reconciliation_contexts WHERE id = $1 FOR SHARE`

if _, err := tx.ExecContext(ctx, query, sourceContextID); err != nil {

return fmt.Errorf("lock source context for share: %w", err)

}

return nil

func lockSourceContextForShare(ctx context.Context, tx *sql.Tx, sourceContextID uuid.UUID) error {

const query = `SELECT 1 FROM reconciliation_contexts WHERE id = $1 FOR SHARE`

var locked int

if err := tx.QueryRowContext(ctx, query, sourceContextID).Scan(&locked); err != nil {

return fmt.Errorf("lock source context for share: %w", err)

}

return nil

}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/configuration/services/command/tx_helpers.go` around lines 39 - 46, The lockSourceContextForShare function currently uses tx.ExecContext which doesn't detect a missing row; change it to use tx.QueryRowContext(ctx, query, sourceContextID) and Scan into a dummy variable, returning an explicit error when Scan returns sql.ErrNoRows (or wrap that error) so the zero-row case fails; update the test suite by adding a case in internal/configuration/services/command/tx_helpers_test.go that calls lockSourceContextForShare with a non-existent UUID and asserts an error is returned.

coderabbitai · 2026-03-21T19:51:25Z

internal/matching/adapters/http/handlers_run.go

+	case errors.Is(err, command.ErrFeeRulesReferenceMissingSchedules):
+		return libHTTP.RespondError(
+			fiberCtx,
+			fiber.StatusUnprocessableEntity,
+			"fee_rules_misconfigured",
+			"fee rules reference fee schedules that do not exist",
+		)
+	case errors.Is(err, command.ErrFeeRulesRequiredForNormalization):
+		return libHTTP.RespondError(
+			fiberCtx,
+			fiber.StatusUnprocessableEntity,
+			"fee_rules_missing",
+			"fee normalization is enabled but no fee rules are configured for this context",
+		)
+	case errors.Is(err, sharedfee.ErrFeeRuleCountLimitExceeded):
+		return libHTTP.RespondError(
+			fiberCtx,
+			fiber.StatusUnprocessableEntity,
+			"fee_rules_misconfigured",
+			"fee rule count exceeds the maximum allowed per context",
+		)


⚠️ Potential issue | 🟡 Minor

Document the new 422 responses on RunMatch.

Lines 444-464 add reachable 422 Unprocessable Entity paths, but the annotation block above RunMatch still advertises only 400/401/403/404/500. The generated OpenAPI will be wrong for clients.

As per coding guidelines, "Add Swagger/OpenAPI annotations (@Summary, @Description, @Tags, @Param, @Success, @Failure, @Router) to all handler functions."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/matching/adapters/http/handlers_run.go` around lines 444 - 464, Update the Swagger/OpenAPI annotation block above the RunMatch handler to document the new 422 responses introduced for error cases ErrFeeRulesReferenceMissingSchedules, ErrFeeRulesRequiredForNormalization, and sharedfee.ErrFeeRuleCountLimitExceeded; add `@Failure` entries (422) with appropriate response keys ("fee_rules_misconfigured", "fee_rules_missing", etc.) and brief descriptions so the generated OpenAPI reflects these Unprocessable Entity paths for the RunMatch function and matches the fiber response logic.

coderabbitai · 2026-03-21T19:51:25Z

internal/matching/ports/source_provider.go

+	ID   uuid.UUID
+	Type SourceType
+	Side sharedfee.MatchingSide


⚠️ Potential issue | 🟡 Minor

Make Side validity explicit to prevent silent empty values.

At Line 32, sharedfee.MatchingSide is a string alias, so an unset value becomes "" and can pass through silently. Please enforce non-empty/known-side validation at provider mapping boundaries (or model an explicit unknown state) to avoid nondeterministic fee-rule behavior.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@internal/matching/ports/source_provider.go` around lines 30 - 32, The Side field on the Source provider struct is using the string alias sharedfee.MatchingSide and can silently be empty; update the mapping/validation where instances of this struct are created or converted (e.g., the provider mapping function that constructs the Source with ID, Type, Side) to explicitly validate Side is non-empty and one of the known values (or change the type to an enum/explicit Unknown variant), and return an error or set a deterministic default when validation fails; reference the Source struct's Side, the SourceType mapping code and sharedfee.MatchingSide when adding the check so uninitialized/"" values cannot propagate.

fredcamaral added 30 commits March 19, 2026 14:55

docs(swagger): regenerate API documentation for fee rule endpoints

12468ef

X-Lerian-Ref: 0x1

build: switch from vendor to module mode

6379e10

Changes .air.toml and docker-compose.yml build flags from -mod=vendor to -mod=mod, allowing direct module resolution without a vendored directory. X-Lerian-Ref: 0x1

feat(bootstrap): wire fee rule repository into configuration use cases

e98aacb

Injects configFeeRule repository into both command and query use cases via WithFeeRuleRepository option, enabling fee rule CRUD and clone operations at runtime. X-Lerian-Ref: 0x1

docs(swagger): regenerate API documentation

1f6a886

Reflects all API surface changes: source Side field (required), fee rule priority uniqueness description, predicates maxItems:50, clone response feeRulesCloned counter, and removal of primarySourceId from RunMatchRequest. X-Lerian-Ref: 0x1

test(e2e): remove primarySourceID parameter from RunMatch client API

85ff07e

Simplify RunMatch, RunMatchCommit, and RunMatchDryRun signatures to use symmetric mode by default. Update all 30 journey tests to drop the now- removed third argument. X-Lerian-Ref: 0x1

lerian-studio added the size/XL label Mar 21, 2026

github-actions bot added area: internal area: migrations area: tests area: documentation and removed size/XL labels Mar 21, 2026

coderabbitai bot requested changes Mar 21, 2026

View reviewed changes

fredcamaral added 10 commits March 21, 2026 16:32

lerian-studio added the size/XL label Mar 21, 2026

coderabbitai bot requested changes Mar 21, 2026

View reviewed changes

fredcamaral closed this Mar 21, 2026

fredcamaral deleted the feat/fee-rules-per-field branch March 22, 2026 10:43

		err = uc.DeleteContext(context.Background(), existing.ID)
		require.ErrorIs(t, err, ErrContextHasChildEntities)

Conversation

fredcamaral commented Mar 21, 2026

Summary

Changes

Testing

Migration Notes

Uh oh!

coderabbitai bot commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Sequence Diagram(s)

Uh oh!

lerian-studio commented Mar 21, 2026

Uh oh!

lerian-studio commented Mar 21, 2026

Uh oh!

lerian-studio commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Unit Test Coverage Report: matcher

Coverage by Package

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

lerian-studio commented Mar 21, 2026

Uh oh!

lerian-studio commented Mar 21, 2026

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Mar 21, 2026 •

edited

Loading

lerian-studio commented Mar 21, 2026 •

edited

Loading

📊 Unit Test Coverage Report: `matcher`