Code Indexing Support for WSO2 Integrator Copilot#2310
Open
yasithrashan wants to merge 1 commit into
Open
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 24 minutes and 48 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThis PR adds three major feature groups to the Ballerina AI extension: (1) a code map feature enabling AI agents to receive codebase high-level summaries in prompts; (2) ripgrep-based grep and glob search tools that allow agents to discover and search code within projects; and (3) context retrieval evaluation that assesses whether agents retrieved sufficient context for their tasks. The PR also includes comprehensive Ballerina sample projects for testing and evaluation purposes, covering healthcare FHIR APIs, hotel reservations, order management, Salesforce-Slack integration, and Shopify-Stripe integration. Text editor tools are refactored to use line-range parameters instead of offset/limit, improving compatibility with the new search-based workflows. Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Nitpick comments (4)
workspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/Ballerina.toml (1)
6-6: 💤 Low valueInconsistent spacing around assignment operator.
Line 6 is missing a space before
=, while lines 2-5 include spacing. Consider adding a space for consistency.♻️ Suggested fix
-distribution= "2201.13.1" +distribution = "2201.13.1"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/Ballerina.toml` at line 6, Normalize spacing around the assignment operator for the 'distribution' key in Ballerina.toml: change the token "distribution= \"2201.13.1\"" to include a single space before and after the '=' so it matches the style used on lines 2–5 (i.e., "distribution = \"2201.13.1\""); update the 'distribution' line only to preserve formatting consistency.workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/test-validation.ts (1)
47-58: 💤 Low valueConsider extracting shared tool event mapping logic.
This mapping duplicates the logic in
result-conversion.ts(lines 46-68). A shared helper would prevent drift between the two implementations.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/test-validation.ts` around lines 47 - 58, The tool-event mapping in test-validation.ts (producing toolEvents from result.events) duplicates logic in result-conversion.ts; extract that mapping into a single helper (e.g., mapEventToToolEvent(event): ToolEvent) in a shared util module and replace the inline .map(...) in both places (the toolEvents creation in test-validation.ts and the mapping in result-conversion.ts) to call that helper; ensure the helper returns the correct discriminated types (ToolCallEvent, ToolResultEvent, EvalsToolResultEvent) and preserves fields toolName, toolOutput, and output as appropriate.workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/evaluator-utils.ts (1)
292-292: ⚡ Quick winUpdate: Claude model identifier is valid; check intent for model mismatch
claude-sonnet-4-5-20250929is a valid Anthropic Claude Sonnet 4.5 model ID. Since this is still different fromclaude-sonnet-4-20250514used elsewhere, document/justify why two different model versions are used (e.g., context retrieval vs evaluation) to avoid accidental drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/evaluator-utils.ts` at line 292, The test is using anthropic('claude-sonnet-4-5-20250929') which is a valid Sonnet 4.5 model but differs from claude-sonnet-4-20250514 used elsewhere; update the repository by adding a short justification or comment near the model selection in evaluator-utils.ts (or central test config) that explains why two different Claude model versions are intentionally used (for example: "claude-sonnet-4-5-20250929 for evaluation, claude-sonnet-4-20250514 for context retrieval") or consolidate to a single constant/ENV-driven model name (e.g., EVAL_MODEL vs RETRIEVAL_MODEL) and reference those symbols where models are selected so usage is explicit and avoids accidental drift.workspaces/ballerina/ballerina-extension/test/data/order_management_system/order_utils/utils.bal (1)
23-25: ⚡ Quick winConsider adding input validation for monetary calculations.
The function accepts any decimal
unitPriceand intquantitywithout validation. Negative or zero values could indicate upstream issues in an order management context. Adding guards would prevent invalid calculations from propagating.🛡️ Proposed validation
public function calculateLineTotal(decimal unitPrice, int quantity) returns decimal { + if unitPrice < 0 { + panic error("Unit price cannot be negative"); + } + if quantity <= 0 { + panic error("Quantity must be positive"); + } return unitPrice * <decimal>quantity; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workspaces/ballerina/ballerina-extension/test/data/order_management_system/order_utils/utils.bal` around lines 23 - 25, The calculateLineTotal function currently computes unitPrice * quantity without guarding against invalid inputs; add validation at the top of calculateLineTotal to ensure unitPrice > 0 and quantity > 0, and update the function signature from returns decimal to returns decimal|error so you can return a descriptive error (e.g., error("invalid input", {unitPrice, quantity})) when validation fails; keep the existing multiplication logic for valid inputs and ensure callers handle the decimal|error return.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workspaces/ballerina/ballerina-extension/src/features/ai/agent/index.ts`:
- Around line 121-130: The Code Map is being fetched using workspacePath which
can miss edits in the temp review snapshot; change the path passed to
langClient.getCodeMap to prefer pendingReview?.reviewState.tempProjectPath first
(e.g. use const codeMapPath = pendingReview?.reviewState.tempProjectPath ??
workspacePath ?? StateMachine.context().projectPath) or move the getCodeMap call
to after temp-project initialization so langClient.getCodeMap(...) uses the
tempProjectPath when present; update references around StateMachine.context(),
pendingReview, and the langClient.getCodeMap invocation accordingly.
In `@workspaces/ballerina/ballerina-extension/src/features/ai/agent/prompts.ts`:
- Around line 218-232: The system prompt and the getUserPrompt behavior are
inconsistent: getUserPrompt emits a `<Codebase High Level Summary>` block when
codeMapMarkdown exists and uses formatCodebaseStructure() for the full fallback,
so update the system prompt text to explicitly state that `<Codebase High Level
Summary>` contains a high-level summary only (not full source) and that
`<codebase_structure>` (or the full structure tag used by
formatCodebaseStructure) will be provided when the full project source is
included; change any wording that claims the complete existing source will
"always" be under the same tag so the agent knows to prefer reading files when
full structure is present and rely only on the summary when only codeMapMarkdown
is supplied (reference symbols: getUserPrompt, codeMapMarkdown,
formatCodebaseStructure).
In
`@workspaces/ballerina/ballerina-extension/src/features/ai/utils/project/temp-project.ts`:
- Around line 189-190: The packagePaths assignment currently maps
StateMachine.context().projectInfo?.children to child.projectPath but can
produce undefined entries; update the packagePaths computation to first extract
children paths by mapping then filtering out undefined/empty values (e.g.,
filter(Boolean) or check typeof === "string"), and if the resulting array is
empty (or children is empty/undefined), fall back to
workspaceTomlValues.workspace.packages; ensure this change is applied where
packagePaths is defined so later calls to path.isAbsolute and path.join receive
only valid strings.
In `@workspaces/ballerina/ballerina-extension/test/ai/evals/code/test-cases.ts`:
- Line 279: Fix the incorrect and inconsistent langlib function names and
duplicate/typo entries in the test-case comments: replace startswith() with
startsWith(), change arr.length() to a consistent array:length() notation,
remove the duplicate push(), add sorting functions like sort() and
codePointCompare() where "unicode ordering" is mentioned, normalize .toString()
to toString() and add float:exp() (or equivalent) where "exponential
calculations" are referenced, remove the duplicate find(), add fromJsonString(),
cloneWithType(), and toJson() where JSON parsing/serialization is described,
update the listed functions to better match the "validation failures" scenario,
and correct cloneReaadOnly() to cloneReadOnly(); apply these fixes to the
comment lines referenced (279, 284, 289, 329, 342, 347, 357).
In
`@workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/evaluator-utils.ts`:
- Around line 176-179: The current loop that creates entries in contentByPath by
iterating parts and mapping trailing segments (using variables parts,
normalized, contentByPath, file.content) causes collisions when different files
share the same basename; instead, ensure the full normalized path is always
stored (set contentByPath with normalized first) and only add shorter keys
(e.g., basename or trailing segments) as fallbacks when an exact key does not
already exist—i.e., for each file put contentByPath.set(normalized,
file.content) and then for each parts.slice(i).join('/') only call
contentByPath.set(...) if !contentByPath.has(...) so earlier exact/full-path
wins and loose matches are used only as fallback.
In
`@workspaces/ballerina/ballerina-extension/test/data/healthcare_sample/mapping.bal`:
- Line 34: The default fallback string for the gender field is misspelled
causing the cast to <uscore311:USCorePatientProfileGender> to fail; update the
expression that sets gender (currently using patient?.gender ?: "unkown") to use
the correct fallback value "unknown" so the runtime cast to
USCorePatientProfileGender succeeds.
- Line 45: The call to mapGivenToName(patient.name[0].given) can index into an
empty given array; update the code to guard against empty given arrays by either
(A) checking that patient.name[0].given has length > 0 before calling
mapGivenToName (e.g., only call mapGivenToName(patient.name[0].given) when
patient.name[0].given.length > 0 and otherwise provide a safe fallback/empty
value), or (B) change mapNameToGiven so it never returns an empty string[] for
missing names (return ()/nil instead) and adjust mapGivenToName to handle
nil/absent given safely; locate references to mapGivenToName and mapNameToGiven
to implement the chosen guard/fallback.
In
`@workspaces/ballerina/ballerina-extension/test/data/hotel_reservation/utils.bal`:
- Around line 52-56: The current table query in utils.bal that builds
reservations from roomReservations (using Reservation, rCheckin, rCheckout,
userCheckinUTC, userCheckoutUTC) only finds fully-contained reservations; update
the where clause in that query to use standard interval-overlap logic (replace
"userCheckinUTC <= rCheckin && userCheckoutUTC >= rCheckout" with a condition
equivalent to "userCheckinUTC < rCheckout && userCheckoutUTC > rCheckin") so
partially overlapping reservations are treated as conflicts for
getAvailableRoom/getAvailableRoomTypes.
In
`@workspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/Ballerina.toml`:
- Around line 7-10: The dependency org for the local package mismatch: in
order_service/Ballerina.toml the [[dependency]] for name "order_utils" uses org
"yasithrashan" but functions.bal imports "wso2/order_utils" and the local
order_utils/Ballerina.toml declares org "wso2"; update the dependency org in
order_service/Ballerina.toml to "wso2" (or alternatively change the import in
functions.bal to match "yasithrashan/order_utils") so the package name and org
for order_utils are consistent across order_service/Ballerina.toml,
order_utils/Ballerina.toml, and the import in functions.bal.
In
`@workspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/modules/db/db_operations.bal`:
- Around line 12-20: The query result stream opened via dbClient->query(q)
(resultStream) must always be closed even if resultStream.next() errors; wrap
the call to resultStream.next() and subsequent logic in a try block and move
check resultStream.close() into a finally block (or equivalent) so close() is
always invoked; update the code paths around resultStream.next(), the row
variable, and the return of row.value (and the OrderNotFoundError path) to work
with the try/finally structure so the stream is closed on both success and
error.
In
`@workspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration_errors/functions.bal`:
- Around line 184-194: In getSlackUserIdFromEmail change the URL construction to
URL-encode the email before appending it to the Slack endpoint (use
ballerina/url:encode(email, "UTF-8")) and add an import for ballerina/url if
missing; ensure the encoded value is used in the slackHttpClient->get call
instead of raw email so query injection/invalid-character issues are prevented.
In
`@workspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration_errors/main.bal`:
- Around line 67-74: Deduplication currently marks the lead
(processedLeadIds[leadId] = true) inside the lock before downstream work,
causing permanent false positives if later steps fail; change the flow so the
map is updated only after the entire processing succeeds (move the
processedLeadIds[leadId] = true assignment to after the downstream queries and
Slack notification), or alternatively wrap the downstream work in a try/catch
and remove processedLeadIds[leadId] in the error path (or use a finally-style
cleanup) to ensure failed attempts can be retried; refer to the processedLeadIds
map, the leadId variable, and the existing lock block when making this change.
In
`@workspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/functions.bal`:
- Line 190: In getSlackUserIdFromEmail, the email is concatenated into the query
string unencoded; import ballerina/url and call url:encode(email, "UTF-8")
before building the "/api/users.lookupByEmail?email=" URL, handle the returned
string|error by logging the error (e.g., log:printWarn with the original email
and enc error) and returning early on error, then pass the encoded string to
slackHttpClient->get instead of the raw email.
In
`@workspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/main.bal`:
- Around line 68-74: The code marks processedLeadIds[leadId] = true inside the
lock before the lead is actually processed, which causes failed processing to be
permanently skipped on retries; move the processedLeadIds[leadId] = true
assignment out of the lock to after the full success path (after lead query,
record fetches, and successful Slack dispatch) or, alternatively, keep the
current placement but ensure any failure path (exceptions or non-success results
from the lead query/fetch/Slack dispatch) removes processedLeadIds[leadId]
inside a finally/catch so retries can attempt processing again; update the logic
around the lock/lead processing flow and use the same processedLeadIds and
leadId symbols to locate the change.
In
`@workspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/connections.bal`:
- Around line 4-6: The variable declaration uses an undefined type
`shopifyListenerConfig`; replace it with the correct Shopify listener config
type `shopify:ListenerConfig` (or the appropriate imported alias if you imported
the Shopify module with a different prefix) in the `listenerConfig` declaration
so it reads like: listenerConfig of type shopify:ListenerConfig with the same
property `apiSecretKey: shopifyConfig.apiSecretKey`; ensure the module prefix
`shopify` matches the import used elsewhere in the code.
- Line 16: Module-level use of the `check` expression on the `stripe` variable
(stripe:Client stripe = check new (configuration);) can cause
initialization/error propagation problems; move the client creation into an
initialization function (e.g., a new createStripeClient or init function) and
perform the `check` there, or if you intend the program to abort on failure,
replace the module-level `check` with `checkpanic` inside a controlled init
routine; ensure you update references to the `stripe` symbol to use the client
returned/assigned by that init function.
In
`@workspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/main.bal`:
- Line 19: There's a typo in the remote function declaration: change the
incorrect keyword "remotee" to "remote" on the function declaration for
onCustomersDisable (remotee function onCustomersDisable(shopify:CustomerEvent
event) returns error? -> remote function onCustomersDisable(...)); update the
keyword in that declaration so the remote method name onCustomersDisable
compiles correctly.
- Around line 7-9: The log call may throw if event?.id is null because it calls
toString() directly; change the logging to use a null-safe conversion (e.g.,
compute an id string with event?.id?.toString() or a conditional/default like
"<unknown>" before calling log:printInfo) and update the log:printInfo
invocation accordingly so it never calls toString() on a nil value; reference
the event variable and the log:printInfo call in the snippet to locate where to
apply the fix.
---
Nitpick comments:
In
`@workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/evaluator-utils.ts`:
- Line 292: The test is using anthropic('claude-sonnet-4-5-20250929') which is a
valid Sonnet 4.5 model but differs from claude-sonnet-4-20250514 used elsewhere;
update the repository by adding a short justification or comment near the model
selection in evaluator-utils.ts (or central test config) that explains why two
different Claude model versions are intentionally used (for example:
"claude-sonnet-4-5-20250929 for evaluation, claude-sonnet-4-20250514 for context
retrieval") or consolidate to a single constant/ENV-driven model name (e.g.,
EVAL_MODEL vs RETRIEVAL_MODEL) and reference those symbols where models are
selected so usage is explicit and avoids accidental drift.
In
`@workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/test-validation.ts`:
- Around line 47-58: The tool-event mapping in test-validation.ts (producing
toolEvents from result.events) duplicates logic in result-conversion.ts; extract
that mapping into a single helper (e.g., mapEventToToolEvent(event): ToolEvent)
in a shared util module and replace the inline .map(...) in both places (the
toolEvents creation in test-validation.ts and the mapping in
result-conversion.ts) to call that helper; ensure the helper returns the correct
discriminated types (ToolCallEvent, ToolResultEvent, EvalsToolResultEvent) and
preserves fields toolName, toolOutput, and output as appropriate.
In
`@workspaces/ballerina/ballerina-extension/test/data/order_management_system/order_utils/utils.bal`:
- Around line 23-25: The calculateLineTotal function currently computes
unitPrice * quantity without guarding against invalid inputs; add validation at
the top of calculateLineTotal to ensure unitPrice > 0 and quantity > 0, and
update the function signature from returns decimal to returns decimal|error so
you can return a descriptive error (e.g., error("invalid input", {unitPrice,
quantity})) when validation fails; keep the existing multiplication logic for
valid inputs and ensure callers handle the decimal|error return.
In
`@workspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/Ballerina.toml`:
- Line 6: Normalize spacing around the assignment operator for the
'distribution' key in Ballerina.toml: change the token "distribution=
\"2201.13.1\"" to include a single space before and after the '=' so it matches
the style used on lines 2–5 (i.e., "distribution = \"2201.13.1\""); update the
'distribution' line only to preserve formatting consistency.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d11b10ae-acd1-4af1-ae08-f19d6dccfc1e
⛔ Files ignored due to path filters (1)
common/config/rush/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (82)
workspaces/ballerina/ballerina-core/src/interfaces/extended-lang-client.tsworkspaces/ballerina/ballerina-extension/package.jsonworkspaces/ballerina/ballerina-extension/src/core/extended-language-client.tsworkspaces/ballerina/ballerina-extension/src/features/ai/activator.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/AgentExecutor.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/index.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/prompts.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/tool-registry.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/glob.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/grep.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/text-editor.tsworkspaces/ballerina/ballerina-extension/src/features/ai/agent/tools/utils/rg-utils.tsworkspaces/ballerina/ballerina-extension/src/features/ai/executors/base/AICommandExecutor.tsworkspaces/ballerina/ballerina-extension/src/features/ai/utils/project/temp-project.tsworkspaces/ballerina/ballerina-extension/test/ai/evals/code/result-management/report-generator.tsworkspaces/ballerina/ballerina-extension/test/ai/evals/code/result-management/result-conversion.tsworkspaces/ballerina/ballerina-extension/test/ai/evals/code/result-management/result-persistence.tsworkspaces/ballerina/ballerina-extension/test/ai/evals/code/test-cases.tsworkspaces/ballerina/ballerina-extension/test/ai/evals/code/types/result-types.tsworkspaces/ballerina/ballerina-extension/test/ai/evals/code/types/test-types.tsworkspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/evaluator-utils.tsworkspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/test-validation.tsworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/Ballerina.tomlworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/Config.tomlworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/encounter_api_config.balworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/mapping.balworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/modules/db/persist_client.balworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/modules/db/persist_db_config.balworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/modules/db/persist_types.balworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/modules/db/script.sqlworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/patient_api_config.balworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/persist/model.balworkspaces/ballerina/ballerina-extension/test/data/healthcare_sample/service.balworkspaces/ballerina/ballerina-extension/test/data/hotel_reservation/Ballerina.tomlworkspaces/ballerina/ballerina-extension/test/data/hotel_reservation/service.balworkspaces/ballerina/ballerina-extension/test/data/hotel_reservation/tests/Config.tomlworkspaces/ballerina/ballerina-extension/test/data/hotel_reservation/tests/service_test.balworkspaces/ballerina/ballerina-extension/test/data/hotel_reservation/types.balworkspaces/ballerina/ballerina-extension/test/data/hotel_reservation/utils.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/Ballerina.tomlworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/Ballerina.tomlworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/configurations.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/functions.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/main.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/modules/db/db_client.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/modules/db/db_config.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/modules/db/db_operations.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/modules/db/db_types.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/modules/messaging/kafka_config.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/modules/messaging/kafka_operations.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/modules/messaging/kafka_producer.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/modules/messaging/kafka_types.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/service.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_service/types.balworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_utils/Ballerina.tomlworkspaces/ballerina/ballerina-extension/test/data/order_management_system/order_utils/utils.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/Ballerina.tomlworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/agents.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/config.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/connections.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/data_mappings.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/functions.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/main.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/types.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration_errors/Ballerina.tomlworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration_errors/agents.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration_errors/config.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration_errors/connections.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration_errors/data_mappings.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration_errors/functions.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration_errors/main.balworkspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration_errors/types.balworkspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/Ballerina.tomlworkspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/agents.balworkspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/automation.balworkspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/config.balworkspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/connections.balworkspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/data_mappings.balworkspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/functions.balworkspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/main.balworkspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/types.balworkspaces/ballerina/ballerina-visualizer/src/views/AIPanel/styles.tsx
Comment on lines
+121
to
+130
| // Fetch Code Map at query submission time to capture the current project state. | ||
| // TODO: The language server does not capture file delete events yet. Once that is fixed, | ||
| // we can use incremental updates — fetching the Code Map only for changed files and | ||
| // merging into the existing bal.md, instead of regenerating the full Code Map each time. | ||
| const langClient = StateMachine.context().langClient; | ||
| const workspacePath = StateMachine.context().workspacePath || StateMachine.context().projectPath; | ||
| if (langClient) { | ||
| try { | ||
| const codeMapResponse = await langClient.getCodeMap({ | ||
| projectPath: workspacePath, |
Contributor
There was a problem hiding this comment.
Fetch the Code Map from the reused temp project during review continuation.
When pendingReview?.reviewState.tempProjectPath exists, the executor will edit that temp snapshot, but this fetch still targets workspacePath/projectPath. Follow-up prompts can therefore be built from stale project state and miss previously generated review changes. Prefer pendingReview?.reviewState.tempProjectPath ?? workspacePath ?? StateMachine.context().projectPath here, or move Code Map generation after temp-project initialization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/ballerina/ballerina-extension/src/features/ai/agent/index.ts`
around lines 121 - 130, The Code Map is being fetched using workspacePath which
can miss edits in the temp review snapshot; change the path passed to
langClient.getCodeMap to prefer pendingReview?.reviewState.tempProjectPath first
(e.g. use const codeMapPath = pendingReview?.reviewState.tempProjectPath ??
workspacePath ?? StateMachine.context().projectPath) or move the getCodeMap call
to after temp-project initialization so langClient.getCodeMap(...) uses the
tempProjectPath when present; update references around StateMachine.context(),
pendingReview, and the langClient.getCodeMap invocation accordingly.
Comment on lines
+218
to
+232
| export function getUserPrompt(params: GenerateAgentCodeRequest, tempProjectPath: string, projects: ProjectSource[], codeMapMarkdown?: string) { | ||
| const content = []; | ||
|
|
||
| content.push({ | ||
| type: 'text' as const, | ||
| text: formatCodebaseStructure(projects) | ||
| }); | ||
| // Add codebase high level summary if available, otherwise fall back to full project structure | ||
| if (codeMapMarkdown) { | ||
| content.push({ | ||
| type: 'text' as const, | ||
| text: `<Codebase High Level Summary>\n${codeMapMarkdown}\n</Codebase High Level Summary>` | ||
| }); | ||
| } else { | ||
| content.push({ | ||
| type: 'text' as const, | ||
| text: formatCodebaseStructure(projects) | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Align the prompt contract with the new Code Map path.
When codeMapMarkdown is present, this no longer sends full source in <codebase_structure>, but the system prompt above still says the complete existing source code will always be provided there. That contradiction can push the agent to act on summary-only context instead of reading files first. Update the instructions to explicitly distinguish the Code Map path from the full-structure fallback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/ballerina/ballerina-extension/src/features/ai/agent/prompts.ts`
around lines 218 - 232, The system prompt and the getUserPrompt behavior are
inconsistent: getUserPrompt emits a `<Codebase High Level Summary>` block when
codeMapMarkdown exists and uses formatCodebaseStructure() for the full fallback,
so update the system prompt text to explicitly state that `<Codebase High Level
Summary>` contains a high-level summary only (not full source) and that
`<codebase_structure>` (or the full structure tag used by
formatCodebaseStructure) will be provided when the full project source is
included; change any wording that claims the complete existing source will
"always" be under the same tag so the agent knows to prefer reading files when
full structure is present and rely only on the summary when only codeMapMarkdown
is supplied (reference symbols: getUserPrompt, codeMapMarkdown,
formatCodebaseStructure).
Comment on lines
+189
to
+190
| const packagePaths = StateMachine.context().projectInfo?.children?.map(child => child.projectPath) | ||
| ?? workspaceTomlValues.workspace.packages; |
Contributor
There was a problem hiding this comment.
Validate projectInfo.children paths before preferring them over workspace.toml.
child.projectPath is optional in the shared contract, so this map can yield undefined values that are used by path.isAbsolute and path.join a few lines later. Also, an empty children array skips the fallback entirely and returns no package sources. Filter out missing paths and fall back to workspaceTomlValues.workspace.packages when the filtered list is empty.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@workspaces/ballerina/ballerina-extension/src/features/ai/utils/project/temp-project.ts`
around lines 189 - 190, The packagePaths assignment currently maps
StateMachine.context().projectInfo?.children to child.projectPath but can
produce undefined entries; update the packagePaths computation to first extract
children paths by mapping then filtering out undefined/empty values (e.g.,
filter(Boolean) or check typeof === "string"), and if the resulting array is
empty (or children is empty/undefined), fall back to
workspaceTomlValues.workspace.packages; ensure this change is applied where
packagePaths is defined so later calls to path.isAbsolute and path.join receive
only valid strings.
| }, | ||
| { | ||
| // findAll(), substring(), push(), findGroups(), string.length(), push(), startswith(), fromJsonString(), cloneWithType(), arr.length() | ||
| // findAll(), substring(), push(), findGroups(), string.length(), push(), startswith(), fromJsonString(), cloneWithType(), arr.length() |
Contributor
There was a problem hiding this comment.
Fix langlib function comment inconsistencies and typos.
Several langlib test case comments contain errors that should be corrected:
- Line 279:
startswith()should bestartsWith()(camelCase);arr.length()should use consistent notation likearray:length();push()is listed twice - Line 284: Prompt mentions "unicode ordering" but comment doesn't list sorting functions like
sort()orcodePointCompare() - Line 289:
.toString()has inconsistent dot prefix (should betoString()); prompt mentions "exponential calculations" but comment doesn't listfloat:exp()or equivalent - Line 329:
find()is listed twice - Line 342: Prompt describes JSON parsing and serialization but comment doesn't list
fromJsonString(),cloneWithType(), ortoJson() - Line 347: Prompt mentions validation failures but listed functions don't clearly align with the described error handling scenario
- Line 357:
cloneReaadOnly()has a typo — should becloneReadOnly()
These comments appear to document which langlib functions should be exercised during evaluation. Ensuring accuracy will help maintain test reliability.
Also applies to: 284-284, 289-289, 329-329, 342-342, 347-347, 357-357
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@workspaces/ballerina/ballerina-extension/test/ai/evals/code/test-cases.ts` at
line 279, Fix the incorrect and inconsistent langlib function names and
duplicate/typo entries in the test-case comments: replace startswith() with
startsWith(), change arr.length() to a consistent array:length() notation,
remove the duplicate push(), add sorting functions like sort() and
codePointCompare() where "unicode ordering" is mentioned, normalize .toString()
to toString() and add float:exp() (or equivalent) where "exponential
calculations" are referenced, remove the duplicate find(), add fromJsonString(),
cloneWithType(), and toJson() where JSON parsing/serialization is described,
update the listed functions to better match the "validation failures" scenario,
and correct cloneReaadOnly() to cloneReadOnly(); apply these fixes to the
comment lines referenced (279, 284, 289, 329, 342, 347, 357).
Comment on lines
+176
to
+179
| const parts = normalized.split('/'); | ||
| for (let i = 1; i < parts.length; i++) { | ||
| contentByPath.set(parts.slice(i).join('/'), file.content); | ||
| } |
Contributor
There was a problem hiding this comment.
Loose path matching may cause incorrect file content lookups.
When multiple files share trailing path segments (e.g., modules/a/types.bal and modules/b/types.bal), all partial segments like types.bal map to the same key. The last file processed overwrites earlier entries, causing lookups to return wrong content.
Consider matching on full normalized paths first, then falling back to basename only if no exact match exists.
Suggested approach
function extractFileReadSection(toolEvents: readonly ToolEvent[], initialSource: SourceFile[]): string {
const contentByPath = new Map<string, string>();
+ const contentByBasename = new Map<string, string>();
for (const file of initialSource) {
const normalized = file.filePath.replace(/\\/g, '/');
contentByPath.set(normalized, file.content);
- // Also index by just the filename and relative segments for loose matching
- const parts = normalized.split('/');
- for (let i = 1; i < parts.length; i++) {
- contentByPath.set(parts.slice(i).join('/'), file.content);
- }
+ // Index by basename for fallback (may have collisions)
+ const basename = normalized.split('/').pop();
+ if (basename && !contentByBasename.has(basename)) {
+ contentByBasename.set(basename, file.content);
+ }
}
+ // Helper to find content
+ const findContent = (fileName: string): string | undefined => {
+ const normalized = fileName.replace(/\\/g, '/');
+ return contentByPath.get(normalized) ?? contentByBasename.get(normalized.split('/').pop() || '');
+ };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@workspaces/ballerina/ballerina-extension/test/ai/evals/code/utils/evaluator-utils.ts`
around lines 176 - 179, The current loop that creates entries in contentByPath
by iterating parts and mapping trailing segments (using variables parts,
normalized, contentByPath, file.content) causes collisions when different files
share the same basename; instead, ensure the full normalized path is always
stored (set contentByPath with normalized first) and only add shorter keys
(e.g., basename or trailing segments) as fallbacks when an exact key does not
already exist—i.e., for each file put contentByPath.set(normalized,
file.content) and then for each parts.slice(i).join('/') only call
contentByPath.set(...) if !contentByPath.has(...) so earlier exact/full-path
wins and loose matches are used only as fallback.
Comment on lines
+68
to
+74
| lock { | ||
| if processedLeadIds.hasKey(leadId) { | ||
| log:printInfo("Duplicate lead conversion event, skipping", leadId = leadId); | ||
| return; | ||
| } | ||
| processedLeadIds[leadId] = true; | ||
| } |
Contributor
There was a problem hiding this comment.
Lead is marked processed before processing completes.
processedLeadIds[leadId] is set inside the lock before the lead query, record fetches, and Slack dispatch run. If any of those steps fails, a subsequent retry of the same event is treated as a duplicate and skipped, so the notification is never delivered. Consider marking the lead as processed only after successful dispatch, or removing it from the cache on failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@workspaces/ballerina/ballerina-extension/test/data/salesforce_slack_integration/main.bal`
around lines 68 - 74, The code marks processedLeadIds[leadId] = true inside the
lock before the lead is actually processed, which causes failed processing to be
permanently skipped on retries; move the processedLeadIds[leadId] = true
assignment out of the lock to after the full success path (after lead query,
record fetches, and successful Slack dispatch) or, alternatively, keep the
current placement but ensure any failure path (exceptions or non-success results
from the lead query/fetch/Slack dispatch) removes processedLeadIds[leadId]
inside a finally/catch so retries can attempt processing again; update the logic
around the lock/lead processing flow and use the same processedLeadIds and
leadId symbols to locate the change.
Comment on lines
+4
to
+6
| shopifyListenerConfig listenerConfig = { | ||
| apiSecretKey: shopifyConfig.apiSecretKey | ||
| }; |
Contributor
There was a problem hiding this comment.
Undefined type shopifyListenerConfig.
The type shopifyListenerConfig is not defined or imported. Based on the Shopify module import, this should likely be shopify:ListenerConfig.
🔧 Proposed fix
-shopifyListenerConfig listenerConfig = {
+shopify:ListenerConfig listenerConfig = {
apiSecretKey: shopifyConfig.apiSecretKey
};📝 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.
Suggested change
| shopifyListenerConfig listenerConfig = { | |
| apiSecretKey: shopifyConfig.apiSecretKey | |
| }; | |
| shopify:ListenerConfig listenerConfig = { | |
| apiSecretKey: shopifyConfig.apiSecretKey | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@workspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/connections.bal`
around lines 4 - 6, The variable declaration uses an undefined type
`shopifyListenerConfig`; replace it with the correct Shopify listener config
type `shopify:ListenerConfig` (or the appropriate imported alias if you imported
the Shopify module with a different prefix) in the `listenerConfig` declaration
so it reads like: listenerConfig of type shopify:ListenerConfig with the same
property `apiSecretKey: shopifyConfig.apiSecretKey`; ensure the module prefix
`shopify` matches the import used elsewhere in the code.
| } | ||
| }; | ||
|
|
||
| stripe:Client stripe = check new (configuration); |
Contributor
There was a problem hiding this comment.
Module-level check expression may cause initialization issues.
Using check at module level can lead to improper error propagation. Consider moving this initialization to a function or using checkpanic if the program should terminate on failure.
🔧 Alternative approach
-stripe:Client stripe = check new (configuration);
+stripe:Client stripe = checkpanic new (configuration);📝 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.
Suggested change
| stripe:Client stripe = check new (configuration); | |
| stripe:Client stripe = checkpanic new (configuration); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@workspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/connections.bal`
at line 16, Module-level use of the `check` expression on the `stripe` variable
(stripe:Client stripe = check new (configuration);) can cause
initialization/error propagation problems; move the client creation into an
initialization function (e.g., a new createStripeClient or init function) and
perform the `check` there, or if you intend the program to abort on failure,
replace the module-level `check` with `checkpanic` inside a controlled init
routine; ensure you update references to the `stripe` symbol to use the client
returned/assigned by that init function.
Comment on lines
+7
to
+9
| if ((event?.first_name == () && event?.last_name == ()) || event?.email == ()) { | ||
| log:printInfo("Skipping customer creation in Stripe for Shopify customer with missing details: " + (event?.id.toString())); | ||
| return; |
Contributor
There was a problem hiding this comment.
Potential null pointer exception when logging event ID.
Line 8 calls toString() on event?.id, which may be null. If id is null, this will cause a runtime error.
🛡️ Proposed fix
- log:printInfo("Skipping customer creation in Stripe for Shopify customer with missing details: " + (event?.id.toString()));
+ log:printInfo("Skipping customer creation in Stripe for Shopify customer with missing details: " + (event?.id?.toString() ?: "unknown"));📝 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.
Suggested change
| if ((event?.first_name == () && event?.last_name == ()) || event?.email == ()) { | |
| log:printInfo("Skipping customer creation in Stripe for Shopify customer with missing details: " + (event?.id.toString())); | |
| return; | |
| if ((event?.first_name == () && event?.last_name == ()) || event?.email == ()) { | |
| log:printInfo("Skipping customer creation in Stripe for Shopify customer with missing details: " + (event?.id?.toString() ?: "unknown")); | |
| return; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@workspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/main.bal`
around lines 7 - 9, The log call may throw if event?.id is null because it calls
toString() directly; change the logging to use a null-safe conversion (e.g.,
compute an id string with event?.id?.toString() or a conditional/default like
"<unknown>" before calling log:printInfo) and update the log:printInfo
invocation accordingly so it never calls toString() on a nil value; reference
the event variable and the log:printInfo call in the snippet to locate where to
apply the fix.
| log:printInfo("Customer created in Stripe for Shopify customer: " + (event?.email ?: "")); | ||
| } | ||
|
|
||
| remotee function onCustomersDisable(shopify:CustomerEvent event) returns error? { |
Contributor
There was a problem hiding this comment.
Syntax error: remotee should be remote.
Line 19 contains a typo. The keyword should be remote, not remotee. This will prevent compilation.
🐛 Required fix
- remotee function onCustomersDisable(shopify:CustomerEvent event) returns error? {
+ remote function onCustomersDisable(shopify:CustomerEvent event) returns error? {📝 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.
Suggested change
| remotee function onCustomersDisable(shopify:CustomerEvent event) returns error? { | |
| remote function onCustomersDisable(shopify:CustomerEvent event) returns error? { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@workspaces/ballerina/ballerina-extension/test/data/shopify_stripe_integration_errors/main.bal`
at line 19, There's a typo in the remote function declaration: change the
incorrect keyword "remotee" to "remote" on the function declaration for
onCustomersDisable (remotee function onCustomersDisable(shopify:CustomerEvent
event) returns error? -> remote function onCustomersDisable(...)); update the
keyword in that declaration so the remote method name onCustomersDisable
compiles correctly.
yasithrashan
force-pushed
the
feature/wso2-integrator-copilot-code-indexing
branch
from
3570689 to
e483566
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
$title
Resolves: https://github.com/wso2-enterprise/integration-engineering/issues/893
key Changes