feat: report artifacts table, paper validation job, and REST endpoints by PatrickFanella · Pull Request #627 · PatrickFanella/augr

PatrickFanella · 2026-04-16T23:52:31Z

Summary

Implements persistent report artifacts and the automated daily paper-trading validation report job.

Changes

Migration 000029 — `report_artifacts` table

New table with composite unique key (strategy_id, report_type, time_bucket)
Descending index on (strategy_id, report_type, completed_at) for fast latest-report lookups
Status enum: pending | completed | error

`ReportArtifactRepo` (`internal/repository/postgres/report_artifact.go`)

Upsert — insert or update-on-conflict; idempotent per (strategy, type, bucket)
GetLatest — returns most recent status=completed artifact for a strategy+type
List — paginated list with ReportArtifactFilter (strategy_id, report_type, status)

`paper_validation_report` automation job (`internal/automation/jobs_reports.go`)

Scheduled daily at 17:00 ET Mon–Fri (after market close), weekdays only
Iterates all active paper strategies; failures are isolated (one bad strategy won't block others)
Loads latest backtest run → deserialises backtest.Metrics → calls papervalidation.GenerateReport
Persists ReportArtifact with status=completed; on error writes status=error artifact
0–120s jitter between strategies to spread DB/CPU load
Gracefully skips registration when ReportArtifactRepo is nil (e.g. smoke env)

REST endpoints (`internal/api/report_handlers.go`)

GET /api/v1/strategies/{id}/reports/latest — latest completed report; includes stale_seconds
GET /api/v1/strategies/{id}/reports — paginated history; supports ?report_type= and ?status= filters
Returns 501 Not Implemented when repo not wired (graceful degradation)

Wiring

OrchestratorDeps gains ReportArtifactRepo, BacktestConfigRepo, BacktestRunRepo
api.Deps / api.Server gain ReportArtifacts *pgrepo.ReportArtifactRepo
runtime.go constructs and distributes the repo to both orchestrator and API server

Pre-existing fix

Removed duplicate validateFallbackProvider declaration in internal/config/validate.go (caused build failure before this PR)

Schema bump

RequiredSchemaVersion: 28 → 29

Run task migrate before deploying.

Tests

internal/repository/postgres/report_artifact_test.go — struct JSON round-trip, filter defaults
internal/automation/jobs_reports_test.go — nil repo guard, no paper strategies, no backtest configs, generation flow
internal/api/report_handlers_test.go — 501 when unconfigured, 400 on invalid UUID, stale_seconds serialization

Copilot

Pull request overview

Adds persistent storage and API access for generated strategy reports, plus a scheduled automation job that produces daily paper-validation reports and saves them as artifacts.

Changes:

Introduces report_artifacts table (migration 000029) and bumps RequiredSchemaVersion to 29.
Adds a PostgreSQL ReportArtifactRepo (upsert, latest lookup, paginated listing) and wires it into the runtime, orchestrator, and API server.
Implements the paper_validation_report daily job and new REST endpoints for latest report + report history.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
migrations/000029_report_artifacts.up.sql	Creates `report_artifacts` table + index for latest-report lookups
migrations/000029_report_artifacts.down.sql	Drops `report_artifacts` table
internal/repository/postgres/schema_version.go	Bumps `RequiredSchemaVersion` to 29
internal/repository/postgres/schema_version_test.go	Updates schema version tests for new required version
internal/repository/postgres/report_artifact.go	Adds repo + model for persisting and querying report artifacts
internal/repository/postgres/report_artifact_test.go	Adds unit tests for JSON round-trip and filter defaults
internal/config/validate.go	Removes duplicate `validateFallbackProvider` definition
internal/automation/orchestrator.go	Adds report-related deps and registers report jobs
internal/automation/jobs_reports.go	Implements daily paper-validation report generation + persistence
internal/automation/jobs_reports_test.go	Adds tests covering report job behavior and generation flow
internal/api/server.go	Wires repo into API server and registers report routes
internal/api/report_handlers.go	Implements `latest` and `list` report endpoints
internal/api/report_handlers_test.go	Adds handler tests for unconfigured repo, invalid UUID, and stale seconds serialization
internal/api/settings_test.go	Updates expected schema versions to 29
cmd/tradingagent/runtime.go	Constructs `ReportArtifactRepo` and wires it into API + orchestrator
cmd/tradingagent/runtime_test.go	Updates schema mismatch error expectations for version 29

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-16T23:57:15Z

+func TestGenerateOneReport_Success(t *testing.T) {
+	t.Parallel()
+
+	stratID := uuid.New()
+	configID := uuid.New()
+	metricsJSON := mustMarshal(t, backtest.Metrics{
+		TotalReturn:     0.15,
+		SharpeRatio:     1.5,
+		MaxDrawdown:     0.08,
+		WinRate:         0.55,
+		StartTime:       time.Now().Add(-30 * 24 * time.Hour),
+		EndTime:         time.Now(),
+		StartEquity:     10000,
+		EndEquity:       11500,
+		TotalBars:       30,
+		Volatility:      0.20,
+		ProfitFactor:    2.0,
+		AvgWinLossRatio: 1.5,
+		CalmarRatio:     1.8,
+		SortinoRatio:    1.2,
+	})
+	// Empty trade log so ComputeTradeAnalytics is skipped (no +Inf).
+	tradeLogJSON := json.RawMessage(`[]`)
+
+	orch := newReportTestOrchestrator(
+		[]domain.Strategy{{ID: stratID, Name: "test", Status: "active", IsPaper: true, CreatedAt: time.Now().Add(-90 * 24 * time.Hour)}},
+		[]domain.BacktestConfig{{ID: configID, StrategyID: stratID}},
+		[]domain.BacktestRun{{ID: uuid.New(), BacktestConfigID: configID, Metrics: metricsJSON, TradeLog: tradeLogJSON}},
+	)
+
+	// ReportArtifactRepo is nil → will fail at persist, but NOT at report generation.
+	err := orch.generateOneReport(context.Background(), stratID, "test", time.Now().Truncate(24*time.Hour), time.Now())
+	if err == nil {
+		t.Fatal("expected error (nil repo), got nil")
+	}


TestGenerateOneReport_Success currently asserts that generateOneReport returns an error (nil repo) and does not verify any “success” outcome. Renaming the test to reflect what it actually validates (e.g., generation succeeds up to persist, then persist fails) would make failures easier to interpret.

Copilot · 2026-04-16T23:57:15Z

+// stubReportArtifactRepo satisfies the report handler's usage of
+// *pgrepo.ReportArtifactRepo via duck-typing at the Server field level.
+// Since the handlers reference the concrete repo type directly, the test
+// overrides the Server.reportArtifacts field with a real (nil-pool) repo
+// and exercises the handler path that checks nil.


The file-level comment claims there is a stubReportArtifactRepo and that the test overrides Server.reportArtifacts with a “real (nil-pool) repo”, but the tests don’t do either (they rely on reportArtifacts being nil). Updating/removing this comment would avoid misleading future readers.

Suggested change

// stubReportArtifactRepo satisfies the report handler's usage of

// *pgrepo.ReportArtifactRepo via duck-typing at the Server field level.

// Since the handlers reference the concrete repo type directly, the test

// overrides the Server.reportArtifacts field with a real (nil-pool) repo

// and exercises the handler path that checks nil.

// These tests exercise the "not configured" handler path by using the

// default test server setup, where Server.reportArtifacts is left nil.

Copilot · 2026-04-16T23:57:16Z

+		}
+		artifacts = append(artifacts, *a)
+	}
+	return artifacts, rows.Err()


