feat(amazonq): /doc: add support for infrastructure diagrams #6561

vikshe · 2025-02-11T22:06:02Z

Amazon Q /doc: Add support for infrastructure diagrams

Treat all work as PUBLIC. Private feature/x branches will not be squash-merged at release time.
Your code changes must meet the guidelines in CONTRIBUTING.md.
License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

KevinDing1 · 2025-02-14T16:39:09Z

packages/core/src/amazonqDoc/session/sessionState.ts

+    protected override async prepareProjectZip(
+        workspaceRoots: string[],
+        workspaceFolders: CurrentWsFolders,
+        telemetry: TelemetryHelper,


Can we refactor the prepareProjectZip to make the telemetry optional? As this is so called toolkit telemetry, and we are trying to get rid of it in /doc

avi-alpert · 2025-02-14T16:46:46Z

packages/core/src/amazonq/util/files.ts

+            let isExcludeFile = fileSize >= maxFileSizeBytes
+            // When useAutoBuildFeature is on, only respect the gitignore rules filtered earlier and apply the size limit
+            if (!isExcludeFile && !useAutoBuildFeature) {
+                isExcludeFile = isDevFile || (!isCodeFile_ && (!isIncludeInfraDiagram || !isInfraDiagramFileExt))


With the addition of this complex conditional logic, could we add unit tests to ensure that infra diagrams are included when intended and excluded when not intended?

as I understand it should be integration test.
can you specify where this test should be added ? do we have existing infrastructure for it ?

a unit test should work, you can add here packages/amazonq/test/unit/amazonqFeatureDev/util/files.test.ts

jpinkney-aws · 2025-02-14T18:02:32Z

packages/core/src/amazonqDoc/controllers/chat/controller.ts

                }
                const rightPath = path.join(uploadId, zipFilePath)
-                await openDiff(pathInfos.absolutePath, rightPath, tabId, this.scheme)
+                if (rightPath.toLowerCase().endsWith('.svg')) {


Should this .svg be pulled out into a constants file just in case you want to add more someday

jpinkney-aws · 2025-02-14T18:10:10Z

packages/core/src/shared/utilities/workspaceUtils.ts

+    const maxSizeLimitInBytes = options?.maxSizeLimitInBytes ?? maxRepoSizeBytes
+
+    const excludePatterns = [...getGlobalExcludePatterns()]
+    if (extraExcludeFilePatterns.length) {


is this a short form for detecting that its not empty ?

KevinDing1 · 2025-02-14T16:40:12Z

packages/core/src/amazonq/session/sessionState.ts

 } from '../commons/types'
 import { prepareRepoData, getDeletedFileInfos, registerNewFiles } from '../util/files'
 import { uploadCode } from '../util/upload'
+import { TelemetryHelper } from '../util/telemetryHelper'


Can we get rid of TelemetryHelper?

KevinDing1 · 2025-02-18T16:59:55Z

packages/core/src/test/amazonqDoc/controller.test.ts

Could you add new test cases to cover the infra generation and also keep the original test? I saw you changed the file amount of the mock data to 2, does this mean we lost coverage to all the flow if our service only generate README file?

reverted the changes to test as discussed.

If the flow generates 2 files instead of 1 how do we lose the coverage since it does the assertions based on changes in all files? are we planning to add each separate flow tests if there are generated 1, 2, 3... files ?

justinmk3 · 2025-02-18T18:19:21Z

packages/core/src/shared/utilities/workspaceUtils.ts

-    const storage: Awaited<ReturnType<typeof collectFiles>> = []
+    options?: {
+        maxSizeLimitInBytes?: number // 200 MB default
+        useGitIgnoreFileAsFilter?: boolean // default true


Suggested change

useGitIgnoreFileAsFilter?: boolean // default true

excludeByGitIgnore?: boolean // default true

justinmk3 · 2025-02-18T18:19:58Z

packages/core/src/shared/utilities/workspaceUtils.ts

+        extraExcludeFilePatterns?: string[] // default DefaultExtraExcludePatterns
+        extraFileFilterFn?: CollectFilesFileFilter


what purpose is the word "extra" serving here?

Suggested change

extraExcludeFilePatterns?: string[] // default DefaultExtraExcludePatterns

extraFileFilterFn?: CollectFilesFileFilter

excludePatterns?: string[] // default DefaultExtraExcludePatterns

filterFn?: CollectFilesFileFilter

justinmk3 · 2025-02-18T18:28:06Z

packages/core/src/amazonq/session/sessionState.ts

+        span: Span<AmazonqCreateUpload>,
+        options: PrepareRepoDataOptions
+    ) {
+        return await prepareRepoData(workspaceRoots, workspaceFolders, span, options)


can you explain why the prepareProjectZip method is needed, if it just passes stuff to this one?

for inheritance. this method is overridden here: DocPrepareCodeGenState

justinmk3 · 2025-02-18T18:33:30Z

packages/core/src/shared/utilities/workspaceUtils.ts

    return vscode.workspace.textDocuments.some((doc) => doc.isDirty)
 }

+export const DefaultExtraExcludePatterns = [


codebase conventions

is the word "extra" giving useful info? please name things in a meaningful way.

Suggested change

export const DefaultExtraExcludePatterns = [

export const defaultExcludePatterns = [

justinmk3 · 2025-02-18T18:36:13Z

packages/core/src/shared/utilities/workspaceUtils.ts

+    return getGlobDirExcludedPatterns().map((pattern) => `**/${pattern}/*`)
+}
+
+export function excludePatternsAsString(patterns: string[]): string {


why is this public?

justinmk3 · 2025-02-18T18:36:41Z

packages/core/src/shared/utilities/workspaceUtils.ts

+    return excludePatternsAsString(allPatterns)
+}
+
+export function getGlobalExcludePatterns() {


why is this public? (export)

justinmk3 · 2025-02-18T18:37:17Z

packages/core/src/shared/utilities/workspaceUtils.ts

+    fileContent: string
+    zipFilePath: string
+}
+export type CollectFilesFileFilter = (relativePath: string) => boolean // returns true if file should be filtered out


please think of a more meaningful, less verbose, name.

justinmk3 · 2025-02-18T18:37:53Z

packages/core/src/shared/utilities/workspaceUtils.ts

+
 /**
- * collects all files that are marked as source
+ * search and collect source files


what is a "source file" ?

justinmk3 · 2025-02-18T18:39:01Z

packages/core/src/shared/utilities/workspaceUtils.ts

+
+    const useGitIgnoreFileAsFilter = options?.useGitIgnoreFileAsFilter ?? true
+    const extraExcludeFilePatterns = options?.extraExcludeFilePatterns ?? DefaultExtraExcludePatterns
+    const maxSizeLimitInBytes = options?.maxSizeLimitInBytes ?? maxRepoSizeBytes


"max" and "limit" are redundant.

Suggested change

const maxSizeLimitInBytes = options?.maxSizeLimitInBytes ?? maxRepoSizeBytes

const maxSizeBytes = options?.maxSizeBytes ?? maxRepoSizeBytes

justinmk3 · 2025-02-18T18:39:43Z

packages/core/src/shared/utilities/workspaceUtils.ts

+    '**/LICENSE.md',
+]
+
 export function getExcludePattern(defaultExcludePatterns: boolean = true) {


Suggested change

export function getExcludePattern(defaultExcludePatterns: boolean = true) {

export function getExcludePattern(useDefaults: boolean = true) {

@justinmk3 Addressed all your code review comments

vikshe · 2025-02-19T14:18:11Z

Please merge, all code review comments were addressed

jpinkney-aws · 2025-02-19T14:24:59Z

packages/core/src/amazonq/util/files.ts

+export type PrepareRepoDataOptions = {
+    telemetry?: TelemetryHelper
+    zip?: ZipStream
+    isIncludeInfraDiagram?: boolean


Can this be changed to something like includeInfraDiagram? Having this start with is is kind of confusing

sure, will update it in my next PR
Thank you

Problem: - Amazon Q does not have support for infrastructure diagrams Solution: - Add support for them ![infra_diagram](https://github.com/user-attachments/assets/79693ab0-d95d-415e-8daf-04d59fed8573) --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. Co-authored-by: Viktor Shesternyak <[email protected]>

* fix(appbuilder): pass in the negative version of --use-container when using build quickpick (aws#6603) ## Problem When users use appbuilder to build their lambda functions, they choose between using their samconfig file or manually selecting the build parameters/flags. The problem is that when the user selects build flags and intentionally doesn't select the ```--use-container``` flag, the command will still be run with --use-container if the samconfig file has ```use_container``` is set to true. ## Solution Whenever the user manually selects the build flags and doesn't select ```--use-container```, we add the negative version of ```--use-container```, which is ```--no-use-container```. This serves as an override if the samconfig file has ```--use-container``` set to true. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. * ci(jscpd): merge target branch in jscpd to avoid false negatives. (aws#6572) ## Problem - Follow up to aws#6564 (review). ## Solution - It appears that there is an undocumented "feature" that GHA don't run when there is a merge conflict. See [here](https://github.com/orgs/community/discussions/11265) - This means we don't have to handle the failure case where a merge fails. - Add fake config identity to mitigate this error: <img width="913" alt="image" src="https://github.com/user-attachments/assets/cd426ec7-e1ca-4d13-a3b1-3985b5593c07" /> ## Notes Going to let this sit and make sure it works as changes are merged into master. --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Co-authored-by: Justin M. Keyes <[email protected]> * ci: fix and enable post-release notification (aws#6613) - Enable for prod runs - Fix script slightly because the way codebuild runs bash and the way my local runs bash seems to not be the same. - Tested on dev release pipeline --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. * refactor: notify.txt typos (aws#6616) --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. * config(amazonq): update polling config for codefix (aws#6617) ## Problem Increase in codefix timeouts ## Solution Increase default timeout and lower the polling frequency --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. * fix(amazonq): auto-review removes existing issues (aws#6535) ## Problem Auto-reviews often produce less code issues than manual reviews, but the current behavior is to remove all the issues in the file when processing the new ones. This means that issues discovered by manual reviews but not auto-reviews will silently disappear if auto-reviews is enabled. ## Solution - Auto-reviews should not clear the previous issues, but instead merge in the new results to the existing group. - Fixed a related issue with the `ignoreIssue` command being flaky --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. * feat(amazonq): /doc: add support for infrastructure diagrams (aws#6561) Problem: - Amazon Q does not have support for infrastructure diagrams Solution: - Add support for them ![infra_diagram](https://github.com/user-attachments/assets/79693ab0-d95d-415e-8daf-04d59fed8573) --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. Co-authored-by: Viktor Shesternyak <[email protected]> * refactor(cleanup): remove dead code (aws#6619) This does nothing anymore --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: nkomonen-amazon <[email protected]> * telemetry(auth): Update metrics to better debug Auth dropoff (aws#6625) ## Problem We noticed that there was an auth dropoff between `session_start` with a brand new clientId, versus when the `auth_userState` metric indicated `isFirstUse` meaning the user is net new. We went from 12.7k for `session_start` to 9.8k for `auth_userState`, these should have been basically the same. ## Solution Add in certain metrics to help debug where the discrepancy is coming from: - When we determine the user is a first time user, we will also check if the clientId is newly generated. If this is not the case we know there is a discrepancy here - The relevant metric will be `function_call` with a `functionName: isFirstUse`, `result: Failed`, and a `reason: ClientIdAlreadyExisted` - When we determine the user is a first time user, if we detected that they had previous auth connections, this will indicate a likely cause for the discrepancy - The relevant metric will be `function_call` with `reason: UnexpectedConnections` - We will emit metrics when the Auth Login page loads since that also had a discrepancy and the telemetry did not exist - The relevant metric is `webview_load` and it will indicate when the Auth Login/Reauth page has actually loaded - Previously we were observing the telemetry for the command `aws.amazonq.focusChat`, but all this did was emit when called and didn't confirm the UI actually loaded. - We will also add the `isFirstUse` metric source value in to some other existing metrics --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: nkomonen-amazon <[email protected]> --------- Signed-off-by: nkomonen-amazon <[email protected]> Co-authored-by: Frederic Mbea <[email protected]> Co-authored-by: Hweinstock <[email protected]> Co-authored-by: Justin M. Keyes <[email protected]> Co-authored-by: Maxim Hayes <[email protected]> Co-authored-by: Tai Lai <[email protected]> Co-authored-by: Viktor Shesternyak <[email protected]> Co-authored-by: Viktor Shesternyak <[email protected]> Co-authored-by: Nikolas Komonen <[email protected]>

vikshe requested review from a team as code owners February 11, 2025 22:06

vikshe marked this pull request as draft February 11, 2025 22:06

vikshe force-pushed the features/infra-d branch from 8c959d1 to 8e95d4c Compare February 11, 2025 22:48

vikshe marked this pull request as ready for review February 14, 2025 16:23

KevinDing1 reviewed Feb 14, 2025

View reviewed changes

avi-alpert reviewed Feb 14, 2025

View reviewed changes

jpinkney-aws reviewed Feb 14, 2025

View reviewed changes

vikshe force-pushed the features/infra-d branch 2 times, most recently from 1bde21e to 6671b6a Compare February 17, 2025 19:17

vikshe closed this Feb 17, 2025

vikshe reopened this Feb 17, 2025

vikshe force-pushed the features/infra-d branch 4 times, most recently from e38a3f3 to 5d51397 Compare February 18, 2025 04:03

vikshe closed this Feb 18, 2025

vikshe reopened this Feb 18, 2025

avi-alpert approved these changes Feb 18, 2025

View reviewed changes

KevinDing1 reviewed Feb 18, 2025

View reviewed changes

vikshe force-pushed the features/infra-d branch 2 times, most recently from 13158fc to 55e239e Compare February 18, 2025 17:48

KevinDing1 approved these changes Feb 18, 2025

View reviewed changes

justinmk3 reviewed Feb 18, 2025

View reviewed changes

feat(amazonq): /doc: add support for infrastructure diagrams

4472edd

vikshe force-pushed the features/infra-d branch from 55e239e to 4472edd Compare February 18, 2025 19:46

vikshe enabled auto-merge February 18, 2025 22:30

jpinkney-aws approved these changes Feb 19, 2025

View reviewed changes

jpinkney-aws disabled auto-merge February 19, 2025 15:10

jpinkney-aws merged commit a512982 into aws:master Feb 19, 2025
26 checks passed

ashishrp-aws mentioned this pull request Feb 25, 2025

fix(amazonq): /test file collection usecase adds more patterns to filtering. #6681

Merged

	useGitIgnoreFileAsFilter?: boolean // default true
	excludeByGitIgnore?: boolean // default true

		extraExcludeFilePatterns?: string[] // default DefaultExtraExcludePatterns
		extraFileFilterFn?: CollectFilesFileFilter

	export const DefaultExtraExcludePatterns = [
	export const defaultExcludePatterns = [

	const maxSizeLimitInBytes = options?.maxSizeLimitInBytes ?? maxRepoSizeBytes
	const maxSizeBytes = options?.maxSizeBytes ?? maxRepoSizeBytes

	export function getExcludePattern(defaultExcludePatterns: boolean = true) {
	export function getExcludePattern(useDefaults: boolean = true) {

feat(amazonq): /doc: add support for infrastructure diagrams #6561

feat(amazonq): /doc: add support for infrastructure diagrams #6561

Uh oh!

Conversation

vikshe commented Feb 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vikshe Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmk3 Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmk3 Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmk3 Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vikshe Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vikshe commented Feb 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

vikshe Feb 18, 2025 •

edited

Loading

justinmk3 Feb 18, 2025 •

edited

Loading

justinmk3 Feb 18, 2025 •

edited

Loading

justinmk3 Feb 18, 2025 •

edited

Loading

vikshe Feb 18, 2025 •

edited

Loading