From abe1355ca948a9b8e2b038e11e0970dd7204b751 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 22:02:39 +0000 Subject: [PATCH 1/6] Initial plan for issue From 05b3703d0a159a1f7da067c73a96a917f8e92657 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 22:17:46 +0000 Subject: [PATCH 2/6] Fix notebook target profile to default to Unrestricted instead of workspace setting Co-authored-by: minestarks <16928427+minestarks@users.noreply.github.com> --- language_service/src/state.rs | 51 +++++++++++++++- language_service/src/state/tests.rs | 95 +++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) diff --git a/language_service/src/state.rs b/language_service/src/state.rs index f031b79937..58923e837a 100644 --- a/language_service/src/state.rs +++ b/language_service/src/state.rs @@ -414,7 +414,7 @@ impl<'a> CompilationStateUpdater<'a> { .map(|manifest| manifest.lints) .unwrap_or_default(), }; - let configuration = merge_configurations(¬ebook_configuration, &configuration); + let configuration = merge_configurations_for_notebook(¬ebook_configuration, &configuration); // Compile the notebook and add each cell into the document map let compilation = Compilation::new_notebook( @@ -744,3 +744,52 @@ fn merge_configurations( lints_config: merged_lints, } } + +fn merge_configurations_for_notebook( + compilation_overrides: &PartialConfiguration, + workspace_scope: &Configuration, +) -> Configuration { + let mut merged_lints = workspace_scope.lints_config.clone(); + let mut override_lints = compilation_overrides.lints_config.clone(); + override_lints.retain(|override_lint| { + for merged_lint in &mut merged_lints { + match (merged_lint, override_lint) { + ( + LintOrGroupConfig::Lint(lint_config), + LintOrGroupConfig::Lint(lint_config_override), + ) => { + if lint_config.kind == lint_config_override.kind { + lint_config.level = lint_config_override.level; + return false; + } + } + ( + LintOrGroupConfig::Group(group_config), + LintOrGroupConfig::Group(group_config_override), + ) => { + if group_config.lint_group == group_config_override.lint_group { + group_config.level = group_config_override.level; + return false; + } + } + _ => (), + } + } + true + }); + merged_lints.extend(override_lints); + + Configuration { + // For notebooks, default to Unrestricted instead of workspace target profile + target_profile: compilation_overrides + .target_profile + .unwrap_or(Profile::Unrestricted), + package_type: compilation_overrides + .package_type + .unwrap_or(workspace_scope.package_type), + language_features: compilation_overrides + .language_features + .unwrap_or(workspace_scope.language_features), + lints_config: merged_lints, + } +} diff --git a/language_service/src/state/tests.rs b/language_service/src/state/tests.rs index c057bdc59f..a095a88d7a 100644 --- a/language_service/src/state/tests.rs +++ b/language_service/src/state/tests.rs @@ -853,6 +853,101 @@ async fn update_notebook_reports_errors_from_dependency_of_dependencies() { ); } +#[tokio::test] +async fn notebook_defaults_to_unrestricted_not_workspace_profile() { + let errors = RefCell::new(Vec::new()); + let test_cases = RefCell::new(Vec::new()); + let mut updater = new_updater(&errors, &test_cases); + + // Set workspace target profile to Base + updater.update_configuration(WorkspaceConfigurationUpdate { + target_profile: Some(Profile::Base), + ..WorkspaceConfigurationUpdate::default() + }); + + // Add notebook with code that requires Unrestricted profile + // This should NOT generate errors, even though workspace is set to Base + updater + .update_notebook_document( + "notebook.ipynb", + &NotebookMetadata::default(), // No target_profile override + [ + ( + "cell1", + 1, + r#"operation Foo() : Unit { + use q = Qubit(); + H(q); + if (M(q) == One) { + Reset(q); + } else { + X(q); + } +}"#, + ), + ] + .into_iter(), + ) + .await; + + // Should have no errors because notebook should default to Unrestricted + expect_errors(&errors, &expect!["[]"]); +} + +#[tokio::test] +async fn notebook_respects_explicit_target_profile_override() { + let errors = RefCell::new(Vec::new()); + let test_cases = RefCell::new(Vec::new()); + let mut updater = new_updater(&errors, &test_cases); + + // Set workspace target profile to Unrestricted + updater.update_configuration(WorkspaceConfigurationUpdate { + target_profile: Some(Profile::Unrestricted), + ..WorkspaceConfigurationUpdate::default() + }); + + // Add notebook with explicit Base profile override + let notebook_metadata = NotebookMetadata { + target_profile: Some(Profile::Base), + ..NotebookMetadata::default() + }; + + updater + .update_notebook_document( + "notebook.ipynb", + ¬ebook_metadata, + [ + ( + "cell1", + 1, + r#"operation Foo() : Unit { + use q = Qubit(); + H(q); + if (M(q) == One) { + Reset(q); + } else { + X(q); + } +}"#, + ), + ] + .into_iter(), + ) + .await; + + // Should have errors because notebook explicitly requested Base profile + expect_errors( + &errors, + &expect![[r#" + [ + uri: "cell1" version: Some(1) errors: [ + cannot use a dynamic bool value + [cell1] [M(q) == One] + ], + ]"#]], + ); +} + #[tokio::test] async fn update_doc_updates_project() { let received_errors = RefCell::new(Vec::new()); From de7e95e441e0613babf2c335219cf20efc224a2f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 22:42:01 +0000 Subject: [PATCH 3/6] Address code review feedback: move notebook default to VS Code layer Co-authored-by: minestarks <16928427+minestarks@users.noreply.github.com> --- language_service/src/state.rs | 51 +--------- language_service/src/state/tests.rs | 95 ------------------- vscode/src/language-service/notebook.ts | 3 +- .../suites/language-service/notebook.test.ts | 39 ++++++++ .../test-unrestricted-default.ipynb | 57 +++++++++++ 5 files changed, 99 insertions(+), 146 deletions(-) create mode 100644 vscode/test/suites/language-service/test-workspace/test-unrestricted-default.ipynb diff --git a/language_service/src/state.rs b/language_service/src/state.rs index 58923e837a..f031b79937 100644 --- a/language_service/src/state.rs +++ b/language_service/src/state.rs @@ -414,7 +414,7 @@ impl<'a> CompilationStateUpdater<'a> { .map(|manifest| manifest.lints) .unwrap_or_default(), }; - let configuration = merge_configurations_for_notebook(¬ebook_configuration, &configuration); + let configuration = merge_configurations(¬ebook_configuration, &configuration); // Compile the notebook and add each cell into the document map let compilation = Compilation::new_notebook( @@ -744,52 +744,3 @@ fn merge_configurations( lints_config: merged_lints, } } - -fn merge_configurations_for_notebook( - compilation_overrides: &PartialConfiguration, - workspace_scope: &Configuration, -) -> Configuration { - let mut merged_lints = workspace_scope.lints_config.clone(); - let mut override_lints = compilation_overrides.lints_config.clone(); - override_lints.retain(|override_lint| { - for merged_lint in &mut merged_lints { - match (merged_lint, override_lint) { - ( - LintOrGroupConfig::Lint(lint_config), - LintOrGroupConfig::Lint(lint_config_override), - ) => { - if lint_config.kind == lint_config_override.kind { - lint_config.level = lint_config_override.level; - return false; - } - } - ( - LintOrGroupConfig::Group(group_config), - LintOrGroupConfig::Group(group_config_override), - ) => { - if group_config.lint_group == group_config_override.lint_group { - group_config.level = group_config_override.level; - return false; - } - } - _ => (), - } - } - true - }); - merged_lints.extend(override_lints); - - Configuration { - // For notebooks, default to Unrestricted instead of workspace target profile - target_profile: compilation_overrides - .target_profile - .unwrap_or(Profile::Unrestricted), - package_type: compilation_overrides - .package_type - .unwrap_or(workspace_scope.package_type), - language_features: compilation_overrides - .language_features - .unwrap_or(workspace_scope.language_features), - lints_config: merged_lints, - } -} diff --git a/language_service/src/state/tests.rs b/language_service/src/state/tests.rs index a095a88d7a..c057bdc59f 100644 --- a/language_service/src/state/tests.rs +++ b/language_service/src/state/tests.rs @@ -853,101 +853,6 @@ async fn update_notebook_reports_errors_from_dependency_of_dependencies() { ); } -#[tokio::test] -async fn notebook_defaults_to_unrestricted_not_workspace_profile() { - let errors = RefCell::new(Vec::new()); - let test_cases = RefCell::new(Vec::new()); - let mut updater = new_updater(&errors, &test_cases); - - // Set workspace target profile to Base - updater.update_configuration(WorkspaceConfigurationUpdate { - target_profile: Some(Profile::Base), - ..WorkspaceConfigurationUpdate::default() - }); - - // Add notebook with code that requires Unrestricted profile - // This should NOT generate errors, even though workspace is set to Base - updater - .update_notebook_document( - "notebook.ipynb", - &NotebookMetadata::default(), // No target_profile override - [ - ( - "cell1", - 1, - r#"operation Foo() : Unit { - use q = Qubit(); - H(q); - if (M(q) == One) { - Reset(q); - } else { - X(q); - } -}"#, - ), - ] - .into_iter(), - ) - .await; - - // Should have no errors because notebook should default to Unrestricted - expect_errors(&errors, &expect!["[]"]); -} - -#[tokio::test] -async fn notebook_respects_explicit_target_profile_override() { - let errors = RefCell::new(Vec::new()); - let test_cases = RefCell::new(Vec::new()); - let mut updater = new_updater(&errors, &test_cases); - - // Set workspace target profile to Unrestricted - updater.update_configuration(WorkspaceConfigurationUpdate { - target_profile: Some(Profile::Unrestricted), - ..WorkspaceConfigurationUpdate::default() - }); - - // Add notebook with explicit Base profile override - let notebook_metadata = NotebookMetadata { - target_profile: Some(Profile::Base), - ..NotebookMetadata::default() - }; - - updater - .update_notebook_document( - "notebook.ipynb", - ¬ebook_metadata, - [ - ( - "cell1", - 1, - r#"operation Foo() : Unit { - use q = Qubit(); - H(q); - if (M(q) == One) { - Reset(q); - } else { - X(q); - } -}"#, - ), - ] - .into_iter(), - ) - .await; - - // Should have errors because notebook explicitly requested Base profile - expect_errors( - &errors, - &expect![[r#" - [ - uri: "cell1" version: Some(1) errors: [ - cannot use a dynamic bool value - [cell1] [M(q) == One] - ], - ]"#]], - ); -} - #[tokio::test] async fn update_doc_updates_project() { let received_errors = RefCell::new(Vec::new()); diff --git a/vscode/src/language-service/notebook.ts b/vscode/src/language-service/notebook.ts index eeb05809d3..9a2b6f6e12 100644 --- a/vscode/src/language-service/notebook.ts +++ b/vscode/src/language-service/notebook.ts @@ -149,6 +149,7 @@ function getQSharpConfigMetadata(notebook: vscode.NotebookDocument): object { log.trace("found Q# config metadata: " + dataString); return JSON.parse(dataString); } else { - return {}; + // Default notebooks to unrestricted target profile when no explicit configuration is provided + return { targetProfile: "unrestricted" }; } } diff --git a/vscode/test/suites/language-service/notebook.test.ts b/vscode/test/suites/language-service/notebook.test.ts index 014c8d55b4..51ddda6b34 100644 --- a/vscode/test/suites/language-service/notebook.test.ts +++ b/vscode/test/suites/language-service/notebook.test.ts @@ -112,4 +112,43 @@ suite("Q# Notebook Tests", function suite() { assert.equal(location.range.start.line, 2); assert.equal(location.range.start.character, 10); }); + + test("Notebook defaults to unrestricted target profile", async () => { + const notebook = await vscode.workspace.openNotebookDocument( + vscode.Uri.joinPath( + workspaceFolderUri, + "test-unrestricted-default.ipynb", + ), + ); + + const qsharpCellUri = notebook.cellAt(1).document.uri; + + // Wait for the document to be processed by the language service + await waitForCondition( + () => + !!notebook + .getCells() + .find((cell) => cell.document.languageId === "qsharp"), + vscode.workspace.onDidChangeNotebookDocument, + 100, + "timed out waiting for Q# code cell", + ); + + // Verify that there are no diagnostics, meaning the unrestricted operation is allowed + await waitForCondition( + () => { + const diagnostics = vscode.languages.getDiagnostics(qsharpCellUri); + // Filter out any unrelated diagnostics and only look for target profile related errors + const profileErrors = diagnostics.filter( + (d) => + d.message.includes("dynamic bool") || + d.message.includes("target profile"), + ); + return profileErrors.length === 0; + }, + vscode.languages.onDidChangeDiagnostics, + 1000, + "expected no target profile related diagnostics for unrestricted operations", + ); + }); }); diff --git a/vscode/test/suites/language-service/test-workspace/test-unrestricted-default.ipynb b/vscode/test/suites/language-service/test-workspace/test-unrestricted-default.ipynb new file mode 100644 index 0000000000..9716271462 --- /dev/null +++ b/vscode/test/suites/language-service/test-workspace/test-unrestricted-default.ipynb @@ -0,0 +1,57 @@ +{ + "cells": [ + { + "cell_type": "code", + "execution_count": null, + "id": "1e8e4faa", + "metadata": {}, + "outputs": [], + "source": [ + "import qsharp\n" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "1b55e53c", + "metadata": { + "scrolled": false + }, + "outputs": [], + "source": [ + "%%qsharp\n", + "\n", + "operation TestUnrestrictedDefault() : Unit {\n", + " use q = Qubit();\n", + " H(q);\n", + " if (M(q) == One) {\n", + " Reset(q);\n", + " } else {\n", + " X(q);\n", + " }\n", + "}\n" + ] + } + ], + "metadata": { + "kernelspec": { + "display_name": "Python 3 (ipykernel)", + "language": "python", + "name": "python3" + }, + "language_info": { + "codemirror_mode": { + "name": "ipython", + "version": 3 + }, + "file_extension": ".py", + "mimetype": "text/x-python", + "name": "python", + "nbconvert_exporter": "python", + "pygments_lexer": "ipython3", + "version": "3.11.4" + } + }, + "nbformat": 4, + "nbformat_minor": 5 +} \ No newline at end of file From 2098598c13f6da95de1616a28c022ad74d59c030 Mon Sep 17 00:00:00 2001 From: Mine Starks <16928427+minestarks@users.noreply.github.com> Date: Thu, 12 Jun 2025 15:56:27 -0700 Subject: [PATCH 4/6] Update copilot-setup-steps.yml --- .github/workflows/copilot-setup-steps.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/copilot-setup-steps.yml b/.github/workflows/copilot-setup-steps.yml index 3981e4d207..83ddb8f83d 100644 --- a/.github/workflows/copilot-setup-steps.yml +++ b/.github/workflows/copilot-setup-steps.yml @@ -27,4 +27,4 @@ jobs: - name: Prereqs run: python ./prereqs.py --install - name: First build - run: python ./build.py --no-check --no-test + run: python ./build.py --no-check --no-test --integration-tests From 09df85ee43bd9c66595ccfb7f6787a14c0e6696d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 23:45:57 +0000 Subject: [PATCH 5/6] Update notebook test to use improved timeouts and align with new test helpers Co-authored-by: minestarks <16928427+minestarks@users.noreply.github.com> --- vscode/test/suites/language-service/notebook.test.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/vscode/test/suites/language-service/notebook.test.ts b/vscode/test/suites/language-service/notebook.test.ts index a713aef7dc..e09be71b1e 100644 --- a/vscode/test/suites/language-service/notebook.test.ts +++ b/vscode/test/suites/language-service/notebook.test.ts @@ -127,22 +127,24 @@ suite("Q# Notebook Tests", function suite() { const qsharpCellUri = notebook.cellAt(1).document.uri; - // Wait for the document to be processed by the language service + // Wait for the Q# cell to be detected by the language service await waitForCondition( () => !!notebook .getCells() .find((cell) => cell.document.languageId === "qsharp"), vscode.workspace.onDidChangeNotebookDocument, - 100, + 2000, "timed out waiting for Q# code cell", ); - // Verify that there are no diagnostics, meaning the unrestricted operation is allowed + // Verify that there are no target profile related diagnostics for unrestricted operations + // We use waitForCondition to ensure we give enough time for any diagnostics to appear, + // then verify that none of them are target profile related await waitForCondition( () => { const diagnostics = vscode.languages.getDiagnostics(qsharpCellUri); - // Filter out any unrelated diagnostics and only look for target profile related errors + // Filter for target profile related errors - there should be none const profileErrors = diagnostics.filter( (d) => d.message.includes("dynamic bool") || @@ -151,7 +153,7 @@ suite("Q# Notebook Tests", function suite() { return profileErrors.length === 0; }, vscode.languages.onDidChangeDiagnostics, - 1000, + 3000, "expected no target profile related diagnostics for unrestricted operations", ); }); From 8cddc89a9d082d3e9d0e3a5f9ab8bbe5cb28e918 Mon Sep 17 00:00:00 2001 From: Mine Starks <16928427+minestarks@users.noreply.github.com> Date: Thu, 12 Jun 2025 16:47:32 -0700 Subject: [PATCH 6/6] Update copilot-setup-steps.yml --- .github/workflows/copilot-setup-steps.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/copilot-setup-steps.yml b/.github/workflows/copilot-setup-steps.yml index 83ddb8f83d..3981e4d207 100644 --- a/.github/workflows/copilot-setup-steps.yml +++ b/.github/workflows/copilot-setup-steps.yml @@ -27,4 +27,4 @@ jobs: - name: Prereqs run: python ./prereqs.py --install - name: First build - run: python ./build.py --no-check --no-test --integration-tests + run: python ./build.py --no-check --no-test