Skip to content

feat(ai-suggestions): scheming UI for AI-derived metadata (PR #253 follow-up)#302

Merged
jqnatividad merged 2 commits into
mainfrom
ai-suggestions-ui
May 17, 2026
Merged

feat(ai-suggestions): scheming UI for AI-derived metadata (PR #253 follow-up)#302
jqnatividad merged 2 commits into
mainfrom
ai-suggestions-ui

Conversation

@jqnatividad
Copy link
Copy Markdown
Collaborator

Summary

Completes the AI-suggestions feature from PR #253 by porting the scheming form-layer UI on top of the backend that landed in #301. The push-time pipeline already populates package["dpp_suggestions"]["ai_suggestions"] — this PR adds the form-side affordances to surface and apply those suggestions.

What's added

File Notes
assets/js/scheming-ai-suggestions.js (NEW, 419 lines) Ported verbatim from #253. Polls package_show every 2.5s, reads dpp_suggestions.ai_suggestions, shows per-field AI buttons when suggestions exist, opens popovers with text + source, applies on click. Terminates when STATUS[DONE, ERROR, FAILED].
templates/scheming/snippets/ai_suggestions_{button,asset}.html (NEW) Dataset-level "Get AI Suggestions" CTA gated on scheming_has_ai_suggestion_fields(schema) + the asset loader. Verbatim from #253.
templates/scheming/form_snippets/markdown.html Per-field AI button block gated on scheming_field_supports_ai_suggestion(field).
assets/webassets.yml Registers the new datapusher_plus/ai-suggestions bundle.
helpers.py + plugin.py Four helpers the templates call: _value, _source, _supports_ai_suggestion, _has_ai_suggestion_fields. PR #301's scheming_get_ai_suggestion renamed to _value to match the name templates reference.
jobs/stages/ai_suggestions.py New _reshape_for_ui transforms qsv's verbatim output into the per-field {value, source} schema the JS reads. Sets STATUS=DONE so polling terminates early.
tests/test_ai_suggestions.py 34 tests (was 23). New coverage for the 3 new helpers + JSON-string decoding + bad-input defensiveness.

Output shape (UI contract)

{
  "dpp_suggestions": {
    "ai_suggestions": {
      "description": {"value": "A dataset of widgets.", "source": "qsv describegpt"},
      "tags":        {"value": "widgets, production", "source": "qsv describegpt"},
      "sku":         {"value": "Stock keeping unit",  "source": "qsv describegpt"},
      "STATUS": "DONE",
      "generated_at": "2026-05-17T04:55:00Z"
    }
  }
}

tags comma-joined (scheming's tag field accepts that natively); per-column entries come from qsv's dictionary[i].field / .summary; STATUS=DONE terminates the JS polling loop early (without it the loop runs out the 100s maxPollAttempts clock — works, but burns redundant package_show calls).

Field schema convention

To opt a scheming field into AI suggestions, add ai_suggestion: true to its config:

- field_name: notes
  label: Description
  preset: markdown
  ai_suggestion: true

scheming_field_supports_ai_suggestion(field) reads this; scheming_has_ai_suggestion_fields(schema) walks both dataset_fields and resource_fields to drive the dataset-level CTA.

Backwards compat

The on-disk schema for ai_suggestions changes (verbatim qsv shape → per-field {value, source}). Practical blast radius is zero: enable_ai_suggestions is off by default and #301 merged minutes ago, so production packages with the old shape effectively don't exist. PR #301's helper was renamed in the same edit (scheming_get_ai_suggestion_value).

Test plan

  • 34 new/updated unit tests pass (mocked qsv, mocked dsu, no real subprocess / CKAN required).
  • Full unit suite: 161 passed (was 149, +12 net after the rewrite swap). No regressions.
  • Stage reshape correctness verified by test_process_happy_path_persists_suggestions (asserts the per-field {value, source} shape, comma-joined tags, STATUS=DONE).
  • JS not unit-tested (no JS test infrastructure in this codebase). Needs manual UI verification on a scheming form with ai_suggestion: true fields once an operator has a working qsv describegpt config + opt-in flag enabled.

Tracking

This is the deferred UI port called out in the close comment on #253 and the #259 checkbox. With this landed, the original PR #253 feature set is fully re-implemented across #298 (spatial extent), #301 (AI backend), and this PR (AI UI).

Co-Authored-By: Minhajuddin Mohammed minhajuddin2510@gmail.com

🤖 Generated with Claude Code

…llow-up)

Completes the AI-suggestions feature from PR #253 by porting the
scheming form-layer UI on top of the backend that landed in #301.
The push-time pipeline already populates
``package["dpp_suggestions"]["ai_suggestions"]`` — this PR adds the
form-side affordances to surface and apply those suggestions.

What

* ``assets/js/scheming-ai-suggestions.js`` (419 lines, ported verbatim
  from #253) — the polling controller. Hits ``package_show`` on a
  2.5s loop, reads ``dpp_suggestions.ai_suggestions``, shows per-field
  AI buttons when suggestions exist, opens a popover with the
  suggestion text + source, and applies on click. Terminates polling
  when ``dpp_suggestions.STATUS`` ∈ ``[DONE, ERROR, FAILED]``.

* ``templates/scheming/snippets/ai_suggestions_asset.html`` /
  ``ai_suggestions_button.html`` (also verbatim from #253) — the
  dataset-level "Get AI Suggestions" CTA gated on
  ``scheming_has_ai_suggestion_fields(schema)`` plus the asset
  loader.

* ``templates/scheming/form_snippets/markdown.html`` — adds the
  per-field AI button block (gated on
  ``scheming_field_supports_ai_suggestion(field)``) that renders the
  AI icon alongside each opt-in field's label.

* ``assets/webassets.yml`` — registers the new
  ``datapusher_plus/ai-suggestions`` asset bundle.

* ``helpers.py``: four helpers the templates call.

  - ``scheming_get_ai_suggestion_value(field, data)`` — renamed from
    PR #301's ``scheming_get_ai_suggestion`` to match the name the
    template helpers reference. Reads from the new per-field
    ``{value, source}`` shape (see below).
  - ``scheming_get_ai_suggestion_source(field, data)`` — sister
    helper for the source label (e.g. "qsv describegpt").
  - ``scheming_field_supports_ai_suggestion(field)`` — checks the
    scheming field config for ``ai_suggestion: true``.
  - ``scheming_has_ai_suggestion_fields(schema)`` — schema-level
    "any field opts in?" — drives whether the dataset CTA renders.

* ``plugin.py`` — registers all four helpers via ITemplateHelpers.

* ``jobs/stages/ai_suggestions.py`` —
  ``AISuggestionsStage._reshape_for_ui`` transforms qsv describegpt's
  verbatim output (top-level ``description`` / ``tags``, list-of-dicts
  ``dictionary``) into the per-field ``{value, source}`` schema the JS
  reads:

      ai_suggestions = {
          "description": {"value": "...", "source": "qsv describegpt"},
          "tags":        {"value": "...", "source": "qsv describegpt"},
          "<col>":       {"value": "...", "source": "qsv describegpt"},
          "STATUS":      "DONE",
          "generated_at": "<ISO 8601>",
      }

  ``STATUS=DONE`` terminates the polling JS loop early — without it the
  loop runs out the 100s ``maxPollAttempts`` clock, which works but
  burns redundant ``package_show`` calls. ``tags`` are comma-joined
  (scheming's tag field accepts that natively); per-column entries
  come from qsv's ``dictionary[i].field`` / ``.summary``.

Backwards compat

The on-disk schema for ``ai_suggestions`` changes (verbatim qsv shape
→ per-field ``{value, source}``). Practically zero blast radius:
``enable_ai_suggestions`` is off by default and #301 merged minutes
ago, so production packages with the old shape effectively don't
exist. PR #301's helper was renamed in the same edit
(``scheming_get_ai_suggestion`` → ``_value``).

Test plan

* ``tests/test_ai_suggestions.py`` updated: 34 tests (was 23).
  ``test_process_happy_path_persists_suggestions`` asserts the new
  reshape + STATUS=DONE; new tests cover ``_get_ai_suggestion_source``,
  ``_field_supports_ai_suggestion``, and ``_has_ai_suggestion_fields``
  including JSON-string ``dpp_suggestions`` decoding and bad-input
  defensiveness. Full unit suite: 161 passed (was 149, +12 net after
  the rewrite swap).
* JS not unit-tested (no JS test infrastructure in this codebase) —
  needs manual UI verification on a scheming form with ``ai_suggestion:
  true`` fields once an operator has a working qsv describegpt config.

Co-Authored-By: Minhajuddin Mohammed <minhajuddin2510@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds the scheming-form UI layer for DataPusher Plus “AI suggestions” and aligns the persisted dpp_suggestions.ai_suggestions shape with what the UI expects (per-field {value, source} plus polling termination bookkeeping).

Changes:

  • Persist AI suggestions in a UI-friendly per-field {value, source} map and stamp STATUS=DONE + generated_at.
  • Introduce a new Webassets bundle + JS polling/popover UX for applying AI suggestions in scheming forms.
  • Add new template helpers and extend unit tests for helper behavior and reshape logic.

Reviewed changes

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

Show a summary per file
File Description
tests/test_ai_suggestions.py Expands coverage for the new {value, source} contract and new helpers.
ckanext/datapusher_plus/templates/scheming/snippets/ai_suggestions_button.html Adds dataset-level CTA markup (currently mismatched with JS module behavior).
ckanext/datapusher_plus/templates/scheming/snippets/ai_suggestions_asset.html Adds asset loader snippet for AI suggestions bundle.
ckanext/datapusher_plus/templates/scheming/form_snippets/markdown.html Adds per-field AI button rendering for markdown preset fields.
ckanext/datapusher_plus/plugin.py Registers new template helpers for AI suggestions value/source and schema gating.
ckanext/datapusher_plus/jobs/stages/ai_suggestions.py Reshapes qsv output into {value, source} entries and adds STATUS=DONE.
ckanext/datapusher_plus/helpers.py Adds helper functions to fetch AI suggestion value/source and schema opt-in checks.
ckanext/datapusher_plus/assets/webassets.yml Registers the new datapusher_plus/ai-suggestions bundle.
ckanext/datapusher_plus/assets/js/scheming-ai-suggestions.js Implements polling + popover UI for AI suggestions.
Comments suppressed due to low confidence (2)

ckanext/datapusher_plus/assets/js/scheming-ai-suggestions.js:135

  • Polling termination checks dppSuggestionsData.STATUS, but the stage writes STATUS under dpp_suggestions.ai_suggestions.STATUS (and dpp_suggestions.STATUS is used by the formula-suggestions pipeline). As written, the AI polling loop won’t terminate early and will keep making redundant package_show calls until maxPollAttempts. Update the status lookup to read from the AI suggestions payload (e.g., dppSuggestionsData.ai_suggestions.STATUS).
              // Check if processing is complete
              var currentStatus = dppSuggestionsData.STATUS ? dppSuggestionsData.STATUS.toUpperCase() : null;
              
              if (currentStatus && self.options.terminalStatuses.includes(currentStatus)) {
                console.log("AI Suggestions: Processing complete with status " + currentStatus);

ckanext/datapusher_plus/assets/js/scheming-ai-suggestions.js:269

  • suggestionSource is interpolated directly into popoverContent HTML without escaping. Since this string ultimately comes from dataset data, it can become an XSS vector in the editor UI. Escape suggestionSource before concatenating, or inject it via .text() after creating the DOM nodes (similar to how escapedDisplayValue is handled).
  var popoverContent = 
    '<div class="suggestion-popover-content">' +
      '<div class="ai-suggestion-header">' +
        '<strong>AI Suggestion</strong>' +
        '<div class="ai-suggestion-source">' + suggestionSource + '</div>' +
      '</div>' +

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

Comment thread ckanext/datapusher_plus/assets/js/scheming-ai-suggestions.js
Comment thread ckanext/datapusher_plus/templates/scheming/form_snippets/markdown.html Outdated
Three follow-ups from Copilot's inline review:

* assets/js/scheming-ai-suggestions.js: ``initialize()`` now early-
  returns when the element has no ``data-field-name``. The module
  only knows how to drive per-field AI buttons; any element that
  grabs it without a field name (a future dataset-level CTA, an
  accidental wiring) was being hidden by ``$(el).hide()`` and would
  create popovers keyed by ``undefined``. Mirrors the same guard
  ``scheming-suggestions.js`` already has. Also dropped a leftover
  ``var self = this`` in ``initialize`` that wasn't actually
  referenced — ``_pollForAiSuggestions`` declares its own local.

* templates/scheming/snippets/ai_suggestions_button.html: this
  dataset-level CTA template isn't included from any active form
  template (verified via ``grep -rn``), so it's currently dead code.
  Added a Jinja ``{# ... #}`` comment at the top explaining the
  status: the CTA is unwired, and wiring it needs a dedicated JS
  module that doesn't hide on init plus a defined click semantic.
  Kept the template + ``scheming_has_ai_suggestion_fields`` helper
  so a future PR can add the proper CTA without re-creating them.
  The early-return from the first fix also protects this case if
  someone wires the existing button up: instead of breaking, it
  silently no-ops until the future-work module ships.

* templates/scheming/form_snippets/{markdown,text,select}.html:
  factored the per-field AI button block out of ``markdown.html``
  into a new shared snippet
  ``templates/scheming/snippets/ai_suggestion_field_button.html``,
  and now include it from all three form-snippet presets. Mirrors
  the existing ``suggestion_button.html`` pattern for
  formula-derived suggestions. The shared snippet does its own
  ``scheming_field_supports_ai_suggestion`` gate, so call sites
  emit it unconditionally and non-opted-in fields cost a single
  helper call. A field marked ``ai_suggestion: true`` now gets the
  AI button regardless of preset, matching the PR description's
  claim.

Tests: no test changes needed — helper contract is unchanged, only
call sites moved. 34/34 ``tests/test_ai_suggestions.py`` still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jqnatividad jqnatividad merged commit ad28ebc into main May 17, 2026
1 check passed
@jqnatividad jqnatividad deleted the ai-suggestions-ui branch May 17, 2026 10:12
jqnatividad added a commit that referenced this pull request May 17, 2026
…302 follow-up)

End-to-end verification against LM Studio (gemma-4-e4b on
host.docker.internal:1234) surfaced two real bugs in the AI
suggestions feature that landed in #301 + #302. The unit suite
passed because subprocess.run was mocked with a synthetic JSON
shape; nothing exercised qsv's actual CLI surface or output
schema.

Bug 1: ``--json`` flag doesn't exist in qsv 20.0.0

  PR #301's ``QSVCommand.describegpt`` wrapper unconditionally
  emitted ``--json`` → qsv rejected every real call with
  ``Unknown flag: '--json'``. The correct flag is ``--format JSON``
  (qsv 20.0.0 supports Markdown / TSV / JSON / TOON via a single
  ``--format`` flag).

  Fix: wrapper now emits ``--format JSON`` when ``json_output=True``.
  Test asserts both the presence of ``--format JSON`` and the
  absence of ``--json`` so this regression can't reland silently.

Bug 2: ``_reshape_for_ui`` assumed wrong output schema

  PR #301/#302 assumed qsv emits a flat shape like
  ``{description: "...", tags: [...], dictionary: [{field, summary}]}``.
  Real qsv 20.0.0 output (validated against LM Studio) is a wrapped
  envelope with PascalCase top-level keys::

    {"Dictionary": {"response": {"fields": [...], ...},
                    "reasoning": "...", "token_usage": {...}},
     "Description": {"response": "<markdown>",
                     "reasoning": "...", "token_usage": {...}},
     "Tags": {"response": {"tags": [...], "attribution": "..."},
              "reasoning": "...", "token_usage": {...}, "num_tags": N}}

  Even if Bug 1 were fixed, the reshape would always produce an
  empty dict because none of the assumed top-level keys would
  match. The UI would never see populated suggestions.

  Fix: rewrote ``_reshape_for_ui`` to walk the real envelope:
  ``Description.response`` → top-level ``description``,
  ``Tags.response.tags`` → top-level ``tags`` (comma-joined),
  ``Dictionary.response.fields[i]`` → per-column entries keyed by
  ``name`` with the LLM-generated ``description`` as value and the
  ``label`` folded into source (so the popover surfaces both
  "Stock Keeping Unit (SKU)" and the long description).

  Committed a real qsv JSON envelope as
  ``tests/fixtures/qsv_describegpt_sample.json`` so the
  ``test_process_happy_path_persists_suggestions`` test now
  exercises actual qsv output, not a synthetic shape. Future qsv
  schema changes that break the reshape get caught here.

Wrapper: two new optional params for non-localhost LLMs

  ``api_key`` and ``base_url``. qsv 20.0.0 refuses to start when
  the resolved base URL doesn't contain ``localhost`` AND no API
  key is set (``Neither QSV_LLM_BASE_URL nor QSV_LLM_APIKEY
  environment variables are set``). For DP+ specifically, the
  worker container reaches LM Studio / Ollama on the host via
  ``host.docker.internal`` — not localhost — so this triggers
  every time. Documented workaround: set ``QSV_LLM_APIKEY=NONE``
  in the worker environment OR pass ``api_key="NONE"`` per call;
  qsv accepts ``NONE`` as the explicit
  "this is unauthenticated, get on with it" sentinel.

config_declaration.yaml: expanded ``describegpt_config_path`` docs

  Operators now have a clear path for both cloud LLMs
  (``QSV_LLM_APIKEY`` env) and local LLMs reached via container-host
  hostnames (``QSV_LLM_APIKEY=NONE``). Also clarified that
  ``format = "JSON"`` should be set in the prompt-file — qsv emits
  Markdown by default, which the stage's JSON parse would reject.

Test plan

* 39 ``tests/test_ai_suggestions.py`` tests pass (was 36); +3 for
  the new wrapper params, partial-envelope reshape, and forward-
  compat unknown-key handling. Full unit suite: 166 (was 163).
* **End-to-end verified**: qsvdp 20.0.0 inside the dpp-test
  container → ``host.docker.internal:1234/v1`` → LM Studio
  (gemma-4-e4b) → real JSON envelope → reshape → on-disk
  ``ai_suggestions`` with ``description``, ``tags``, ``sku``,
  ``name``, ``price``, ``quantity_in_stock`` entries, each with
  ``value`` + ``source`` (label folded into source). This is the
  first time the feature has actually been confirmed working
  end-to-end against a real LLM.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jqnatividad added a commit that referenced this pull request May 17, 2026
fix(ai-suggestions): real-qsv schema + CLI flag corrections (PR #301/#302 follow-up)
jqnatividad added a commit that referenced this pull request May 17, 2026
… fixes 1 polling bug) (#304)

* test(js): vitest + jsdom unit tests for scheming-ai-suggestions.js

The 419-line scheming-ai-suggestions.js polling controller is the
JS half of the AI-suggestions feature contract: the Python
``AISuggestionsStage`` writes ``package["dpp_suggestions"]
["ai_suggestions"]``, this JS reads it via ``package_show`` polling
and surfaces per-field buttons. Before this PR there was zero JS
test coverage — manual UI checks were the only validation.

Adds:

* ``package.json`` + ``vitest.config.js`` — minimal Vitest + jsdom
  + jquery devDependencies (~24 packages, no Babel toolchain).
* ``tests/js/setup.js`` — loads jQuery directly into the jsdom
  realm (the standard ``jquery(window)`` returns the jQuery
  namespace object under vitest's ESM-CJS interop, NOT the
  callable ``$`` selector). Stubs ``ckan.module`` to capture the
  module registration. Provides ``buildInstance`` + ``stubAjax``
  helpers so tests can drive the polling state machine with
  ``vi.useFakeTimers()``.
* ``tests/js/scheming-ai-suggestions.test.js`` — 12 tests across
  4 describe blocks:

  - Module registration (3): name, options defaults, exposed methods.
  - ``initialize()`` early-return guard (2): bails when
    ``data-field-name`` is missing (the guard added in PR #302's
    Copilot-review follow-up); hides + starts polling when
    present.
  - ``_pollForAiSuggestions()`` state machine (4): STATUS=DONE
    termination, polls again when ai_suggestions absent, caps at
    maxPollAttempts, retries with exponential backoff on ajax
    error.
  - ``_showAiSuggestionButtons()`` DOM updates (3): reveals +
    sets ``data-suggestion-value`` / ``data-suggestion-source``;
    leaves unmatched buttons hidden; no-ops on empty value.

* ``tests/js/README.md`` — how to run, what's covered, what's
  deferred (popover interactions; real CKAN-on-page integration),
  and how the harness works.
* ``.gitignore`` — node_modules / package-lock.json / coverage.

The harness immediately caught two real bugs:

1. **JS polling bug** (the most important finding):
   ``scheming-ai-suggestions.js`` was reading ``STATUS`` from
   ``dpp_suggestions.STATUS`` (top-level), but
   ``AISuggestionsStage._reshape_for_ui`` writes it INSIDE
   ``ai_suggestions.STATUS``. The terminal status check therefore
   never fires — polling runs out the 100s ``maxPollAttempts``
   clock on every successful AI suggestion run, burning ~40
   redundant ``package_show`` calls. Fixed: JS now reads
   ``dpp_suggestions.ai_suggestions.STATUS`` first, falls back
   to the top-level value for forward-compat with custom plugins
   that mirror the legacy convention.

2. **jQuery deprecation cleanup**:
   ``$(document).ready(handler)`` was deprecated in jQuery 3.0
   in favour of ``$(handler)``. Same behavior, cleaner. Removes
   the TypeScript-style deprecation warning the linter was
   flagging.

Test summary: 12 passed. Run via ``npm test``. Full Python suite
(171 tests) unaffected — JS lives in its own toolchain.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(js-tests): address Copilot review on PR #304

Five findings, all stylistic/portability — applied verbatim.

* setup.js: ``__dirname`` was relying on Vitest's CommonJS compat
  shim (works under Vitest, would break under any ESM-native
  runner). Switched to ``path.dirname(fileURLToPath(import.meta.url))``
  — the proper ESM idiom, runner-agnostic.

* scheming-ai-suggestions.test.js: replaced
  ``Object.defineProperty(window, 'location', ...)`` (fragile in
  strict jsdom configs where ``location`` is non-configurable)
  with ``window.history.pushState({}, '', '/dataset/edit/<id>')`` —
  the browser-equivalent idiom that survives any jsdom version.

* scheming-ai-suggestions.test.js: reworded the misleading
  "no global state touched" comment. The script DOES initialize
  ``window._schemingAiSuggestionsGlobalState`` at top-level load
  time — what the guard prevents is ``initialize()`` going on to
  *mutate* that already-initialized state for elements with no
  ``data-field-name``.

* setup.js: ``stubAjax`` docstring example showed ``STATUS`` at
  the wrong level (top-level ``dpp_suggestions``); updated to
  ``dpp_suggestions.ai_suggestions.STATUS`` matching the contract
  this PR's production fix established.

* vitest.config.js: header comment said the SUT was loaded via
  ``eval``, but setup.js actually uses ``new Function(...)``.
  Updated to match — an earlier draft did use ``eval`` and the
  comment drifted when I switched to ``new Function`` for the
  ``\$``-in-scope reason.

Tests: 12/12 still pass — pure cleanup, no behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(js-tests): address roborev #2213 findings on PR #304

Five LOW findings, all applied.

* test.js (DOM, "leaves unrelated buttons hidden"): replaced the
  manual ``inst = {el, options: factory().options, ...factory()}``
  construction. The options spread was immediately overwritten by
  the trailing ``...factory()`` spread (silently dropped), and the
  factory was called twice yielding two unrelated instances. Just
  uses ``buildInstance`` like the other tests now.

* test.js ("keeps polling when ai_suggestions is absent"): added a
  final ``advanceTimersByTime(2500); expect(calls.length).toBe(2)``
  assertion. Without it, a regression that ignored terminal STATUS
  would still pass — the first response would re-trigger polling
  and the count would hit 2 regardless of whether DONE actually
  stopped the loop.

* setup.js (``stubAjax``): now throws on unexpected extra calls
  (queue exhausted) instead of silently returning. Caught a real
  test gap on the very next run — the "hides element AND starts
  polling" test was passing without queueing a response for the
  polling call initialize() triggers; silently swallowed before,
  now surfaces immediately. Fixed by adding a sentinel ``{}``
  response in that test (documented in the new stubAjax docstring
  as the escape hatch for tests that want to count calls but
  don't care about responses).

* .gitignore + package-lock.json: removed package-lock.json from
  .gitignore and committed it. For a private test harness (no
  ``"private": true`` lie — the package.json already declares
  it) we want dev + CI to install identical transitive versions
  of jsdom / vitest / jquery so test behavior is reproducible.
  Standard npm practice for apps and test harnesses.

* scheming-ai-suggestions.js: switched the STATUS coalesce from
  ``||`` to a nullish check. An empty-string ``ai_suggestions.STATUS``
  (unlikely but possible upstream state — e.g. a half-written
  package extra) was silently falling through to the legacy
  top-level STATUS via ``||``. Treating empty string as "no
  status, keep polling" is the right behavior — switched to an
  explicit ``!== undefined && !== null`` check (functionally
  ``??`` but more explicit for an old-browser ES target).

Tests: 12/12 still pass after the strict-stub fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(js-tests): address low-confidence Copilot comments on PR #304

Two suppressed-as-low-confidence comments from review #4305403052,
both legitimate.

* setup.js: ``stubAjax`` overwrote ``window.$.ajax`` without ever
  restoring it. Since ``window.$`` is the same jQuery instance
  across every test (captured once at module load), the stub
  silently leaked into the next test — a subsequent test that
  didn't call ``stubAjax`` would still see the previous test's
  stub. Captured ``originalAjax = $.ajax`` at module load and
  restore it in the ``beforeEach``. Tests that want the stub now
  have to explicitly call ``stubAjax``; default state matches a
  fresh page load.

* scheming-ai-suggestions.test.js: renamed the polling-termination
  test from ``stops polling when dpp_suggestions.STATUS is in
  terminalStatuses`` to ``stops polling when
  dpp_suggestions.ai_suggestions.STATUS is in terminalStatuses``
  — matches the actual contract this PR established (and the
  fixture the test asserts on). The old name would invite a
  future reader to "fix" the code back to the pre-PR-#304 bug
  thinking the test was pinning the wrong path.

Tests: 12/12 still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants