diff --git a/README.md b/README.md index 9a1f2b9c..bd65dfe8 100644 --- a/README.md +++ b/README.md @@ -89,7 +89,7 @@ go run ./cmd/neocode 核心模块职责: - **`internal/config`** — 配置管理、环境变量、YAML 加载 -- **`internal/context`** — system prompt、消息裁剪与上下文构建 +- **`internal/context`** — 多 section system prompt 组装、消息裁剪与上下文构建 - **`internal/provider`** — LLM 提供商抽象,抹平厂商差异 - **`internal/runtime`** — ReAct 主循环、事件流、会话管理 - **`internal/tools`** — 工具注册表与具体工具实现 @@ -107,7 +107,7 @@ go run ./cmd/neocode ├── internal │ ├── app # 应用装配 │ ├── config # 配置管理 -│ ├── context # 上下文构建 +│ ├── context # system prompt 组装与上下文构建 │ ├── provider # Provider 抽象与实现 │ ├── runtime # ReAct 循环与事件流 │ ├── tools # 工具系统 diff --git a/docs/runtime-provider-event-flow.md b/docs/runtime-provider-event-flow.md index c39c740a..e9e007e2 100644 --- a/docs/runtime-provider-event-flow.md +++ b/docs/runtime-provider-event-flow.md @@ -31,7 +31,7 @@ - 当前 `provider` - 当前 `model` - `context.Builder` 负责统一组装: - - 固定核心 system prompt + - 固定核心 system prompt sections - 从 `workdir` 向上发现的 `AGENTS.md` - 系统状态摘要(`workdir` / `shell` / `provider` / `model` / git branch / git dirty) - 裁剪后的历史消息 @@ -42,7 +42,7 @@ 当前 `system prompt` 按以下顺序拼装: -1. 固定核心指令 +1. 固定核心 sections 2. `Project Rules` section 3. `System State` section @@ -51,6 +51,7 @@ - 规则文件只支持大写文件名 `AGENTS.md` - 多份命中结果按“从全局到局部”的顺序注入 - git 只注入摘要,不注入完整 `git status` +- 各 section 统一由 `internal/context` 内部的 `renderPromptSection` 和 `composeSystemPrompt` 渲染,`runtime` 仍只消费最终字符串 ## 流式桥接 diff --git a/internal/context/builder.go b/internal/context/builder.go index bc57052b..abd9523a 100644 --- a/internal/context/builder.go +++ b/internal/context/builder.go @@ -30,12 +30,12 @@ func (b *DefaultBuilder) Build(ctx context.Context, input BuildInput) (BuildResu return BuildResult{}, err } + sections := append([]promptSection{}, defaultSystemPromptSections()...) + sections = append(sections, renderProjectRulesSection(rules)) + sections = append(sections, renderSystemStateSection(systemState)) + return BuildResult{ - SystemPrompt: composeSystemPrompt( - defaultSystemPrompt(), - renderProjectRulesSection(rules), - renderSystemStateSection(systemState), - ), - Messages: trimMessages(input.Messages), + SystemPrompt: composeSystemPrompt(sections...), + Messages: trimMessages(input.Messages), }, nil } diff --git a/internal/context/builder_test.go b/internal/context/builder_test.go index 4ca49c05..f01a84a7 100644 --- a/internal/context/builder_test.go +++ b/internal/context/builder_test.go @@ -3,6 +3,8 @@ package context import ( stdcontext "context" "fmt" + "os" + "path/filepath" "strings" "testing" @@ -27,8 +29,8 @@ func TestDefaultBuilderBuild(t *testing.T) { if got.SystemPrompt == "" { t.Fatalf("expected non-empty system prompt") } - if !strings.Contains(got.SystemPrompt, defaultSystemPrompt()) { - t.Fatalf("expected default prompt to remain in composed prompt") + if !strings.Contains(got.SystemPrompt, "## Agent Identity") { + t.Fatalf("expected core prompt sections to be included") } if !strings.Contains(got.SystemPrompt, "## System State") { t.Fatalf("expected system state section in composed prompt") @@ -36,6 +38,9 @@ func TestDefaultBuilderBuild(t *testing.T) { if strings.Contains(got.SystemPrompt, "## Project Rules") { t.Fatalf("did not expect project rules section without AGENTS.md") } + if strings.Contains(got.SystemPrompt, "\n\n\n") { + t.Fatalf("did not expect repeated blank lines in composed prompt") + } if !strings.Contains(got.SystemPrompt, input.Metadata.Workdir) { t.Fatalf("expected workdir in system state section") } @@ -60,6 +65,34 @@ func TestDefaultBuilderBuildHonorsCancellation(t *testing.T) { } } +func TestDefaultBuilderBuildComposesPromptSectionsInOrder(t *testing.T) { + t.Parallel() + + root := t.TempDir() + if err := os.WriteFile(filepath.Join(root, ruleFileName), []byte("project-rules"), 0o644); err != nil { + t.Fatalf("write AGENTS.md: %v", err) + } + + builder := NewBuilder() + got, err := builder.Build(stdcontext.Background(), BuildInput{ + Messages: []provider.Message{{Role: "user", Content: "hello"}}, + Metadata: testMetadata(root), + }) + if err != nil { + t.Fatalf("Build() error = %v", err) + } + + identityIndex := strings.Index(got.SystemPrompt, "## Agent Identity") + rulesIndex := strings.Index(got.SystemPrompt, "## Project Rules") + stateIndex := strings.Index(got.SystemPrompt, "## System State") + if identityIndex < 0 || rulesIndex < 0 || stateIndex < 0 { + t.Fatalf("expected all prompt sections, got %q", got.SystemPrompt) + } + if !(identityIndex < rulesIndex && rulesIndex < stateIndex) { + t.Fatalf("expected section order core -> project rules -> system state, got %q", got.SystemPrompt) + } +} + func TestTrimMessagesPreservesToolPairs(t *testing.T) { t.Parallel() diff --git a/internal/context/prompt.go b/internal/context/prompt.go index c3c44e2f..22cc66b7 100644 --- a/internal/context/prompt.go +++ b/internal/context/prompt.go @@ -2,23 +2,83 @@ package context import "strings" -func defaultSystemPrompt() string { - return `You are NeoCode, a local coding agent. +type promptSection struct { + title string + content string +} + +var defaultPromptSections = []promptSection{ + { + title: "Agent Identity", + content: "You are NeoCode, a local coding agent focused on completing the current task end-to-end.\n" + + "Preserve the main loop of user input, agent reasoning, tool execution, result observation, and UI feedback.", + }, + { + title: "Tool Usage", + content: "- Use tools when they reduce uncertainty or are required to complete the task safely.\n" + + "- Inspect tool failures, explain the relevant error, and continue with the safest useful next step.\n" + + "- Do not claim work is done unless the needed files, commands, or verification actually succeeded.", + }, + { + title: "Workspace Safety", + content: "- Stay within the current workspace unless the user clearly asks for something else.\n" + + "- Avoid destructive actions such as deleting files, rewriting unrelated work, or changing history unless explicitly requested.\n" + + "- Respect project rules and local constraints before making changes.", + }, + { + title: "Code Changes", + content: "- Prefer minimal, testable changes that keep module boundaries clear.\n" + + "- Follow the existing architecture and keep provider, runtime, tools, config, and TUI responsibilities separated.\n" + + "- When behavior changes, update the relevant tests or documentation needed to keep the implementation verifiable.", + }, + { + title: "Failure Recovery", + content: "- If blocked, identify the concrete blocker and try the next reasonable path before giving up.\n" + + "- Surface risky assumptions, partial progress, or missing verification instead of hiding them.\n" + + "- When constraints prevent completion, return the best safe result and explain what remains.", + }, + { + title: "Response Style", + content: "- Be concise, accurate, and collaborative.\n" + + "- Keep updates focused on useful progress, decisions, and verification.\n" + + "- Base claims on the current workspace state instead of generic advice.", + }, +} - Be concise and accurate. - Use tools when necessary. - When a tool fails, inspect the error and continue safely. - Stay within the workspace and avoid destructive behavior unless clearly requested.` +func defaultSystemPromptSections() []promptSection { + return defaultPromptSections } -func composeSystemPrompt(parts ...string) string { - sections := make([]string, 0, len(parts)) - for _, part := range parts { - part = strings.TrimSpace(part) +func composeSystemPrompt(sections ...promptSection) string { + rendered := make([]string, 0, len(sections)) + for _, section := range sections { + part := renderPromptSection(section) if part == "" { continue } - sections = append(sections, part) + rendered = append(rendered, part) + } + return strings.Join(rendered, "\n\n") +} + +func renderPromptSection(section promptSection) string { + title := strings.TrimSpace(section.title) + content := strings.TrimSpace(section.content) + + switch { + case title == "" && content == "": + return "" + case title == "": + return content + case content == "": + return "" + default: + var builder strings.Builder + builder.Grow(len(title) + len(content) + len("## \n\n")) + builder.WriteString("## ") + builder.WriteString(title) + builder.WriteString("\n\n") + builder.WriteString(content) + return builder.String() } - return strings.Join(sections, "\n\n") } diff --git a/internal/context/prompt_test.go b/internal/context/prompt_test.go new file mode 100644 index 00000000..73cd3d15 --- /dev/null +++ b/internal/context/prompt_test.go @@ -0,0 +1,76 @@ +package context + +import "testing" + +func TestDefaultSystemPromptSectionsReturnsCachedSections(t *testing.T) { + t.Parallel() + + sections := defaultSystemPromptSections() + if len(sections) != len(defaultPromptSections) { + t.Fatalf("expected %d default sections, got %d", len(defaultPromptSections), len(sections)) + } + if len(sections) == 0 { + t.Fatalf("expected non-empty default sections") + } + if sections[0].title != "Agent Identity" { + t.Fatalf("expected first default section title, got %q", sections[0].title) + } +} + +func TestRenderPromptSectionBranches(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + section promptSection + want string + }{ + { + name: "empty title and content renders empty", + section: promptSection{}, + want: "", + }, + { + name: "content only renders content", + section: promptSection{ + content: "content only", + }, + want: "content only", + }, + { + name: "title only renders empty", + section: promptSection{ + title: "Title Only", + }, + want: "", + }, + { + name: "title and content render heading", + section: promptSection{ + title: "Section", + content: "body", + }, + want: "## Section\n\nbody", + }, + { + name: "title and content are trimmed before rendering", + section: promptSection{ + title: " Section ", + content: "\nbody\n", + }, + want: "## Section\n\nbody", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + got := renderPromptSection(tt.section) + if got != tt.want { + t.Fatalf("renderPromptSection() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/context/source_rules.go b/internal/context/source_rules.go index a666132b..31b89ad1 100644 --- a/internal/context/source_rules.go +++ b/internal/context/source_rules.go @@ -119,15 +119,14 @@ func findExactRuleFile(dir string) (string, error) { return "", nil } -func renderProjectRulesSection(documents []ruleDocument) string { +func renderProjectRulesSection(documents []ruleDocument) promptSection { if len(documents) == 0 { - return "" + return promptSection{} } const totalTruncationNotice = "\n[additional project rules truncated to fit total limit]\n" var builder strings.Builder - builder.WriteString("## Project Rules\n") remaining := maxTotalRuleRunes totalBudgetTruncated := false @@ -162,7 +161,10 @@ func renderProjectRulesSection(documents []ruleDocument) string { } } - return strings.TrimSpace(builder.String()) + return promptSection{ + title: "Project Rules", + content: strings.TrimSpace(builder.String()), + } } func renderRuleDocumentChunk(document ruleDocument) string { diff --git a/internal/context/source_rules_test.go b/internal/context/source_rules_test.go index 12cbda99..fa32a1e7 100644 --- a/internal/context/source_rules_test.go +++ b/internal/context/source_rules_test.go @@ -38,7 +38,7 @@ func TestLoadProjectRulesOrdersGlobalToLocal(t *testing.T) { t.Fatalf("expected global-to-local order, got %+v", documents) } - section := renderProjectRulesSection(documents) + section := renderPromptSection(renderProjectRulesSection(documents)) rootIndex := strings.Index(section, rootRules) localIndex := strings.Index(section, localRules) if rootIndex < 0 || localIndex < 0 || rootIndex >= localIndex { @@ -140,25 +140,29 @@ func TestRenderProjectRulesSectionTruncatesSingleFileAndTotalBudget(t *testing.T largeTotalA := strings.Repeat("b", 7000) largeTotalB := strings.Repeat("c", 7000) - section := renderProjectRulesSection([]ruleDocument{ + section := renderPromptSection(renderProjectRulesSection([]ruleDocument{ {Path: "/repo/AGENTS.md", Content: largeSingle[:maxRuleFileRunes], Truncated: true}, - }) + })) if !strings.Contains(section, "[truncated to fit per-file limit]") { t.Fatalf("expected per-file truncation marker, got %q", section) } - totalSection := renderProjectRulesSection([]ruleDocument{ + totalPromptSection := renderProjectRulesSection([]ruleDocument{ {Path: "/repo/root/AGENTS.md", Content: largeTotalA}, {Path: "/repo/root/app/AGENTS.md", Content: largeTotalB}, }) + totalSection := renderPromptSection(totalPromptSection) if !strings.Contains(totalSection, "[additional project rules truncated to fit total limit]") { t.Fatalf("expected total truncation marker, got %q", totalSection) } if strings.Contains(totalSection, strings.Repeat("c", 6500)) { t.Fatalf("expected total rules section to be truncated") } - body := strings.TrimPrefix(totalSection, "## Project Rules\n") - if runeCount(body) > maxTotalRuleRunes { - t.Fatalf("expected rendered rules body to respect total rune budget, got %d > %d", runeCount(body), maxTotalRuleRunes) + if runeCount(totalPromptSection.content) > maxTotalRuleRunes { + t.Fatalf( + "expected rendered rules body to respect total rune budget, got %d > %d", + runeCount(totalPromptSection.content), + maxTotalRuleRunes, + ) } } diff --git a/internal/context/source_system.go b/internal/context/source_system.go index ebc2a042..6ee12e0d 100644 --- a/internal/context/source_system.go +++ b/internal/context/source_system.go @@ -48,10 +48,8 @@ func collectSystemState(ctx context.Context, metadata Metadata, runner gitComman return state, nil } -func renderSystemStateSection(state SystemState) string { +func renderSystemStateSection(state SystemState) promptSection { lines := []string{ - "## System State", - "", fmt.Sprintf("- workdir: `%s`", promptValue(state.Workdir)), fmt.Sprintf("- shell: `%s`", promptValue(state.Shell)), fmt.Sprintf("- provider: `%s`", promptValue(state.Provider)), @@ -68,7 +66,10 @@ func renderSystemStateSection(state SystemState) string { lines = append(lines, "- git: unavailable") } - return strings.Join(lines, "\n") + return promptSection{ + title: "System State", + content: strings.Join(lines, "\n"), + } } func runGitCommand(ctx context.Context, workdir string, args ...string) (string, error) { diff --git a/internal/context/source_system_test.go b/internal/context/source_system_test.go index df40ca38..19c1b39e 100644 --- a/internal/context/source_system_test.go +++ b/internal/context/source_system_test.go @@ -21,7 +21,7 @@ func TestCollectSystemStateHandlesGitUnavailable(t *testing.T) { t.Fatalf("expected git to be unavailable") } - section := renderSystemStateSection(state) + section := renderPromptSection(renderSystemStateSection(state)) if !strings.Contains(section, "- git: unavailable") { t.Fatalf("expected unavailable git section, got %q", section) } @@ -56,13 +56,16 @@ func TestCollectSystemStateIncludesGitSummary(t *testing.T) { t.Fatalf("expected dirty git state") } - section := renderSystemStateSection(state) + section := renderPromptSection(renderSystemStateSection(state)) if !strings.Contains(section, "branch=`feature/context`") { t.Fatalf("expected branch in system section, got %q", section) } if !strings.Contains(section, "dirty=`dirty`") { t.Fatalf("expected dirty marker in system section, got %q", section) } + if strings.Contains(section, "internal/context/builder.go") { + t.Fatalf("did not expect full git status output in system section, got %q", section) + } } func TestCollectSystemStateReturnsContextError(t *testing.T) {