# Budget Screen — Nested FormArray Restructure

Technical documentation of the Budget screen (`costing` MFE) parent/child grid architecture.

**Audience:** a developer picking up this screen for the first time.
**Scope:** documents the implementation as it exists in the working tree, not the original plan.
**Status:** implementation complete; live end-to-end verification still outstanding (see §18).

---

## 1. Overview

The Budget screen shows two grids:

- **CostingDetail** (parent) — the items being costed.
- **CostingMaterial** (child) — the materials for the *selected* detail row.

Users reported material rows disappearing, data being overwritten, and the two grids falling out
of sync when switching detail rows. Five rounds of fixes each removed a symptom and exposed
another: materials landing on the wrong row, totals no longer updating, hierarchical serial
numbers flattening `1, 1.1, 1.2 → 1, 2, 3`, and finally the POST body sending
`CostingMaterialArray` as a **single object** instead of an array, which made the server persist
one material and return none on the next load.

Every one of those traces to the same thing:

> **The parent↔child relation was a convention maintained by hand, not a structure the form
> expressed.**

It was re-derived three different ways over time — array index, then `RowGuid`, then a store `Map`
— and each re-derivation had its own failure mode. The same material row existed in three places
that had to be kept in agreement by explicit code.

The restructure makes the relation **structural**: a material row is physically inside its detail
row, so it cannot belong to the wrong one.

---

## 2. Old Architecture

### The three copies

| # | Copy | Owner | Notes |
|---|---|---|---|
| 1 | `form.CostingDetailArray[i].CostingMaterialArray` | `BudgetComponent` | canonical — the only thing POSTed |
| 2 | `form.CostingMaterialArray` (sibling `FormControl`) | `BudgetComponent` | flat scratch buffer for the selected row; `PostData:false`, stripped at save |
| 3 | `BudgetGridComponent.BudgetFormArrayData` | `BudgetGridComponent` | the live `FormArray` the user actually typed into |

The scratch buffer existed because of a base-class constraint: `GBBaseFormGroup` builds only
`FormControl`s, so a `Type:"Grid"` field became a `FormControl` holding a **raw JS array**. A
`FormControl`'s value is plain data, so a detail row could not *contain* its materials — the
selected row's materials had to be shuttled through a sibling control. That is also why nearly
every handler was `JSON.parse(JSON.stringify(...))` → mutate → `patchValue(...)`. It was forced,
not sloppy.

### The compensating layer

- **`CostingDetailStore`** (`projects/costing/service/costingdetail.store.ts`) — a per-component
  signal store keying materials by the detail row's `RowGuid` plus a load generation counter.
  Introduced to make *one* copy authoritative.
- **`SeedStoreFromLoadedDetailArray()`** — captured the API's materials into the store at each
  load path, **before** the grid could touch them.
- **`RepairMaterialArrayFromStore()`** — wrote stored materials back into `form.CostingDetailArray`
  for rows the grid had corrupted.
- **`BuildSavePayload()`** — rebuilt the POST body at save time from the store, because every
  earlier repair was undone before it could reach the server.
- **`GetCostingDetails()` / `GetBudgetGridData()`** — received the grids' emissions and re-shuttled
  data between the three copies.
- **`GridRefresh` / `BudgetGridRefresh` signals** — toggled to force a grid to rebuild from its copy.

### The root defect: `fb.group()` and the `[value, validator]` tuple

`gb-formgrid` builds each row with `this.fb.group(GridData)`. Angular's `FormBuilder._createControl`
reads a **raw array** as a `[value, validator, asyncValidator]` tuple:

```
[]            → undefined
[m1]          → m1          (an object, not an array)
[m1, m2, m3]  → m1          (m2 consumed as a "validator", m3 as options — both discarded)
```

Silently, with no error. `budget.json`'s `CostingDetailArray.GridArray` lists
`CostingProcessArray`, `CostingChargesArray`, `AlfrescoArray` but **not** `CostingMaterialArray`, so
materials were never pre-converted to a `FormArray` and hit that tuple path every time.

### Why the repairs never held

`gb-formgrid.onGridChange` emits **before** it writes back through the CVA:

```ts
this.value = value;                      // already mangled
this.GridOutPut.emit(this.value);        // → parent repairs → patchValue(good)
if (this.onChange) this.onChange(value); // → CVA writes the MANGLED value back, one line later
```

Every repair performed in response to the emission was overwritten immediately afterwards. This is
why three successive repair attempts all failed: **none of them was a terminal write.**

### The cost

Three representations, two async emitters (a 10 ms debounce and a ~300 ms batched calculation),
an index-based cursor that the grid could silently reset to 0, and repair code whose correctness
depended on the order in which unrelated components emitted. The screen worked only when every one
of those agreed.

---

## 3. New Architecture

```
form (GBBaseFormGroup)
└── CostingDetailArray : FormArray
    ├── [0] Detail FormGroup
    │   ├── ItemName, CostingDetailQuantity, ... : FormControl
    │   ├── RowGuid : FormControl
    │   └── CostingMaterialArray : FormArray
    │       ├── [0] Material FormGroup
    │       │   ├── CostingMaterialLineNumber, CostingMaterialLevel, ... : FormControl
    │       │   ├── RowGuid, rowState, serverRowId : FormControl
    │       │   ├── CostingChargesArray : FormArray
    │       │   └── CostingProcessArray : FormArray
    │       └── [1] Material FormGroup
    └── [1] Detail FormGroup
        └── CostingMaterialArray : FormArray   ← its own, unrelated to [0]'s
```

**The nested `FormArray` is the single source of truth.** There is no copy, no mirror, no store and
no write-back. Both grids edit *these* controls in place.

This also removes the `fb.group()` hazard at the source: `_createControl` only mis-reads a **raw
array**. An `AbstractControl` — which a `FormArray` is — passes through untouched.

---

## 4. Step-by-Step Implementation

