Improve/render vars in ingestr

[DjamilaBaroudi]

PR Overview

• Improved Ingestr asset display with edit/view mode toggle using "Edit"/"Done" text link for cleaner UI.
• Updated integration tests, changelog, and README.

[greptile-apps]

Comments Outside Diff (1)

1. webview-ui/src/components/asset/IngestrAssetDisplay.vue, line 780-788 (link)

   cleanQuery double-strips YAML block scalars that parseIngestrParameters already removed

   parseIngestrParameters skips the |/> indicator line and only collects the indented body lines, so displayParams.query should never start with | or >. The guard in cleanQuery is therefore dead code. If parseIngestrParameters is changed in the future (e.g., to pass through the YAML verbatim), this silent no-op could mask the issue. Remove the guard or add a comment explaining why it is kept.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: webview-ui/src/components/asset/IngestrAssetDisplay.vue
   Line: 780-788
   
   Comment:
   **`cleanQuery` double-strips YAML block scalars that `parseIngestrParameters` already removed**
   
   `parseIngestrParameters` skips the `|`/`>` indicator line and only collects the indented body lines, so `displayParams.query` should never start with `|` or `>`. The guard in `cleanQuery` is therefore dead code. If `parseIngestrParameters` is changed in the future (e.g., to pass through the YAML verbatim), this silent no-op could mask the issue. Remove the guard or add a comment explaining why it is kept.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/extension/commands/parseAssetCommand.ts
Line: 91

Comment:
**Missing `await` on async `render()` call**

`renderer.render()` is an `async` method but is called without `await`. Any unhandled rejection from the promise would be silently swallowed, and there is no error boundary here to catch it. Since `patchAssetCommand` is already `async`, awaiting the call is straightforward.

```suggestion
      await renderer.render(filePath);
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/bruin/bruinRender.ts
Line: 12-67

Comment:
**Regex-based YAML parsing is fragile and can silently return wrong values**

The `parseIngestrParameters` function builds regexes directly from parameter names and matches against raw YAML text. Two concrete failure cases:

1. If a YAML *value* contains a substring that looks like another key (e.g., `source_table: "schema.incremental_key: foo"`), the regex for `incremental_key` will match inside the string.
2. The multiline-query extraction removes only 2–4 leading spaces (`/^[ ]{2,4}/`), so deeper or mixed-indent YAML indentation will either strip too little or fail to strip at all, corrupting the query.

Using a proper YAML parser (e.g., `js-yaml`, already available in most VS Code extension projects) would handle all these edge cases reliably.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/bruin/bruinRender.ts
Line: 127-139

Comment:
**`isIngestrAsset` triggers an extra render for every `.asset.yml` file, not just ingestr-type assets**

`isIngestrAsset` is determined solely by file extension. Python pipeline tasks, generic YAML-defined assets, and any other asset type stored in a `.asset.yml` file will all enter this branch and fire off an extra `renderIngestrParams` call. The call is harmless (it parses YAML content and finds nothing, then no message is sent), but it doubles the CLI invocations for every YAML asset open in the editor.

Consider checking the asset *type* field in the already-parsed asset data (available via `parseAssetCommand`) before deciding to call `renderIngestrParams`.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: webview-ui/src/components/asset/IngestrAssetDisplay.vue
Line: 780-788

Comment:
**`cleanQuery` double-strips YAML block scalars that `parseIngestrParameters` already removed**

`parseIngestrParameters` skips the `|`/`>` indicator line and only collects the indented body lines, so `displayParams.query` should never start with `|` or `>`. The guard in `cleanQuery` is therefore dead code. If `parseIngestrParameters` is changed in the future (e.g., to pass through the YAML verbatim), this silent no-op could mask the issue. Remove the guard or add a comment explaining why it is kept.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "refactor: streamline edit mode toggle in..." | Re-trigger Greptile}