Skip to content

[DEV-1438] Wire CLI to session-scoped judge-facts endpoints#13

Merged
alexeyzimarev merged 2 commits intomainfrom
alexeyzimarev/dev-1438b-cli-repo-scoped-facts
Apr 13, 2026
Merged

[DEV-1438] Wire CLI to session-scoped judge-facts endpoints#13
alexeyzimarev merged 2 commits intomainfrom
alexeyzimarev/dev-1438b-cli-repo-scoped-facts

Conversation

@alexeyzimarev
Copy link
Copy Markdown
Member

Summary

Matches the server-side pivot in kurrent-io/Kurrent.Capacitor#476: judge facts are now scoped to the session's project (repo) and shared among all team members working on it, rather than siloed per-user.

Changes

  • Endpoint URLs:
    • `GET /api/judge-facts?category=X` → `GET /api/sessions/{id}/judge-facts?category=X`
    • `POST /api/judge-facts` → `POST /api/sessions/{id}/judge-facts`
  • `JudgeFactPayload` drops `source_session_id` (URL-bound now).
  • Prompt template rewritten to frame retained facts as project-level observations, with updated examples and explicit anti-examples:
    • ✅ "This repo's tests rely on Testcontainers, so missing Docker is a frequent failure mode"
    • ✅ "This codebase prefers handler-per-file over mega-handlers"
    • ❌ "Alice tends to force-push" (individual — no longer appropriate under repo scoping)
  • `FetchAllJudgeFactsAsync` takes `sessionId` so per-category GETs hit the correct session-scoped URL.

Sessions without a detected repo get empty fact lists from the server, so judges silently see "(no patterns retained yet)" — no special-casing on the CLI.

Depends on kurrent-io/Kurrent.Capacitor#476.

Test plan

  • `dotnet build src/kapacitor/kapacitor.csproj` — clean, 0 warnings
  • `dotnet publish -c Release` — zero IL3050/IL2026 warnings (AOT-clean)
  • Full unit suite — 205/205 pass
    • Existing `ParseVerdict`, `ExtractRetainFact`, `Aggregate`, `FormatKnownPatterns`, `BuildQuestionPrompt` tests are server-independent and didn't need changes
  • CI
  • End-to-end once #476 lands

🤖 Generated with Claude Code

Matches the server-side pivot in kurrent-io/Kurrent.Capacitor#476: judge
facts are now scoped to the session's **project (repo)** and shared among
all team members working on it, rather than siloed per-user.

Client-side changes:

- Endpoint URLs:
  GET /api/judge-facts?category=X                        -> GET /api/sessions/{id}/judge-facts?category=X
  POST /api/judge-facts                                  -> POST /api/sessions/{id}/judge-facts
- JudgeFactPayload drops source_session_id (URL-bound now).
- Prompt template rewritten to frame retained facts as *project-level*
  observations, with updated examples and an explicit anti-example
  ("Alice tends to force-push" — individual, not codebase-level).
- FetchAllJudgeFactsAsync takes sessionId so per-category GETs hit the
  correct session-scoped URL.

Sessions without a detected repo get empty fact lists from the server,
so judges silently see "(no patterns retained yet)" — no special-casing
needed on the CLI side.

Existing EvalCommand tests continue to pass without changes (ParseVerdict,
ExtractRetainFact, Aggregate, FormatKnownPatterns, BuildQuestionPrompt are
all server-independent). Full suite 205/205, AOT publish clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@linear
Copy link
Copy Markdown

linear bot commented Apr 13, 2026

@qodo-code-review
Copy link
Copy Markdown

Review Summary by Qodo

Wire CLI to session-scoped judge-facts endpoints

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Migrate judge-facts endpoints from user-scoped to session-scoped (repo-level)
• Update API URLs to include sessionId in endpoint paths
• Remove SourceSessionId from JudgeFactPayload (now URL-bound)
• Rewrite prompt template to emphasize project-level patterns over user-level
• Update examples to reflect codebase conventions and anti-examples for individual behavior
Diagram
flowchart LR
  A["User-scoped<br/>judge-facts"] -->|"Migrate to"| B["Session-scoped<br/>judge-facts"]
  B -->|"URL includes"| C["sessionId parameter"]
  D["JudgeFactPayload<br/>with SourceSessionId"] -->|"Remove field"| E["Payload without<br/>SourceSessionId"]
  F["User/individual<br/>patterns"] -->|"Reframe as"| G["Project/codebase<br/>patterns"]
Loading

Grey Divider

File Changes

