-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add insights for terraform plans #1604
Add insights for terraform plans #1604
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes in this pull request introduce several enhancements primarily focused on integrating the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (11)
priv/repo/migrations/20241121212536_add_insight_id_states.exs (1)
4-8
: Consider adding an index for the foreign key.Since this column will be used in JOIN operations between
stack_states
andai_insights
, adding an index would improve query performance.Here's how you can add the index:
def change do alter table(:stack_states) do add :insight_id, references(:ai_insights, type: :uuid, on_delete: :nilify_all) end + create index(:stack_states, [:insight_id]) end
lib/console/schema/stack_state.ex (1)
30-33
: Consider simplifying the insight association handlingThe foreign key constraints are correctly added, but the
cast_assoc
for the insight relationship might be unnecessary since it's abelongs_to
association. Typically,cast_assoc
is more useful for has_many/has_one relationships where you need to handle nested attributes.Consider simplifying to:
- |> cast_assoc(:insight)
Unless you have a specific need to handle nested insight attributes in the same changeset, removing this line would simplify the code without losing functionality.
lib/console/ai/pubsub/protocol.ex (1)
32-37
: Consider performance implications of synchronous preloadThe synchronous database preload operation in the protocol implementation could impact performance, especially in high-throughput scenarios.
Consider using async preload or moving the database operation earlier in the pipeline:
- case Console.Repo.preload(run, [:state]) do + # Option 1: Use async preload + case Console.Repo.preload(run, [:state], force: false) do + # Option 2: Ensure state is preloaded before reaching this point + case run dolib/console/ai/evidence/stack_state.ex (2)
13-13
: Consider optimizing preload performance.The preload operation loads multiple associations (
[:insight, run: [:stack, :cluster, :errors, :repository]]
). Consider:
- Making the preload selective based on what data is actually needed
- Adding documentation about the performance implications
15-31
: Consider externalizing template strings and adding nil checks.Two suggestions for improvement:
- Move the static template text to a module attribute or external template file for better maintainability
- Add nil checks for
run.repository
andrun.git
to prevent runtime errors+ @description_template """ + The Plural stack %{stack_name} has a terraform plan generated and the user will want to understand what it means, in particular: + + * expected blast radius of the change + * if any critical systems can be affected by the change + * whether it's safe to apply + + The plan itself is recorded below: + + ``` + %{plan} + ``` + + It is sourcing %{type} configuration from the git repository at %{repo_url} from the folder %{folder} at ref %{ref}. + """ + defp state_description(%StackState{run: %StackRun{} = run} = state) do + with repo_url <- get_in(run, [:repository, :url]) || "unknown", + folder <- get_in(run, [:git, :folder]) || "unknown", + ref <- get_in(run, [:git, :ref]) || "unknown" do {:user, """ - The Plural stack #{run.stack.name} has a terraform plan generated... + #{String.replace(@description_template, [ + "%{stack_name}", run.stack.name, + "%{plan}", state.plan, + "%{type}", run.type, + "%{repo_url}", repo_url, + "%{folder}", folder, + "%{ref}", ref + ])} """} + end endlib/console/schema/ai_insight.ex (1)
30-30
: Good architectural decision for Terraform plan insightsThe one-to-one relationship between AiInsight and StackState is a clean way to model the interpretation of Terraform plans. This design:
- Maintains clear separation between the plan state and its interpretation
- Allows for future extensibility (e.g., adding more insight types)
- Makes it easy to track and manage the lifecycle of insights
test/console/ai/pubsub/consumer_test.exs (2)
Line range hint
47-47
: Fix typos in test descriptionsThere are typos in two test descriptions where "faield" should be "failed".
- test "it will not run if the run isn't faield" do + test "it will not run if the run isn't failed" do - test "it will not stack if the stack isn't faield" do + test "it will not stack if the stack isn't failed" doAlso applies to: 82-82
99-112
: Consider enhancing test coverageWhile the test effectively verifies the basic flow, consider adding:
- Assertions about the plan content being passed to OpenAI
- Multiple test cases with different plan contents
- Verification of error handling scenarios
Example enhancement:
test "it handles different types of terraform plans" do # Test setup similar to existing test plans = [ { "plan with resource creation", "some plan with resource creation", "expected insight about creation" }, { "plan with resource deletion", "some plan with resource deletion", "expected insight about deletion" } ] for {name, plan, expected} <- plans do expect(Console.AI.OpenAI, :completion, fn _, content -> assert content =~ plan {:ok, expected} end) # Test assertions end end test "it handles OpenAI errors gracefully" do # Test setup expect(Console.AI.OpenAI, :completion, fn _, _ -> {:error, "API error"} end) # Verify error handling endlib/console/graphql/ai.ex (1)
95-95
: LGTM! Consider adding field documentation.The addition of the
stack_state
field is well-structured and consistent with the schema's patterns. Consider adding a@desc
documentation string to clarify the field's purpose in relation to Terraform plan insights.- field :stack_state, :stack_state, resolve: dataloader(Deployments) + @desc "Associated stack state containing Terraform plan information" + field :stack_state, :stack_state, resolve: dataloader(Deployments)lib/console/graphql/deployments/stack.ex (1)
335-338
: Consider standardizing insight field descriptions across objects.The insight field appears in multiple objects (stack, stack_run, stack_state) with slightly different descriptions. Consider documenting the relationship between insights and different contexts to help developers understand when and how insights are generated.
Example documentation pattern:
field :insight, :ai_insight, resolve: dataloader(Deployments), - description: "an insight explaining the state of this stack state, eg the terraform plan it represents" + description: """ + AI-generated insight explaining this stack state. + For Terraform plans, provides interpretation of proposed changes. + Related to insights on stack_run (execution context) and stack (overall context). + """schema/schema.graphql (1)
6198-6199
: LGTM! Consider adding field documentation.The addition of
stackState
field toAiInsight
type is well-structured and aligns with the PR's objective of enhancing terraform plan interpretability. Consider adding a GraphQL description string to document the field's purpose.clusterInsightComponent: ClusterInsightComponent + "The stack state this insight was generated from, if any" stackState: StackState insertedAt: DateTime
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (3)
assets/src/generated/graphql-kubernetes.ts
is excluded by!**/generated/**
assets/src/generated/graphql-plural.ts
is excluded by!**/generated/**
assets/src/generated/graphql.ts
is excluded by!**/generated/**
📒 Files selected for processing (11)
go/client/models_gen.go
(1 hunks)lib/console/ai/evidence/stack_run.ex
(1 hunks)lib/console/ai/evidence/stack_state.ex
(1 hunks)lib/console/ai/pubsub/protocol.ex
(1 hunks)lib/console/graphql/ai.ex
(1 hunks)lib/console/graphql/deployments/stack.ex
(1 hunks)lib/console/schema/ai_insight.ex
(2 hunks)lib/console/schema/stack_state.ex
(2 hunks)priv/repo/migrations/20241121212536_add_insight_id_states.exs
(1 hunks)schema/schema.graphql
(1 hunks)test/console/ai/pubsub/consumer_test.exs
(1 hunks)
🔇 Additional comments (18)
priv/repo/migrations/20241121212536_add_insight_id_states.exs (1)
1-9
: LGTM! The migration is well-structured.
The migration correctly implements the foreign key relationship between stack_states
and ai_insights
tables with appropriate UUID type and nilify strategy for deletions.
lib/console/schema/stack_state.ex (3)
3-3
: LGTM: Appropriate module alias added
The AiInsight
alias is correctly added alongside other schema aliases.
23-23
: LGTM: Valid attributes updated correctly
The insight_id
is appropriately added to the list of valid attributes, maintaining consistency with the new association.
16-18
: Verify the database schema supports this new association
The association declarations look correct. The belongs_to
relationships are properly defined for stack, run, and the new insight association.
Let's verify the database schema supports these associations:
✅ Verification successful
Database schema correctly supports the new association
The database schema fully supports all the declared associations:
- Initial migration (20240402221806) created
stack_states
withstack_id
andrun_id
foreign keys - Recent migration (20241121212536) added
insight_id
foreign key with appropriate reference toai_insights
table
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the database schema has the required foreign key columns
# and if there are any existing migrations that might conflict
# Look for related migration files
fd -e exs . "priv/repo/migrations" | xargs rg -l "stack_states|ai_insights"
# Look for schema definitions to verify foreign keys
rg "create.*table.*stack_states|alter.*table.*stack_states" -A 10
Length of output: 1914
lib/console/ai/pubsub/protocol.ex (2)
29-39
: Implementation aligns well with PR objectives
The implementation correctly handles the stack run state for pending approvals, which is essential for providing insights into Terraform plans. The pattern matching and return values follow the protocol specification correctly.
29-39
: Verify status handling across implementations
Let's ensure there's no overlap in status handling between different implementations.
✅ Verification successful
Let me gather more context about the protocol implementations to ensure we have a complete picture.
No status handling overlap found across implementations
Each implementation handles distinct status cases without any conflicts:
ServiceUpdated
: handlesstale
andfailed
statuses for ServiceStackRunUpdated
: handlespending_approval
status for StackRunStackRunCompleted
: handlesfailed
status for StackRunStackUpdated
: handlesfailed
status for Stack
The implementations are well-segregated based on both the event type and status, with no overlapping conditions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential overlapping status handling across implementations
# Search for all status pattern matches in protocol implementations
echo "Checking status patterns in protocol implementations:"
rg -A 1 "status: [:]?\w+" lib/console/ai/pubsub/
Length of output: 915
Script:
#!/bin/bash
# Get the protocol definition and all its implementations
echo "Protocol definition and implementations:"
ast-grep --pattern 'defprotocol Console.AI.PubSub.Insightful do
$$$
end'
echo -e "\nAll protocol implementations:"
ast-grep --pattern 'defimpl Console.AI.PubSub.Insightful, for: $_'
Length of output: 3498
lib/console/ai/evidence/stack_state.ex (2)
1-7
: LGTM! Well-structured protocol implementation.
The implementation follows Elixir best practices with clear module organization and proper use of aliases.
1-41
: Verify integration with PR comment system.
The implementation successfully addresses the PR objective of enhancing Terraform plan interpretability. However, since the PR mentions "sending these insights along with pull request comments", we should verify this integration.
✅ Verification successful
Based on the code evidence gathered, I can now generate the final response:
Integration with PR comments is properly implemented
The codebase shows a complete implementation for sending AI-generated insights as PR comments:
- The
Console.AI.Evidence
protocol forStackState
generates insights about terraform plans - The
post_comment
function inlib/console/deployments/stacks.ex
handles posting these insights as PR comments through the SCM-specific implementations - The PR comment system supports multiple SCM providers (GitHub, GitLab, Bitbucket) through the dispatcher pattern
- The insights include plan analysis, blast radius assessment, and safety considerations as shown in the state description template
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for PR comment integration code
rg -l "pull.*request.*comment.*insight" --type elixir
Length of output: 55
Script:
#!/bin/bash
# Let's search for PR comment or review-related code more broadly
rg -A 5 "pull.*request|pr.*comment|review.*comment" --type elixir
# Also search for modules that might handle PR interactions
fd -e ex -E "_test.ex$" -x grep -l "github.*api\|pull.*request\|pr.*comment"
# Look for any insight-related code that might be connected to PRs
rg -A 5 "insight.*comment|comment.*insight" --type elixir
Length of output: 61754
Script:
#!/bin/bash
# Let's look for code that handles insights and comments together
ast-grep --pattern 'def post_comment($$$) { $$ insight $$ }'
# Also search for any code that connects insights with PR comments
rg -A 5 "insight.*review|review.*insight" --type elixir
# Look for the implementation of the AI evidence protocol for stack states
rg -A 10 "defimpl Console.AI.Evidence, for: Console.Schema.StackState" --type elixir
Length of output: 1044
Script:
#!/bin/bash
# Let's look for the code that handles PR comments and reviews
rg -A 10 "def review|def post_comment" lib/console/deployments/
# Also check for any AI-related code that might be used in PR comments
rg -A 5 "ai.*review|review.*ai" --type elixir
# Look for the connection between stack states and PR comments
rg -A 10 "def state_description|def fetch_code" lib/console/ai/evidence/
Length of output: 5686
lib/console/schema/ai_insight.ex (2)
8-8
: LGTM: Clean alias addition
The StackState alias is properly added and follows the alphabetical ordering convention.
26-30
: Verify database setup for the new StackState association
The has_one associations are properly defined with consistent formatting. However, let's verify the database setup for the new StackState relationship.
✅ Verification successful
Let me search for more specific information about the migration files.
Database setup for StackState association is properly configured
The database setup for the StackState relationship is correctly implemented:
- Migration exists adding
insight_id
as a foreign key tostack_states
table with proper reference toai_insights
- The
belongs_to :insight, AiInsight
relationship is defined in the StackState schema - The foreign key uses
on_delete: :nilify_all
which is appropriate for this relationship
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the database migration and index for the new StackState relationship
# Check for migration file that adds insight_id to stack_states
fd -e exs -e sql . priv/repo/migrations/ -x grep -l "stack_states.*insight_id"
# Check for proper index definition
fd -e exs -e sql . priv/repo/migrations/ -x grep -l "create.*index.*stack_states.*insight_id"
# Verify the StackState schema definition
rg "schema \"stack_states\"" -A 10
Length of output: 892
Script:
#!/bin/bash
# Let's look for recent migrations that might contain stack_states table creation
fd -e exs . priv/repo/migrations/ -x grep -l "create table.*stack_states"
# Also check for any references to insight_id in stack_states table
fd -e exs . priv/repo/migrations/ -x grep -A 5 -B 5 "table.*stack_states"
# Let's also verify if there are any other foreign key references in the StackState schema
rg "belongs_to|references" lib/console/schema/stack_state.ex
Length of output: 926
lib/console/ai/evidence/stack_run.ex (2)
56-56
: LGTM! Good improvement in code flexibility.
The change from hardcoded "terraform" to dynamic #{run.type}
makes the code more generic and maintainable, allowing it to handle different types of runs consistently.
56-56
: Verify supported run types.
Since we're making the code more generic to handle different run types, let's verify all supported types to ensure proper handling throughout the codebase.
✅ Verification successful
Let me try a more focused search to find the run types.
Let me try one more focused search to find the run types in the codebase.
Let me try one more search to find the run types in the schema definitions.
Let me try one final search to find the stack run types.
Based on the search results, I can now provide a definitive response about the supported run types.
Verified supported run types: terraform, ansible, and custom
The codebase shows that Stack.Type
is defined as an enum in lib/console/schema/stack.ex
with three possible values:
- terraform (0)
- ansible (1)
- custom (2)
This matches the usage in stack_runs
schema where field :type
is defined as Stack.Type
, confirming these are the only supported run types throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all possible run types and their handling
# Search for type definitions and pattern matches on run.type
ast-grep --pattern 'type: $_'
# Search for specific handling of run types
rg -A 2 'run.type'
# Look for type-specific handling in pattern matches
rg 'case.*type.*do'
Length of output: 101834
Script:
#!/bin/bash
# Search for run types in Elixir files
rg -t elixir "run\.type" -B2 -A2
# Search for type definitions or case statements with run types
rg -t elixir "type: ['\"].*['\"]" -B1 -A1
# Look for run type pattern matches
ast-grep --pattern 'type: $_'
Length of output: 100204
Script:
#!/bin/bash
# Search for run types in Elixir files, focusing on schema definitions and enums
rg -t elixir "type: :" -B2 -A2
# Search for run type definitions in schema files
rg -t elixir "@type.*run" -B2 -A2
# Look for run type usage in pattern matches
rg -t elixir "case.*run\.type" -B2 -A2
Length of output: 107065
Script:
#!/bin/bash
# Search for run type definitions in schema files
rg -t elixir "^ @type.*run" -B2 -A2
# Search for run type enum definitions
rg -t elixir "@type.*run.*::" -B2 -A2
# Search for run type in schema files
rg -t elixir "schema.*run" -B2 -A2
Length of output: 3630
Script:
#!/bin/bash
# Search for Stack.Type enum definition
rg -t elixir "defenum.*Type" -B2 -A2
# Search for stack run type usage in schema
rg -t elixir "field :type.*Stack\.Type" -B2 -A2
# Search for stack type in migrations
rg "add :type" migrations
Length of output: 4214
test/console/ai/pubsub/consumer_test.exs (1)
89-91
: Good choice using async: false
Setting async: false
is appropriate here since we're testing OpenAI integration and database operations that could have side effects.
lib/console/graphql/ai.ex (2)
97-97
: LGTM! Formatting improvement.
The reformatting enhances code readability by maintaining consistent alignment with other fields.
95-97
: Verify the stack_state relationship configuration.
Let's ensure the relationship between stack_state
and ai_insight
is properly defined in both directions.
✅ Verification successful
Relationship between stack_state and ai_insight is properly configured
The verification shows that the relationship is correctly defined in both directions:
- In
lib/console/schema/stack_state.ex
:belongs_to :insight, AiInsight
- In
lib/console/schema/ai_insight.ex
:has_one :stack_state, StackState, foreign_key: :insight_id
The GraphQL type definition in lib/console/graphql/deployments/stack.ex
also correctly exposes this relationship through the insight
field on the stack_state
type.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify stack_state type definition and relationship configuration
# Check for stack_state type definition
echo "Checking for stack_state type definition..."
rg -A 10 "object :stack_state" --type elixir
# Check for belongs_to relationship in stack_state schema
echo "Checking for belongs_to relationship..."
rg "belongs_to.*:ai_insight" --type elixir
# Check for migration adding insight_id
echo "Checking for migration adding insight_id..."
fd -e exs -x rg "add.*insight_id.*stack_states" {}
Length of output: 4096
Script:
#!/bin/bash
# Let's check for the schema definition to verify the relationship
echo "Checking for stack_state schema definition..."
rg "schema.*StackState" --type elixir -A 10
# Check for any references to insight_id in stack_states
echo "Checking for insight_id references..."
rg "insight_id.*stack_states|stack_states.*insight_id" --type elixir
# Check for any recent migrations related to stack_states
echo "Checking recent migrations..."
fd ".*_stack_states.exs" --type f
Length of output: 407
Script:
#!/bin/bash
# Let's try a broader search for the schema definition
echo "Checking for StackState module..."
rg "defmodule.*StackState" --type elixir -A 10
# Look for any schema definitions related to stack_states
echo "Checking for stack_states schema..."
rg "schema \"stack_states\"" --type elixir -A 10
# Check for any belongs_to or has_one relationships between stack_states and ai_insights
echo "Checking for relationships..."
rg "belongs_to :insight|has_one :stack_state" --type elixir
# Look for migrations with a broader pattern
echo "Checking migrations with broader pattern..."
fd -e exs -x rg "create table.*stack_states|alter table.*stack_states" {}
Length of output: 2944
lib/console/graphql/deployments/stack.ex (1)
335-338
: LGTM! The insight field addition enhances Terraform plan interpretability.
The implementation follows GraphQL best practices with proper use of dataloader and clear documentation.
Let's verify the consistency of insight field implementation across related objects:
✅ Verification successful
Consistent implementation of insight field confirmed across schema objects
The insight field is consistently implemented across all relevant objects:
- stack, stack_run, stack_state
- service, service_component
- cluster
All implementations follow the same pattern:
- Type:
:ai_insight
- Resolver:
dataloader(Deployments)
- Description: contextual explanation for each object type
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent implementation of insight field across objects
# Test: Search for insight field definitions in GraphQL schema
# Expect: Consistent implementation pattern (type, resolver, description)
rg --type elixir 'field :insight.*:ai_insight.*dataloader\(Deployments\)' -A 2
Length of output: 2599
go/client/models_gen.go (2)
Line range hint 1-1
: Note: This is generated code
This file is automatically generated by gqlgen. Manual modifications should be avoided.
152-152
: LGTM: New field addition looks correct
The addition of StackState *StackState
field to the AiInsight
struct is properly typed and follows the existing field pattern with the json:"stackState,omitempty"
tag.
lib/console/ai/pubsub/protocol.ex
Outdated
case Console.Repo.preload(run, [:state]) do | ||
%StackRun{state: %StackState{} = state} -> {:ok, state} | ||
_ -> :ok | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add error handling for database operations
The current implementation doesn't handle potential database errors explicitly, which could lead to unexpected behavior.
Consider adding explicit error handling:
- case Console.Repo.preload(run, [:state]) do
- %StackRun{state: %StackState{} = state} -> {:ok, state}
- _ -> :ok
+ try do
+ case Console.Repo.preload(run, [:state]) do
+ %StackRun{state: %StackState{} = state} -> {:ok, state}
+ _ -> :ok
+ end
+ rescue
+ e in [Ecto.QueryError] ->
+ Logger.error("Failed to preload stack state: #{Exception.message(e)}")
+ :ok
+ end
Committable suggestion skipped: line range outside the PR's diff.
def generate(%StackState{run: %StackRun{} = run} = state), | ||
do: {:ok, [state_description(state) | fetch_code(run)]} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for missing run data.
The pattern matching %StackState{run: %StackRun{} = run}
will fail if run
is nil. Consider adding a fallback clause to handle this case gracefully.
+def generate(%StackState{run: nil}), do: {:error, "No run data available"}
def generate(%StackState{run: %StackRun{} = run} = state),
do: {:ok, [state_description(state) | fetch_code(run)]}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
def generate(%StackState{run: %StackRun{} = run} = state), | |
do: {:ok, [state_description(state) | fetch_code(run)]} | |
def generate(%StackState{run: nil}), do: {:error, "No run data available"} | |
def generate(%StackState{run: %StackRun{} = run} = state), | |
do: {:ok, [state_description(state) | fetch_code(run)]} | |
defp fetch_code(%StackRun{} = run) do | ||
with {:ok, f} <- Stacks.tarstream(run), | ||
{:ok, msgs} <- code_prompt(f, run.git.folder, "I'll also include the relevant #{run.type} code below, listed in the format #{file_fmt()}") do | ||
msgs | ||
else | ||
_ -> [] | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve error handling and add documentation.
The current implementation silently fails by returning an empty list for any error. Consider:
- Adding logging for errors
- Documenting the expected format of the response
- Being more specific about which errors to catch
+ @doc """
+ Fetches relevant code from the stack run.
+
+ Returns a list of code snippets in the format specified by file_fmt/0.
+ In case of errors, returns an empty list.
+ """
defp fetch_code(%StackRun{} = run) do
with {:ok, f} <- Stacks.tarstream(run),
{:ok, msgs} <- code_prompt(f, run.git.folder, "I'll also include the relevant #{run.type} code below, listed in the format #{file_fmt()}") do
msgs
else
- _ -> []
+ {:error, reason} = error ->
+ Console.Logger.warn("Failed to fetch code: #{inspect(error)}")
+ []
end
end
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
defp fetch_code(%StackRun{} = run) do | |
with {:ok, f} <- Stacks.tarstream(run), | |
{:ok, msgs} <- code_prompt(f, run.git.folder, "I'll also include the relevant #{run.type} code below, listed in the format #{file_fmt()}") do | |
msgs | |
else | |
_ -> [] | |
end | |
end | |
@doc """ | |
Fetches relevant code from the stack run. | |
Returns a list of code snippets in the format specified by file_fmt/0. | |
In case of errors, returns an empty list. | |
""" | |
defp fetch_code(%StackRun{} = run) do | |
with {:ok, f} <- Stacks.tarstream(run), | |
{:ok, msgs} <- code_prompt(f, run.git.folder, "I'll also include the relevant #{run.type} code below, listed in the format #{file_fmt()}") do | |
msgs | |
else | |
{:error, reason} = error -> | |
Console.Logger.warn("Failed to fetch code: #{inspect(error)}") | |
[] | |
end | |
end |
93c59e4
to
d51260e
Compare
These are often hard to interpret too so an insight could be useful to have, will also see if i can send them w/ the pr comments
d51260e
to
a6161e7
Compare
d32c48b
to
b74d740
Compare
These are often hard to interpret too so an insight could be useful to have, will also see if i can send them w/ the pr comments
Fixes PROD-2913
Test Plan
unit tests
Checklist
Summary by CodeRabbit
Release Notes
New Features
stack_state
field in AI insights, providing additional context.insight
field tostack_state
object for detailed stack information.AiInsight
andStackState
.Bug Fixes
Tests
StackRunUpdated
events in a pending approval state.