feat: inital steps towards phase5 manual ranking improvement#130
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughImplements Phase 5 inline candidate comparison (route-type gate, sponsor-fit scoring, pairwise resolution with succession and UK-presence adjustments), adds drain/hydration scripts, refines SQL optimistic-lock precision, updates docs, and adds tests and a generated drain comparison report. ChangesPhase 5 inline resolution pipeline
Queue maintenance scripts
Sequence Diagram(s)sequenceDiagram
participant Caller
participant scoreCandidate
participant routeTypeCompatible
Caller->>scoreCandidate: candidate, sponsor
scoreCandidate->>routeTypeCompatible: sponsor.route, candidate.type
alt route incompatible with company type
routeTypeCompatible-->>scoreCandidate: false
scoreCandidate-->>Caller: -Infinity
else route compatible
scoreCandidate->>scoreCandidate: normalise locality, compute feature scores
scoreCandidate-->>Caller: sum of locality + status contributions
end
sequenceDiagram
participant Resolver
participant scoreCandidate as scoreCandidate (existing)
participant scoreCandidate2 as scoreCandidate (proposed)
participant SuccessionMatch
Resolver->>scoreCandidate: existing candidate
scoreCandidate-->>Resolver: score_existing
Resolver->>scoreCandidate2: proposed candidate
scoreCandidate2-->>Resolver: score_proposed
Resolver->>SuccessionMatch: canonicalise names, check previous_company_names
SuccessionMatch-->>Resolver: succession forward/reverse
Resolver->>Resolver: apply SUCCESSION_WEIGHT adjustments and UK_PRESENCE_WEIGHT
alt adjusted_delta >= SCORE_MARGIN
Resolver-->>Resolver: promote or keep
else adjusted_delta < SCORE_MARGIN
Resolver-->>Resolver: inconclusive
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/scripts/drain-review-queue.ts`:
- Around line 191-200: The SELECT DISTINCT ON (organisation_name) query is
non-deterministic for tied counts because ORDER BY ends with "route" only;
update the ORDER BY in that query (the block producing { organisation_name,
town_city, route }) to include town_city as a final tie-breaker (e.g. ORDER BY
organisation_name, n DESC, route, COALESCE(town_city, '') or ORDER BY ... route,
town_city NULLS LAST) so rows with equal n and route are chosen
deterministically.
- Around line 551-554: The code increments the wrong counter when a mapping is
missing: replace the increment of orphaned (orphaned += 1) with the stale
counter (stale += 1) so the branch that calls markResolved(r.id,
`drain_${strategy}_stale`, changedBy) and logs "stale (no mapping row)"
correctly updates the stale tally; locate the block where mapping is checked
(the mapping variable) and the orphaned/stale counters and change the increment
to stale += 1.
In `@apps/web/scripts/hydrate-queue-proposed-profiles.ts`:
- Around line 161-165: The current fetch branch treats all non-OK responses the
same so 401/403 (invalid API key) keeps the loop running; update the non-OK
branch in the fetch logic that uses res and path to special-case authentication
failures: if res.status === 401 || res.status === 403, log a clear message
including the status and path and return a distinct fatal/auth result (e.g., {
kind: 'auth_error', status: res.status }) or throw an Error so the caller of
this function can abort the whole hydration process immediately; keep the
existing return { kind: 'error' } for other non-OK statuses and still return {
kind: 'ok', data: await res.json() } for success.
In `@apps/web/src/lib/phase5/compare-candidates.ts`:
- Around line 115-124: The UK-presence boost is currently applied based only on
types (isUkEstablishment / isForeignEntity) which can wrongly favor unrelated
pairs; change the two-if block so the boost is only added when the two
candidates are the same legal entity — e.g., require an identity guard such as
canonical-name equality or a previous-name linkage before adding
UK_PRESENCE_WEIGHT to s_e or s_p; update the checks around
isUkEstablishment(existing.type) && isForeignEntity(proposed.type) and its
symmetric counterpart to include a same-entity predicate (for example:
(existing.canonicalName === proposed.canonicalName ||
hasPreviousNameLink(existing, proposed)) && ...), so only confirmed same-entity
pairs receive the boost.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5f50d35e-8294-498e-9bed-499927221df7
📒 Files selected for processing (9)
apps/web/scripts/drain-review-queue.tsapps/web/scripts/hydrate-queue-proposed-profiles.tsapps/web/src/lib/phase5/compare-candidates.test.tsapps/web/src/lib/phase5/compare-candidates.tsapps/web/src/lib/phase5/score-candidate.test.tsapps/web/src/lib/phase5/score-candidate.tsapps/web/src/lib/phase5/sql.tsdocs/phase5-drain-comparison.mddocs/phase5-sweep-algorithm.md
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/lib/phase5/score-candidate.ts
- apps/web/src/lib/phase5/score-candidate.test.ts
Comment on lines
+582
to
+598
| const result = await applyPromotion( | ||
| mapping, | ||
| proposedResolution, | ||
| changedBy, | ||
| applyDeps, | ||
| ); | ||
|
|
||
| if (!result.ok) { | ||
| lockMissed += 1; | ||
| console.log( | ||
| ` ${idx} ${r.organisation_name} → lock_missed (mapping verified_at changed; queue row stays unresolved)`, | ||
| ); | ||
| continue; | ||
| } | ||
|
|
||
| swapped += 1; | ||
| await markResolved(r.id, `drain_${strategy}_swap`, changedBy); |
There was a problem hiding this comment.
Resolve the queue row in the same write unit as the promotion.
Lines 582-598 do two separate writes: first applyPromotion, then markResolved. If the process dies after the promotion succeeds but before the queue update, the mapping is already swapped while the queue row stays unresolved; the next run will then classify that row as stale instead of recording a successful drain.
Please fold the queue resolution into the same DB transaction/CTE as the promotion, or add a recovery path that marks rows resolved when the live mapping already matches the proposed number.
- this ensures we no longer keep populating data into the human review table
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Refactor
Tests
Documentation