> Steps 1–3 are in commit `a26db7336`. Steps 4–6 and the §12 fixes are in the current working tree.

### Step 1 — Nested FormArray

**Files:** `projects/costing/transaction/budget/budget.component.ts`

**Added:**

| Method | Purpose |
|---|---|
| `InstallNestedDetailArray()` | Captures both grids' `field` config, then swaps the flat `FormControl` for a real `FormArray` |
| `BuildDetailFormArray(rows)` | API array → `FormArray<FormGroup>` |
| `BuildDetailFormGroup(row)` | One detail row, with its materials nested |
| `BuildMaterialFormArray(materials)` | Materials, each with *their* nested arrays as `FormArray`s |
| `LoadDetailArray(rows)` | The single entry point every load path uses |
| `DetailFormArray`, `SelectedMaterialArray`, `SelectedDetailRowGuid` | Accessors |

```ts
private InstallNestedDetailArray(): void {
  if (this.DetailFieldConfig) return;              // idempotent

  this.DetailFieldConfig   = (this.form.get('CostingDetailArray') as any)?.field;
  this.MaterialFieldConfig = (this.form.get('CostingMaterialArray') as any)?.field;

  const existing = this.form.get('CostingDetailArray')?.value;
  this.form.setControl('CostingDetailArray', this.BuildDetailFormArray(existing));

  this.form.removeControl('CostingMaterialArray');  // the scratch buffer (Step 6)
  this.cdr.markForCheck();                          // (§12)
}
```

**Why required:** `GBBaseFormGroup` extends `FormGroup`, so `setControl()` is available and the
base class — used by every screen in the product — needs no change. The `field` config that
`gb-formgrid` used to read off `ngControl.control.field` must be captured *first*, because it is
then passed to the new grids as an `@Input` instead.

**Two constraints that shape everything else:**

1. **`patchValue` cannot be used to load.** `FormArray.patchValue` only patches controls that
   already exist — it never adds or removes them. Patching a 5-row response into an empty
   `FormArray` silently loads nothing. Every load path must call `LoadDetailArray()`, which
   *rebuilds*.
2. **Every nested array must itself be a `FormArray`.** A material carries its own
   `CostingChargesArray`/`CostingProcessArray`; if those were left as raw arrays, `fb.group()`
   would flatten them one level down — the same bug, one level deeper.

```ts
group[key] = Array.isArray(value)
  ? this.formBuilder.array(value.map((entry: any) =>
      entry && typeof entry === 'object' ? this.formBuilder.group({ ...entry })
                                         : this.formBuilder.control(entry)))
  : this.formBuilder.control(value);
```

**Behaviour preserved:** `budget.json` unchanged; API request/response unchanged; `RowGuid` stamped
where absent (the server does not persist it); `rowState`/`serverRowId` stamped because
`gb-budgetgrid`'s `removedata()` reads `rowState` to choose between a physical `removeAt` and a
soft delete — metadata the old `populateFormArray()` used to add during its rebuild.

`BuildMaterialFormArray` also tolerates the shapes the *old* broken flow persisted, so records
saved before the fix still load:

```ts
const rows = Array.isArray(materials) ? materials : (materials ? [materials] : []);
```

### Step 2 — `gb-budgetdetailgrid`

**Files (new):** `libs/gbdirectives/src/lib/gbbudgetdetailgrid/gbbudgetdetailgrid.component.{ts,html,scss}`

A Budget-owned parent grid that renders a `FormArray` **directly**. Deliberately **not** a
`ControlValueAccessor`: no `writeValue`, no internal copy, no change emission. It edits the
caller's controls, so there is nothing to synchronise and nothing that can go stale.

Inputs: `detailArray`, `FieldConfig`, `FormData`, `SelectedIndex`, `MenuId`, `HeaderId`,
`ObjectHeaderTypeId`. Outputs: `SelectedIndexChange`, `RowRemoved`, `AttachmentOutPut`.

**Why a new component rather than changing `gb-formgrid`:** see §5.

**Behaviour preserved:** the DOM structure and class names mirror `gb-formgrid`
(`.Grid-sticky-header` > `.Grid-styled-table` > `thead`/`tbody`) so the screen looks identical;
`PicklistPatchValue` reproduces `gb-formgrid`'s `LinkId`/`LinkValue` semantics including the
`RepeatNotAllowed` duplicate dialog; the delete action is kept (`gb-formgrid` defaults
`DeleteRowNeeded = true`), and no add-row is rendered (`IsAddrowRequired: false`).

### Step 3 — `gb-budgetgrid` edits the nested array directly

**Files:** `libs/gbdirectives/src/lib/gbbudgetgrid/gbbudgetgrid.component.{ts,html}`

**Added:** `@Input() materialArray: FormArray`, `@Input() FieldConfig`, `ResolveRowsArray()`,
`AttachNestedMaterialArray()`.

