feat: change llm model and refactor code structure by sehwan505 · Pull Request #33 · DevSymphony/sym-cli

sehwan505 · 2025-12-01T04:52:59Z

LLM 모델 변경
프롬프트 개선
LLM 사용 시 빌더 패턴 적용

…tecture

Copilot

Pull request overview

This PR refactors the LLM integration by introducing a builder pattern for LLM requests, changing the default models, improving prompts for more reliable responses, and adding support for MCP (Model Context Protocol) session integration. The key architectural change is replacing the direct Complete() method with a more flexible Request().Execute() pattern that supports different model tiers and reasoning levels.

Introduces builder pattern for LLM client with fast (gpt-4o-mini) and power (intended as o3-mini) model tiers
Enhances validation prompts with clearer instructions and confidence levels
Adds MCP session support for using host LLM instead of direct OpenAI API
Removes model selection flags from CLI commands (centralized in code)

Reviewed changes

Copilot reviewed 19 out of 20 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`internal/llm/client.go`	Major refactor introducing builder pattern, dual-model support, and MCP integration
`internal/validator/llm_validator.go`	Enhanced prompts with confidence levels and improved JSON parsing logic
`internal/validator/validator.go`	Updated to use new builder API
`internal/mcp/server.go`	Added MCP session support for LLM client and context propagation
`internal/converter/converter.go`	Added parallel rule processing and updated to builder API
`internal/adapter/*/converter.go`	Updated all adapters to use new builder API with power model
`internal/cmd/*.go`	Removed model selection flags, updated example documentation
`internal/adapter/checkstyle/converter.go`	Added property validation to prevent invalid configurations
`cmd/test-linter/main.go`	New test utility for ESLint adapter
`go.mod`, `go.sum`	Removed unused doublestar dependency, reordered entries
`tests/e2e/*.go`	Removed explicit model specifications from tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copilot · 2025-12-01T05:04:45Z

internal/cmd/convert.go

+  # Convert for Java with specific model
+  sym convert -i user-policy.json --targets checkstyle,pmd --openai-model gpt-5-mini


This example references the removed --openai-model flag and uses the non-existent 'gpt-5-mini' model. Since the flag was removed in line 59 of the diff, this example is now invalid and should be removed or updated to reflect the current CLI interface.

Suggested change

# Convert for Java with specific model

sym convert -i user-policy.json --targets checkstyle,pmd --openai-model gpt-5-mini

# Convert for Java linters

sym convert -i user-policy.json --targets checkstyle,pmd

Copilot · 2025-12-01T05:04:45Z

internal/validator/llm_validator.go

+// parseJSON parses JSON string into the target struct
+func parseJSON(jsonStr string, target interface{}) error {
+	decoder := strings.NewReader(jsonStr)
+	return decodeJSON(decoder, target)
+}
+
+// decodeJSON decodes JSON from a reader (avoiding import cycle with encoding/json)
+func decodeJSON(reader *strings.Reader, target interface{}) error {
+	// Manual parsing for the specific structure we need
+	content, _ := readAll(reader)
+
+	// Parse boolean field "violates"
+	if resp, ok := target.(*jsonValidationResponse); ok {
+		resp.Violates = strings.Contains(strings.ToLower(content), `"violates":true`) ||
+			strings.Contains(strings.ToLower(content), `"violates": true`)
+
+		resp.Confidence = extractJSONField(content, "confidence")
+		resp.Description = extractJSONField(content, "description")
+		resp.Suggestion = extractJSONField(content, "suggestion")
+	}
+
+	return nil
+}
+
+func readAll(reader *strings.Reader) (string, error) {
+	var builder strings.Builder
+	buf := make([]byte, 1024)
+	for {
+		n, err := reader.Read(buf)
+		if n > 0 {
+			builder.Write(buf[:n])
+		}
+		if err != nil {
+			break
+		}
+	}
+	return builder.String(), nil
+}


This custom JSON parsing implementation is unnecessarily complex and error-prone. The comment on line 293 mentions 'avoiding import cycle with encoding/json', but there is no import cycle issue here. The standard library's json.Unmarshal() should be used instead: json.Unmarshal([]byte(jsonStr), target). This would eliminate the need for parseJSON, decodeJSON, and readAll functions while being more reliable.

Copilot · 2025-12-01T05:04:45Z

internal/validator/llm_validator.go


-	// Call LLM
-	response, err := v.client.Complete(ctx, systemPrompt, userPrompt)
+	// Call LLM with low reasoning (needs thought for code validation)


[nitpick] Corrected spelling in comment. Should be 'some thought' not just 'thought' for clarity, or simply 'reasoning' since the parameter is already 'reasoning'. Consider: 'Call LLM with reasoning (needs some thought for code validation)' or 'Call LLM with reasoning for code validation'.

Suggested change

// Call LLM with low reasoning (needs thought for code validation)

// Call LLM with reasoning for code validation

Copilot · 2025-12-01T05:04:46Z

