JD6TFM

Names are precise (CheckSide, PullRequestInfo, GateCondition, newAiDuplicates, statusLine) and the magic values are named (TIMEOUT_MS, MAX_CLONES, MAX_AI, DISCORD_LIMIT, STATUS_CONTEXT, COMMENT_MARKER). Code is free of dead code in the files read. Two things pull it down. First, checks.ts repeats the commit-status pattern in publishStatus (checks.ts line 205) and publishResult (line 232), with the same GitHub target and sha handling in each. Second, the request.head.sha! and request.pullRequest!.number assertions recur in those helpers, even though githubTarget already guards them. In github.ts parsePullRequestEvent, the (pr.user as { login?: unknown }) cast is written out twice to build the optional author. On the front end, settings.js copies the same copy-to-clipboard button pattern in three places (secretCallout, the webhook URL row, and setupSteps). No duplication or lint measurement was supplied, so this rests on reading the code.

The layers are well separated: gate.ts is pure, check-summary.ts is pure Markdown, GitHubClient wraps the API, gate-alerts.ts is a state-change detector, and the webhook route verifies the signature before doing any work. Error handling sits at the boundaries: a failed scan becomes an errored check, a failed GitHub publish is recorded rather than thrown, and failed alert deliveries are logged. The concerns are: Checks.run (checks.ts ~lines 120–165) is a 45-line method that mixes scanning, comparison, summary, persistence, publishing and alerting. publishResult takes seven positional parameters. The github?.client guard in run is always true whenever github is defined, which makes the reader check twice. The settings view's renderSettings refresh is a view.querySelector('#alerts-extra') hack, which is fragile to change. Splitting run into scan, evaluate and publish steps would make it safer for a newcomer to change.

The task's core scenarios are implemented and wired end to end. A PR webhook (signed with X-Hub-Signature-256) starts a check that scans base and head, evaluates the gate, and posts a commit status plus one updated-in-place PR comment (src/routes/webhooks.ts, src/checks.ts run(), src/check-summary.ts). Threshold breaches fire check.failed alerts, and project scans fire breached/recovered only on state change (src/gate-alerts.ts). The gate thresholds are user-settable and unset thresholds are skipped (src/gate.ts evaluateGate). The PR comment is useful as a product: a metrics table, ✅/❌ per threshold, new clones linked to lines, and AI-style duplicates with fixes and collapsible assistant prompts. Judgement calls are coherent: a skipped threshold is reported rather than failed, alert URLs must be public https with redirects off, secrets are encrypted at rest and never returned, and checks are kept out of project trends via context 'check'. The one-time secret display and the setup steps in the UI (public/js/views/settings.js) guide the user. Points to tighten: the completion note's test count (153), coverage (~99%) and the 'checked live against a real GitHub repository' and Chrome claims were not verified in this review. The PR path depends on the GitHub client and the webhook route, and neither was exercised by a probe here. Also, a comment is posted per check, so a push with many checks is not deduplicated across shas beyond the single PR comment. No interactive probe was registered, so the UI is judged from the code only.

The task's core scenarios are present end to end. Each scan runs a second jscpd pass with --kind=renamed,similar (src/options.ts toNearMissArgs, src/scanner.ts insights()). Clone pairs are grouped into blocks (src/blocks.ts, shared with the dashboard hotspots). Each block is rated by weighted signals (src/insights/signals.ts) into high/medium/low. Suggestions name concrete targets, e.g. parameterize, extract-shared-module with a path, local helper, or test fixture (src/insights/suggestions.ts). Ready-to-paste prompts and an AGENTS.md guardrail snippet are generated (src/insights/prompts.ts). Results reach the API via report.insights, the dashboard data, and GET /projects/:id/scans/:scanId/insights (src/routes/dashboard.ts). The dashboard shows an 'AI-era insights' card (public/js/views/insights.js) with rating words plus icons, signals, linked copies, LCS-based differences, suggestions, copy-prompt and guardrail copy buttons. Judgement calls are coherent: the weights sum to a score, data/prose formats are excluded (NON_CODE_FORMATS in src/insights/index.ts), the near-miss pass is failure-tolerant (nearMiss: 'failed' with a UI banner), and the option insights: false turns it off. Caveats: the README and done note both hedge the rating as a heuristic, which is honest. The guardrail is only a snippet in the report and card, not written into the repo's AGENTS.md. Every scan now pays for a second jscpd run, which the done note does not mention. The note's 129 tests, ~99% coverage, jscpd grade A and Chrome check are unverified in this review, and no probe results were available to me. The code I read shows no obvious product defect.

