Skip to content

Commit 0ca6252

Browse files
committed
refactor: introduce ContextRetriever for enhanced testability
Introduce the ContextRetriever interface and concrete implementations to facilitate better testability by abstracting context retrieval in file and I/O reader scenarios. This change allows for more granular testing and decoupling of context acquisition from business logic. While this improvement brings enhanced testability and maintainability, it also introduces an additional layer of indirection, which can slightly impact the initial grasping of the context flow. However, this cost is mitigated by the benefits of having a more testable and modular codebase.
1 parent 01f1e43 commit 0ca6252

15 files changed

+385
-242
lines changed

.mockery.yaml

+3-1
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,6 @@ dir: "mocks"
88
outpkg: "mocks"
99
packages:
1010
github.com/intility/cwc/internal:
11-
github.com/intility/cwc/pkg/templates:
11+
github.com/intility/cwc/pkg/templates:
12+
github.com/intility/cwc/pkg/config:
13+
github.com/intility/cwc/pkg/systemcontext:

cmd/cwc.go

+77-30
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ package cmd
22

33
import (
44
"fmt"
5+
"github.com/intility/cwc/pkg/config"
6+
"github.com/intility/cwc/pkg/filetree"
7+
"github.com/intility/cwc/pkg/systemcontext"
58
"github.com/intility/cwc/pkg/templates"
9+
"github.com/intility/cwc/pkg/ui"
610
"github.com/spf13/cobra"
711
"os"
812
"path/filepath"
@@ -11,7 +15,8 @@ import (
1115
)
1216

1317
const (
14-
longDescription = `The 'cwc' command initiates a new chat session,
18+
warnFileSizeThreshold = 100000
19+
longDescription = `The 'cwc' command initiates a new chat session,
1520
providing granular control over the inclusion and exclusion of files via regular expression patterns.
1621
It allows for specification of paths to include or exclude files from the chat context.
1722
@@ -60,13 +65,24 @@ func CreateRootCommand() *cobra.Command {
6065
Long: longDescription,
6166
Args: cobra.MaximumNArgs(1),
6267
RunE: func(cobraCmd *cobra.Command, args []string) error {
63-
deps := createDefaultDeps(args, chatOpts.TemplateName, chatOpts.TemplateVariables)
68+
cfgProvider := config.NewDefaultProvider()
69+
clientProvider := internal.NewOpenAIClientProvider(cfgProvider)
70+
templateLocator := getTemplateLocator(cfgProvider)
71+
promptResolver := internal.NewArgsOrTemplatePromptResolver(templateLocator, args, chatOpts.TemplateName)
6472

6573
if isPiped(os.Stdin) {
74+
contextRetriever := systemcontext.NewIOReaderContextRetriever(os.Stdin)
75+
smGenerator := internal.NewTemplatedSystemMessageGenerator(
76+
templateLocator,
77+
chatOpts.TemplateName,
78+
chatOpts.TemplateVariables,
79+
contextRetriever,
80+
)
81+
6682
nic := internal.NewNonInteractiveCmd(
67-
deps.clientProvider,
68-
deps.promptResolver,
69-
deps.systemMessageGenerator,
83+
clientProvider,
84+
promptResolver,
85+
smGenerator,
7086
)
7187

7288
err := nic.Run()
@@ -77,10 +93,24 @@ func CreateRootCommand() *cobra.Command {
7793
return nil
7894
}
7995

96+
contextRetriever := systemcontext.NewFileContextRetriever(
97+
cfgProvider,
98+
chatOpts.IncludePattern,
99+
chatOpts.ExcludePattern,
100+
chatOpts.Paths,
101+
printContext)
102+
103+
smGenerator := internal.NewTemplatedSystemMessageGenerator(
104+
templateLocator,
105+
chatOpts.TemplateName,
106+
chatOpts.TemplateVariables,
107+
contextRetriever,
108+
)
109+
80110
interactiveCmd := internal.NewInteractiveCmd(
81-
deps.promptResolver,
82-
deps.clientProvider,
83-
deps.systemMessageGenerator,
111+
promptResolver,
112+
clientProvider,
113+
smGenerator,
84114
chatOpts,
85115
)
86116

@@ -103,35 +133,52 @@ func CreateRootCommand() *cobra.Command {
103133
return rootCmd
104134
}
105135

136+
func printContext(fileTree string, files []filetree.File) {
137+
ui.PrintMessage(fileTree, ui.MessageTypeInfo)
138+
for _, file := range files {
139+
printLargeFileWarning(file)
140+
}
141+
}
142+
143+
func printLargeFileWarning(file filetree.File) {
144+
if len(file.Data) > warnFileSizeThreshold {
145+
largeFileMsg := fmt.Sprintf(
146+
"warning: %s is very large (%d bytes) and will degrade performance.\n",
147+
file.Path, len(file.Data))
148+
149+
ui.PrintMessage(largeFileMsg, ui.MessageTypeWarning)
150+
}
151+
}
152+
106153
type defaultDeps struct {
107-
configProvider internal.ConfigProvider
154+
configProvider config.ConfigProvider
108155
clientProvider internal.ClientProvider
109156
templateLocator templates.TemplateLocator
110157
promptResolver internal.PromptResolver
111158
systemMessageGenerator internal.SystemMessageGenerator
112159
}
113160

114-
func createDefaultDeps(args []string, templateName string, templateVars map[string]string) *defaultDeps {
115-
cfgProvider := internal.NewDefaultProvider()
116-
clientProvider := internal.NewOpenAIClientProvider(cfgProvider)
117-
templateLocator := getTemplateLocator(cfgProvider)
118-
promptResolver := internal.NewArgsOrTemplatePromptResolver(templateLocator, args, templateName)
119-
systemMessageGenerator := internal.NewTemplatedSystemMessageGenerator(
120-
templateLocator,
121-
templateName,
122-
templateVars,
123-
)
124-
125-
return &defaultDeps{
126-
configProvider: cfgProvider,
127-
clientProvider: clientProvider,
128-
templateLocator: templateLocator,
129-
promptResolver: promptResolver,
130-
systemMessageGenerator: systemMessageGenerator,
131-
}
132-
}
133-
134-
func getTemplateLocator(cfgProvider internal.ConfigProvider) templates.TemplateLocator {
161+
//func createDefaultDeps(args []string, templateName string, templateVars map[string]string) *defaultDeps {
162+
// cfgProvider := internal.NewDefaultProvider()
163+
// clientProvider := internal.NewOpenAIClientProvider(cfgProvider)
164+
// templateLocator := getTemplateLocator(cfgProvider)
165+
// promptResolver := internal.NewArgsOrTemplatePromptResolver(templateLocator, args, templateName)
166+
// systemMessageGenerator := internal.NewTemplatedSystemMessageGenerator(
167+
// templateLocator,
168+
// templateName,
169+
// templateVars,
170+
// )
171+
//
172+
// return &defaultDeps{
173+
// configProvider: cfgProvider,
174+
// clientProvider: clientProvider,
175+
// templateLocator: templateLocator,
176+
// promptResolver: promptResolver,
177+
// systemMessageGenerator: systemMessageGenerator,
178+
// }
179+
//}
180+
181+
func getTemplateLocator(cfgProvider config.ConfigProvider) templates.TemplateLocator {
135182
var locators []templates.TemplateLocator
136183

137184
configDir, err := cfgProvider.GetConfigDir()

internal/client.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package internal
22

33
import (
44
"fmt"
5+
"github.com/intility/cwc/pkg/config"
56

67
"github.com/sashabaranov/go-openai"
78
)
@@ -11,10 +12,10 @@ type ClientProvider interface {
1112
}
1213

1314
type OpenAIClientProvider struct {
14-
cfg ConfigProvider
15+
cfg config.ConfigProvider
1516
}
1617

17-
func NewOpenAIClientProvider(provider ConfigProvider) *OpenAIClientProvider {
18+
func NewOpenAIClientProvider(provider config.ConfigProvider) *OpenAIClientProvider {
1819
return &OpenAIClientProvider{cfg: provider}
1920
}
2021

internal/cmd.go

-83
This file was deleted.

internal/interactive.go

+1-72
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import (
66
"github.com/sashabaranov/go-openai"
77

88
"github.com/intility/cwc/pkg/chat"
9-
"github.com/intility/cwc/pkg/filetree"
10-
"github.com/intility/cwc/pkg/pathmatcher"
119
"github.com/intility/cwc/pkg/ui"
1210
)
1311

@@ -46,18 +44,7 @@ func (c *InteractiveCmd) Run() error {
4644
return fmt.Errorf("error creating openaiClient: %w", err)
4745
}
4846

49-
files, fileTree, err := c.gatherAndPrintContext()
50-
if err != nil {
51-
return err
52-
} else if len(files) == 0 { // No files found, terminating or confirming to proceed
53-
if !askConfirmation("No files found matching the given criteria.\n", ui.MessageTypeWarning) {
54-
return nil
55-
}
56-
}
57-
58-
contextStr := createContext(fileTree, files)
59-
60-
generatedSystemMessage, err := c.smGenerator.GenerateSystemMessage(contextStr)
47+
generatedSystemMessage, err := c.smGenerator.GenerateSystemMessage()
6148
if err != nil {
6249
return fmt.Errorf("error creating system message: %w", err)
6350
}
@@ -116,61 +103,3 @@ func (c *InteractiveCmd) printMessageChunk(chunk *chat.ConversationChunk) {
116103

117104
ui.PrintMessage(chunk.Content, ui.MessageTypeInfo)
118105
}
119-
120-
func (c *InteractiveCmd) gatherAndPrintContext() ([]filetree.File, string, error) {
121-
// gatherAndPrintContext gathers file context based on provided options and prints it out.
122-
files, rootNode, err := c.gatherContext()
123-
if err != nil {
124-
return nil, "", err
125-
}
126-
127-
for _, file := range files {
128-
printLargeFileWarning(file)
129-
}
130-
131-
fileTree := filetree.GenerateFileTree(rootNode, "", true)
132-
133-
ui.PrintMessage("The following files will be used as context:\n", ui.MessageTypeInfo)
134-
ui.PrintMessage(fileTree, ui.MessageTypeInfo)
135-
136-
return files, fileTree, nil
137-
}
138-
139-
func (c *InteractiveCmd) gatherContext() ([]filetree.File, *filetree.FileNode, error) {
140-
var excludeMatchers []pathmatcher.PathMatcher
141-
142-
// add exclude flag to excludeMatchers
143-
if c.chatOptions.ExcludePattern != "" {
144-
excludeMatcher, err := pathmatcher.NewRegexPathMatcher(c.chatOptions.ExcludePattern)
145-
if err != nil {
146-
return nil, nil, fmt.Errorf("error creating exclude matcher: %w", err)
147-
}
148-
149-
excludeMatchers = append(excludeMatchers, excludeMatcher)
150-
}
151-
152-
excludeMatchersFromConfig, err := excludeMatchersFromConfig()
153-
if err != nil {
154-
return nil, nil, err
155-
}
156-
157-
excludeMatchers = append(excludeMatchers, excludeMatchersFromConfig...)
158-
159-
excludeMatcher := pathmatcher.NewCompoundPathMatcher(excludeMatchers...)
160-
161-
includeMatcher, err := pathmatcher.NewRegexPathMatcher(c.chatOptions.IncludePattern)
162-
if err != nil {
163-
return nil, nil, fmt.Errorf("error creating include matcher: %w", err)
164-
}
165-
166-
files, rootNode, err := filetree.GatherFiles(&filetree.FileGatherOptions{
167-
IncludeMatcher: includeMatcher,
168-
ExcludeMatcher: excludeMatcher,
169-
PathScopes: c.chatOptions.Paths,
170-
})
171-
if err != nil {
172-
return nil, nil, fmt.Errorf("error gathering files: %w", err)
173-
}
174-
175-
return files, rootNode, nil
176-
}

internal/noninteractive.go

+1-18
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@ package internal
22

33
import (
44
"fmt"
5-
"io"
6-
"os"
7-
85
"github.com/intility/cwc/pkg/chat"
96
"github.com/intility/cwc/pkg/errors"
107
"github.com/intility/cwc/pkg/ui"
@@ -34,12 +31,7 @@ func (c *NonInteractiveCmd) Run() error {
3431
return fmt.Errorf("error creating openaiClient: %w", err)
3532
}
3633

37-
systemCtx, err := c.readContextFromStdIn()
38-
if err != nil {
39-
return fmt.Errorf("error reading context from stdin: %w", err)
40-
}
41-
42-
generateSystemMessage, err := c.smGenerator.GenerateSystemMessage(systemCtx)
34+
generateSystemMessage, err := c.smGenerator.GenerateSystemMessage()
4335
if err != nil {
4436
return fmt.Errorf("error creating system message: %w", err)
4537
}
@@ -58,15 +50,6 @@ func (c *NonInteractiveCmd) Run() error {
5850
return nil
5951
}
6052

61-
func (c *NonInteractiveCmd) readContextFromStdIn() (string, error) {
62-
inputBytes, err := io.ReadAll(os.Stdin)
63-
if err != nil {
64-
return "", fmt.Errorf("error reading from stdin: %w", err)
65-
}
66-
67-
return string(inputBytes), nil
68-
}
69-
7053
func (c *NonInteractiveCmd) printChunk(chunk *chat.ConversationChunk) {
7154
ui.PrintMessage(chunk.Content, ui.MessageTypeInfo)
7255
}

0 commit comments

Comments
 (0)