List returns rows.Err() directly without wrapping, unlike other postgres repos in this codebase that add context (e.g., postgres: list ... rows: %w). Wrapping here would make debugging production DB/scan issues much easier.

Suggested change

return artifacts, rows.Err()

if err := rows.Err(); err != nil {

return nil, fmt.Errorf("postgres: list report artifacts rows: %w", err)

}

return artifacts, nil

Copilot · 2026-04-16T23:57:16Z

+
+	stale := 0.0
+	if artifact.CompletedAt != nil {
+		stale = math.Round(time.Since(*artifact.CompletedAt).Seconds())


stale_seconds can become negative if completed_at is in the future (clock skew, bad data). Consider clamping to >= 0 (e.g., max(0, ...)) so clients don’t see confusing negative “staleness” values.

Suggested change

stale = math.Round(time.Since(*artifact.CompletedAt).Seconds())

stale = math.Max(0, math.Round(time.Since(*artifact.CompletedAt).Seconds()))

Copilot · 2026-04-16T23:57:16Z

+func containsStr(s, substr string) bool {
+	return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsSubstring(s, substr))
+}
+
+func containsSubstring(s, sub string) bool {
+	for i := 0; i+len(sub) <= len(s); i++ {
+		if s[i:i+len(sub)] == sub {
+			return true
+		}
+	}
+	return false
+}