Naming is precise throughout: groupBlocks, sameCopy, differences, likelihoodOf, effortOf, and NON_CODE_FORMATS each say what they do. The block-grouping logic was moved out of dashboard.ts into src/blocks.ts and is now shared, so there is no copy-paste between hotspots and insights (src/blocks.ts:1, src/dashboard.ts:79-95). The remaining blemishes are small. targetFile (src/insights/suggestions.ts) strips a :lang suffix with an inline regex, while testFixture calls extname(input.copies[0].file) without the strip, so the two paths handle embedded-code file names inconsistently. linkedInsights in src/dashboard.ts has a long inline intersection return type that repeats the shape of Insights. analyze in src/insights/index.ts re-checks copies[0].text several times. None of this is dead code, and there is no measured duplication to flag.

The pipeline is easy to follow: Scanner.insights runs the near-miss pass with a deadline and degrades to nearMiss: 'failed', and SourceReader confines reads to the workspace. Several things would slow a newcomer down. scanner.ts keeps this.binary! and this.version! non-null assertions, which the earlier verdicts already asked to remove. isWholeFunction in src/insights/code.ts uses firstCodeLine(lines)! and /^\s*/.exec(...)![0]. The scoring thresholds 60 and 35 in likelihoodOf (signals.ts) and the effort cut-offs 40 and 160 in effortOf (suggestions.ts) are unnamed literals, and the 0.8 similarity threshold is only in the README. readCopy makes three separate source reads per copy, including a one-line read for next. The differences LCS loop in src/insights/diff.ts nests a while inside a while and is hard to follow without a comment. Functions are mostly short, and the front end is split into small view helpers, though insightView in public/js/views/insights.js nests five levels of h(...) calls.

The task adds four test files: test/insights.test.ts, test/insights-code.test.ts, test/insights-api.test.ts and an extended test/dashboard-api.test.ts. The assertions are concrete. They check exact scores (90, 55), exact signal codes and ordering, exact difference strings like 'amount → total', exact suggestion strategies and targets, guardrail text, and that the standard report is unchanged (clones 0). Negative paths are covered: unreadable files still rate, data/prose formats are excluded, path traversal is rejected in SourceReader, insights can be turned off, and the endpoint returns insights:null when disabled. The HTTP level drives the real scanner with a zip fixture, so the level mix is reasonable. Gaps: there is no test file for signals.ts or suggestions.ts, so the rating thresholds (high from 60, medium from 35) are only exercised indirectly through the 90 and 55 scores, with no boundary test. The new blocks.ts groupBlocks is only covered indirectly through dashboard hotspots, with no direct test. The near-miss failure path is tested only at the buildInsights(nearMiss: null) level, with no HTTP test for a timeout or failed pass. The dashboard UI (insights.js: copy buttons, the flagged/low split, the failed banner, the no-insights hint) has no automated test, and the author's live Chrome check is manual only. In test/insights-api.test.ts the after(() => app.close()) sits inside an it block rather than a describe-level hook, the same defect flagged in task 1. The dashboard-api test still uses the loose startLine >= 1 assertion, which was flagged in task 2 and is not fixed here.

Names are mostly precise (sameCopy, redundantLines, counterpart, hostFile, Groups), and the server modules are tidy. Several small blemishes remain. Groups.find uses a non-null assertion (this.parent.get(root)!, src/dashboard.ts). setChildren in public/js/dom.js:22-24 relies on element.replaceChildren() ?? element, which works only because replaceChildren() returns undefined, so it reads as a trick. externalLink and locationLink in public/js/views/hotspots.js:10 overlap in purpose, and clear duplicates replaceChildren. There are stray trailing blank lines in public/js/views/summary.js and public/js/views/trend.js. Duplication was not measured (no analysis probe), so this rests on reading the code. No dead code stood out.

Error handling at the boundaries is solid: ApiError for missing clones and scans (src/routes/dashboard.ts:34), the 401 handling in public/js/api.js:35-37, and a strict CSP with the asset map loaded at startup (src/routes/ui.ts). The routes are small and read well. The weak spots are long functions. trendChart in public/js/charts.js:14-111 is about 95 lines, with a nested render that holds the scales, the tooltip, the keyboard handling and the SVG assembly, so a newcomer has to scroll to follow it. blockHotspots in src/dashboard.ts mixes grouping, region merging and ranking in one body. Some magic values are unnamed, such as the 80px label spacing, the 14px tooltip offset, and the 86_400 and 7 * 86_400 time breakpoints in public/js/format.js. pollUntilDone in public/js/views/dashboard.js rerenders with new URLSearchParams(), which silently drops the selected ?scan= query, a subtle trap for a later change.