1. src/kapacitor/Commands/EvalCommand.cs ✨ Enhancement +11/-10

Wire session-scoped judge-facts API endpoints

• Updated FetchAllJudgeFactsAsync to accept sessionId parameter and construct session-scoped
 endpoint URLs
• Modified PostJudgeFactAsync to use session-scoped endpoint and removed SourceSessionId from
 payload
• Updated comments to reflect DEV-1438 and explain session-scoped repo-level fact storage
• Changed method call to pass sessionId to both fetch and post operations

src/kapacitor/Commands/EvalCommand.cs


2. src/kapacitor/Models.cs ✨ Enhancement +6/-8

Remove SourceSessionId from JudgeFactPayload

• Removed SourceSessionId property from JudgeFactPayload record
• Updated XML documentation to explain session-scoped repo-level fact storage
• Clarified that facts are derived from session repo scope and shared across team members

src/kapacitor/Models.cs


3. src/kapacitor/Resources/prompt-eval-question.txt 📝 Documentation +8/-7

Reframe judge-facts prompt guidance to project-level scope

• Changed section header from "Known patterns" to "Known patterns for this project"
• Rewrote description to emphasize facts are project/codebase-level, not user-level
• Updated positive examples to focus on codebase conventions (Testcontainers, handler-per-file
 patterns, env vars)
• Added explicit anti-example showing individual behavior ("Alice tends to force-push") is no longer
 appropriate
• Clarified that facts are shared across all evaluators on the same repository

src/kapacitor/Resources/prompt-eval-question.txt


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown

qodo-code-review bot commented Apr 13, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Unescaped sessionId in GET🐞
Description
FetchAllJudgeFactsAsync interpolates the raw sessionId into the URL path for GET
/api/sessions/{id}/judge-facts, so session IDs containing reserved URL characters (e.g. '/') will
produce an invalid path and fail to load known patterns.
Code

src/kapacitor/Commands/EvalCommand.cs[254]

+                using var resp = await httpClient.GetWithRetryAsync($"{baseUrl}/api/sessions/{sessionId}/judge-facts?category={category}");
Evidence
The eval command takes a positional sessionId token verbatim (no escaping/validation) and uses it
directly inside the new session-scoped judge-facts path. If the token contains reserved path
characters, it will be interpreted as additional path segments by the HTTP client/server, breaking
fact retrieval for that sessionId.

src/kapacitor/Commands/EvalCommand.cs[249-256]
src/kapacitor/ArgParsing.cs[10-29]
src/kapacitor/Program.cs[120-137]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`FetchAllJudgeFactsAsync` builds the judge-facts URL via string interpolation: `/api/sessions/{sessionId}/judge-facts...`. Because `sessionId` comes from CLI args/env without validation, reserved URL characters (notably `/`) can corrupt the request path and cause judge-facts retrieval to fail.

## Issue Context
This PR introduced new session-scoped judge-facts endpoints, which adds new places where `sessionId` is embedded in the URL path.

## Fix Focus Areas
- src/kapacitor/Commands/EvalCommand.cs[249-268]

## Suggested fix
- Encode `sessionId` as a path segment, e.g.:
 - `var sid = Uri.EscapeDataString(sessionId);`
 - use `$"{baseUrl}/api/sessions/{sid}/judge-facts?category={Uri.EscapeDataString(category)}"`
- (Optional) centralize URL construction in a helper to avoid repeating escaping logic.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Review response on PR #13: FetchAllJudgeFactsAsync (and the other session-
scoped URLs in this file) interpolated sessionId directly into the path
without escaping. Most sessionIds are GUIDs, but meta-session slugs are
free-form and KAPACITOR_SESSION_ID could in principle carry anything, so
reserved URL characters would corrupt the path.

Encode once at the top of HandleEval and reuse the escaped form for the
four session-scoped URLs in EvalCommand: /eval-context (existing), /evals
(existing), /judge-facts GET, /judge-facts POST. The category query
parameter is also escaped for hygiene even though the canonical four
categories are safe ASCII.

Note: the same raw-interpolation pattern exists in other CLI commands
(RecapCommand, WhatsDoneCommand, etc.). Not fixing those here to keep the
PR focused; a follow-up could centralize URL construction.

Full suite 205/205, AOT publish clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@alexeyzimarev alexeyzimarev merged commit 1571222 into main Apr 13, 2026
2 checks passed
@alexeyzimarev alexeyzimarev deleted the alexeyzimarev/dev-1438b-cli-repo-scoped-facts branch April 13, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant