Pr Perfect

Ship to Boss

Create PRs that serve as briefing documents — detailed enough for both humans and coding agents to review, verify, and merge with confidence.

Arguments

The skill accepts free-form ARGUMENTS at invocation. Only one flag needs special handling:

Detection: treat the flag as present if any of its aliases appears as a whole token in ARGUMENTS (case-insensitive). When present, Step 4 is replaced by the Local-only variant below — everything else (Steps 1–3) runs identically.

Why This Format Matters

The PR description is not just documentation. The reviewer (and their coding agent) uses it to verify changes make sense, spot conflicts, and confirm nothing breaks. Every table, every file listing, every verification bullet exists so the reviewer can cross-reference the description against the actual diff. A vague PR description forces the reviewer to reverse-engineer intent from raw code — that's slow and error-prone.

Workflow

Execute these steps in order. Do not skip any step.

Step 1: Forensic Commit Analysis

Study every commit that will be in the PR. This is the foundation — a lazy read here produces a lazy description.

# What commits are we working with?
git fetch origin
git log origin/main..HEAD --oneline --reverse

# For EACH commit, read the full message and file list
git show <sha> --stat --format="%s%n%n%b"

# Total diff summary
git diff origin/main..HEAD --stat

For each commit, note:

• What problem it solves (not what it changes — WHY it changes)
• Which files it touches
• Whether it's a fix, feature, test, or refactor

Step 2: Group Into Themes

Commits rarely map 1:1 to PR sections. Look for natural groupings:

• Multiple commits touching the same subsystem = one section
• A fix + its test = one section
• An independent feature = its own section

Aim for 2-5 numbered sections. Each section needs a clear narrative arc: problem existed → here's what was broken → here's the fix → here's why it's safe.

Step 3: Write the PR Description

Read references/pr-examples.md for the exact style from the last merged PRs. Then write the description following this template structure:

Title Format

feat(scope): short theme 1, short theme 2, short theme 3

Keep under 70 characters. Use the primary scope (e.g., ai, telemetry, test). Comma-separate if multiple themes. Use conventional commit prefix (feat, fix, test, refactor).

Body Structure

## Summary

One paragraph (2-3 sentences) stating what this PR does at the highest level.
Lead with the impact, not the implementation. Mention how many themes/areas are covered.

### 1. Section Title — Descriptive Subtitle

Start with the PROBLEM. What was broken, missing, or wrong? Use **bold** to highlight
the failure mode or gap. Be specific — "the web-search toggle silently vanished" not
"there was a bug in web search."

**Fix:** Describe the solution. If it involves a new pattern or contract, use a table:

| Column1 | Column2 | Column3 |
|---|---|---|
| data | data | data |

### 2. Next Section Title

Same pattern: problem → fix → evidence it's safe.

(repeat for each theme)

## Changes by file

| Layer | File | Changes |
|---|---|---|
| Backend | `file.rs` | +N: brief description of what changed |
| Frontend | `Component.tsx` | +N/−M: brief description |

Every file in the diff MUST appear in this table. Group by layer (Backend, Frontend,
Agents, Config, Tests). Use the `+N` / `+N/−M` format to show line counts.

## Commits

sha1 type(scope): commit message 1 sha2 type(scope): commit message 2 ...

List ALL commits in chronological order (oldest first). Use short SHA (7 chars).

## Verification

Bullet list of how to verify this works. Be specific:
- "All 11 previously failing tests now pass" (not "tests pass")
- "New config fields backward compatible (`#[serde(default)]`)" (not "backward compatible")
- "Zero changes to SSE contract" (not "nothing else changed")

**N files changed, +X, −Y. Safe to squash merge.**

Step 4: Create Branch and PR (default)

Use this variant unless the user passed --no-branch (see Arguments section).

# Create feature branch from current HEAD
git checkout -b feature/<descriptive-name> HEAD

# Push to remote
git push -u origin feature/<descriptive-name>

# Write body to temp file first — avoids all shell quoting issues
# (single quotes, backticks, dollar signs in body content break heredoc-in-substitution)
cat > /tmp/pr_body.md <<'PREOF'
<body>
PREOF

# Create PR using body file — gh reads it directly, no shell parsing of content
gh pr create --base main --title "<title>" --body-file /tmp/pr_body.md
rm -f /tmp/pr_body.md

Branch naming: feature/<primary-theme> using kebab-case. Keep it short but descriptive.

After PR creation, switch back to main:

git checkout main

Report the PR URL to the user.

Step 4 (Local-only variant): --no-branch

Runs when ARGUMENTS contains --no-branch, --local-only, --no-pr, or --draft-only. Do NOT create a branch. Do NOT push. Do NOT call `gh pr create`. The user is working on main directly and only wants the engineered description on disk.

# Write the body to PR_DESCRIPTION.md at the repo root. Overwrites any prior
# draft. The title goes on the first line as an H1 so the file stands on its
# own if pasted into GitHub's web UI.
cat > PR_DESCRIPTION.md <<'PREOF'
# <title>

<body>
PREOF

Then tell the user exactly three things: 1. The full path of PR_DESCRIPTION.md. 2. That no branch was created, no push happened, and no PR was opened. 3. How to turn it into a PR later if they want — typically: gh pr create --base main --head <their-branch> --title "<title>" --body-file PR_DESCRIPTION.md

Do NOT commit PR_DESCRIPTION.md. It is a scratch artifact for the user — add it to .gitignore only if the user asks. Leave git status dirty so they see it.

Style Rules

These rules are non-negotiable — they're what makes this team's PRs consistent.

1. German-enterprise tone: Technical, precise, no fluff. Tables over paragraphs when data is structured. Numbers over adjectives ("11 failures" not "several issues").

2. Problem-first sections: Every section starts with what was wrong. The reader should understand the pain before seeing the fix.

3. Bold failure modes: Use **bold** for the specific thing that broke or was missing. "The toggle silently vanished" — the bold text is what the reviewer's eye catches.

4. Pipe tables for structured data: Methods, tools, safety systems, file changes — anything with 2+ columns gets a markdown table.

5. File-by-file accountability: The "Changes by file" table is mandatory. Every file in the diff must be listed. This is what the reviewer's agent uses to cross-check.

6. Concrete verification: Each bullet in Verification must be falsifiable. The reviewer should be able to check each one by running a command or reading the diff.

7. Footer: Always end with the line count summary and "Safe to squash merge." (or explain why not, if applicable).

8. No Co-Authored-By: Never add co-author lines to commits or PR descriptions.

Edge Cases

• Single commit PR: Still use the full template. One numbered section is fine.
• Only test changes: Lead with what was broken in the test suite, not "added tests."
• Mixed fix + feature: Feature sections before fix sections (features are the headline).
• Large PRs (20+ files): Group the file table by subsystem, add a "Total" row.

PR Examples — Team Style Reference

---

PR #7

Title: feat(telemetry): complete observability stack — correlation IDs, Langfuse tool tracing, config seeding

Summary

Production-critical completion of the 3-system observability stack (correlation IDs, Langfuse LLM tracing, Langfuse infrastructure). The original implementation had working foundations but two integration gaps that left agent→backend correlation and tool-level Langfuse tracing inoperative in production. This PR closes both gaps across all 3 Go agents and the Rust backend, and seeds Langfuse configuration defaults for zero-guesswork admin setup.

Correlation ID chain — agent operations now traced end-to-end

The Go agents had correlation ID generation and HTTP header forwarding code, but every context.Background() created bare contexts — the X-Correlation-ID header was always empty, making the entire propagation pipeline dead code. All 3 agents now generate fresh UUID v4 correlation IDs at every operation boundary:

• ADFS Agent: 11 operation contexts replaced — config fetch, heartbeat, alert checks, rescan triggers, reconnect loops, event flushes, startup checks
• Scan Agent: heartbeat ticks, poll/dispatch cycles, and WebSocket connections now carry unique IDs per operation
• Updater Agent: heartbeat and WebSocket contexts instrumented
• New ContextWithTimeout() / ContextWithCancel() helpers in all 3 telemetry packages — clean single-call API instead of verbose 3-line context composition
• 6 new unit tests (2 per agent) verifying UUID generation, deadline propagation, and cancellation

Langfuse tool call tracing — every AI action now visible

ToolCallTrace was built and fully tested in llm_trace.rs but never imported or wired into the AI agent loop (ai_chat.rs). Langfuse showed LLM calls (prompt/response/tokens) but zero tool execution spans — meaning "which Cypher query ran and did it succeed" was invisible in the tracing dashboard. Now all 10 tool types emit Langfuse spans nested under each session trace:

Langfuse configuration seeding

Added INSERT OR IGNORE defaults for langfuse_enabled, langfuse_host, langfuse_public_key, langfuse_secret_key in config_db_schema.sql — admins can discover required keys without raw SQL guesswork. Idempotent on every startup, disabled by default.

Changes by layer

Go Agents (ADFS, Scan, Updater)

Backend (Rust)

Verification

• All 27 Go telemetry tests pass (9 per agent, including 6 new)
• All 3 Go agents compile clean (go build ./...)
• Rust backend compiles clean (cargo check)
• Zero behavioral changes — all additive instrumentation
• Langfuse remains disabled by default until admin configures credentials

Merge notes

• No conflicts expected — changes are isolated to telemetry infrastructure and the tool-call match block in ai_chat.rs
• No deployment steps required — Langfuse stays disabled until admin runs infrastructure/langfuse/setup.ps1 and sets credentials
• Safe to squash merge — all changes are additive, no destructive refactors

---

PR #8

Title: feat(ai): agent loop safety rails, orchestrator prompt fix, Langfuse exit tracking

Summary

Production-critical hardening of the AI agent loop and prompt architecture. Three interconnected improvements that make the agent safe, observable, and correctly configured.

1. Orchestrator Prompt — Single Source of Truth

The orchestrator prompt (the short behavioral directive at the end of the system prompt) was defined in 3 places — i18n hardcode, config DB, and prompts DB — but the chat UI only used the i18n one, making the admin config page's setting completely inert. The per-message textarea below the chat input kept resetting on every new tab and could never be permanently cleared (backend had a hardcoded fallback).

Fix: Removed the chat-level textarea entirely. The backend now reads the orchestrator prompt server-side with a clean priority chain: per-request override → admin config DB (ai_orchestrator_prompt) → seeded prompts DB default. The admin config page is now the single source of truth. Also fixed the format suffix (HTML/PDF/web-search instructions) being duplicated in both the user message AND the orchestrator prompt — it now only appears in the user message.

2. Agent Loop Safety Rails — 7 Systems

The agent loop (run_agent_loop in ai_chat.rs) had no context management, no budget enforcement, no timeout, and no circuit breaker. Complex queries could run for 50+ minutes with unbounded token consumption.

All systems are purely additive — the agent always works, tools/prompts/SSE contract unchanged. The only difference is optimization and safety.

3. Langfuse Exit Tracking

The Langfuse trace output was missing the status_label, so the dashboard couldn't distinguish why an agent run ended. Fixed:

• Added status_label and exit_reason to trace output JSON
• Added agent_completion_quality score: 1.0 for normal completion, 0.5 for safety-rail exits (timeout, token budget, max iterations)
• Bug fix: choice variable needed mut for reasoning block sanitization to compile

Changes by file

New Admin Config Controls

Under "Agent-Limits" section in AI Config page:

• Agent-Timeout (Sekunden): Default 600s (10 min), range 30-3600
• Max Tokens pro Lauf: Default unlimited, range 50K-2M
• Fehler-Abbruchgrenze: Default 5, range 2-20

Verification

