refactor(store): isolate semantic scan worker execution

Closed
  • Is something wrong?
#1,382 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

  • #1619 by @Reaan06 — closed without merging

Assessment

Difficulty
3/5
Estimated time
Half a day
Newbie friendliness
45/100
Issue type
Refactor
Clarity
Clearly specified
Activity status
Active
Tech stack
go
Domain
database

Research direction

Start at internal/store/relations.go:1311 scanProject, focusing on the semantic worker block at :1562–1663, and read internal/store/scan_semantic_test.go plus scan_batch_test.go and scan_page_test.go to learn the expected dry-run/apply and accounting behavior. Extract only semantic worker execution and result accounting into a private function after candidate pairs are chosen, keeping page loading, pair selection and structural inserts in scanProject. Done means go test ./internal/store/... passes with unchanged public API, SQL semantics and response fields. Note an open linked PR (#1619) already targets this issue, so confirm with maintainers before starting.

Written by the indexing model from the issue text.

Description

priority:low status:needs-review type:refactor
🔍 Problem Description

internal/store/relations.go:1311–1664 implements scanProject as one roughly 350-line function. It combines page loading, candidate selection, relation insertion and caps, semantic-pair deduplication, and a concurrent semantic judgment pool. This puts distinct failure and accounting paths into one unit and makes semantic changes hard to review without rechecking structural scan behavior. This is a maintainability proposal, not a claim that the scan currently misbehaves.

💡 Proposed Solution

Extract only semantic worker execution and result accounting into a private, focused function after candidate pairs have been selected. Keep page/query logic, pair selection/caps and structural inserts in scanProject. Preserve dry-run/apply semantics, worker concurrency, timeouts, panic/error handling, counters and result ordering. Do not combine this with a behavioral performance fix.

Acceptance: existing scan batching/page and semantic tests still pass; focused cases in internal/store/scan_semantic_test.go cover dry-run versus apply, valid/invalid verdicts, runner errors/panics, capped pairs, and concurrent accounting. No public API, SQL semantics or response fields change; prefer net simpler code rather than shifting the same complexity to more layers.

📦 Affected Area

Store (database, queries)

🔄 Alternatives Considered

Split the entire scan pipeline at once (larger review and regression surface) or leave all paths together. A bounded semantic execution seam permits one behavior-preserving work unit with existing tests.

📎 Additional Context

Source revalidated on main at a43183c2 (the semantic worker pool still lives inside scanProject): internal/store/relations.go:1311–1664, particularly the semantic worker block :1562–1663; tests in internal/store/scan_semantic_test.go, scan_batch_test.go, and scan_page_test.go. The closed #590 and #721 tracked different behavioral bugs; this proposal does not reopen them. No implementation or tests were run for this proposal.

Dominant language
Go
Stars
7.1k
Forks
729
Avg merge
10h 51m
Merged PRs (30d)
277

Getting set up

Open in Codespaces

Starts the project's dev container in your browser, under your own GitHub account.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Gentleman-Programming/engram

All issues in Gentleman-Programming/engram

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.