This test helper reimplements substring search (containsStr/containsSubstring) rather than using strings.Contains, which makes the intent harder to read and maintain. Replacing it with strings.Contains would simplify the test and reduce custom code.

Copilot · 2026-04-16T23:57:17Z

+    report_type       TEXT NOT NULL DEFAULT 'paper_validation',
+    time_bucket       TIMESTAMPTZ NOT NULL,
+    status            TEXT NOT NULL DEFAULT 'pending',
+    report_json       JSONB,


The migration defines report_type and status as free-form TEXT. Since the runtime and PR description treat these as a closed set (e.g., pending|completed|error), it would be safer to add a CHECK constraint (or enum type) so invalid values can’t be persisted (including via ad-hoc SQL), and optionally enforce completed_at presence when status='completed'.

Copilot · 2026-04-16T23:57:17Z

+    prompt_tokens     INT DEFAULT 0,
+    completion_tokens INT DEFAULT 0,
+    latency_ms        INT DEFAULT 0,


prompt_tokens, completion_tokens, and latency_ms are nullable (they only have DEFAULT 0). In Go these scan into non-pointer int fields, so a NULL in the DB would cause scan errors. Consider making these columns NOT NULL DEFAULT 0 to match the struct shape.

Suggested change

prompt_tokens INT DEFAULT 0,

completion_tokens INT DEFAULT 0,

latency_ms INT DEFAULT 0,

prompt_tokens INT NOT NULL DEFAULT 0,

completion_tokens INT NOT NULL DEFAULT 0,

latency_ms INT NOT NULL DEFAULT 0,

Copilot · 2026-04-16T23:57:17Z

+);
+
+CREATE INDEX idx_report_artifacts_strategy_type
+    ON report_artifacts (strategy_id, report_type, completed_at DESC);


GetLatest filters on status='completed' and orders by completed_at DESC, but the index is on (strategy_id, report_type, completed_at DESC) without status. Consider making this a partial index WHERE status = 'completed' (and optionally completed_at IS NOT NULL) to reduce index size and improve latest-report lookups.

Suggested change

ON report_artifacts (strategy_id, report_type, completed_at DESC);

ON report_artifacts (strategy_id, report_type, completed_at DESC)

WHERE status = 'completed';

Copilot · 2026-04-16T23:57:17Z

