Skip to content

Complete SOW-0055 ingest write-model cleanup#63

Open
ktsaou wants to merge 1 commit into
masterfrom
codex/sow-0055-ingest-write-model-complexity
Open

Complete SOW-0055 ingest write-model cleanup#63
ktsaou wants to merge 1 commit into
masterfrom
codex/sow-0055-ingest-write-model-complexity

Conversation

@ktsaou

@ktsaou ktsaou commented Jun 7, 2026

Copy link
Copy Markdown
Member

Summary

  • complete SOW-0055 by reducing targeted ingest write-model function complexity
  • add characterization coverage for catalog migration, writer re-emits, log FTS idempotency, rollup carry-forward, and resolver atomicity
  • track the remaining pre-existing writer file-length debt in SOW-0060

Validation

  • gofmt clean on touched Go files
  • git diff --check clean
  • strict Lizard target scans clean for production files and SOW-0055 characterization tests
  • go test ./internal/ingest -count=1
  • go test -race ./internal/ingest -count=1 -timeout=10m
  • golangci-lint run --timeout=5m ./internal/ingest/...
  • local Codacy on changed files: Semgrep and Trivy clean; only pre-existing writer file-length debt remains and is tracked
  • ./scripts/gates.sh passed in 809s
  • external review converged with no actionable findings

Summary by cubic

Finish SOW-0055 by reducing ingest write-model complexity across internal/ingest while preserving behavior and data integrity. Add characterization tests for catalog migration, writer re-emits, rollup carry-forward, and resolver atomicity; move SOW-0055 to done/ and open SOW-0060 to track writer.go file-size split.

  • Refactors
    • internal/ingest/writer.go: extracted dispatchEvent with sub-dispatchers; apply only tracks max seq; unknown kinds return explicit errors.
    • internal/ingest/catalog_migrate.go: replaced inline UPDATEs with per-table helpers preserving original SQL and sign semantics.
    • internal/ingest/resolver.go: organized link passes into named steps inside one transaction to keep link+notify atomicity.
    • Rollups: unified hour/day refresh via refreshRollupBucketPass; split backfill orchestration into focused helpers.
    • Tests + SOWs: added SOW‑0055 characterization suites under internal/ingest/sow55_*_test.go; moved SOW‑0055 doc to done/; opened SOW‑0060 to split oversized writer.go.

Written for commit 28aac0d. Summary will update on new commits.

Review in cubic

@CLAassistant

CLAassistant commented Jun 7, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@codacy-production

codacy-production Bot commented Jun 7, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 140 complexity · 41 duplication

Metric Results
Complexity 140
Duplication 41

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Reduce ingest write-model complexity with behavior-preserving helper extraction and characterization coverage. Move SOW-0055 to completed and track residual writer file-size debt in SOW-0060.
@ktsaou ktsaou force-pushed the codex/sow-0055-ingest-write-model-complexity branch from 2e24d94 to 28aac0d Compare June 7, 2026 21:17

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

1 issue found across 13 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="internal/ingest/sow55_characterization_resolver_test.go">

<violation number="1" location="internal/ingest/sow55_characterization_resolver_test.go:96">
P2: Rollback characterization assertion is too weak: it only rejects `root_session_id == parentID` instead of asserting the child remains self-rooted, so some regressions would pass undetected.</violation>
</file>

Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.

Re-trigger cubic

Comment on lines +96 to +98
if got := scanString(t, db, `SELECT IFNULL(root_session_id,'') FROM sessions WHERE id=?`, childID); got == parentID {
t.Errorf("child root_session_id = %q (= parent), want still self (linkage rolled back)", got)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Rollback characterization assertion is too weak: it only rejects root_session_id == parentID instead of asserting the child remains self-rooted, so some regressions would pass undetected.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At internal/ingest/sow55_characterization_resolver_test.go, line 96:

<comment>Rollback characterization assertion is too weak: it only rejects `root_session_id == parentID` instead of asserting the child remains self-rooted, so some regressions would pass undetected.</comment>

<file context>
@@ -0,0 +1,130 @@
+	if got := scanString(t, db, `SELECT IFNULL(parent_session_id,'') FROM sessions WHERE id=?`, childID); got != "" {
+		t.Errorf("child parent_session_id = %q, want empty (linkage must roll back on notify failure)", got)
+	}
+	if got := scanString(t, db, `SELECT IFNULL(root_session_id,'') FROM sessions WHERE id=?`, childID); got == parentID {
+		t.Errorf("child root_session_id = %q (= parent), want still self (linkage rolled back)", got)
+	}
</file context>
Suggested change
if got := scanString(t, db, `SELECT IFNULL(root_session_id,'') FROM sessions WHERE id=?`, childID); got == parentID {
t.Errorf("child root_session_id = %q (= parent), want still self (linkage rolled back)", got)
}
if got := scanInt(t, db, `SELECT COUNT(*) FROM sessions WHERE id=? AND root_session_id=id`, childID); got != 1 {
t.Errorf("child root_session_id changed, want still self (linkage rolled back)")
}

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants