feat(assistant): add context build seam#31
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR introduces a context build lifecycle pipeline that enables extensions to contribute bounded system context during prompt generation. It adds orchestration and types to assemble base prompts with auto-activated skills, dispatches a ChangesContext Build Lifecycle & Extension Contributions
Sequence DiagramsequenceDiagram
participant Client
participant Runtime
participant ContextBuild as buildModelContext
participant ContextBase as modelContextBase
participant Lifecycle
participant Extensions
participant Provider
Client->>Runtime: Prompt()
Runtime->>ContextBuild: buildModelContext()
ContextBuild->>ContextBase: build base system prompt & estimate tokens
ContextBuild->>Lifecycle: dispatchContextBuild(base, result)
Lifecycle->>Extensions: Emit context_build lifecycle event
Extensions-->>Lifecycle: Return contributions
ContextBuild->>ContextBuild: Parse/validate & append contributions
ContextBuild-->>Runtime: contextBuildResult (prompt, usage, breakdown)
Runtime->>Provider: make completion request
Provider-->>Runtime: completion response
Runtime-->>Client: assistant response
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
==========================================
+ Coverage 58.13% 58.44% +0.30%
==========================================
Files 165 166 +1
Lines 15973 16160 +187
==========================================
+ Hits 9286 9444 +158
- Misses 5719 5742 +23
- Partials 968 974 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
internal/assistant/lifecycle.go (1)
81-89: ⚡ Quick winAvoid double-logging lifecycle dispatch failures.
dispatchLifecyclealready logs the error, so every observational failure gets logged twice once there and once again indispatchObservationalLifecycle.🪵 Suggested cleanup
- if err != nil && runtime.logger != nil { - runtime.logger.Debug( - "extension lifecycle event failed", - slog.String("event", string(name)), - slog.Any("error", err), - ) - } - return result, errAlso applies to: 139-150
🤖 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 `@internal/assistant/lifecycle.go` around lines 81 - 89, dispatchObservationalLifecycle is double-logging lifecycle errors because dispatchLifecycle already logs failures; remove the extra runtime.logger.Debug/Info/Error call in dispatchObservationalLifecycle (the block that logs "extension lifecycle event failed" with slog.String("event", string(name)) and slog.Any("error", err)) and rely on dispatchLifecycle's logging instead. Apply the same cleanup to the analogous block at the other location noted (lines where a similar log appears for the other lifecycle dispatch), referencing the functions dispatchLifecycle and dispatchObservationalLifecycle and the runtime.logger usage to locate the duplicated logging.
🤖 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 `@internal/assistant/context_build.go`:
- Around line 205-233: The two fmt.Fprintf calls in appendContextContributions
(writing attributes and the source/role/tokens block into the strings.Builder)
currently ignore errors by assigning them to blank identifiers; update the
function to properly handle these returns: either switch to using
builder.WriteString/strconv to avoid fmt errors, or change
appendContextContributions to return error and capture the err from each
fmt.Fprintf (remove the underscore assignments), returning the first non-nil
error; also update callers to handle the new error return if you choose the
latter. Ensure you reference appendContextContributions, contextBuildResult,
contextContribution, and result.SystemPrompt when making the change so the
builder writes and error handling are applied to those exact spots.
---
Nitpick comments:
In `@internal/assistant/lifecycle.go`:
- Around line 81-89: dispatchObservationalLifecycle is double-logging lifecycle
errors because dispatchLifecycle already logs failures; remove the extra
runtime.logger.Debug/Info/Error call in dispatchObservationalLifecycle (the
block that logs "extension lifecycle event failed" with slog.String("event",
string(name)) and slog.Any("error", err)) and rely on dispatchLifecycle's
logging instead. Apply the same cleanup to the analogous block at the other
location noted (lines where a similar log appears for the other lifecycle
dispatch), referencing the functions dispatchLifecycle and
dispatchObservationalLifecycle and the runtime.logger usage to locate the
duplicated logging.
🪄 Autofix (Beta)
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: CHILL
Plan: Pro
Run ID: b906fc09-01b8-4f7e-88a7-c880063db253
📒 Files selected for processing (7)
docs/extension-api.mdinternal/assistant/context_build.gointernal/assistant/context_build_test.gointernal/assistant/lifecycle.gointernal/assistant/runtime.gointernal/assistant/runtime_events.gointernal/assistant/usage.go
💤 Files with no reviewable changes (1)
- internal/assistant/usage.go
|



Summary
Validation