Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ refrax refactor --output="./out" --ai=deepseek test/test_data/java/person

### Checking Results

`refrax` does not inherently verify whether the applied changes are correct or cause any issues.
`refrax` does not inherently verify whether the applied changes are correct or cause any issues.
To address this, you can use the `--check` option to validate the changes. Multiple `--check` options can be provided, as shown below:

```sh
Expand Down
8 changes: 7 additions & 1 deletion internal/critic/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,13 @@ func (c *Critic) thinkLong(m *protocol.Message) (*protocol.Message, error) {
return nil, fmt.Errorf("failed to parse task from message: %w", err)
}
artifacts, err := c.agent.Review(tsk)
return artifacts.Marshal().Message, err
if err != nil {
return nil, fmt.Errorf("agent review failed: %w", err)
}
if artifacts == nil {
return nil, fmt.Errorf("agent review returned nil artifacts")
}
return artifacts.Marshal().Message, nil
}

func agentCard(port int) *protocol.AgentCard {
Expand Down
3 changes: 3 additions & 0 deletions internal/domain/a2a.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,9 @@ func (j *Job) Marshal() *protocol.MessageSendParams {
}

func (a *Artifacts) Marshal() *protocol.MessageSendParams {
if a == nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yegor256 I don't think we need to allow a nil receiver here. By doing this, we’re just hiding the original problem: 'Why do we have a nil here at all?'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@volodya-lombrozo I have no idea, to be honest...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yegor256 can we just remove it then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@volodya-lombrozo I can try, but the bug may not be fixed in this case

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yegor256 Maybe you can try it locally?

go install

refrax ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@volodya-lombrozo tests fail if I remove this check for nil. I can simply drop this PR, but I don't know how to fix the bug otherwise (since this PR is made by Claude Code, not me)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yegor256 I suggest closing it. I've successfully reproduced the issue. Thus, I can fix the bug properly

return protocol.NewMessageSendParams().WithMessage(protocol.NewMessage().WithMessageID(uuid.NewString()))
}
msg := protocol.NewMessage().WithMessageID(uuid.NewString())
if a.Descr != nil {
msg.AddPart(a.Descr.Marshal())
Expand Down
7 changes: 7 additions & 0 deletions internal/domain/a2a_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,10 @@ func TestMarshalAndUnmarshalJob(t *testing.T) {
require.NoError(t, err, "Unmarshaling job should not return an error")
assert.Equal(t, before, after, "Jobs should be equal after marshaling and unmarshaling")
}

func TestArtifactsMarshalDoesNotPanicOnNilReceiver(t *testing.T) {
var artifacts *Artifacts
result := artifacts.Marshal()
require.NotNil(t, result, "Marshal should not return nil even with nil receiver")
require.NotNil(t, result.Message, "Message should not be nil even with nil receiver")
}
9 changes: 4 additions & 5 deletions internal/prompts/critic/critic.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ Identify issues such as:
* unnecessary comments inside methods,
* redundant code.

Do **not** suggest any changes that alter functionality.
Do **not** suggest moving code between files (e.g., extract class or interface).
Do **not** suggest renaming methods or classes.
Do **not** suggest any changes that alter functionality.
Do **not** suggest moving code between files (e.g., extract class or interface).
Do **not** suggest renaming methods or classes.
Do **not** suggest upgrading or replacing libraries.

Consider the following imperfections reported by static analysis:
Expand All @@ -22,6 +22,5 @@ Consider the following imperfections reported by static analysis:
(none)
{{- end }}

Respond with plain text, one suggestion per line.
Respond with plain text, one suggestion per line.
If there are no significant issues, respond with: {{ .NotFound }}

1 change: 0 additions & 1 deletion internal/prompts/facilitator/choose.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,3 @@ Answer format (one per line):

Example:
src/test/java/com/example/service/Example.java: Fix the typo in the class comment

1 change: 0 additions & 1 deletion internal/prompts/facilitator/group.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,3 @@ Answer in the following format:

Example:
src/test/java/com/example/service/Example.java: Fix the typo in the class comment

1 change: 0 additions & 1 deletion internal/prompts/fixer/fix.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,3 @@ File: {{ .FilePath }}
## Final check (silent)
- No leading/trailing blank lines outside the code.
- No backtick fences, no “diff” syntax, no headings.

1 change: 0 additions & 1 deletion internal/prompts/reviewer/review.md.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,3 @@ Answer in the following format:

Example:
src/test/java/com/example/service/Example.java: Fix the typo in the class comment

3 changes: 3 additions & 0 deletions internal/reviewer/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ func (r *A2AReviewer) thinkLong(_ *protocol.Message) (*protocol.Message, error)
if err != nil {
return nil, fmt.Errorf("failed to task: %w", err)
}
if artifacts == nil {
return nil, fmt.Errorf("review returned nil artifacts")
}
suggestions := artifacts.Suggestions
r.log.Info("number of processed suggestions: %d", len(suggestions))
logSuggestions(r.log, suggestions)
Expand Down
Loading