`AttachNestedMaterialArray()` replaces the old rebuild. It clears per-row caches and the
absolute-index visibility sets (which belong to the *previous* row's array), points
`BudgetFormArrayData` at the caller's array, stamps `ownerKey`, and schedules recalculation and
viewport measurement:

```ts
this.BudgetFormArrayData = this.ResolveRowsArray();
this.FormValue           = this.BudgetFormArrayData.value;
this.ownerKey            = this.DetailKey;
// No onGridChange(): there is no second copy to push back to.
```

**Why required:** 37 methods reached their rows through `BudgetGridForm.get('BudgetFormArray')`.
Under the nested architecture that control is not the owner, so every one had to be routed through
`ResolveRowsArray()`. Two more (`getCostingProcessArray`, `getCostingProcesscostArray`) were missed
in the first pass and fixed during Step 6.

**Template change:** the outer `[formGroup]="BudgetGridForm"` + `formArrayName="BudgetFormArray"`
were removed; each row binds its own `[formGroup]`. This matters — wrapping the caller's array in a
new `FormGroup` would call `setParent()` on it and **detach it from its detail row**, so edits
would bubble to the wrapper and `detailRow.value.CostingMaterialArray` would go stale.

### Step 4 — Direct `form.value` POST and direct total updates

**Files:** `budget.component.ts`, `budget.component.html`, `gbbudgetgrid.component.ts`

Both save call sites changed from `this.BuildSavePayload()` to `this.form.value`. See §8.

`GridTotalOutput` rewritten from *stringify → mutate clone → `patchValue(wholeArray)` → toggle
`GridRefresh`* to a direct write on the target detail `FormGroup`, located by `RowGuid` with an
index fallback (§9).

One subtlety: it does **not** use `patchValue`, because `patchValue` silently drops keys with no
matching control, and a detail row is built from the keys the API returned. The old clone-mutate
created the key implicitly, so `addControl` reproduces that:

```ts
for (const key of Object.keys(patch)) {
  const control = row.get(key);
  if (control) control.setValue(patch[key], { emitEvent: false });
  else row.addControl(key, this.formBuilder.control(patch[key]), { emitEvent: false });
}
```

**Behaviour preserved:** the three total-comparison branches and every assigned field are
byte-identical to the previous implementation, so the numbers are unchanged.

### Step 5 — Verification

**Files:** `projects/costing/transaction/budget/budget.roundtrip.spec.ts` (new),
`projects/costing/tsconfig.spec.json`, `projects/costing/tsconfig.app.json`, `angular.json`

The test harness for this project did not work at all. Three independent faults, each of which
produced "0 tests" rather than an error — see §15.

### Step 6 — Legacy cleanup

**Files:** `budget.component.ts`, `budget.component.html`, `gbbudgetgrid.component.ts`,
`projects/costing/service/costingdetail.store.ts` (deleted)

Removed the entire compensating layer once it was unreachable. Full list in §13.

Three non-mechanical parts of this step:

1. **`BudgetCopyFrom` was rewritten, not deleted** — one line inside the legacy mirror block was
   the only thing actually copying the source materials (§11).
2. **`resetForm` needed a replacement for `store.clear()`** — `form.reset()` does not remove
   `FormArray` controls (§10).
3. **`ownerKey` was deliberately kept** (§9).

---

## 5. Why a Budget-owned Detail Grid

`gb-formgrid` was **intentionally left unchanged**.

- It is a `ControlValueAccessor` over a **`FormControl`**, used in **248 places** (227 via
  `formControlName`).
- Its column config is read *only* from `ngControl.control.field`; `Field` is a plain property, not
  an `@Input`, so there is no input-driven mode to switch it into.
- Making it `FormArray`-aware would change the binding contract for every consumer.

Replacing it on this screen only was feasible because the detail grid is small: `CostingDetailArray`
declares 79 columns of which **12 are visible** — 7 read-only displays, 1 numeric
(`CostingDetailQuantity`), 2 picklists (`ItemName`, `UOMName`), 1 dropdown, 1 attachment.

`gb-budgetdetailgrid` therefore reproduces `gb-formgrid`'s *rendering and behaviour* while changing
only the *data binding*. Where the two must agree, the new grid ports `gb-formgrid`'s logic rather
than inventing its own — column visibility, read-only rules, picklist inputs and the attachment
modal are all direct ports (§12).

**A rejected alternative:** adding `"CostingMaterialArray"` to `budget.json`'s `GridArray`. That
would fix the outer array but `patchSubItems` builds each element with `fb.group(SubItemData)`, so
every material's own nested `CostingChargesArray` would be mangled one level down. Strictly worse.

---

## 6. Material Grid

`gb-budgetgrid` now receives the selected row's real array:

```html
<gb-budgetgrid [materialArray]="SelectedMaterialArray"
               [FieldConfig]="MaterialFieldConfig"
               [DetailKey]="SelectedDetailRowGuid"
               (TotalEmit)="GridTotalOutput($event)">
```

```ts
public get SelectedMaterialArray(): FormArray | null {
  const row = this.DetailFormArray?.at(this.CostingDetailIndex) as FormGroup | undefined;
  return (row?.get('CostingMaterialArray') as FormArray) ?? null;
}
```

Switching detail rows is a **reference swap**. `ngOnChanges` sees the new `materialArray` and calls
`AttachNestedMaterialArray()`. Nothing is flushed on the way out — the outgoing row's controls were
edited in place and are already correct.

**Removed:** the `ControlValueAccessor` implementation (`writeValue`, `registerOnChange`,
`registerOnTouched`, `setDisabledState`, `onChange`, `onTouched`, the `@Self() ngControl`
injection), the internal `BudgetGridForm` + `BudgetFormArray` copy, `InitiateFormGrid`,
`populateFormArray`, `patchFormArray`, `GridOutPut`/`emitGridOutput`, and the 10 ms
`gridChangeSubject` debounce.

**How each feature is preserved:**

| Feature | How |
|---|---|
| **Calculations** | The formula code is untouched; it now operates on the real array instead of a copy. `triggerFullRecalculation()` runs 300 ms after attach. Guards were added so a stale index cannot resolve to `undefined`. |
| **Hierarchy / indent / outdent** | `handleLevelFront`/`handleLevelBack`/`handleNestedSubArray` already set `CostingMaterialLevel` on the live control. `LevelIndent` gained the `onGridChange` notification every sibling mutation already made. |
| **Insert / delete** | Operate directly on the caller's array. `removedata()` still reads `rowState` for physical-vs-soft delete, which is why `BuildMaterialFormArray` stamps it. |
| **Picklist** | Unchanged bindings; `FormValue` (a plain snapshot of the rows) still feeds `[FormData]`. |
| **Attachment** | Unchanged; addressed by `RowGuid`/`HeaderId`, never by row index. |
| **Validation** | Unchanged — validators live on the controls. |
| **Selection** | `selectedRowIndex` reset on attach. |
| **Virtual scroll** | Unchanged, plus an ordering fix: `updateViewportHeight()` now assigns the height, calls `detectChanges()` to flush `[style.height.px]`, and only then calls `checkViewportSize()`. CDK caches `getBoundingClientRect()` as `_viewportSize`, so measuring first cached the *outgoing* row's height and truncated the rendered range — that was the "1.1.1 row invisible until you click indent" bug. |
| **`onGridChange`** | Kept, reduced to refreshing the `FormValue` snapshot the picklist cells read. It is no longer a write-back. |

---

## 7. Parent-Child Ownership

```
CostingDetailArray
├── [0] Detail "NEW PROJECT"          RowGuid: aaa…
│   └── CostingMaterialArray
│       ├── [0] Material  1     level 1
│       └── [1] Material  1.1   level 2
└── [1] Detail "Bearing 6314"         RowGuid: bbb…
    └── CostingMaterialArray
        ├── [0] Material  2     level 1
        └── [1] Material  2.1   level 2
```

Selecting Detail 2 changes **which `FormArray` the getter returns**. Material `1.1` is not moved,
copied, cleared or reloaded — it is still at `CostingDetailArray[0].CostingMaterialArray[1]`,
because that is the only place it has ever been.

**Before**, switching rows meant: flush the grid's copy into the leaving row, update the store
bucket, move the cursor, then load the new row's materials into the scratch control and force the
grid to rebuild. Five ordered operations, any of which could be pre-empted by a debounced emission
that was still in flight. The entire handler is now:

```ts
public CostingDetailIndexOutPut(event: any) {
  const newIndex = Number(event);
  if (Number.isNaN(newIndex) || newIndex < 0 || newIndex >= (this.DetailFormArray?.length ?? 0)) return;
  this.CostingDetailIndex = newIndex;
}
```

A material row **cannot** be attributed to the wrong detail row, because it is physically inside
the right one.

---

## 8. Save / POST Flow

```
CostingDetailArray : FormArray
        │  .value  (Angular walks the tree)
        ▼
{ CostingDetailArray: [ { …, CostingMaterialArray: [ {…}, {…} ] } ] }
        │
        ▼
formsaveservice('budget', this.form.value)  →  Costing.Budget.Save
```

```ts
this.service.formsaveservice('budget', this.form.value)
```

`this.form.value` on a nested `FormArray` already produces the exact API shape.
`budget.service.ts` is untouched.

**Why the repair layer is gone:** the payload was only ever wrong because `fb.group()` mis-read a
raw array as a `[value, validator]` tuple. There is no raw array on this path any more, so there is
nothing to corrupt and nothing to repair. `BuildSavePayload()` existed solely because repairs
performed earlier in the cycle were overwritten by the CVA write-back one line later; with no CVA
and no second copy, the terminal-write problem does not exist.

The scratch control was removed outright. That is payload-safe: `budget.json` marks
`CostingMaterialArray` `PostData: false`, and `budget.service.ts` skips such fields when building
`criteria`, so it never reached the API. It is not `Required`, so validation does not read it
either.

---

## 9. Async Calculation and `ownerKey`

`ownerKey`/`DetailKey` was **deliberately retained** after the store was deleted.

The original plan called for removing it, on the reasoning that "the parent patches totals onto
`detailArray.at(selectedIndex)`, which cannot misroute because there is only one array". That holds
for **edits** — they are synchronous and land on the control the user typed into. It does **not**
hold for **totals**, which are produced by an asynchronous batched calculation that yields to the
browser once per 50-row batch:

```
t=0    user is on Detail 1 → calculation starts, ~300 ms of work
t=120  user clicks Detail 2 → CostingDetailIndex = 1
t=310  calculation finishes and emits its totals
       ↳ reading CostingDetailIndex here would write Detail 1's totals onto Detail 2
```

So the grid stamps the owning row **once, at attach time**, and carries it through:

```ts
this.ownerKey = this.DetailKey;              // in AttachNestedMaterialArray()
...
this.TotalEmit.emit({ ...totals, OwnerKey: owner });
```

Stamping at *attach* rather than at *emit* is the crux: Angular applies input bindings **before**
`ngOnChanges`, so reading the live `DetailKey` when the emission fires would label the old row's
data with the new row's key and defeat the guard entirely.

The parent resolves by key, falling back to the index:

```ts
let target = -1;
if (event.OwnerKey) {
  target = detailArray.controls.findIndex(
    (row) => (row as FormGroup).get('RowGuid')?.value === event.OwnerKey
  );
}
if (target === -1) target = this.CostingDetailIndex;
```

The fallback matters: identity-only matching dropped every total when `OwnerKey` was empty — that
was the "calculations stopped working" report. A key rather than an index because an index is valid
only for one array at one moment, and inserts, deletes and reloads all happen on this screen.

`Generation` (the store's staleness counter) **was** removed — it had no source once the store was
gone.

---

## 10. Add New / Reset

**Symptom:** after loading a Budget document, Add New left every detail row and its materials on
screen, values intact.

**Two independent causes.**

**(a) The form reset walks straight past a `FormArray`.** `GBBaseFormGroup.reset()`
(`libs/common/src/lib/gbformgroup/gbformgroup.ts`) handles only two shapes:

```ts
if (control instanceof FormControl) { … }
else if (control instanceof FormGroup) { … }
// a FormArray is neither → skipped in silence
```

While `CostingDetailArray` was a `FormControl` holding a raw array, reset set it to its
`DefaultValue` (undefined → empty) and the grid cleared. As a real `FormArray` it is skipped
entirely — not even nulled. Note also that `AddNew` never calls the component's own `resetForm()`;
that runs only from `handleFormResult` after a successful Save/Delete. The Add New path depends
entirely on `gbformaction`'s `performAddNew()` calling `FormDetails.reset()`.

**Why `FormArray.reset()` alone would not have been enough either:** Angular's own
`FormArray.reset()` nulls the values but **keeps the controls**, which would leave a grid full of
blank rows rather than an empty grid.

Replacing the control is the only thing that actually removes them — and because materials are
*inside* their detail row, dropping the row drops its materials with it:

```ts
private ClearDetailArray(): void {
  this.LoadDetailArray([]);   // setControl with a fresh, empty FormArray
}
```

Called from `resetForm()` and, for Add New, in the existing microtask (`performAddNew()` emits
`'AddNew'` *before* calling `FormDetails.reset()`; everything it does after the emit is synchronous,
so the microtask lands last):

```ts
Promise.resolve().then(() => {
  this.MakeHeaderUUId();
  this.ClearDetailArray();
});
```

**(b) The material grid would not have detached.** `AttachNestedMaterialArray()` opened with
`if (!this.materialArray) return;` placed *before* the reassignment of `BudgetFormArrayData`. The
`rows` getter reads that field, so once the detail array emptied and `SelectedMaterialArray` went
`null`, the grid held its reference to the row that had just been discarded and kept rendering the
previous document's materials. The guard is now a proper detach: state is cleared and
`BudgetFormArrayData` is reassigned through `ResolveRowsArray()` (which yields an empty array),
*then* it returns before the recalculation and viewport timers.

**Fixing only the parent would have cleared the top grid and left the bottom one populated.**

---

## 11. Copy From

Copy From replaces the materials of **only the currently selected** detail row, from the *first*
row of the copied-from costing. That asymmetry is intentional and preserved.

```ts
const target = this.DetailFormArray?.at(this.CostingDetailIndex) as FormGroup | undefined;
if (!target) { this.CopyFrom = false; return; }

target.setControl(
  'CostingMaterialArray', this.BuildMaterialFormArray(sourceDetail.CostingMaterialArray)
);

target.get('CostingChargesArray')?.patchValue([]);
target.get('CostingProcessArray')?.patchValue([]);
```

**Why `setControl` and not `patchValue`:** the source and target rows rarely hold the same number of
materials, and `patchValue` on a `FormArray` can only patch controls that already exist,
positionally. Copying 5 materials onto a row that has 2 would patch 2 and silently discard 3;
copying onto an empty row would load nothing at all. `setControl` swaps the whole array.

No refresh toggle is needed: `SelectedMaterialArray` now returns the new `FormArray` reference,
which is what drives the material grid's `ngOnChanges`.

The charges/process blanking is carried over verbatim from the previous implementation. It remains
flagged in code with a `NOTE:` — **it is still unconfirmed with the API team whether this blanking
is intended behaviour**.

---

## 12. Rendering / UI Fixes

Six defects found after the restructure. All were in the new detail grid or its bindings; none was
caused by the nesting itself.

### 12.1 Duplicate Item column

**Root cause.** `budget.json` declares two *visible* columns at `SNo: 2` with the same label, made
mutually exclusive by a dependency gate on the header field `CostingAnalysisType`:

| value | Type | shown when |
|---|---|---|
| `ItemName` | Picklist | `CostingAnalysisType == 1` |
| `CostingDetailProductDescription` | ReadOnly | `CostingAnalysisType ∈ {0,3,4}` |

`gb-formgrid` gates every `<th>`/`<td>` on
`IsVisible && (DependendFieldName == undefined || DependendFieldValue == undefined || DependendFieldValue.split(',').includes(FormData[DependendFieldName]))`.
The new grid filtered on `IsVisible` alone, so both rendered — and since `CostingAnalysisType`
defaults to `3`, the `ItemName` cell was always the blank one.

**Fix.** `IsColumnVisible(column)` reproducing that predicate, used by the `columns` getter so
header and body cannot desync. Null-safe: a missing value is coerced to `''`, which matches neither
list — the same outcome `gb-formgrid` produces.

**Final behaviour.** Exactly one Item column at every `CostingAnalysisType`.

### 12.2 Rows not rendering until the cursor moved

**Root cause.** `BudgetComponent` is `OnPush` and injects `ChangeDetectorRef`, but **none** of the
~10 sites that install `CostingDetailArray` nudged change detection. Loads arrive in RxJS
`subscribe` callbacks, which do *not* mark an `OnPush` component dirty, so the template was never
re-evaluated and the new `FormArray` from `setControl` never reached the child's input. Hovering
appeared to fix it because `markForCheck()` from an event in any descendant marks **ancestors**
dirty too. The app is zone-based (`provideZoneChangeDetection({ eventCoalescing: true })`); this was
plain `OnPush`, not a zone escape.

**Fix.** `this.cdr.markForCheck()` at the end of `LoadDetailArray()` — every load path and
`ClearDetailArray()` funnel through it — and at the end of `InstallNestedDetailArray()`, which calls
`setControl` directly.

**Final behaviour.** Rows render as soon as the response lands.

### 12.3 UOM picklist rendering blank

**Root cause.** In grid mode (`GridIndex !== -1`) `gb-newpicklist` resolves its own row **out of an
array**:

```ts
private handlePicklistDataInGrid(): void {
  if (!Array.isArray(this.FormData) || this.GridIndex < 0 || this.GridIndex >= this.FormData.length) return;
```

That method is the only place its internal `Field` is assigned, and both halves of its template are
`*ngIf="Field && …"`. The new grid passed `[FormData]="row.value"` — a single object — so
`Array.isArray` was false, `Field` stayed `undefined`, and the cell rendered **nothing**.

**Fix.** `[FormData]="detailArray.value"`, matching `gb-formgrid`'s `[FormData]="value"`
(`= GridArray.value`) and `gb-budgetgrid`'s `[FormData]="FormValue"` (`= materialArray.value`).
`AbstractControl.value` is a stored property, so its reference changes only when the form value
does — which is what `gb-newpicklist`'s `ngOnChanges` (`changes['GridIndex'] || changes['FormData']`)
needs to re-read.

**Final behaviour.** UOM displays its value. It remains **non-clickable**, which is correct: the
column's `PicklistDetail.ReadOnly: true` makes `OnPicklistClick` return early *by design* — UOM is
populated from the Item picklist, not selected directly.

### 12.4 Item picklist not editable

**Root cause.** Two faults. The parent passed `[FormEditable]="ReadOnlyPicklist"`, a flag used
everywhere else as `[gbReadOnly]="!ReadOnlyPicklist"` — it gates the **header** selection picklists
and flips to `false` once a document loads, so loading a document marked every grid cell read-only.
And `IsCellReadOnly` treated the field-level `ReadOnly: true` as "every non-number cell is
read-only", a rule that exists nowhere in `gb-formgrid` — `Field.ReadOnly` is not referenced in that
component at all.

**Fix.** Reproduce `gb-formgrid`: `@Input() MenuId`, inject `FormActionservice`, and

```ts
public FormEdit: Signal<boolean> = computed(() => {
  const menuData = this.formactionservice.isMenuEditable();
  return menuData.find((obj: { id: any }) => obj.id === this.MenuId)?.editable ?? true;
});
```

with `IsCellReadOnly(column, rowIndex)` = `!FormEdit() || IsReadOnlyByPrimaryId(...) || column.ReadOnly`.
`gb-formgrid` reads `MenuId` off `ngControl.control.MenuId`; a `FormArray` built by a plain
`FormBuilder` carries no such property, so the parent passes it.

**Final behaviour.** Cell editability matches the old screen, driven by the same service.

### 12.5 Attachment icon did nothing

**Root cause.** `gb-formgrid` **hosts the attachment dialog itself** — its icon calls an internal
`openAttachmentModal(i)` and its own template renders `<gb-attachment>` in a modal. The new grid
instead emitted an `AttachmentOpen` output that **nothing bound**, so the click went nowhere.

This was not fallout from Step 6's removal of `TailRowIndex`/`SubtailRowIndex` — those carried the
parent's detail cursor, not a row identity. The flow needs only the row index, from which it reads
two values off the row itself.

**Fix.** Port `openAttachmentModal`, the three state fields (`AttachmentModelWindow`,
`AttachmentObjectId`, `CurrentRowId`), `AttachmentDataget`, the modal markup and its styles:

```ts
const rawId = row.get(this.FieldConfig?.GridId ?? '')?.value ?? 0;   // CostingDetailId
this.AttachmentObjectId = rawId == -1 ? 0 : rawId;                   // unsaved → 0
this.CurrentRowId       = row.get('RowGuid')?.value ?? '';           // identity before save
this.AttachmentModelWindow = true;
```

**Final behaviour.** The icon opens the same dialog as the old screen, addressed identically.

### 12.6 Unwanted `more_vert` icon

**Root cause.** `gb-formgrid` has branches for 27 column types and **no `dropdown` branch**. The
`["Attachments","CommentDTO"]` column (`Type: "Dropdown"`) therefore falls through every branch and
renders an **empty cell**. The `more_vert` icon existed only in the new grid — a placeholder added
during implementation.

**Fix.** Removed the block; the cell is now empty, as in the old grid.

**Final behaviour.** No `more_vert`; the attachment column is unaffected.

---

## 13. Removed Legacy Code

| Removed | File | Reason |
|---|---|---|
| `CostingDetailStore` (whole class, 207 lines) | `projects/costing/service/costingdetail.store.ts` *(deleted)* | Existed only to make one of three copies authoritative. With one copy there is nothing to arbitrate. |
| `providers: [CostingDetailStore]`, `store` field, import | `budget.component.ts` | Store gone. |
| `BuildSavePayload()` | `budget.component.ts` | Rebuilt the POST from the store because earlier repairs were overwritten. `form.value` is now correct by construction. |
| `SeedStoreFromLoadedDetailArray()` | `budget.component.ts` | Captured materials before the grid could corrupt them. Nothing corrupts them now. |
| `RepairMaterialArrayFromStore()` | `budget.component.ts` | Wrote stored materials back over corrupted rows. |
| `GetCostingDetails()` | `budget.component.ts` | Received `gb-formgrid`'s emission and re-shuttled data between copies. No emission exists. |
| `GetBudgetGridData()` | `budget.component.ts` | Same, for the material grid. |
| `GridOutput()` + `delay()` | `budget.component.ts` | Already dead; re-read `CostingDetailIndex` at call time. |
| `CostingMaterialArray` scratch control | `budget.component.ts` (`removeControl`) | The flat buffer for the selected row's materials. `PostData:false`, so removing it cannot change the payload. |
| Scratch-buffer `patchValue` writes (4 load paths) | `budget.component.ts` | Duplicated data the nested array already holds. |
| `GridRefresh`, `BudgetGridRefresh` signals + 13 toggles | `budget.component.ts` | Forced a grid to rebuild from its copy. |
| `writeValue`, `registerOnChange`, `registerOnTouched`, `setDisabledState` | `gbbudgetgrid.component.ts` | The CVA contract, i.e. the second copy. |
| `implements ControlValueAccessor`, `onChange`, `onTouched`, `@Self() ngControl` | `gbbudgetgrid.component.ts` | Same. |
| `InitiateFormGrid()` | `gbbudgetgrid.component.ts` | Rebuilt an internal `FormArray` from a copied value on every refresh. |
| `populateFormArray()` | `gbbudgetgrid.component.ts` | Built the internal copy row by row. Its `rowState`/`serverRowId` stamping moved into `BuildMaterialFormArray`. |
| `patchFormArray()` | `gbbudgetgrid.component.ts` | A second full clear-and-rebuild after `populateFormArray`. |
| `BudgetGridForm` + `BudgetFormArray` | `gbbudgetgrid.component.ts` | The internal copy's host `FormGroup`. |
| `@Output() GridOutPut`, `emitGridOutput()` (+9 call sites) | `gbbudgetgrid.component.ts` | The write-back channel. Unbound by the parent. |
| `gridChangeSubject` + 10 ms debounce | `gbbudgetgrid.component.ts` | Debounced the write-back. |
| `@Input() GridRefresh` | `gbbudgetgrid.component.ts` | Rebuild trigger. |
| `@Input() TailRowIndex`, `@Input() SubtailRowIndex` | `gbbudgetgrid.component.ts` | Index-based parent/child mapping. `SubtailRowIndex` was written but never read; `TailRowIndex` never read. |
| `@Input() Generation`, `ownerGeneration` | `gbbudgetgrid.component.ts` | The store's staleness counter — no source once the store was gone. |
| `UsesNestedArray` getter, legacy `ngOnChanges` branch | `gbbudgetgrid.component.ts` | Only one path remains. |
| `import { url } from 'inspector'`, `import { eventNames } from 'process'` | `budget.component.ts` | Unused Node imports (IDE auto-import accidents) in a browser component. |

`onGridChange` was **kept** but reduced: it no longer emits or writes back, it only refreshes the
`FormValue` snapshot the picklist cells read.

---

## 14. Files Changed

### Production

| File | Change |
|---|---|
| `projects/costing/transaction/budget/budget.component.ts` | Nested builders, load/clear, direct totals, `form.value` save, Copy From rewrite, legacy removal, `markForCheck` |
| `projects/costing/transaction/budget/budget.component.html` | `gb-budgetdetailgrid` replaces `gb-formgrid`; `[materialArray]`/`[DetailKey]` on the material grid; `[MenuId]` |
| `libs/gbdirectives/src/lib/gbbudgetdetailgrid/gbbudgetdetailgrid.component.ts` | New grid — column gating, editability, picklist patching, attachment modal |
| `libs/gbdirectives/src/lib/gbbudgetdetailgrid/gbbudgetdetailgrid.component.html` | Table markup, picklist binding, attachment modal |
| `libs/gbdirectives/src/lib/gbbudgetdetailgrid/gbbudgetdetailgrid.component.scss` | `gb-formgrid`-matching styles + modal chrome |
| `libs/gbdirectives/src/lib/gbbudgetgrid/gbbudgetgrid.component.ts` | Nested array input, CVA removal, `ResolveRowsArray`, viewport ordering |
| `libs/gbdirectives/src/lib/gbbudgetgrid/gbbudgetgrid.component.html` | Per-row `[formGroup]`; modals rebased; `[itemSize]="ITEM_SIZE"` |
| `projects/costing/service/costingdetail.store.ts` | **Deleted** |

`gb-formgrid`, `GBBaseFormGroup`, `budget.service.ts` and `budget.json` are **not** modified.

### Tests

| File | Change |
|---|---|
| `projects/costing/transaction/budget/budget.roundtrip.spec.ts` | **New** — 30 specs |

### Build / test configuration

| File | Change | Why |
|---|---|---|
| `projects/costing/tsconfig.app.json` | `exclude: ["**/*.spec.ts"]` | The `include` globs sweep up specs, pulling jasmine globals into the app build. **Required whenever a non-empty spec exists**, or `ng build costing` fails. |
| `projects/costing/tsconfig.spec.json` | `include` → `**/*.spec.ts`; `types` += `node` | The old `src/**` matched nothing (screens live in `transaction/`). `node` is needed for `libs/common`'s runtime `require()`. |
| `angular.json` (costing `test` target only) | builder → `@angular/build:karma`; `include` += `../**/*.spec.ts` | See §15. |

---

## 15. Testing and Verification

### Unblocking the harness

The costing test target could not run any spec outside `src/`. Three independent faults, each
producing "0 tests" rather than an error:

1. `tsconfig.spec.json` included `src/**/*.spec.ts`, but Budget lives in `transaction/`.
2. The karma spec-finder globs with `cwd: projectSourceRoot` (`projects/costing/src`) — despite its
   schema documenting "relative to project root" — so nothing outside `src/` was reachable. Fixed
   with `"../**/*.spec.ts"`.
3. The webpack karma builder could not compile the styles: shared `features/**` SCSS uses
   `@import url('….scss')`, which Sass leaves for css-loader to parse as plain CSS, where `//`
   comments are syntax errors. The app never hit this because it builds with esbuild. Fixed by
   switching the costing test target to `@angular/build:karma`, the esbuild runner.

### Automated results (current session)

| Check | Result |
|---|---|
| `npx tsc --noEmit -p projects/costing/tsconfig.app.json` | **Clean** |
| `npx ng test costing --watch=false --browsers=ChromeHeadless` | **33 SUCCESS, 0 failed** (30 in this spec + 3 pre-existing app-shell specs) |
| `npx ng build costing --no-ssr` | **Passed** after Step 6. **Not re-run** after the §12.3–12.6 fixes — see §18. |

### What the specs cover

| Suite | Guards |
|---|---|
| *Budget detail grid columns* (7) | One Item column at each `CostingAnalysisType`; no duplicate labels; both hidden when the value matches neither list; `IsVisible` honoured; SNo ordering; missing header field does not throw |
| *the shape that kept breaking* (6) | `CostingMaterialArray` posts as an array of 24 not an object; two materials stay two rows; `CostingMaterialLevel` `[1,2,2]` preserved; nested charges/process stay arrays; every API field round-trips; exactly three client-only keys added (`RowGuid`, `rowState`, `serverRowId`) |
| *each detail row owns its own materials* (4) | Row isolation; edit confinement; add-to-selected-only; `RowGuid` stamping |
| *totals land on the row they were computed for* (6) | Late batch applied by `OwnerKey` after a switch; index fallback when unstamped; deleted owner does not throw; missing total column created; no write when unchanged; empty event/array safe |
| *Add New starts from a genuinely empty grid* (4) | Rows and nested materials dropped; `SelectedMaterialArray` null so the grid detaches; fresh load after clear; `reset()` alone insufficient |
| *records saved by the old broken code still load* (3) | Object → one-row array; missing → `[]`; null rows skipped |

### Live verification — NOT yet completed

The following require the running app against a backend and **have not been performed**:

| Scenario | Status |
|---|---|
| GET → Edit → POST → GET round trip, payload diff | **Not run** |
| Load paths: Order, Quotation, Enquiry, Version Load, Copy From | **Not run** |
| Add New (after the §10 fix) | **Not run** |
| Hierarchy (indent/outdent, `1.1.1` visibility) | Partially exercised during development; not formally re-verified |
| Calculations (qty/rate/value/tax, detail totals = sum of materials) | **Not run** |
| Attachment dialog (after the §12.5 fix) | **Not run** |
| Picklists — UOM display, Item selection populating UOM/Qty via `LinkId`/`LinkValue` | **Not run** |

The payload diff is the highest-value check: this is the first change that alters how the body is
*built* rather than repairing it afterwards. Capture `Costing.Budget.Save` before and after on the
same record; the only permitted difference is `CostingMaterialArray` being a correct array rather
than a truncated object.

---

## 16. Before vs After

| | Before | After |
|---|---|---|
| Representations of a material row | 3 (form nested, scratch control, grid copy) | **1** |
| Parent↔child link | Convention — array index, then `RowGuid`, then a store `Map` | **Structural** — physically nested |
| Detail-row switch | Flush copy → update store → move cursor → reload scratch → force rebuild | Reference swap |
| Save | `BuildSavePayload()` reassembling from the store | `this.form.value` |
| Payload correctness | Repaired after the fact, and overwritten by the CVA one line later | Correct by construction |
| `fb.group()` tuple hazard | Live on every emission | Cannot occur — no raw arrays |
| Detail grid | `gb-formgrid` (CVA over a `FormControl`) | `gb-budgetdetailgrid` (renders a `FormArray`) |
| Material grid | CVA + internal copy + 10 ms debounced write-back | Edits the caller's controls in place |
| Change propagation | `GridRefresh` toggles forcing rebuilds | Input reference change |
| Automated tests | None runnable in this project | 33 |
| Budget-related LOC | — | **≈ −480 net** (919 deleted / 438 added) |

```
BEFORE                                   AFTER

form.CostingDetailArray[i]               form.CostingDetailArray : FormArray
   .CostingMaterialArray  ◄──repair──┐      └── Detail FormGroup
form.CostingMaterialArray (scratch) ──┤          └── CostingMaterialArray : FormArray
BudgetGridComponent copy ─────────────┘                  └── Material FormGroup
   ▲         write-back (debounced 10 ms)
   └── CostingDetailStore (RowGuid → materials)     one array, edited in place
```

---

## 17. Risks and Design Decisions

**`gb-formgrid` was not changed.** 248 consumers, and its column config is readable only from
`ngControl.control.field`. A Budget-specific grid was cheaper and safer than a binding-contract
change across the product. The cost is duplicated rendering logic between the two grids — mitigated
by porting `gb-formgrid`'s logic rather than reinventing it, and by comments pointing at the source.
The same reasoning kept `GBBaseFormGroup` untouched in §10, even though its `reset()` genuinely
cannot see `FormArray`s: it is the base class for every screen in the product, so the gap is handled
where the `FormArray` shape is owned.

**`ownerKey` remains.** Removing it would reintroduce a real race for asynchronously-computed
totals (§9). It is the one surviving piece of the old owner-stamping protocol, and it has a spec.

**Loads rebuild rather than patch.** `FormArray.patchValue` cannot add or remove controls, so a
response with a different row count than the current array would silently load partial data.
`LoadDetailArray()` is the single entry point; **any new load path must call it**.

**`setControl` for Copy From.** Same reason, at row scope (§11).

**`markForCheck` is required, not optional.** `BudgetComponent` is `OnPush` and every load lands in
a `subscribe`. Any *new* code path that replaces `CostingDetailArray` outside `LoadDetailArray()`
must nudge change detection, or the grid will silently not update.

**Known open questions** — both carry `NOTE:` comments in code and need the API team:

1. Does `Costing.Budget.Save` expect soft-deleted material rows present-with-flag, or absent?
   Current behaviour (posting them with `rowState: 'Deleted'`) is preserved as-is.
2. Is the `CostingChargesArray`/`CostingProcessArray` blanking on the Copy From target row
   intentional save behaviour? Preserved verbatim.

---

## 18. Final Status

**Complete**

- Nested `FormArray` architecture (Steps 1–4), with the nested array as the single source of truth.
- Legacy layer removed (Step 6): store, scratch buffer, repair/mirror methods, CVA path, refresh
  signals — 919 lines deleted against 438 added.
- Test harness unblocked and 30 regression specs added; **33/33 green**.
- Post-restructure fixes: Add New reset, duplicate Item column, OnPush rendering, UOM picklist
  binding, cell editability, attachment modal, `more_vert` removal.
- `tsc --noEmit` clean.

**Outstanding**

1. **`ng build costing --no-ssr` has not been re-run** since the attachment and picklist fixes.
   Those added a component import, a new template block and new SCSS — none of which `tsc`
   validates. This must pass before the change is considered done; an invalid SCSS selector once
   broke the whole stylesheet and reached the user precisely because only `tsc` had been run.
2. **Live GET → Edit → POST → GET has not been performed** (§15). The payload diff, the five load
   paths, Add New, Copy From, calculations, hierarchy, attachments and picklists all still need a
   browser pass.
3. **Two API questions remain open** (§17) and block any change to soft-delete or charges/process
   handling.

**Not included in this work.** The working tree also contains unrelated edits — `gbcharges`,
`libs/telemetry`, `gbconfig.json`, `itemmaster.json`, four `projects/sales/**` files — plus a debug
`console.log` in `budget.service.ts` and a `budget.json` edit removing the UOM picklist's
`ReadOnly: true`. Neither of the last two belongs to this restructure and both should be reverted
before committing.