+// scannable abstracts pgx.Row and pgx.Rows.
+type scannable interface {
+	Scan(dest ...any) error
+}
+
+func scanReportArtifact(row pgx.Row) (*ReportArtifact, error) {


This file introduces a new scannable interface and two near-identical scan helpers (scanReportArtifact / scanReportArtifactRow). The postgres package already has a scanner interface used to share scan logic across pgx.Row and pgx.Rows (see internal/repository/postgres/strategy.go), so this can be simplified to a single scan helper taking that existing interface and the extra interface/duplication removed.

PatrickFanella · 2026-04-17T01:24:59Z

@copilot apply changes based on the comments in this thread

Copilot · 2026-04-17T01:30:21Z

@copilot apply changes based on the comments in this thread

Implemented the requested review-thread updates in commit 0932a54: test rename/cleanup, misleading test comment fix, rows.Err() context wrapping, stale-seconds clamp to non-negative, migration constraints/index improvements, and shared scanner helper simplification in the report artifact repo. No UI changes were involved.

PatrickFanella · 2026-04-17T01:30:30Z

@copilot resolve the merge conflicts in this pull request

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

PatrickFanella · 2026-04-17T02:24:18Z

@copilot apply changes based on the comments in this thread

- Add paper_validation_report automation job (runs 17:00 ET Mon-Fri) that iterates active paper strategies, loads latest backtest run, calls papervalidation.GenerateReport, and persists a ReportArtifact - Guard nil ReportArtifactRepo in registerReportJobs/persistErrorArtifact - Add BacktestConfigRepo and BacktestRunRepo to OrchestratorDeps - Add GET /api/v1/strategies/{id}/reports/latest (stale_seconds field) - Add GET /api/v1/strategies/{id}/reports (paginated, filterable) - Wire ReportArtifactRepo into api.Deps and OrchestratorDeps in runtime - Update reviewed migration SQL and repo implementation from PR feedback

Copilot AI review requested due to automatic review settings April 16, 2026 23:52

Copilot started reviewing on behalf of PatrickFanella April 16, 2026 23:53 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

Copilot started work on behalf of PatrickFanella April 17, 2026 01:25 View session

Copilot finished work on behalf of PatrickFanella April 17, 2026 01:31

PatrickFanella requested a review from Copilot April 17, 2026 01:55

Copilot started reviewing on behalf of PatrickFanella April 17, 2026 01:56 View session

Copilot AI reviewed Apr 17, 2026

View reviewed changes

Comment thread internal/repository/postgres/report_artifact.go Outdated

Comment thread internal/automation/jobs_reports.go Outdated

Comment thread internal/automation/jobs_reports.go Outdated

PatrickFanella enabled auto-merge (squash) April 17, 2026 02:24

PatrickFanella disabled auto-merge April 17, 2026 02:24

PatrickFanella closed this Apr 17, 2026

PatrickFanella reopened this Apr 17, 2026

PatrickFanella force-pushed the feat/report-artifacts-and-paper-validation-job branch from 23e3725 to 1e73558 Compare April 17, 2026 02:35

github-actions Bot added the size:xl Extra large effort label Apr 17, 2026

PatrickFanella merged commit 72366e2 into main Apr 17, 2026
3 checks passed

PatrickFanella deleted the feat/report-artifacts-and-paper-validation-job branch April 17, 2026 02:43

-	return artifacts, rows.Err()
+	if err := rows.Err(); err != nil {
+		return nil, fmt.Errorf("postgres: list report artifacts rows: %w", err)
+	}
+	return artifacts, nil

	stale = math.Round(time.Since(*artifact.CompletedAt).Seconds())
	stale = math.Max(0, math.Round(time.Since(*artifact.CompletedAt).Seconds()))

	ON report_artifacts (strategy_id, report_type, completed_at DESC);
	ON report_artifacts (strategy_id, report_type, completed_at DESC)
	WHERE status = 'completed';

Conversation

PatrickFanella commented Apr 16, 2026

Summary

Changes

Migration 000029 — report_artifacts table

ReportArtifactRepo (internal/repository/postgres/report_artifact.go)

paper_validation_report automation job (internal/automation/jobs_reports.go)

REST endpoints (internal/api/report_handlers.go)

Wiring

Pre-existing fix

Schema bump

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

PatrickFanella commented Apr 17, 2026

Uh oh!

Copilot AI commented Apr 17, 2026

Uh oh!

PatrickFanella commented Apr 17, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PatrickFanella commented Apr 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Migration 000029 — `report_artifacts` table

`ReportArtifactRepo` (`internal/repository/postgres/report_artifact.go`)

`paper_validation_report` automation job (`internal/automation/jobs_reports.go`)

REST endpoints (`internal/api/report_handlers.go`)