refactor(engine): extract the stranded linked-reference parsers out of the D1 repository layer - #5773
Conversation
…f the D1 repository layer (JSONbored#4882) `src/db/repositories.ts` is a ~386KB, D1-query-heavy repository-access file, and JSONbored#4882 calls out the exact candidate stranded inside it: "a pure regex-based parser living inside the very large D1-query-heavy repository-access file". That parser is the linked-issue/PR reference extractor -- pure regex logic with no D1 or `Env` dependency, yet four core modules reach into the database layer purely to get at it. Move it to `@loopover/engine` as `github/linked-references.ts` (regexes byte-for-byte identical, so the live gate's behavior is provably unchanged), and re-export it from `src/db/repositories.ts` as a thin shim so every existing importer keeps working. This also converges a real divergence. Because the engine cannot import from `src/`, `signals/predicted-gate-engine.ts` carried a hand-written second copy that had drifted: it was missing the inline-code-span guard. This repo's own PR template contains "(e.g. `Closes JSONbored#123`)" on the line contributors are told to fill out, not replace -- so the predicted gate read a linked issue where the live gate correctly sees none, telling contributors their PR was safe right before the linked-issue hard rule closed it. The engine copy also lacked the MAX_LINKED_ISSUE_NUMBERS overflow cap that linked-issue-hard-rules.ts relies on. Both gates now resolve one shared module. Tests cover the moved module at 100% statements/branches/functions/lines, including both sides of the code-span overlap predicate, the cross-repo qualified form, overflow at default/explicit/fractional/ negative limits, and regression tests pinning that the predicted gate and the live gate now agree on the unedited PR template.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
loopover-orb
Bot
added
the
gittensor:bug
|
Caution 🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥 🛑 LoopOver review result - reject/close recommendedReview updated: 2026-07-14 14:00:41 UTC
🛑 Suggested Action - Reject/Close Review summary Nits — 4 non-blocking
Why this is blocked
📋 Copy for AI agents — paste into your coding agent
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
[BETA] Chat with GittensoryAsk Gittensory a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/gittensory-commands 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5773 +/- ##
==========================================
- Coverage 95.07% 95.07% -0.01%
==========================================
Files 582 583 +1
Lines 46216 46211 -5
Branches 14820 14820
==========================================
- Hits 43940 43935 -5
Misses 1516 1516
Partials 760 760
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Gittensory is closing this pull request on the maintainer's behalf (No linked issue detected). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
1 participant
Closes #4882
Summary
#4882 asks for a sweep of the largest I/O-bound files for pure logic stranded inside them, and names its own first candidate: "a pure regex-based parser living inside the very large D1-query-heavy repository-access file."
That file is
src/db/repositories.ts(~386KB, D1-query-heavy), and that parser is the linked-issue / linked-PR reference extractor —extractLinkedIssueNumbersWithOverflow,extractLinkedIssueNumbers,extractLinkedPrNumbers,MAX_LINKED_ISSUE_NUMBERS. It is pure regex logic with no D1 orEnvdependency, yet four core modules (src/github/backfill.ts,src/review/enrichment-wire.ts,src/review/linked-issue-hard-rules.ts,src/signals/engine.ts) reach into the database layer purely to get at it.This PR moves it into
@loopover/engine, which also closes a real divergence — the reason this was the right candidate to move first.The divergence this closes
The engine cannot import from
src/, sopackages/loopover-engine/src/signals/predicted-gate-engine.tscarried a hand-written second copy of the parser. That copy had drifted: it was missing the inline-code-span guard the live gate's copy has, and it silently lacked theMAX_LINKED_ISSUE_NUMBERS(50) overflow cap thatlinked-issue-hard-rules.tsrelies on.The consequence: a closing keyword wrapped in backticks is not a real GitHub closing directive, and the live gate correctly ignores it — but the engine's copy counted it. So the miner's predicted gate could report a linked issue for a body the live gate reads as having none, telling a contributor their PR was safe immediately before the linked-issue hard rule closed it. The pre-submit oracle was wrong in exactly the failure mode it exists to prevent.
What changed
packages/loopover-engine/src/github/linked-references.ts— the parsers, moved with their regexes byte-for-byte identical, so the live gate's behavior is provably unchanged. It sits in the engine's existinggithub/directory next toconstants.tsandsanitize-public-comment.ts.packages/loopover-engine/src/signals/predicted-gate-engine.ts— the hand-written duplicate is deleted; it imports the shared parser. The two gates can no longer disagree.src/db/repositories.ts— the stranded implementation is gone; the file re-exports from the engine as a thin shim, so all four existing importers keep working unchanged. Imported by relative source path, matching the convention insrc/signals/check-summary.ts.No public API changed, and no host-side gate behavior changed.
Other stranded-pure-logic candidates found in the same sweep
Recorded so the next slice of #4882 need not re-derive them. All are pure, top-level, and D1-free inside
src/db/repositories.ts::6900-7290) —buildProductUsageDailyRollupRecord,buildProductUsageActivationFunnel,buildProductUsageRetentionRollups,productUsageRetentionRate,intersectionCount,addProductUsageUtcDays. The largest pure cluster in the file.sanitizeProductUsageMetadata/sanitizeProductUsageJson/sanitizeProductUsageString.parseAutonomyPolicy,parseGatePack,parseTypeLabelSet,normalizeReviewNagPolicy, …) — deliberately left alone: they belong to thesettings/slice tracked by Finish thesettings/slice extraction #4879, and moving them here would collide with it.loginMatchesis not a candidate despite looking pure — it builds a Drizzlesqlfragment and belongs with the query layer.Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateNew suite
test/unit/linked-references.test.ts(12 tests) covers the moved module at 100% statements / branches / functions / lines (29/29, 19/19, 7/7, 21/21):owner/repo-qualified, plus the cross-repo form that must not count;Infinity;npm run engine-parity:drift-checkpasses (22 duplicated pairs agree), as dotest:engine-parity,test:live-gate-parity,test:driver-parity, and the@loopover/engineworkspace suite.If any required check was skipped, explain why:
Safety
UI Evidencesection with JPG/PNG screenshots. Not applicable here.UI Evidence
Not applicable — no UI, frontend, docs, or extension surface is touched. The diff is three backend/engine TypeScript files plus one new unit-test file.
Notes
packages/loopover-engine/src/index.ts: both consumers import it by relative source path per the existing convention, so adding an unused export there would widen the public surface for no caller.