Repository navigation
Multiagent and CI - #244
Closed
marynabieliaieva wants to merge 92 commits into
Closed
marynabieliaieva wants to merge 92 commits into
marynabieliaieva wants to merge 92 commits into
Conversation
…LOG_LEVEL
Two boot blockers when running ./scripts/dev.sh from a clean checkout:
1. The API imports reviewer-core's raw source via the tsconfig path alias, so
Node resolves `openai` relative to reviewer-core/src — but dev.sh only
installed deps for server and client, never reviewer-core, so boot crashed
with ERR_MODULE_NOT_FOUND ('openai'). e2e.sh already handled this; dev.sh
now does the same (npm ci in reviewer-core when node_modules is missing).
openai is a real runtime dep (OpenRouter SDK + zod→json-schema helper used
by both the OpenAI and Anthropic paths), so removing it was not an option.
2. With openai resolving, the next crash was env validation: .env ships
`LOG_LEVEL=` empty, and z.enum(...).optional() rejects '' (it permits
undefined, not an empty string). Coerce '' → undefined so it falls through
to the 'info' default; also unblocks any .env files already on disk.
Verified: pnpm dev now reaches a healthy /health.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Self-contained evals/ package (statistics-grade skill/agent/workflow evals) plus the OpenRouter engine: LiteLLM translating proxy so the tool tiers can run on cheap non-Anthropic models. Only the evals/ folder — nothing else. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- add agent_runs.cost_usd column (migration 0010) and thread it through the run repository, run executor, and shared contracts (RunMeta/PrMeta) - expose latest completed run cost on the PR list endpoint - new RunCostBadge component; show cost in PR rows, run history, review run accordion, and verdict banner - add per-package CLAUDE.md/INSIGHTS.md docs and engineering-insights skill Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add per-severity counter chips (e.g. CRITICAL 2 · WARNING 1) to the FindingsPanel toolbar. Clicking a chip filters the list to that severity; clicking it again clears the filter. Counts always reflect the full finding list, and the row is hidden when a run has no findings. Pure client-side derivation from already-fetched findings — no new API or LLM calls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
client: FindingCard borderColor-vs-borderLeftColor React warning survives the all-longhand trick; SeverityBadge labels are uppercased by CSS only. e2e: the app scrolls in an inner <main>, so agent-browser scroll / screenshot --full only ever capture the top — grow the viewport instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r preview Match the lab design: each PR row shows per-severity finding chips (CRITICAL/WARNING/SUGGESTION counts, colored, zero counts hidden). Clicking a chip deep-links to the PR's findings tab pre-filtered to that severity (?tab=findings&severity=…, picked up by FindingsPanel). Hovering the cell shows a preview card (title, category, file:line, confidence, truncated rationale), lazily fetched from the existing /pulls/:id/reviews endpoint and cached by TanStack Query. Server: the pulls list now returns findings_counts summed over each agent's latest review — the same rule the client preview applies, so chips and preview always agree. Contract extended in the canonical server copy of @devdigest/shared and hand-mirrored to the client copy. The preview card is position:fixed anchored to the cell rect — the list container clips absolutely-positioned children. No new LLM calls anywhere. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Match the lab design for the Agent runs tab: a settled timeline run whose review has findings now shows per-severity chips (plus the blockers note) instead of the plain "N finding(s)" text, and hovering them opens a "N findings in this run" preview card. Runs without a matched review keep the old text line. The preview card is extracted into a shared component (src/components/findings-preview) used by both the PR list's FINDINGS column and the timeline; it stays position:fixed anchored to the hovered rect so container overflow can't clip it. Reviews reach RunHistory via a new optional prop from FindingsTab (matched by run_id) — data the tab already had, so no new requests and no LLM calls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng trap Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…-l01 Adding markdown files and costs feature implementation
…indings HW-1: PR findings
Everything that had accumulated uncommitted on this branch, pushed to the fork so it is not stranded on one machine: - the skills feature (server module, agent Skills tab, /skills UI, seeds) - mechanical enforcement: eslint configs in all three packages, dependency-cruiser layer rules - the module promotions (pulls/settings) and shared container repositories - client error boundaries (error.tsx, global-error.tsx, not-found.tsx) - .claude/skills additions and per-package INSIGHTS updates Recorded as one commit deliberately: this is a snapshot of work in progress, not a reviewed changeset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Scan a cloned repo for the conventions it already follows, triage the
candidates, and merge the accepted ones into a `repo-conventions` skill
linked to a reviewing agent.
Three stages, and only the middle one is a model:
SAMPLE configs + repoIntel.getConventionSamples(), rendered with a
1-based line gutter. Pure code — the model never browses.
PROPOSE one structured call on the model from FEATURE_MODELS.conventions.
VERIFY re-read the cited file: the path must have been sampled, the
snippet must be substantial and must really occur there. A wrong
line number is corrected; an invented snippet is dropped. The
snippet shown in the UI is sliced from the file, never from the
model's reply.
Server — `modules/conventions/`:
GET /repos/:id/conventions
POST /repos/:id/conventions/extract
POST /repos/:id/conventions/skill (draft only, persists nothing)
PATCH /conventions/:id (accept / reject / edit)
DELETE /conventions/:id
Migration 0015 adds category / rationale / evidence_line / status /
created_at with CHECK constraints and a repo+created index. A re-scan
replaces only `pending` rows, so an accepted or rejected rule is never
re-litigated.
Client — `/repos/[repoId]/conventions`: triage filters with counts, evidence
deep-linked to the exact line on GitHub, per-candidate selection so one board
can yield several skills, and a Create-skill modal that links the result to an
agent through the additive link endpoint.
Skills for the API Contract Reviewer: good/bad examples added to
contract-breaking-change and response-shape-guard, a new semver-discipline
seed skill, and docs/skills/deprecation-policy.md for the import path.
Measured on a live scan of angular-osf (deepseek-v4-flash, ~$0.001, 17 files):
field ORDER in the response schema is generation order, so moving
category/confidence after the rule took the model from 1 category and a flat
0.90 confidence to 5 categories spanning 0.50-0.95. 13 proposed, 2 dropped by
the evidence gate. Full write-up and the quality roadmap in
docs/specs/conventions.md.
Includes the skills module and the container's shared repositories, which the
conventions feature depends on and which were not yet committed upstream.
Tests: 26 server (21 unit on the gate + skill assembly, 5 integration),
34 client. Both typechecks clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two CI failures introduced by 641b637, which copied the whole migrations directory over upstream's instead of merging with it. 1. The journal rewrote history instead of appending. Entry 10 was replaced with `0010_polite_sasquatch` — a file that exists on no branch — and upstream's `0011_nasty_pretty_boy` was dropped entirely, so every fresh-database lane crashed with "No file …0010_polite_sasquatch.sql found" while already-migrated databases kept passing. The regenerated snapshots had also lost the critical_count / warning_count / suggestion_count columns that src/db/schema/runs.ts still declares. Repaired by restoring upstream's entries and meta/0011_snapshot.json, renumbering this branch's migrations 0011-0015 to 0012-0016 after them, re-adding the lost columns to the renumbered snapshots, and relinking the first renumbered snapshot's prevId. Journal and files now agree: 17 entries, 17 files, tags in order. 2. `fflate` is imported by src/modules/skills/service.ts but appeared in neither package.json nor the lockfile. It passed locally only because a stray copy sat in node_modules; a clean `pnpm install --frozen-lockfile` surfaced it as TS2307. Verified: `pnpm install --frozen-lockfile` clean, typecheck clean, and the full suite green — 158 tests across 25 files, including the integration lane that runs every migration against a fresh testcontainer. Both traps recorded in server/INSIGHTS.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…branch Merge refactor/architecture-plan-wave-0-3 into feat/conventions-extractor-clean. The two branches were complementary halves of the same feature set: - conventions side (kept as base): conventions module + UI, skills backend, hooks/skills.ts, seed-skills.ts, migrations 0010-0016, run cost badge, severity counters on agent_runs, newer CLAUDE.md/INSIGHTS.md. - refactor side (brought in): /skills pages + agent SkillsTab + import drawer, run-executor skill wiring (buildSkillBlocks), seeded Test Quality and API Contract reviewer agents + prompts, pulls module service refactor with the FINDINGS column rollup, error boundaries, eslint configs, extra skills. Conflict resolutions of note: - agent_runs schema: union — severity-count columns (ours) + index/status CHECK declarations (theirs), matching migrations 0012/0013. - run-executor: skills, cost and severity counts all feed the run now. - pulls routes: theirs' thin service delegation (score + cost + findings_counts rollup) replaces the inline implementation. - migrations: ours' 0000-0016 chain is canonical; theirs' divergent 0010_polite_sasquatch dropped. - VerdictBanner adapted to ours' RunCostBadge API; its test updated. Verified: typecheck server/client/reviewer-core; 156 server unit + 58 DB integration + 145 client + 25 engine tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tegration/all-features Integration/all features
Three student pull requests were merged straight into main and left it holding completed course work: ai-agentic-engineering-neo#101 (mp-l01), ai-agentic-engineering-neo#112 (hw_1_findings) and ai-agentic-engineering-neo#137 (integration/all-features) — 228 files, +43308/-706 since the last starter commit 66727c8. The starter has to ship unsolved, so this restores main's tree to 66727c8 exactly. History is left intact: the three merges stay in the log, and the full merged state is preserved on the integration/all-features branch (376ac49). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds costUsd to agent_runs, sums it per PR in the pulls list endpoint, and surfaces it in the UI: a compact badge in the PR list, a detailed badge (cost + token delta) in the run timeline, and a COST stat tile in the run trace drawer. Missing cost data renders as "—", distinct from a genuinely free ($0) run.
- Match suites with a trailing slash (skills/<name>/, agents/<name>/) so that `dependency-checker` no longer also runs the deliberately failing `-lite` control. - LiteLLM main-stable refuses to start without a master key: opt in to the documented no-auth mode for this ephemeral gateway and bind it to 127.0.0.1 only. - Use 127.0.0.1 (not localhost) for the proxy URL so IPv6 resolution can't miss the IPv4 bind. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The criterion "does NOT flag new PgCheckoutRepository()" has nothing to quote, so the Gemini judge returned evidence:null and failed it even though the agent output was correct (one A6 finding, nothing about service.ts). Rephrase it as a quotable positive: the findings contain exactly one item and the summary counts line shows it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
- run.ts: set a tall 1440x3200 viewport once per run. agent-browser's default window is ~568px and `find ... click` does not scroll, so cards/checkboxes below the fold gave "Element not found" in CI or a silent no-op locally. - 08-skills: `click --text X` is not valid in agent-browser 0.38 (--text is parsed as a selector); use `find text X click` / `find role button ... --exact` instead. - 09-conventions: the modal's "Create skill" button shares its name with the page toolbar button behind the overlay; scope the click to `[role=dialog] button`. - e2e/INSIGHTS.md: record both gotchas. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
"Filter skills…" is the input's placeholder, not page text, so `wait --text` never matches it. Check for the element with `wait --fn` instead (same approach as 09-conventions). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The workflow installed the newest agent-browser on every run. Locally (0.38.1) all 13 flows pass, but in CI flows 11 and 12 failed with "Element not found" for `find role checkbox --name <doc>` even though the checkbox is on screen. Pin the version the flows were verified against and print it in the log; bump it deliberately with a full local run. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…re set viewport - 09-conventions: all convention cards share one useUpdateConvention mutation, so `pending` disables every Accept button until the PATCH returns. On slower CI the second back-to-back Accept hit a disabled button (agent-browser still reports "Done") and only one candidate was accepted. Wait for "1 of 4 accepted" and an enabled Accept button before the second click. - run.ts: `set viewport` hangs on a fresh agent-browser daemon; open about:blank first. - e2e/INSIGHTS.md: record the pinning and the disabled-click gotcha. Verified locally: 13/13 flows pass from a fresh daemon (agent-browser 0.38.1). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`find ... click` does not wait for its target, and `wait --url` / `networkidle` do not guarantee the fetched list has rendered. In CI flow 13 clicked the PR row while the list still showed "Loading pull requests…". Add `wait --text <target>` before every click on fetched content: the PR row (04, 05, 13 x2) and the agent card (08, 11 x2). Static sidebar links are unchanged. Verified locally: 13/13 flows pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Flow 13 now fails in CI because the PR list stays on "Loading pull requests…" on the second visit — a server-side stall the failure screenshot cannot explain. Print the tails of api.log and web.log on failure and upload both files with the screenshots. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- API rate limit is now configurable (API_RATE_LIMIT_MAX, default 120 per IP per minute as
before). The e2e suite drives every flow from one loopback IP and, on a fast CI runner, hit the
cap: GET /repos/:id/pulls answered 429 ("retry in 33 seconds") and flow 13's PR list stayed on
"Loading pull requests…". e2e-web.yml and scripts/e2e.sh raise it to 10000 for e2e only.
- Review run: write the run_traces document BEFORE marking agent_runs done. Anything that polls
the status (the UI, reviews.it.test.ts via waitForPrRuns) could see `done` and read a trace
that did not exist yet — the intermittent "trace.config is undefined" failure in tests (postgres).
Verified locally: e2e 13/13; server typecheck; reviews/runs/skills-in-prompt/project-context-run
integration tests 27/27.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- AGENTS.md: "Harness evals — which one to run after a change" table (skill / agent / CLAUDE.md|AGENTS.md / engine / A/B measurement), mirroring the CI mapping. - eval:quality is green again: engineering-insights SKILL.md gets two section headings (text unchanged); the link checker ignores Markdown inside code spans/fences, so a documented template like `[<date> <label>](<file>)` in workflow-retro is no longer resolved as a path. - Workflow evals: a dedicated `dispatch` case for the architecture-reviewer subagent and a positive `activation` case for engineering-insights next to the existing near-miss negative; the multi-package trace now only asserts the nested INSIGHTS.md reads. Verified locally: eval:quality 0 failures; workflow evals 6/6. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In CI (Gemini) the positive engineering-insights case activated the skill, then kept executing it until maxTurns (4); the SDK's "Reached maximum number of turns" error failed the case before activation was checked. Positive activation cases now stop as soon as the skill engages (same as trace cases do); negative cases still run to the end to prove a non-activation. Verified locally: both activation cases pass (positive stops after 2 turns). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Export-to-CI feature: server `ci` module (manifest/workflow generation, export via PR or files, pull-based ingest), agent-runner package, CI Runs page and the agent CI tab with the Export Wizard. Security hardening: - generated workflow pins every action to a full commit SHA, checks out with persist-credentials: false and passes GITHUB_REPOSITORY_ID to the runner - user-edited workflows are validated (W1 triggers, W2 permissions, W4 pinned uses, W7 untrusted expressions in run) and rejected with 422 workflow_invalid - ingest caps artifact size (zip and inflated), requires a single devdigest-result.json, uses a strict artifact schema and cross-checks repository, repository id, head SHA, PR number and agent slug against GitHub's own run metadata - fix: tab=ci rendered ConfigTab and CiTab together Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…eement block) - server: new multi-agent module (POST /pulls/:id/multi-agent-run, GET /pulls/:id/multi-agent, GET /agents/run-estimates); reuses ReviewService.runReview per agent, reviews module unchanged - schema: agent_runs.multi_run_id + index (migration 0017) - contracts: extended MultiAgentRun/AgentColumn/Conflict, mirrored to client - client: PR-page checkbox picker, Configure run screen, results page (Columns/Tabs, disagreement block, trace drawer), sidebar entry - FindingCard and RunTraceDrawer promoted to client/src/components - tests: helpers unit, endpoint integration, results view component; e2e flow 14 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Brings in multi-agent review (agent_runs.multi_run_id, migration 0017). Conflicts resolved: - schema/runs.ts, modules/index.ts: kept both sides (CI columns + multiRunId). - migrations: lesson_07's 0017 kept; export-to-ci's CI migration regenerated with drizzle-kit as 0018 (SQL identical to its former 0017). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
writeFileAt split the path on '/', which never matches on Windows, so nested dirs were not created and 6 tests failed with ENOENT. Use dirname(). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Add DevDigest CI review
head.repo.fork == false is always false in a repo that is itself a GitHub fork, so the review job was skipped even for same-repo PRs. Compare head.repo.full_name to github.repository; PRs from other repos still get no secrets. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Add DevDigest CI review
Add DevDigest CI review
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
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 |
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
5 participants
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.
Multiagent
multiagents_compress_lv1.mp4
CI
pull_requestsfor them; the PR number is now confirmed against the PR's head SHA and commits