All three brief scenarios are delivered in code. Duplication rates come from the scan summary with a change against the previous scan (public/js/views/summary.js, heroCard). The trend is a chart with a table view and click-to-open (public/js/views/trend.js, public/js/charts.js). Ranked hotspots are files by duplicated lines and blocks by redundant lines, each expandable to exact ranges (src/dashboard.ts buildHotspots; public/js/views/hotspots.js). Locations link to the scanned commit on GitHub, GitLab, Bitbucket and Codeberg (src/links.ts), and a 'View code' dialog works for archive uploads too via GET /projects/:id/scans/:scanId/clones/:cloneId (src/routes/dashboard.ts). The block grouping that merges overlapping ranges (sameCopy, 50% overlap) is a thoughtful judgement call for jscpd's inconsistent ranges. The UI builds the DOM with text nodes, under a strict CSP. Deductions: ?scan= on a queued, running or failed scan calls history.report before any status check (src/routes/dashboard.ts), so it can error instead of showing the 'no completed scans' state. The README dashboard routes were added after a blank line, so they land outside the endpoint table. I did not verify the note's test count, the coverage figure or the Chrome walkthrough, and I did not run probes.

The new server-side tests are concrete and mostly well targeted. test/dashboard.test.ts asserts exact block ranking, redundantLines (lines × (occurrences−1)), exact file order and counterpart links, and the overlap-merging rule with exact ranges (lines 127–146). It also has a negative case that separate code touching in one file stays apart (line 149), a limit/totals check (line 153), and a null/empty case for archives and empty reports. test/dashboard-api.test.ts covers the empty dashboard, the trend and hotspot values over real git repos, ?scan= with limit/points, SCAN_NOT_COMPLETED / SCAN_NOT_FOUND / INVALID_REQUEST (limit=0), clone 404, the 401 path, and path traversal on /app/ (lines 135–140). test/links.test.ts checks exact URLs for four hosts plus several rejection cases. Gaps: the browser layer is essentially untested. public/js/charts.js (trendChart keyboard and click selection, inlineBar) and the views have no automated tests; the only UI coverage is the static-serving check and format.js. The player's manual Chrome check is not a regression test. Over HTTP, the hotspot URLs for hosted repos are never asserted (every HTTP test uses a local repo, so url is null), and the archive/upload path is not exercised with dashboard output. The points trend cap is never truncated (points=2 with only 2 scans), and some checks are weak, e.g. assert.ok(location.startLine >= 1 && ...) at dashboard-api.test.ts:~68. No skipped or tautological tests were found. The task-3 done-note cites 110 passing tests and 99.4% coverage, but I did not run them and no probe was registered.

The suite is proportionate and mostly well specified. It has unit tests for accounts (scrypt round-trip, same answer for wrong password and unknown email, expired/signed-out tokens), history (interrupted-scan recovery, cross-project isolation, deleted-project results), compare (added/removed by fingerprint, trend rules), and db migrations/rollback. It also has HTTP-level tests in test/projects-api.test.ts that drive real git repos with exact status codes and codes, e.g. the second-scan change, the ref override, the archive upload, the failed-scan error text, and the 202 poll loop. Assertions check concrete values, not just that code runs. Gaps: no test that a failed scan is skipped when choosing the base for change (the change is only tested with consecutive completed scans); no HTTP test for DELETE cascading to scans or for PATCH option validation; no HTTP test for a malformed before cursor; no HTTP test for session expiry (only the unit test). The SCANNER_BUSY test (projects-api.test.ts, 'scan capacity') depends on the first background scan still being in flight when the second request arrives, which makes it timing-sensitive. The in-test after(() => app.close()) inside the it callback is the same pattern I flagged in task 1, and it is still there. The task 1 request to add a SCAN_FAILED/SCANNER_BUSY HTTP test is only partly addressed: SCANNER_BUSY now exists at HTTP level, but it is only tested for projects, not the anonymous /scans route. No coverage or test-run output was probed, so the claimed ~99% line coverage is unverified.

Naming is precise (ScanHistory, ScanJobs, summarizeChange, interruptUnfinished, unmatched), and the module split is clean. Dead code is absent, and the one clear duplication is that repository parsing exists twice: repositoryFrom in src/http.ts:31 and parseRepository in src/projects.ts:43 both turn a string or {url, ref} into a RepositoryRequest. Several non-null assertions remain: this.find(id)! in src/history.ts (create), and input.name! in src/projects.ts (create). In src/history.ts fail() also calls new Date() twice for one timestamp. No duplication or lint measurements were supplied, so this rests on reading the code.

Most functions are short and single-purpose. Error handling sits at the boundaries: auth, validation, the background job's catch, and the cleanup in finally. Magic values are named (SCRYPT params, MIN_PASSWORD, MAX_PASSWORD, SESSION TTL via DAY). The weakest spot is the POST /projects/:projectId/scans handler in src/routes/projects.ts, which holds about 30 lines with nested branches for multipart and JSON, workspace cleanup, the wait flag and the Location header; a newcomer has to read all of it to change either path. ScanHistory.report() also re-queries the row it already fetched through get. Neither is a serious defect.

