Fix Angular 21 change detection for simulation time, import-csv, connect-database, and ravel-select-horizontal-dim

[Copilot]

• Rebase PR onto develop branch (merged develop into branch)
• Fix CommunicationService: convert t and deltaT to BehaviorSubject<string>
• Fix Header Component: subscribe to t$ and deltaT$, call detectChanges()
• Fix Header Template: use component properties instead of commService.t/deltaT
• Fix Import CSV Component: add this.cdr.detectChanges() at end of load() method
• Fix Connect Database Component: add this.cdRef.detectChanges() at end of getTables()
• Fix Ravel Select Horizontal Dim Component: already in develop via PR Fix dialog rendering in 8 components after Angular 20→21 migration #637

Original prompt

Problem

After the Angular 20→21 migration, four components still have change detection issues where data doesn't update in the UI after async operations complete:

1. Header Component - Simulation Time Not Updating

File: gui-js/libs/ui-components/src/lib/header/header.component.html (lines 118-121)

The template displays t and deltaT values directly from commService:

<div class="time-info toolButton" classType="SimTime">
  <span class="time">t:{{ commService.t }}</span>
  <span class="delta-time">&Delta;t:{{ commService.deltaT }}</span>
</div>

File: gui-js/libs/core/src/lib/services/communication/communication.service.ts (lines 284-293)

The service updates these values but doesn't trigger change detection:

private updateSimulationTime(t: number, deltaT: number) {
  if (Number(this.t) >= this.runUntilTime) {
    this.showPlayButton$.next(true);
  }

  this.t = t.toFixed(2);
  this.deltaT = deltaT.toFixed(2);
}

Root Cause: In Angular 21, the header component needs to manually trigger change detection after commService.t and commService.deltaT are updated. The component already has ChangeDetectorRef injected and uses it for other observables, but not for the simulation time values.

Solution: Subscribe to simulation time updates in the header component and call changeDetectorRef.detectChanges() when they change. Convert t and deltaT to BehaviorSubjects in the service.

2. Import CSV Component - Tab Not Switching After File Load

File: gui-js/libs/ui-components/src/lib/import-csv/import-csv.component.ts (lines 318-347)

The load() method updates selectedTabIndex but doesn't trigger change detection:

async load(selectTab) {
  if (this.url.value === '') return;

  this.setParameterNameFromUrl();

  if (this.url.value.includes('://')) {
    const savePath = await this.electronService.downloadCSV({ windowUid: this.csvDialog.$prefix(), url: this.url.value });
    this.url.setValue(savePath);
    this.files = [savePath];
  }

  const fileUrlOnServer = await this.csvDialog.url();

  if (!this.url.value || this.url.value !== fileUrlOnServer) {
    await this.csvDialog.url(this.url.value);
    await this.csvDialog.guessSpecAndLoadFile();
    await this.getCSVDialogSpec();
    this.updateForm();
  }
  await this.csvDialog.loadFile();
  await this.parseLines();

  for (const tab of this.tabs) {
    tab.disabled = false;
  }

  this.fileLoaded = true;

  this.selectedTabIndex = selectTab;
}

Root Cause: After all the async operations complete, selectedTabIndex is set but Angular 21 doesn't detect the change. The component has ChangeDetectorRef injected but doesn't call it after the async load completes.

Solution: Call this.cdr.detectChanges() at the end of the load() method after selectedTabIndex is updated.

3. Connect Database Component - Table Selector Not Populating

File: gui-js/libs/ui-components/src/lib/connect-database/connect-database.component.ts (lines 44-47, 60-76)

The getTables() method updates this.tables array but doesn't trigger change detection:

async getTables() {
  this.ravel.db.connect(this.dbType,this.connection,"");
  this.tables=await this.ravel.db.tableNames();
}

async selectFile() {
  let options: OpenDialogOptions = {
    defaultPath: ':models',
    filters: [
      { extensions: ['sqlite'], name: 'SQLite' },
      { extensions: ['*'], name: 'All Files' },
    ],
    properties: ['openFile'],
  };
  //if (defaultPath) options['defaultPath'] = defaultPath;
  let filePath = await this.electronService.openFileDialog(options);
  if (typeof filePath==='string')
    this.connection=`db=${filePath}`;
  else
    this.connection=`db=${filePath[0]}`;
  await this.getTables();
}

Template: gui-js/libs/ui-components/src/lib/connect-database/connect-database.html (lines 13-18)

