Skip to content

refactor(engine): extract the stranded linked-reference parsers out of the D1 repository layer - #5771

Closed
glorydavid03023 wants to merge 1 commit into
JSONbored:mainfrom
glorydavid03023:refactor/engine-linked-reference-parsers
Closed

refactor(engine): extract the stranded linked-reference parsers out of the D1 repository layer#5771
glorydavid03023 wants to merge 1 commit into
JSONbored:mainfrom
glorydavid03023:refactor/engine-linked-reference-parsers

Conversation

@glorydavid03023

Copy link
Copy Markdown
Contributor

Summary

Closes #4882.

This is the sweep that issue asks for, aimed at the file it names — src/db/repositories.ts, the ~386KB, D1-query-heavy repository-access layer — and it moves the candidate that issue names: "a pure regex-based parser living inside the very large D1-query-heavy repository-access file."

That parser is the linked-issue / linked-PR reference extractor: extractLinkedIssueNumbersWithOverflow, extractLinkedIssueNumbers, extractLinkedPrNumbers and MAX_LINKED_ISSUE_NUMBERS. It is pure regex logic with no D1 or Env dependency, 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.

Moving it into @loopover/engine also fixes a real, live divergence, which is why this one was worth moving first.

The divergence this closes

Because the engine cannot import from src/, packages/loopover-engine/src/signals/predicted-gate-engine.ts carried a hand-written second copy of the parser — and that copy had drifted. It was missing the inline-code-span guard the live gate's copy has:

Body text Live gate (src/db/repositories.ts) Predicted gate (engine copy)
- [ ] I linked a currently open issue this PR resolves (e.g. `Closes #123`) … no linked issue linked issue #123

That string is line 10 of this repo's own .github/pull_request_template.md. The template says to fill it out, not replace it — so the overwhelmingly common contributor PR body still contains it verbatim. The result: gittensory_predict_gate and the miner's local preflight told a contributor "you have a linked issue", while the live gate saw none and auto-closed the PR under the linked-issue hard rule. The pre-submit oracle was wrong in exactly the failure mode it exists to prevent.

The engine copy also silently lacked the MAX_LINKED_ISSUE_NUMBERS (50) overflow cap that linked-issue-hard-rules.ts relies on.

