refactor(engine): extract the pull-request-target-key parser into loopover-engine - #5762
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5762 +/- ##
=======================================
Coverage 95.07% 95.07%
=======================================
Files 581 582 +1
Lines 46181 46183 +2
Branches 14811 14812 +1
=======================================
+ Hits 43905 43907 +2
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:
|
|
Tip 🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩 ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-14 12:49:32 UTC
✅ Suggested Action - Approve/Merge
Review summary Nits — 4 non-blocking
Linked issue satisfactionPartially addressed 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.
|
…5770) Empirically confirms how a moved file is represented to git's diff and the codecov patch gate, so the engine-extraction phases (content-lane, settings, signals) don't trip patch-99% on unchanged behavior. A pure 'git mv' with no content change stages as an R100 rename (0 insertions / 0 deletions) — verified against a real content-lane file — so patch coverage has nothing to measure and history carries forward. Disabling rename detection (which is how the diff degrades when the move commit also edits the file) re-surfaces the same file as 117 added lines that must clear 99% patch. Documents both the pure-rename path and the extract-as-new path taken by the already-merged #5762, and a move-only-commit protocol for every subsequent phase. Verification writeup only; the throwaway test move was reverted (no code moved). Closes #4878
Summary
Extracts the pure
parsePullRequestTargetKeyparser out of the D1-query-heavysrc/db/repositories.tsand into@loopover/engineas a standalone, independently-tested function — the "sweep for pure logic stranded inside I/O-bound files" work from #4882. The parser ("<owner>/<repo>#<number>"→{ repoFullName, pullNumber } | null) has zero side effects, zero imports, and was module-private with only two in-file call sites, so it moves cleanly: the engine gains the function + a barrel export, andrepositories.tsimports it (no behavior change at the two call sites).Identified in the same file during the sweep and left as follow-up candidates (kept out to honor "one direction of movement per PR"):
intersectionCount,maxIso,escapeSqlLikePattern.Closes #4882
Scope
type(scope): short summaryConventional Commit format.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #4882).Validation
git diff --check— cleannpm run actionlint— no workflow changesnpm run typecheck— passes (whole project; the new@loopover/engineimport inrepositories.tsresolves against the built engine)npm run test:coverage— the new suitetest/unit/parse-pull-request-target-key.test.tsdrives the moved function through the engine barrel and covers 100% of statements/branches/functions/lines onparse-pull-request-target-key.ts(verified viacoverage-summary.json);codecov/patchmeasures only the diff, and the two consumer call sites are unchangednpm run test:workers— unaffected (no worker changes)npm run build:mcp/test:mcp-pack— unaffected (no MCP changes)npm run ui:openapi:check/ui:lint/ui:typecheck/ui:build— unaffected (no UI/OpenAPI changes)npm audit --audit-level=moderate— no dependency changes#, repo half without/, non-integer / non-positive pull number, valid, and last-#split) is exercisedIf any required check was skipped, explain why:
src/db/repositories.ts(−1 def, +1 import) andpackages/loopover-engine/src/**(+ a test). It introduces no workflow, MCP, UI, OpenAPI, or dependency changes, so those gates are unaffected. I validated the parts that touch the change: whole-projecttypecheck, the engine build, the engine's own suite (550/550), and the new function's test at 100% coverage.Safety
UI Evidencesection — N/A (no UI/frontend/docs/extension changes).Notes
src/dbis outsidecheck-engine-parity's scope (review/settings/signals+ named twins), so no parity twin is introduced.