Skip to content

test(core-components): promote duplicate-dom-ids-regression to @stable (#1109) - #1327

Merged
Victor-w-Madeira merged 4 commits into
mainfrom
test/issue-1109-promote-duplicate-dom-ids-stable
Aug 6, 2026
Merged

test(core-components): promote duplicate-dom-ids-regression to @stable (#1109)#1327
Victor-w-Madeira merged 4 commits into
mainfrom
test/issue-1109-promote-duplicate-dom-ids-stable

Conversation

@Victor-w-Madeira

@Victor-w-Madeira Victor-w-Madeira commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Closes #1109.

core-components/duplicate-dom-ids-regression.spec.ts was merged without @stable
deliberately
(#1102 / PR #1106): the upstream fix it asserts — langflow#14312 (LE-2037 /
langflow-ai/langflow#14096) — existed only on the release-1.11.2 branch, while
langflowai/langflow-nightly:latest is built from the highest release-* branch. Both
cases hard-failed on the nightly by design, and tagging them then would have opened a
daily-failure issue every weekday and tripped auto-remove-stable into stripping the tag
and committing to main.

With nightly.yml disabled, that left the spec running nowhere except a PR's
impacted-specs job — and not reliably even there: @agents makes
scripts/provider-dependent-specs.mjs classify it provider-dependent, so a PR that
merely changed a helper it imports would have excluded it. #1109 was the tracker for
the other half of that decision.

The trigger fired — on the ref that decides it

Not main. The nightly is cut from the release line, so the check that matters is
release-1.12.0, and the helper now resolves on both at the same sha:

$ gh api .../get-node-scoped-dom-id.ts?ref=main            --jq .sha  → a8a903e
$ gh api .../get-node-scoped-dom-id.ts?ref=release-1.11.2  --jq .sha  → a8a903e
$ gh api .../get-node-scoped-dom-id.ts?ref=release-1.12.0  --jq .sha  → a8a903e

1. Promoted tests

# Test What it validates
1 two API Request nodes expose the same field without duplicating its DOM id On a two-node canvas, popover-anchor-input-url_input resolves to exactly 2 elements (the data-testid stays unscoped — the contract 132 call sites across 45 specs depend on), both carry a non-empty DOM id, the two ids differ, and the canvas-scoped input/textarea/select sweep returns []
2 two Agent nodes expose the same field without duplicating its DOM id The same contract on textarea_str_system_prompt across two dragged Agent nodes. The Agent is placed and inspected, never executed — no LLM call, no credential

2. Green for the right reason, not merely green

The issue's acceptance criterion was that the ids read …-APIRequest-<suffix> /
…-Agent-<suffix>, not just that the assertions pass. Captured from the force-fail runs,
which print the real values:

popover-anchor-input-url_input-APIRequest-P55sP  /  …-APIRequest-c5ewx
textarea_str_system_prompt-Agent-k1lgI           /  …-Agent-mBubM

popover-anchor-input-input_value on the Agent — a pre-fix duplicate recorded in the spec
doc's measurement table — is scoped as well. The field testid still resolved to 2
elements in both cases, so the contract half stayed green across the fix.

3. Force-failability audit

A promotion is where a spec that cannot fail becomes a blind spot in the daily, so the file
was read for dead assertions, if (visible) bodies and silent early-return chains before
trusting the green. It has none — every assertion is unconditional and
collectDuplicateFormFieldIds throws when the canvas root is missing rather than returning
an empty list. Nothing needed hardening; the mutations below prove all three assertions are
live, per test, isolated with --grep.

4. Scope, and the one logic change

The promotion itself is tag + doc + checklist bullet. An independent review of that commit
found two races, both fixed here. Neither can cause a wrong verdict — both fail hard
and are never a vacuous pass — but on the daily a hard failure strips @stable
automatically and commits to main, so they are worth closing before promotion rather than
triaging after it.

  1. The field read did not wait. expectFieldIdsUniquePerNode read the ids through
    evaluateAll, which resolves the selector once, with no auto-wait. The only prior
    gate was .react-flow__node reaching 2 — that proves the nodes mounted, not their
    parameter fields, so a field arriving one commit later read as got 0. Now gated on the
    field testid reaching 2 first. Force-failed at toHaveCount(3) on both cases.
  2. Case B did not check between its two drags. Case A gates on the first node rendering
    before adding the second, and says why in its own comment; Case B fired both dragTo
    calls back to back and let the final toHaveCount(2) catch whatever went missing — 15 s
    later, without naming which drag was lost. [Daily #1296] modelInputComponent — the canvas renders no node at all, so the model-name trigger is never reachable (hard failure, no outage cover) #1304 measured 4/20 swallowed sidebar
    interactions on this nightly line, so the path is live. The loop now asserts the running
    count after each drop. Force-failed at index + 2: "Agent drag 1/2 did not land on the
    canvas — Expected 2, Received 1"
    .

The same review found five defects introduced by the promotion's own doc edit — a
truncated opening sentence in Tags, a measurement row left unanchored when the bullet
naming its build was rewritten away, literal DOM ids recorded as if a per-node nonce were a
contract, a circular duplicate claim between Tags and Preconditions, and an @agents
note that understated the tooling. All repaired in the same commit.

5. @agents — the tag is a subject, the tooling reads it as a dependency

Not changed here, but it has two live consequences and the PR should name both.
scripts/provider-dependent-specs.mjs treats @agents as a provider declaration,
file-scoped, so it marks both cases even though neither executes the Agent:

Neither is a bug in that mechanism; both rest on a premise this file does not meet. The
spec doc now records it. Changing the classifier or the tag is a separate question and is
not smuggled into a promotion.

Validation (nightly 1.12.0.dev18, --retries=0 --workers=1)

  • Bursts — before the review fixes: 7.7 s / 7.7 s / 7.3 s, plus 7.5 s after the tag
    edit. After the first: 7.9 s / 6.7 s / 6.6 s. After the second: 7.3 s / 7.0 s / 6.9 s.
    2 passed every time, 10 runs in total ✅
  • CI, independently: the impacted-specs lane ran this spec on 1.12.0.dev18 after
    Collect models2 passed (10.4 s)
  • --trace=on ✅ — 8.4 s, no hang (this spec places nodes, it does not load the Simple
    Agent template whose canvas hangs tracing)
  • Zero 🚨 Backend Error on every run ✅
  • Force-fail, executed, one mutation per assertion and isolated per test:
    • new Set(ids).size.toBe(3) → 1 failed in both tests, printing the real
      node-scoped ids
    • duplicate sweep count > 1count > 0 → 1 failed in both (Case A named both
      url_input ids; Case B all four)
    • testid stability toHaveLength(2)toHaveLength(3) → 1 failed in both
    • the new field-mount gate toHaveCount(2)toHaveCount(3) → 1 failed in both
    • the new inter-drag gate toHaveCount(index + 1)toHaveCount(index + 2) → 1 failed,
      naming the drag
    • reverted each time: grep FFMUT = 0, and the spec diff was empty before the tag edit ✅
  • Flow cleanup verified — 26 flows on the instance, all starter projects, zero orphans
    after 6 executions ✅
  • npm run typecheck ✅ · npm run lint 0 errors ✅ · QA-CHECKLIST generated-block guard ✅ ·
    check:checklist-coverage ✅ · --grep @stable now selects both tests ✅
  • QA-CHECKLIST.md: the manual §2.1 bullet only ([-][x]). coverage:summary was not
    run and no generated block is committed (Guard QA-CHECKLIST generated blocks from PR edits (stop recurring merge conflicts) #741).

#1109)

The upstream fix this spec asserts (langflow#14312 / LE-2037) has reached the
release line the nightly is cut from: get-node-scoped-dom-id.ts now resolves on
release-1.12.0 at the same sha as release-1.11.2 and main (a8a903e).

Both cases run green on langflowai/langflow-nightly:latest (1.12.0.dev18) and,
per the issue's criterion, for the right reason -- the ids read
popover-anchor-input-url_input-APIRequest-<suffix> and
textarea_str_system_prompt-Agent-<suffix>, while the field data-testid still
resolves to 2 elements.

Tag, spec doc and checklist bullet only; no test logic changed.
Copilot AI lite review requested due to automatic review settings August 6, 2026 04:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Ready to approve

Only minor doc wording cleanup is suggested; the functional change is limited to adding @stable tags and aligning the checklist/doc accordingly.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR promotes the existing duplicate DOM-id regression coverage for node parameter fields to @stable, ensuring it runs in the weekday daily-stable.yml lane now that the upstream fix is present in the nightly release line.

Changes:

  • Add @stable to both tests in core-components/duplicate-dom-ids-regression.spec.ts.
  • Update the corresponding QA checklist bullet from [-] to [x].
  • Refresh the spec doc to reflect the promotion, updated validation baseline (1.12.x), and the release-line rationale.
File summaries
File Description
tests/tests-automations/regression/core-components/duplicate-dom-ids-regression.spec.ts Adds @stable tags to both regression tests.
QA-CHECKLIST.md Updates the manual Part II bullet to mark the coverage as validated ([x]).
docs/core-components/duplicate-dom-ids-regression.md Updates tags/validation context and documents why/when @stable was added.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

**`@stable` is deliberately absent, and this is the reason.** The upstream fix landed on the **`release-1.11.2`** branch (PR merged 2026-07-29). `langflowai/langflow-nightly:latest` — the image `daily-stable.yml` runs against — is built from the highest `release-*` branch, currently `release-1.12.0`, where the helper `get-node-scoped-dom-id.ts` is **verifiably absent** (it 404s on both `main` and `release-1.12.0`). Both cases therefore hard-fail on today's nightly *by design*. Tagging them `@stable` would open a `daily-failure` issue every weekday and trigger the `auto-remove-stable` path, which would strip the tag and commit to `main` — burning triage cycles to rediscover something already known.

`@stable` should be added once the fix reaches `main` / the 1.12 line and a run against the nightly confirms both cases green. Verify with:
**`@stable` was deliberately withheld until the fix reached the 1.12 line, and it now has (#1109).** When this spec was merged (#1102 / PR #1106) the upstream fix existed only on the **`release-1.11.2`** branch, while `langflowai/langflow-nightly:latest` — the image `daily-stable.yml` runs — is built from the highest `release-*` branch. Both cases therefore hard-failed on the nightly *by design*, and tagging them `@stable` then would have opened a `daily-failure` issue every weekday and tripped `auto-remove-stable` into stripping the tag and committing to `main`. The helper now resolves on `main` **and** on `release-1.12.0` at the same sha (`a8a903e`), and both cases run green on `1.12.0.dev18`:
…dit (#1109)

From an independent review of the promotion.

expectFieldIdsUniquePerNode read the field ids through evaluateAll, which
resolves the selector once with no auto-wait. The only prior gate was
.react-flow__node reaching 2, which proves the NODES mounted, not their
parameter fields, so a field arriving one commit later read as `got 0`. That
is a hard, well-named failure rather than a vacuous pass -- but on the daily a
hard failure strips @stable automatically and commits to main, so the race is
worth closing rather than triaging later. Force-failed at toHaveCount(3) on
both cases.

Doc repairs, all introduced by the promotion edit itself:

- the Tags section opened on a truncated sentence ("and it now has");
- rewriting Preconditions removed the only bullet identifying which build the
  "build carrying langflow#14312" measurement row referred to, leaving that row
  unanchored and undatable; it is now named inline;
- the row added for 1.12.0.dev18 recorded literal DOM ids whose suffix is a
  per-node nonce regenerated on every mount -- four runs produced four different
  pairs -- so it read as if a literal suffix were a contract;
- Tags and Preconditions both asserted the no-credential claim and pointed at
  each other; Preconditions now carries the measurement (green with no provider
  configured locally, green after Collect models on the PR lane) and Tags the
  rationale;
- the @agentS note understated the tooling: provider-dependent-specs.mjs reads
  the tag file-scoped as a provider declaration, which both forces the Collect
  models sweep on a PR that changes this spec and excludes it from a PR that
  only changes a helper it imports.

Also records that the canvas sweep only widens coverage on Case B: a two-node
API Request canvas carries exactly 2 form controls with an id, the same two the
field assertion already covers, while the two-Agent canvas carries 4.
Last item of the independent review. Case A gates on the first node rendering
before adding the second, with a comment explaining why the sidebar interaction
is fire-and-forget; Case B fired both dragTo calls back to back and let the
final toHaveCount(2) catch whatever went missing, 15 s later and without saying
which of the two drags was lost.

#1304 measured 4/20 swallowed sidebar interactions on this nightly line, so the
path is live. The loop now asserts the running node count after each drop and
names the drag in the failure message. Force-failed at index + 2:
"Agent drag 1/2 did not land on the canvas -- Expected 2, Received 1".
@Victor-w-Madeira
Victor-w-Madeira merged commit 22bee70 into main Aug 6, 2026
7 checks passed
@Victor-w-Madeira
Victor-w-Madeira deleted the test/issue-1109-promote-duplicate-dom-ids-stable branch August 6, 2026 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Promote duplicate-dom-ids-regression to @stable once langflow#14312 reaches the 1.12 line

2 participants