Make getCurrentActivityForCell not depend on current position - #3627
Conversation
It's fundamentally passing in a URI and cellID, so resolve those instead of trying to compare them to the active position.
|
This is more for hygiene than to fix a bug, though it is a small behavioral change (if you manage to open another notebook without navigating to it, which is challenging). |
There was a problem hiding this comment.
Pull request overview
Resolves notebook activities from the supplied URI and cell ID instead of stored progress position.
Changes:
- Resolve the owning course and unit from the notebook URI.
- Pass
vscode.Uridirectly from Copilot learning tools.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
source/vscode/src/learning/service.ts |
Resolves cell activity locations from workbook URIs. |
source/vscode/src/gh-copilot/learningTools.ts |
Passes the notebook URI object to the service. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
source/vscode/src/learning/service.ts:546
- The method now returns a full
LearningState, but its doc comment still says it builds aCurrentActivity. Update the contract description so callers are not misled.
getLearningStateForCell(
source/vscode/src/gh-copilot/learningTools.ts:201
- This branch is keyed to whether any notebook editor is active, not whether the target URI is a workbook. If the current course is notebook-based but no notebook editor is active, execution falls through to
readUserCode()and returns the raw.ipynbJSON; conversely, an unrelated active notebook makes a Q# exercise return empty code. Detect the workbook fromuriand use the editor only when it matches.
const editor = vscode.window.activeNotebookEditor;
if (editor) {
let code = "";
if (editor.notebook.uri.toString() === uri.toString()) {
source/vscode/src/gh-copilot/learningTools.ts:382
- This still derives the unit counters from
progress.currentPosition, which is the service's stored position. When the selected cell belongs to another unit—the mismatch this PR is intended to support—the returnedpositiondescribes the selected unit while these counters describe the stored unit. Usestate.position.location.unitIdas the lookup key.
const unitId = progress.currentPosition?.unitId;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
source/vscode/src/gh-copilot/learningTools.ts:199
- This now treats any active notebook as evidence that the current learning activity is notebook-based. If a learner is on a Q# course while an unrelated notebook is active,
uripoints to the Q# exercise but this branch returns an emptycodevalue instead of callingreadUserCode(). Gate the cell-based path on the active course kind (while still allowing the editor itself to be absent) so Q# activities continue to read their source file.
const editor = vscode.window.activeNotebookEditor;
if (editor) {
Dhairya Patel (HABER7789)
left a comment
There was a problem hiding this comment.
I have tested in the dev host. The state now correctly follows the visible notebook tab across units. No issues found.
It's fundamentally passing in a URI and cellID, so resolve those instead of trying to compare them to the active position.
Bonus: tidy up the hint tool so it uses a single state, rather than using two and trying to detect conflicts.