<select (change)="setTable($event)">
  <option disabled selected>--Select Table--</option>
  @for (t of tables; track t) {
    <option [value]="t">{{t}}</option>
  }
</select>

Root Cause: After getTables() completes, the tables array is populated but Angular 21 doesn't detect that the array reference changed. The component has ChangeDetectorRef injected but doesn't use it.

Solution: Call this.cdRef.detectChanges() at the end of getTables() method.

4. Ravel Select Horizontal Dim Component - Numerical Columns Not Populating

File: gui-js/libs/ui-components/src/lib/ravel-select-horizontal-dim/ravel-select-horizontal-dim.component.ts (lines 36-38)

The ngOnInit() method populates dataCols but doesn't trigger change detection:

async ngOnInit() {
  this.dataCols=await this.ravel.db.numericalColumnNames();
}

Template: gui-js/libs/ui-components/src/lib/ravel-select-horizontal-dim/ravel-select-horizontal-dim.html (lines 5-13)

<div class="selectors">
  @for (d of dataCols; track d) {
    <div class...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

*This pull request was created from Copilot chat.*
>

<!-- Reviewable:start -->
- - -
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/highperformancecoder/minsky/639)
<!-- Reviewable:end -->


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->

## Summary by CodeRabbit

* **Bug Fixes**
  * Improved UI responsiveness for simulation time display and overall component rendering through more reliable state management.
  * Strengthened change detection mechanism across multiple data components to ensure consistent and timely display of dynamically loaded content and updates.
  * Fixed potential rendering delays and timing issues that could affect tab selection interactions and data table population during rapid user actions.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

[highperformancecoder]

@copilot Please rebase this PR to develop branch, not master

[highperformancecoder]

@CodeRabbit review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ab68e5d9-6eee-43fe-88b1-9b3511971cf4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Converted simulation time state from plain string properties to BehaviorSubject<string> instances in the communication service, and updated multiple UI components to subscribe to these reactive streams and trigger explicit change detection after asynchronous operations.

Changes

Cohort / File(s)	Summary
Reactive State Management gui-js/libs/core/src/lib/services/communication/communication.service.ts	Replaced t and deltaT string properties with BehaviorSubject<string> instances (t$, deltaT$); updated updateSimulationTime() to read current time via .value and push updates via .next().
Component State and Subscriptions gui-js/libs/ui-components/src/lib/header/header.component.ts	Added t and deltaT public properties; extended ngOnInit to subscribe to commService.t$ and commService.deltaT$, updating local state and triggering change detection on each emission.
Template Binding Updates gui-js/libs/ui-components/src/lib/header/header.component.html	Updated simulation time display bindings from commService.t and commService.deltaT to local component properties t and deltaT.
Change Detection Triggers gui-js/libs/ui-components/src/lib/connect-database/connect-database.component.ts, gui-js/libs/ui-components/src/lib/import-csv/import-csv.component.ts, gui-js/libs/ui-components/src/lib/ravel-select-horizontal-dim/ravel-select-horizontal-dim.component.ts	Added explicit detectChanges() calls after asynchronous operations to ensure immediate view synchronization.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hopping through RxJS streams so bright,
Time flows as subjects, what a delight!
From strings to behaviors, reactively spun,
Change detection syncs when async is done,
Simulation hops in perfect time! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and specifically describes the main changes: fixing Angular 21 change detection issues across four components (simulation time, import-csv, connect-database, ravel-select-horizontal-dim).
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch copilot/fix-header-change-detection

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

gui-js/libs/ui-components/src/lib/header/header.component.ts (1)

81-89: Collapse the two simulation-time subscriptions into one CD trigger.

Line 81-89 currently runs detectChanges() twice per tick (t$ then deltaT$). A combined subscription reduces redundant change detection work.

Proposed refactor