internal/llm/client.go

 		Messages: []openAIMessage{
-			{Role: "system", Content: systemPrompt},
-			{Role: "user", Content: userPrompt},
+			{Role: "user", Content: r.system + "\n\n" + r.user},


Combining system and user prompts into a single user message changes the semantic meaning. OpenAI's API treats 'system' and 'user' roles differently - the system message sets the behavior/context for the assistant, while user messages are the actual user input. This combination may reduce the effectiveness of the system prompt. Consider preserving the two-message structure as shown in the removed code (lines 219-221 in the old version).

Suggested change

{Role: "user", Content: r.system + "\n\n" + r.user},

{Role: "system", Content: r.system},

{Role: "user", Content: r.user},

Copilot · 2025-12-01T05:04:46Z

internal/llm/client.go

-	Temperature float64         `json:"temperature,omitempty"`
+	Model           string          `json:"model"`
+	Messages        []openAIMessage `json:"messages"`
+	MaxTokens       int             `json:"max_completion_tokens,omitempty"`


The field name changed from 'max_tokens' to 'max_completion_tokens'. While 'max_completion_tokens' is the newer OpenAI API parameter name, verify that this is supported by all models being used (gpt-4o-mini and the intended o3-mini). Some older API versions or models may still require 'max_tokens'.

Copilot · 2025-12-01T05:04:46Z

internal/converter/converter.go


-	// Call LLM
-	response, err := c.llmClient.Complete(ctx, systemPrompt, userPrompt)
+	// Call LLM with power model + low reasoning (needs some thought for linter selection)


The comment says 'low reasoning' but the code uses llm.ReasoningMedium. These should match - either update the comment to say 'medium reasoning' or change the code to use llm.ReasoningLow.

Suggested change

// Call LLM with power model + low reasoning (needs some thought for linter selection)

// Call LLM with power model + medium reasoning (needs some thought for linter selection)

Copilot · 2025-12-01T05:04:46Z

internal/llm/client.go

 	defaultMaxTokens   = 1000
-	defaultTemperature = 0.3
-	defaultTimeout     = 30 * time.Second
+	defaultTemperature = 1.0


Temperature of 1.0 is quite high for structured output like JSON. The previous value was 0.3, which is more appropriate for deterministic, structured responses. High temperature (1.0) increases randomness and may result in less consistent JSON formatting, which could cause parsing errors. Consider keeping temperature at 0.3 or at most 0.5 for reliability.

Suggested change

defaultTemperature = 1.0

defaultTemperature = 0.3

Copilot · 2025-12-01T05:04:47Z

internal/validator/llm_validator.go

+
+	jsonStr := response[startIdx : endIdx+1]
+
+	// Parse JSON properly using encoding/json


This comment is misleading because the code does not actually use encoding/json - it uses custom string parsing. Either update the comment to reflect the actual implementation ('Parse JSON using custom string parsing') or refactor to actually use encoding/json as suggested in Comment 3.

Suggested change

// Parse JSON properly using encoding/json

// Parse JSON using custom function, fallback to string-based parsing

sehwan505 added 8 commits December 1, 2025 10:35

fix: update prompt for converter

ba56723

feat: enhance linter converters with validation and option registries

1988aa0

chore: update llm version

0edbb76

feat: improve LLM validation prompt and response handling

cd113d4

refactor: enhance LLM client with new request handling

1878e32

refactor: update LLM client usage

ae0f5bd

refactor: remove deprecated linter files and consolidate linter archi…

a7f375a

…tecture

fix: unused parameter in linter test

29addcc

sehwan505 marked this pull request as ready for review December 1, 2025 04:59

sehwan505 requested a review from Copilot December 1, 2025 05:01

Copilot started reviewing on behalf of sehwan505 December 1, 2025 05:01 View session

Copilot finished reviewing on behalf of sehwan505 December 1, 2025 05:03

Copilot AI reviewed Dec 1, 2025

View reviewed changes

sehwan505 merged commit 7c09c29 into main Dec 2, 2025
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: change llm model and refactor code structure#33

feat: change llm model and refactor code structure#33
sehwan505 merged 8 commits intomainfrom
feature/llm-use

sehwan505 commented Dec 1, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Copilot AI Dec 1, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		# Convert for Java with specific model
		sym convert -i user-policy.json --targets checkstyle,pmd --openai-model gpt-5-mini

	// Call LLM with low reasoning (needs thought for code validation)
	// Call LLM with reasoning for code validation

	{Role: "user", Content: r.system + "\n\n" + r.user},
	{Role: "system", Content: r.system},
	{Role: "user", Content: r.user},

	// Call LLM with power model + low reasoning (needs some thought for linter selection)
	// Call LLM with power model + medium reasoning (needs some thought for linter selection)


		jsonStr := response[startIdx : endIdx+1]

		// Parse JSON properly using encoding/json

	// Parse JSON properly using encoding/json
	// Parse JSON using custom function, fallback to string-based parsing

Conversation

sehwan505 commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sehwan505 commented Dec 1, 2025 •

edited

Loading