Skip to content

Recurring review comments: 8 proposed changes #6319

Open

Description

@rtibblesbot

❌ This issue is not open for contribution. Visit Contributing guidelines to learn about the contributing process and how to find suitable issues.

Reviewers keep making these points on PRs to this repo, each on 3+ PRs by 2+ authors in the last 90 days. Each proposes an AGENTS.md line or a lint rule that would have prevented them.

Acceptance criteria

Each change is made, or declined in a comment here.

  • Tests assert outcomes through translator references and roles, not literals, internals or presence
  • Prefer lodash and existing utilities over hand-rolled helpers
  • Accessible names, live regions and focus follow ARIA/WCAG patterns
  • Document parse/build state shapes and non-obvious helpers with JSDoc
  • Rebuilt XML must be valid against the XSD and round-trip losslessly
  • Reuse shared XML helpers in QTIEditor code and tests
  • Batch backfill commands and use cached sizes
  • Keep the PR in scope

Proposed changes

Tests assert outcomes through translator references and roles, not literals, internals or presence (5 PRs, 3 authors)

AGENTS.md Testing: "Assert the resulting state, such as document HTML, the role-scoped text (getByRole('status')), or saved DB rows equal to the input, not call chains, fetch strategy or toBeInTheDocument() presence. Reference strings through their translator (tr.$tr(...)/str$()), never literals. Never query KDS/keen-ui internal classes; use within(getByTestId(...))."

Prefer lodash and existing utilities over hand-rolled helpers (3 PRs, 2 authors)

AGENTS.md Code: "Before writing a helper, use lodash (lodash/shuffle, lodash/flatMap), existing composables (shared/composables/useFetch), or a maintained library (@internationalized/number for locale numbers)."

Accessible names, live regions and focus follow ARIA/WCAG patterns (6 PRs, 3 authors)

AGENTS.md a11y: "Live regions (role="status"/aria-live) stay mounted empty and only their text changes, with one writer per region. Key sibling v-ifs. Never unmount the focused control. Use KDS widgets (KDropdownMenu, KTabsPanel, KCard :to) for their ARIA wiring. Accessible names start with the visible text. Focus stays visible on every state." Lint: vuejs-accessibility plugin.

Document parse/build state shapes and non-obvious helpers with JSDoc (4 PRs, 2 authors)

Add to AGENTS.md: "Every QTIEditor interaction's parse.js declares a @typedef for its state and JSDoc on parse and build. The JSDoc says which direction it converts (XML → state or state → XML) and what it hydrates or drops. Every utils/ module and every non-obvious algorithm or prop gets a doc comment. When you change behaviour, update any doc comment that describes it."

Rebuilt XML must be valid against the XSD and round-trip losslessly (5 PRs, 2 authors)

Add to AGENTS.md: "Every interaction's build output must pass validate_qti_item for blank and minimal states, including empty ids and empty passages. Any item the build can't reproduce faithfully must be marked read-only through isSupportedBody or an allowlist. Add a test that runs parse → build on blank and edge-case input through the XSD."

Reuse shared XML helpers in QTIEditor code and tests (6 PRs, 2 authors)

Add to AGENTS.md: "Parse and build XML with serialization/xml.js (parseXML, buildXmlNode, getContentHTML) and QTIDeclaration.fromXML. Keep all XML logic in the interaction's parse.js. On the backend, reuse the secure parser in utils/assessment/qti/validation.py; never reach for innerHTML or regex."

Batch backfill commands and use cached sizes (3 PRs, 3 authors)

Add to AGENTS.md: "Management-command backfills process rows in batches, following the existing backfill commands. Endpoints don't recompute per-channel aggregates on every request; reuse the cached_data values. Never fetch unpaginated channel lists into IndexedDB."

Keep the PR in scope (3 PRs, 2 authors)

Add to AGENTS.md: "Unrelated fixes go in their own PR against unstable. Don't change established behaviour, such as stored-value formatting, unless the issue asks you to."

Activity

  1. changed the title [-]Recurring review comment: Tests assert outcomes through translator references and roles, not literals, internals or presence[/-] [+]Recurring review comments: 3 proposed changes[/+] on Oct 18, 2026
  2. changed the title [-]Recurring review comments: 3 proposed changes[/-] [+]Recurring review comments: 8 proposed changes[/+] on Oct 25, 2026
  3. rtibblesbot commented on Oct 25, 2026

    @rtibblesbot
    ContributorAuthor

    This week's pass added 5 proposed changes:

    • Document parse/build state shapes and non-obvious helpers with JSDoc
    • Rebuilt XML must be valid against the XSD and round-trip losslessly
    • Reuse shared XML helpers in QTIEditor code and tests
    • Batch backfill commands and use cached sizes
    • Keep the PR in scope

    Written by rtibblesbot, an LLM-based coding agent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions