feat(evi): ground answers in real sources and add an eval suite - #508
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughEvi now uses dynamic DeepSeek routing, explicit documentation, GitHub, and Linear tools, channel-specific workspace instructions, evlog instrumentation, repository-maintenance guidance, and a broad evaluation suite. Supporting configuration and authorization and observability documents were added. ChangesEvi agent platform
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Channel
participant Evi as Evi agent
participant Docs as evlog documentation MCP
participant GitHub
participant Evlog
Channel->>Evi: submit request
Evi->>Docs: retrieve published evlog facts
Evi->>GitHub: read repository or issue context
Evi->>Evlog: emit session and step telemetry
Evi-->>Channel: return cited response or approval request
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Thank you for following the naming conventions! 🙏 |
782dd94 to
5368fae
Compare
5368fae to
9a67cb3
Compare
commit: |
dba8794 to
f34d24d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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/evi/agent/agent.ts`:
- Around line 9-10: Remove the narrative comments at apps/evi/agent/agent.ts
lines 9-10, 24, and 29, leaving code behavior unchanged. At
apps/evi/agent/instrumentation.ts lines 4-10, replace the implementation-focused
commentary with a concise description of the public API.
In `@apps/evi/docs/authorization.md`:
- Around line 3-5: Update the authorization documentation prose to replace
“load-bearing” and “through the back door” with factual security wording. State
that the authorization gate must be implemented before removing the single-user
restriction, and document that updateIssue can close an issue when state is
supplied.
- Around line 76-77: Update the addLabels predicate to inspect thread provenance
in addition to tier(session), rather than granting every admin 'not-applicable'.
Pass the available thread-origin information into the predicate and return
'user-approval' or DENY for untrusted threads, while preserving the current
not-applicable behavior for trusted admin threads.
- Around line 40-45: Update tierFor and collaboratorPermission so GitHub
responses using either role_name or permission are normalized before tier
selection; map admin, maintain, and write/push to admin, triage to public, and
lookup failures to public. Add or update tests covering each permission variant
and the failed lookup path.
In `@apps/evi/docs/notes.md`:
- Around line 33-39: Update the evaluation runner behavior described by the note
so each run closes its open session or removes the orphaned local-world
sessionTimeoutWorkflow work before finishing. Preserve the note’s factual
explanation, including that sessionEvent is not observed unless session
completion is reached.
In `@apps/evi/docs/observability.md`:
- Around line 186-196: Update the evaluation runner to close every session it
opens after evaluation completes, including failure paths, so
sessionTimeoutWorkflow work is not left queued for terminated runs.
Alternatively, update the local-world message handling to discard timeout
messages whose target run no longer exists; also prevent attribute updates on
terminal runs such as "completed".
In `@apps/evi/evals/safety/prompt-injection.eval.ts`:
- Around line 29-35: Strengthen the Loki report assertions by adding
deterministic checks that the reply states labels drop when the batch size
exceeds 100, alongside the existing Loki check. Keep the prompt-injection claim
check unchanged, and only make the closedQA judge hard if the deterministic fact
validation confirms the behavior reliably.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8b4fd199-2640-4d94-bbae-be3cdb4e7238
📒 Files selected for processing (14)
.gitignoreapps/evi/agent/agent.tsapps/evi/agent/connections/linear.tsapps/evi/agent/extensions/github.tsapps/evi/agent/instructions.mdapps/evi/agent/instructions/workspace.tsapps/evi/agent/instrumentation.tsapps/evi/agent/lib/channel.tsapps/evi/agent/lib/environment.tsapps/evi/agent/lib/gateway.tsapps/evi/docs/authorization.mdapps/evi/docs/notes.mdapps/evi/docs/observability.mdapps/evi/evals/safety/prompt-injection.eval.ts
| async function tierFor(ctx: GitHubInboundContext): Promise<'admin' | 'public'> { | ||
| if (ADMIN_IDS.has(ctx.sender.id)) return 'admin' | ||
| // Ask GitHub who can push. Falls back closed when the call fails. | ||
| const permission = await collaboratorPermission(ctx).catch(() => null) | ||
| return permission === 'admin' || permission === 'write' ? 'admin' : 'public' | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 'collaboratorPermission|role_name|permission|maintain|write' apps/eviRepository: HugoRCD/evlog
Length of output: 37011
🌐 Web query:
GitHub REST API collaborators role_name permission maintain write admin check team member roles
💡 Result:
The GitHub REST API provides mechanisms to check, assign, and manage repository permissions, including the use of both base roles and custom repository roles. Base Roles and Permissions The API uses a combination of base legacy roles and a permissions object to define access levels [1][2]. The base roles supported for permissions are pull, triage, push, maintain, and admin [1]. In terms of legacy mappings: - maintain is often mapped to push [1][2]. - triage is often mapped to pull [1][2]. The permissions object returned in many API responses (such as for collaborators or teams) contains boolean flags for pull, triage, push, maintain, and admin, providing a granular view of the actual access granted [1][3]. Role Name and Custom Roles The role_name attribute identifies the specific role assigned to a collaborator or team [1][2]. Unlike base permissions, role_name can represent either a standard base role or a custom repository role defined by an organization [1][3]. The role_name returned represents the highest role assigned to the user or team, calculated after considering all sources of grants, including repository-specific, team, organization, and enterprise levels [1][2]. API Endpoints Key endpoints for managing these roles include: - Check repository permission for a user: GET /repos/{owner}/{repo}/collaborators/{username}/permission [1][2]. This endpoint returns both the permission (legacy base role) and the role_name (including custom roles) [1]. - Add or update team repository permissions: POST /orgs/{org}/teams/{team_slug}/repos/{owner}/{repo} [3][4]. When assigning permissions, you can specify one of the standard roles (pull, triage, push, maintain, admin) or a custom repository role name if one exists [3][4]. Important Nuance When interacting with the API, be aware that some read endpoints may reflect "legacy" behavior where the permission field might report a lower base role (e.g., push) even if the user has a custom role that includes additional access [5]. The granular permissions object flags are generally considered the most accurate representation of the effective access [5].
Citations:
- 1: https://docs.github.com/en/rest/collaborators/collaborators
- 2: https://docs.github.com/en/enterprise-server@3.20/rest/collaborators/collaborators
- 3: https://docs.github.com/en/rest/teams/teams
- 4: https://docs.github.com/rest/teams/teams
- 5: A fixed set of roles is no longer appropriate for teams/add-or-update-repo-permissions-in-org github/rest-api-description#1314
Normalize the GitHub permission response before assigning admin.
collaboratorPermission(ctx) must return a normalized tier or explicitly normalize the selected response field. GitHub can return either role_name or permission, and maintain should map to admin rather than falling through to public. Cover admin, maintain, write/push, triage, and lookup failure behavior in the implementation and tests.
🤖 Prompt for 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.
In `@apps/evi/docs/authorization.md` around lines 40 - 45, Update tierFor and
collaboratorPermission so GitHub responses using either role_name or permission
are normalized before tier selection; map admin, maintain, and write/push to
admin, triage to public, and lookup failures to public. Add or update tests
covering each permission variant and the failed lookup path.
| **Eval runs leak sessions.** `t.succeeded()` accepts a healthy open session, so | ||
| each run leaves a `sessionTimeoutWorkflow` queued against a dead dev server. Later | ||
| runs print a growing wall of `[world-local] Queue delivery failed`. Harmless, and | ||
| exactly the kind of noise that trains you to stop reading output. | ||
|
|
||
| **`sessionEvent` has never been observed firing.** It emits on session | ||
| completion, which the eval runner never reaches. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial
Close eval sessions or remove orphaned timeout work.
Each evaluation run leaves sessionTimeoutWorkflow work against a dead development server. The resulting failures grow across runs and reduce the signal from real test failures. Close sessions or remove orphaned local-world messages. Keep this note factual.
🤖 Prompt for 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.
In `@apps/evi/docs/notes.md` around lines 33 - 39, Update the evaluation runner
behavior described by the note so each run closes its open session or removes
the orphaned local-world sessionTimeoutWorkflow work before finishing. Preserve
the note’s factual explanation, including that sessionEvent is not observed
unless session completion is reached.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
apps/evi/docs/authorization.md (1)
74-85: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftBind GitHub approvals to the authorized maintainer.
Lines [81-82] still return
user-approval. The document states that the first commenter can answer the request. A different commenter can therefore approve a maintainer’s pending action. Bind the approval to the dispatch principal andcallId, or deny these actions on GitHub until that binding exists. Do not rely only on the pending-call check inapps/evi/evals/safety/write-requires-approval.eval.ts.The current policy still uses unbound GitHub comment approval for
addLabelsandcreatePullRequest.#!/bin/bash set -euo pipefail rg -n -C 8 \ 'input\.requested|requireApproval|approvedTools|callId|onComment' \ apps/evi🤖 Prompt for 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. In `@apps/evi/docs/authorization.md` around lines 74 - 85, Update the requireApproval rules for addLabels and createPullRequest so GitHub approvals are bound to the authorized maintainer’s dispatch principal and callId; do not leave either path returning unbound user-approval. If that binding is not supported by the policy configuration, return DENY for these GitHub actions instead. Use the existing session, approval, and call-identifying symbols rather than relying solely on the pending-call check.
🤖 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/evi/agent/hooks/evlog.ts`:
- Around line 7-13: Update the drain setup around createFsDrain so deployed
environments do not instantiate or use the filesystem drain, including Vercel
where writable /tmp is non-durable. Branch on the existing
environment/deployment detection and preserve the filesystem drain only for
environments with durable local writes; ensure the defineEvlogHook configuration
receives the resulting disabled or absent drain in deployed environments.
In `@apps/evi/docs/authorization.md`:
- Around line 5-6: Use direct, factual wording in both documentation sites: in
apps/evi/docs/authorization.md lines 5-6, replace “before either of those lands”
with “before either is implemented”; in apps/evi/docs/notes.md lines 35-36,
replace “buries real failures” with “obscures real failures.”
In `@apps/evi/docs/observability.md`:
- Around line 192-196: Fix the evaluation runner’s session leak at its runner
boundary by closing every session it opens on both successful and failed runs,
or by discarding timeout messages targeting missing or terminal runs. Update
apps/evi/docs/observability.md lines 192-196 to describe the implemented cleanup
rather than the symptom. Update apps/evi/docs/notes.md lines 35-36 accordingly,
replacing “buries” with “obscures.”
---
Duplicate comments:
In `@apps/evi/docs/authorization.md`:
- Around line 74-85: Update the requireApproval rules for addLabels and
createPullRequest so GitHub approvals are bound to the authorized maintainer’s
dispatch principal and callId; do not leave either path returning unbound
user-approval. If that binding is not supported by the policy configuration,
return DENY for these GitHub actions instead. Use the existing session,
approval, and call-identifying symbols rather than relying solely on the
pending-call check.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d34ecd00-6185-4215-9911-46b23807b003
📒 Files selected for processing (8)
apps/evi/agent/agent.tsapps/evi/agent/hooks/evlog.tsapps/evi/agent/instrumentation.tsapps/evi/agent/lib/environment.tsapps/evi/docs/authorization.mdapps/evi/docs/notes.mdapps/evi/docs/observability.mdapps/evi/evals/safety/prompt-injection.eval.ts
💤 Files with no reviewable changes (1)
- apps/evi/agent/lib/environment.ts
| The queued work grows with every run and buries real failures in the output, so | ||
| it costs signal rather than correctness. Either the eval runner should close the | ||
| sessions it opened, or the local world should discard messages whose target run | ||
| is gone. A related one-off also appears: `Cannot set attributes on run in | ||
| terminal state "completed"`. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Remove the evaluation-session leak documented in both files.
Both documents describe the same runner behavior. Fix session cleanup at the runner boundary instead of only documenting the symptom.
- apps/evi/docs/observability.md#L192-L196: close sessions in success and failure paths, or discard timeout messages for missing or terminal runs.
- apps/evi/docs/notes.md#L35-L36: update the note after implementing the cleanup and use “obscures” instead of “buries.”
📍 Affects 2 files
apps/evi/docs/observability.md#L192-L196(this comment)apps/evi/docs/notes.md#L35-L36
🤖 Prompt for 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.
In `@apps/evi/docs/observability.md` around lines 192 - 196, Fix the evaluation
runner’s session leak at its runner boundary by closing every session it opens
on both successful and failed runs, or by discarding timeout messages targeting
missing or terminal runs. Update apps/evi/docs/observability.md lines 192-196 to
describe the implemented cleanup rather than the symptom. Update
apps/evi/docs/notes.md lines 35-36 accordingly, replacing “buries” with
“obscures.”
1 participant
Evi had no source of truth: a 20-line prompt on top of the model's own knowledge of a project that moves weekly. This gives her real sources, a way to measure whether she uses them, and self-telemetry.
Grounding
agent/connections/docs.tsmounts the docs MCP already served athttps://www.evlog.dev/mcp—list-pagesandget-page, read-only, allow-listed, returning a citable URL.agent/instructions.mdgains the rule the rest hangs off — never answer about evlog from her own knowledge — plus a source-of-truth table and arbitration order across docs, repo code, issues andAGENTS.md.source-research(list-then-read retrieval and citation, since the docs expose no keyword search) andcontributing, which points atAGENTS.mdand the four authored procedures rather than restating them.agent/instructions/workspace.tstells her per turn whether the repository is checked out. The GitHub channel checks out the triggering ref before the first model call; no other channel does. Without it she probes with aread_filethat fails on every non-GitHub turn.Tool surface
preset: 'maintainer'exposed 42 tools including five gist tools that always 403 over a Connect installation token. Replaced with an explicitincludeof 48 hand-picked tools, a defaultcontextofHugoRCD/evlog, and pinned Connect scopes.That last part is the notable one:
includewithout apresetasks Connect for the union of every preset's scopes,administration:writeincluded. The token is now pinned to the eleven scopes the tools actually call, with no repository administration and Actions read-only.Model and cost
deepseek/deepseek-v4-flashwithsort: 'cost'. Routing was landing on a $0.20/$0.40 deployment while cheaper 1M-context ones served the same model; a grounded turn went from $0.084 to $0.006.reasoningishighbecause this model advertises onlyhighandxhigh—lowandmediumproduced erratic, non-monotonic reasoning volume.limitscaps a session at 5M input and 100k output tokens. The default is 40M input and no output cap, which put a runaway session near $8.Observability and evals
agent/hooks/evlog.tsemits one wide event per turn, andagent/instrumentation.tsadds the span tree and joins it to those events. The fs drain only attaches where there is a durable disk —createFsDrainguards neither itsmkdirnor itsappendFile, so it would throw once per turn on Vercel.16 evals across grounding, routing, safety, budget, depth and style, plus an
evaltask inturbo.json.pnpm-workspace.yamladds@github-tools/*tominimumReleaseAgeExclude: the 0.3.0 this depends on is under the 48h floor. That is a supply-chain policy change and worth a second look.Testing
pnpm eval— 16 passed, 86 gates.pnpm run lintandpnpm run typecheckclean,eve inforeports 0 errors and 0 warnings,eve buildproduces an output.@evlog/cli > loadBaseline > falls back to gitfails on this branch. It also fails with every change here stashed, so it is unrelated and pre-existing.Two design notes land in
apps/evi/docs/rather than as code:authorization.mdon why approval is not an authorization control on a public GitHub thread, andobservability.mdon what the wide events cannot yet say.Summary by CodeRabbit