The core scenarios are implemented end to end and match the README. Accounts: scrypt-hashed passwords, SHA-256-hashed Bearer session tokens with TTL, and a decoy hash so unknown emails and wrong passwords take similar time (src/accounts.ts:70-95). Projects are owner-scoped, and other users get PROJECT_NOT_FOUND (src/routes/projects.ts). Scans are stored with seq numbers, timestamps, summary and full report, and each completed scan stores its change from the previous completed one (src/history.ts complete()). Pagination, compare and failed-scan retention are present. The anonymous POST /scans from task 1 is preserved (src/app.ts). Restart recovery marks unfinished scans SCAN_INTERRUPTED (src/history.ts interruptUnfinished, src/server.ts). Judgement calls are coherent: 202 with a Location header by default, ?wait=true for synchronous use, request options layered over project defaults. Weaknesses: I could not run the 89-test suite or the coverage claim, so those figures are unverified. The 'change' object is computed at completion time and not recomputed if an earlier scan later fails, which is acceptable. No UI, which is consistent with the task scope. Overall it is a solid, well-scoped product increment that serves the 'compare over time' goal.

The committed code delivers the task's core scenarios. POST /scans accepts a public git repo (JSON, with optional ref) or an uploaded zip/tar/tar.gz/tar.zst archive (multipart), runs the real jscpd 5 binary with a service-owned config, and returns a structured report with summary, per-format stats, per-file hotspots, clones with locations and fragments, and warnings. Failures use a stable {error:{code,message,details}} shape with specific messages and HTTP statuses. Reliability is handled by a per-scan workspace that is always removed, a concurrency limiter with a bounded queue, and timeouts on both clone and scan. Safety covers https-only git with credential, private-host and allowlist checks, archive magic-byte detection, and entry and size limits. The hotspot ranking and the 'duplicatedLines counts each line once' logic are judgement calls that serve the user well. I did not run the tests or probes, and no probes were registered, so the 99% coverage and 62-test claims in the note are unverified. The note's claims match the code I read: app.ts, scanner.ts, jscpd.ts, sources/git.ts and report.ts. Minor gaps: the scan is synchronous, so there is no job ID or polling, which is acceptable for this task but limits the 'history' later tasks; the README's options table exposes raw jscpd flags with no sanity limits beyond validation.

Naming is consistently descriptive (validateRepository, cloneFailure, coveredLines, hotspots, safeTarget, toJscpdArgs), and the code has no visible dead code or copy-paste. The one repeated pattern is the Rule table in options.ts, which is a deliberate declarative structure rather than duplication. Minor deductions: non-null assertions (this.binary!, this.version!) in scanner.ts scan() and a lazy if (!this.binary || !this.version) await this.init() that overlaps with the startup init path. The (error as Error).message cast in zip.ts is a small wart. No duplication tool measurements were supplied, so this rests on reading the code.

Functions are short and single-purpose (runJscpd, extractArchive, buildReport, readUpload), and boundary errors are mapped to stable ScanError codes with actionable messages. Limits and timeouts come from config, and the spawned-process and workspace cleanup is in finally blocks. Deductions: cloneRepository wraps a closure git() with a deadline ternary that returns undefined, which is harder to follow than it should be, and the zip/tar extractors carry mutable failure state across callbacks. The archive magic-byte checks are inline literal arrays rather than named constants. Otherwise a newcomer could change this safely.

The suite is substantial and mostly behavior-focused. It has 62 tests across app, archive, git, jscpd, options, report, limiter and config files, and it runs the real jscpd binary and real local git repos. Assertions check concrete values: exact source objects and clone line ranges (test/report.test.ts), exact error codes and messages (test/app.test.ts, test/git.test.ts), and the exact jscpd argv (test/options.test.ts). Error paths and boundaries are well covered: zip-slip and symlink entries, archive bomb size and entry limits, corrupt archives, a SIGKILL crash, scan timeouts, a missing or unreadable report, and a non-running binary. Gaps: the jscpd binary is exercised through the HTTP layer only for a timeout (test/app.test.ts 'reports scan timeouts and engine failures'), and that test name overstates what it checks because it covers only the timeout path. There is no HTTP-level test for SCAN_FAILED or SCANNER_BUSY, even though the limiter is unit-tested. Two after(() => app.close()) calls sit inside it bodies in the limits block (test/app.test.ts), which is a non-standard pattern that may not close apps reliably. No tests were run in the grading window, so pass status is taken from the participant's own claim (62 tests, ~99% coverage) and not verified.