What changed

  • New packages/loopover-engine/src/github/linked-references.ts — the parsers, moved verbatim (regexes byte-for-byte identical to the originals, so the live gate's behavior is provably unchanged). Placed in the engine's existing github/ directory, alongside constants.ts and sanitize-public-comment.ts.
  • packages/loopover-engine/src/signals/predicted-gate-engine.ts — its hand-written duplicate is deleted; it now imports the shared parser. The predicted gate and the live gate can no longer disagree.
  • src/db/repositories.ts — the stranded implementation is gone; the file re-exports from the engine via a thin shim, so all four existing importers keep working unchanged. Imported by relative source path, matching this repo's established engine-consumption convention (src/signals/check-summary.ts).

No public API changed, and no gate-decision behavior changed on the host side.

Other stranded-pure-logic candidates found in the same sweep

Recorded here so the next slice of #4882 doesn't have to re-derive them; all are pure, top-level, and D1-free inside src/db/repositories.ts:

  1. Product-usage rollup analytics (~:6900-7290) — buildProductUsageDailyRollupRecord, buildProductUsageActivationFunnel, buildProductUsageRetentionRollups, productUsageRetentionRate, intersectionCount, addProductUsageUtcDays. The largest pure cluster in the file; a natural standalone engine module.
  2. Product-usage sanitizerssanitizeProductUsageMetadata / sanitizeProductUsageJson / sanitizeProductUsageString.
  3. Settings-column parsers (parseAutonomyPolicy, parseGatePack, parseTypeLabelSet, normalizeReviewNagPolicy, …) — deliberately left alone here: they belong to the settings/ slice tracked by Finish the settings/ slice extraction #4879, and moving them in this PR would collide with it.

loginMatches is not a candidate despite looking pure — it builds a Drizzle sql fragment and belongs with the query layer.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked a currently open issue this PR resolves (e.g. Closes #123) — a linked open issue is required for every contributor PR.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

npm run test:ci was run in full and is green. New suite test/unit/linked-references.test.ts covers the moved module at 100% statements / branches / functions / lines (29/29, 19/19, 7/7, 21/21), including:

  • every supported closing keyword, bare and owner/repo#N-qualified, and the cross-repo form that must not count;
  • the inline-code-span guard, with a span overlapping the match, a span before it, and a span after it (both sides of the overlap predicate);
  • why the spans cannot simply be blanked out first (closes `nothing` #5 must not become a match);
  • dedupe, #0, and a 400-digit number that overflows to Infinity;
  • overflow at the default cap and at an explicit / fractional / negative limit;
  • regression tests asserting the predicted gate and the live gate now return the same answer for the unedited PR-template line, and across the whole closing-keyword grammar.

If any required check was skipped, explain why:

  • None skipped.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests.
  • API/OpenAPI/MCP behavior is updated and tested where needed.
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks.
  • Visible UI changes include a UI Evidence section below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs.

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

  • The regexes are moved byte-for-byte, so the live gate's linked-issue behavior is provably unchanged; the only behavior change is that the predicted gate stops disagreeing with it.
  • The new module is intentionally not re-exported from packages/loopover-engine/src/index.ts: nothing in this PR consumes it through the published package entry point (both consumers import it by relative source path, per the existing convention), and adding an unused export there would widen the public surface for no caller.

…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-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.07%. Comparing base (97c98b3) to head (cacaedc).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5771      +/-   ##
==========================================
- 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              
Flag Coverage Δ
shard-1 43.93% <82.60%> (-0.02%) ⬇️
shard-2 35.79% <78.26%> (+0.02%) ⬆️
shard-3 32.24% <91.30%> (-0.01%) ⬇️
shard-4 32.94% <100.00%> (+0.01%) ⬆️
shard-5 31.72% <82.60%> (-0.01%) ⬇️
shard-6 44.64% <82.60%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...es/loopover-engine/src/github/linked-references.ts 100.00% <100.00%> (ø)
...opover-engine/src/signals/predicted-gate-engine.ts 100.00% <ø> (ø)
src/db/repositories.ts 96.67% <ø> (-0.05%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 14, 2026
@loopover-orb

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Caution

🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥🟥

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-14 13:44:24 UTC

4 files · 1 AI reviewer · 1 blocker · readiness 100/100 · CI green · clean

🛑 Suggested Action - Reject/Close

Review summary
This PR extracts the pure regex-based linked-issue/PR reference parsers out of src/db/repositories.ts into a new packages/loopover-engine/src/github/linked-references.ts module, replaces the drifted hand-written copy in predicted-gate-engine.ts with the shared implementation, and keeps repositories.ts working via a re-export. The move is well-scoped, closes a real behavioral divergence (missing inline-code-span guard in the predicted-gate copy caused false-positive linked-issue detection on the unedited PR template), and is backed by direct regression tests proving convergence between the two gates on the exact template string. The predicted-gate-engine.ts signature change drops the unused `repoFullName`-qualified second regex pass that existed only in the old hand-written copy — this is intentional since the shared function now handles both bare and qualified forms in one pass.

Nits — 4 non-blocking
  • src/db/repositories.ts:7833 imports via a relative `../../packages/loopover-engine/src/...` path instead of the `@​loopover/engine` package alias used at the top of the file — the comment justifies this by pointing to an existing convention (src/signals/check-summary.ts), but it's worth confirming that convention is actually followed consistently rather than another one-off.
  • The re-export block in src/db/repositories.ts keeps the file large and doesn't reduce its line count meaningfully relative to the file's ~7800+ lines — the extraction addresses the 'stranded pure logic' issue but the host file remains a long-file smell per the external size report.
  • Consider exporting `extractLinkedIssueNumbersWithOverflow` and `MAX_LINKED_ISSUE_NUMBERS` from predicted-gate-engine.ts's public surface (or via `predictedGateEngineInternals`) if downstream consumers might want overflow visibility, matching the live-gate's capability now that both share the same implementation.
  • If `@​loopover/engine` is the intended public import surface (per the top-of-file import in repositories.ts), consider re-exporting `linked-references` from the engine's package entrypoint and importing it that way instead of a relative path into `packages/loopover-engine/src/...`, for consistency.

Why this is blocked

  • No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. No linked issue detected — If this PR is intended to solve an issue, link it explicitly in the PR body.
Signal Result Evidence
Code review ❌ 1 blocker 1 reviewer
Linked issue ✅ Linked #4882
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 273 registered-repo PR(s), 165 merged, 20 issue(s).
Contributor context ✅ Confirmed Gittensor contributor glorydavid03023; Gittensor profile; 273 PR(s), 20 issue(s).
Gate result ❌ Blocking Repo-configured hard blocker found.
Improvement ✅ Minor risk: clean · value: minor — Code changes are accompanied by test evidence. LLM value judgment: moderate — The PR does exactly what the linked issue (#4882) asks — moves pure logic out of a D1-coupled file — and additionally fixes a real, demonstrated behavioral divergence between the live and predicted gates with a concrete regression test proving the fix.
Linked issue satisfaction

Partially addressed
The PR does exactly what the issue's example calls out — moving the pure regex-based linked-reference parser out of the D1-heavy repositories.ts into the engine package, with tests moved alongside it, satisfying the explicit acceptance criterion for that function. However, the issue also asks for a broader sweep across 'the largest I/O-bound files (the repository-access layer, the queue processor,

Review context
  • Author: glorydavid03023
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 273 PR(s), 20 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
[BETA] Chat with Gittensory

Ask 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.

  • @gittensory ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @gittensory chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @gittensory mention with a real question is routed to the closest matching read-only command automatically -- no exact syntax required.

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.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

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.

@loopover-orb loopover-orb Bot closed this Jul 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sweep for pure logic stranded inside I/O-bound files

1 participant