Code review example: colorIndex is bounded against the series count, not the palette length in Angular

Retired from the DiffDojo practice pool in October 2026; the review below is the canonical answer. An AI wrote this pull request for the task underneath. Read the diff and decide what you would say before you scroll to the review.

Angular · Edge cases & error handling · intermediate

The task

Pin chart series to fixed brand colors

1. Each series pins itself to a fixed brand color via colorIndex; the legend must reject any series whose colorIndex falls outside the palette range, i.e. outside [0, PALETTE.length), before any row is rendered.
2. Because validation guarantees every colorIndex is in range, rows() may index PALETTE directly with no per-row bounds check.
3. The legend renders one row per series, in the same order as the input array (row i is series i): each row shows the series label and the hex of its pinned palette color. Labels are user-authored display text and need not be unique.

Switches the legend from auto-cycling palette colors by position to explicit per-series pinning: each series carries a colorIndex into the fixed brand palette so a metric keeps the same color across every chart. A validation pass up front guarantees every index is in range, so the render path indexes the palette directly. Verified against the sample dashboards.

The series list is user-authored dashboard config; a dashboard routinely has more series than the palette has colors, and each series' colorIndex is hand-entered.

The diff

The review

Issue 1: colorIndex is bounded against the series count, not the palette length

src/app/charts/chart-legend.component.ts, line 41

1. The bound is the wrong length. The check rejects s.colorIndex >= series.length, but series.length is how many series the dashboard config happens to contain, which has nothing to do with how many colors exist. The array rows() indexes is PALETTE, whose length is fixed at 8.
2. A larger config defeats the check. A dashboard with 9 series where one series has colorIndex: 8 passes validation (8 >= 9 is false), because the config's own size vouches for the bad index. rows() then does PALETTE[8] on an 8-element array, gets undefined, and undefined.hex throws Cannot read properties of undefined (reading 'hex').
3. The requirement names the right bound. "Reject any series outside [0, PALETTE.length)" is the spec, and the code checks a different length entirely, so it is objectively wrong. Requirement 2 explicitly lets rows() skip its own bounds check *because* validation was supposed to guarantee this, so the whole safety of the render path rests on this one comparison. (The bug is symmetric: with fewer series than colors it also wrongly *rejects* a valid pin, 3 series with colorIndex: 5 fails 5 >= 3 even though color 5 exists.)

The fix: bound against the array you are about to read: if (!Number.isInteger(s.colorIndex) || s.colorIndex < 0 || s.colorIndex >= PALETTE.length).

The fix

  private validated = computed(() => {
    const series = this.series();
    for (const s of series) {
      if (!Number.isInteger(s.colorIndex) || s.colorIndex < 0 || s.colorIndex >= PALETTE.length) {
        throw new Error(`series "${s.label}" references out-of-range colorIndex ${s.colorIndex}`);
      }
    }
    return series;
  });

Why it matters

Ask yourself: when you bound an index, are you bounding against the array you are about to read, or against some other length that merely looks related? series.length and PALETTE.length are both plausible-looking numbers sitting right next to the index; only one of them is the size of the array being indexed. In TypeScript the bad read is quiet at first, PALETTE[8] is undefined, not an error, so the failure surfaces one line later as a Cannot read properties of undefined thrown mid-render, which in Angular tears down the component tree for the whole chart.
Real-world stakes: this is the mechanism behind the CrowdStrike outage of 19 July 2024. A Falcon rapid-response channel file (Channel File 291) referenced a 21st input field, but the sensor's interpreter was given only 20, and a validation gap let the out-of-range index through. Reading the missing field was an out-of-bounds access that blue-screened about 8.5 million Windows machines worldwide. The mechanism is mundane and language-independent; only the blast radius changes, here a broken dashboard, there a kernel panic.

Read more: angular cannot read properties of undefined template, array index bound check wrong length, crowdstrike channel file 291 out of bounds, off by one input validation

The verdict

Request changes. colorIndex is bounded against the series count, not the palette length

How to review Angular code · All code review examples