-import { Subject, takeUntil } from 'rxjs';
+import { combineLatest, Subject, takeUntil } from 'rxjs';
@@
-      this.commService.t$.pipe(takeUntil(this.destroy$)).subscribe((t) => {
-        this.t = t;
-        this.changeDetectorRef.detectChanges();
-      });
-
-      this.commService.deltaT$.pipe(takeUntil(this.destroy$)).subscribe((deltaT) => {
-        this.deltaT = deltaT;
-        this.changeDetectorRef.detectChanges();
-      });
+      combineLatest([this.commService.t$, this.commService.deltaT$])
+        .pipe(takeUntil(this.destroy$))
+        .subscribe(([t, deltaT]) => {
+          this.t = t;
+          this.deltaT = deltaT;
+          this.changeDetectorRef.detectChanges();
+        });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@gui-js/libs/ui-components/src/lib/header/header.component.ts` around lines 81
- 89, The two subscriptions to this.commService.t$ and this.commService.deltaT$
cause detectChanges() to run twice; replace them with a single combined
subscription (e.g., combineLatest([this.commService.t$,
this.commService.deltaT$]) or an equivalent operator) that pipes
takeUntil(this.destroy$), assigns both this.t and this.deltaT inside the
subscription callback, and then calls this.changeDetectorRef.detectChanges()
once; update references to the existing destroy$, t, deltaT, commService
observables and changeDetectorRef so behavior remains identical but with a
single CD trigger.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@gui-js/libs/core/src/lib/services/communication/communication.service.ts`:
- Around line 285-291: The run-until check is using the previous observable
value (Number(this.t$.value)) which can overshoot; change the condition to use
the current loop variable t (or Number(t) if t might be a string) when comparing
against runUntilTime so showPlayButton$.next(true) fires immediately; leave the
subsequent updates to this.t$.next(t.toFixed(2)) and
this.deltaT$.next(deltaT.toFixed(2)) as-is.

---

Nitpick comments:
In `@gui-js/libs/ui-components/src/lib/header/header.component.ts`:
- Around line 81-89: The two subscriptions to this.commService.t$ and
this.commService.deltaT$ cause detectChanges() to run twice; replace them with a
single combined subscription (e.g., combineLatest([this.commService.t$,
this.commService.deltaT$]) or an equivalent operator) that pipes
takeUntil(this.destroy$), assigns both this.t and this.deltaT inside the
subscription callback, and then calls this.changeDetectorRef.detectChanges()
once; update references to the existing destroy$, t, deltaT, commService
observables and changeDetectorRef so behavior remains identical but with a
single CD trigger.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: f56f932c-c6dc-4f34-a964-a458fc0f0226

📥 Commits

Reviewing files that changed from the base of the PR and between 7ba2cff and 58792a4.

📒 Files selected for processing (6)

• gui-js/libs/core/src/lib/services/communication/communication.service.ts
• gui-js/libs/ui-components/src/lib/connect-database/connect-database.component.ts
• gui-js/libs/ui-components/src/lib/header/header.component.html
• gui-js/libs/ui-components/src/lib/header/header.component.ts
• gui-js/libs/ui-components/src/lib/import-csv/import-csv.component.ts
• gui-js/libs/ui-components/src/lib/ravel-select-horizontal-dim/ravel-select-horizontal-dim.component.ts

[Copilot]

Pull request overview

Addresses remaining Angular 21 change-detection gaps where UI state wasn’t updating after async operations (simulation time in header, CSV import tab switching, DB table list, and ravel horizontal-dim column list).

Changes:

• Converted CommunicationService simulation time fields from plain strings to BehaviorSubject<string> (t$, deltaT$) and updated the simulation-time update logic to publish via .next().
• Updated Header component/template to subscribe to t$ / deltaT$, store local t / deltaT, and manually trigger change detection.
• Added detectChanges() calls after async UI-state updates in Import CSV, Connect Database, and Ravel Select Horizontal Dim.

Reviewed changes

Copilot reviewed 35 out of 37 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
gui-js/libs/core/src/lib/services/communication/communication.service.ts	Publishes simulation time via BehaviorSubject instead of mutating string fields.
gui-js/libs/ui-components/src/lib/header/header.component.ts	Subscribes to simulation time streams and triggers manual change detection.
gui-js/libs/ui-components/src/lib/header/header.component.html	Binds to component t / deltaT instead of service fields.
gui-js/libs/ui-components/src/lib/import-csv/import-csv.component.ts	Forces CD after async load so selectedTabIndex update renders.
gui-js/libs/ui-components/src/lib/connect-database/connect-database.component.ts	Forces CD after async table-name load so dropdown populates.
gui-js/libs/ui-components/src/lib/ravel-select-horizontal-dim/ravel-select-horizontal-dim.component.ts	Forces CD after async numerical column-name load so list populates.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

@copilot Please rebase this PR to develop branch, not master

Done in b7b8f63. The develop branch has been merged into this PR branch so all develop changes are included. However, I don't have the ability to change the PR's target base from master to develop via the available tools — you'll need to do that manually in the GitHub UI (Edit → Change base → develop).