• cargo check — clean
• npx tsc --noEmit — clean
• All new fields backward compatible (#[serde(default)], all default to 0 = "use code default")
• SSE contract unchanged — existing frontends work without update
• Langfuse remains disabled by default until admin configures credentials

Commits

bf1f3ca fix(ai): fix reasoning sanitization mutability and add Langfuse exit tracking
b1405b6 feat(ai): add 7 agent loop safety rails
b71738c fix(ai): single source of truth for orchestrator prompt

Safe to squash merge.

---

PR #10

Title: feat(ai): web search config fix, test infrastructure sweep, Langfuse tracing upgrade

Summary

Three layers of production hardening: a user-facing web search config bug that silently hid features, a test infrastructure sweep that resolves all 11 failing tests, and a Langfuse tracing upgrade that adds full API field coverage with observation nesting.

1. Web Search Config — is_enabled vs is_ready

Selecting "Bing Search API" as the web search provider caused the web-search toggle to silently vanish from the chat UI. Root cause: WebSearchConfig::is_enabled() conflated admin intent (show the feature) with provider readiness (API key configured). When the Bing API key was missing, is_enabled() returned false, hiding the entire feature instead of showing a "not configured" warning.

Fix: Split into a 3-method contract:

The frontend now shows the web-search option with an amber warning state when enabled but not ready, with inline config error text and provider-specific setup hints (e.g. Credential Manager key name for Bing Search API). bulk.rs sends aiWebSearchAvailable, aiWebSearchReady, and aiWebSearchConfigError to the frontend.

2. Test Infrastructure — All 11 Failures Resolved

The test suite had accumulated 11 failures across 8 files from API signature changes, role model updates, and missing service registrations.

API contract drift (6 files):

• contract_tests.rs: enrollment token API changed to consume_enrollment_token(id, agent_id, ip) — test still used old signature
• credentials.rs, workers.rs, system_architecture.rs: Role::Viewer removed/renamed to Role::Requester — 7 permission-denied tests referenced a non-existent role
• deep_report.rs, fs_cleanup.rs: {guid} in format strings triggered compiler warnings (needed {{guid}})

Infrastructure gaps (8 files):

• TestApp missing WorkerRegistry and WsConnectionManager — handlers crashed before permission checks could run (4 tests)
• global_search, temporal, system_architecture: required Neo4jPool even for permission-check tests — changed to Option<Neo4jPool> (5 tests)
• provisioning: test referenced non-existent /templates/user route
• credentials: test was environment-specific (domain-joined machines returned different credential names)
• scheduler: cron day-of-week convention wrong (1=Sun not 1=Mon)
• config_db: fresh-table assertion failed because AI safety rail defaults now seed rows at startup

3. Safety Rail Test Coverage

Unit tests for the compaction and config systems added in PR #8:

4. Langfuse Tracing Upgrade — Full API Coverage + Nesting

The Langfuse client was sending minimal trace/generation/span events, missing most of the API's fields. Upgraded to full coverage:

New fields: tags, release, version, public, environment on traces. parent_observation_id, prompt_name, prompt_version on generations and spans. Cost override fields on usage. New EventEvent struct for discrete point-in-time observations.

Observation nesting: Tool call spans are now nested under their parent LLM generation via parent_observation_id — the Langfuse dashboard shows the full call tree instead of flat spans.

Wiring in `ai_chat.rs`: Traces tagged with context/mode, release from build consts, environment from langfuse_environment config key. LLM messages passed as generation input for full prompt visibility in the dashboard.

Changes by file

Commits

b9e07d6 fix(ai): split WebSearchConfig::is_enabled into is_enabled/is_ready/config_error
caa8ec1 feat(ui): show web-search with warning state when provider not ready
5eeac14 test(ai): update web search tests for is_enabled/is_ready contract
2b9f032 fix(tests): align test assertions with updated API signatures and role model
abef0a8 test(ai): add unit tests for agent loop safety rails and config fields
6fb2620 fix(test): resolve all 11 test failures with infrastructure fixes
036e39b feat(telemetry): upgrade Langfuse tracing with full API fields, nesting, and event support

Verification

• All 11 previously failing tests now pass
• Safety rail compaction + config roundtrip tests pass
• Web search warning state renders correctly in OutputFormatPanel and WebSearchPopup
• Langfuse events include full field set when tracing is enabled
• All new fields backward compatible (#[serde(default)])
• Zero changes to SSE contract, tool definitions, or prompt system

27 files changed, +1441, −116. Safe to squash merge.