`createCommand()` and `createCommands()` are byte-identical implementations in `url_builder.ts` — DRY violation

[Sam-Bolling]

Finding

createCommand() and createCommands() have identical method bodies — both produce the same URL from the same input. The semantic distinction (single vs. bulk POST) lives in the request body, not the URL path.

Review Source: Senior developer code review of clean-pr
Severity: P3-Minor
Category: Code Quality
Ownership: Ours

---

Problem Statement

CSAPIQueryBuilder.createCommand() (line 2295) and CSAPIQueryBuilder.createCommands() (line 2326) in url_builder.ts contain byte-identical implementations. Both call this.assertResourceAvailable('controlStreams') followed by return this.buildResourceUrl('controlStreams', controlStreamId, 'commands'). The OGC API uses a single endpoint for both single-resource and bulk command creation — multiplicity is determined by the request body, not the URL.

Note — Intentional design, not copy-paste accident: docs/research/requirements/contribution-definition.md (Ref 15) explicitly lists "POST create (bulk)" for Commands and names "Command bulk submission" as an acceptance criterion. The plural createCommands() was a deliberate API design choice providing a semantically distinct entry point for discoverability. The problem is not that both methods exist — it is that both methods independently re-implement the same body instead of one delegating to the other. This strengthens Option A (delegation) over Option B (removal).

Affected code:

// src/ogc-api/csapi/url_builder.ts, line 2295
createCommand(controlStreamId: string): string {
  this.assertResourceAvailable('controlStreams');
  return this.buildResourceUrl('controlStreams', controlStreamId, 'commands');
}

// src/ogc-api/csapi/url_builder.ts, line 2326
createCommands(controlStreamId: string): string {
  this.assertResourceAvailable('controlStreams');
  return this.buildResourceUrl('controlStreams', controlStreamId, 'commands');
}

Scenario:

// A future developer fixes a bug in createCommand but forgets createCommands:
createCommand(controlStreamId: string): string {
  this.assertResourceAvailable('controlStreams');
  const url = this.buildResourceUrl('controlStreams', controlStreamId, 'commands');
  return url; // added logging, schema check, etc.
}

// createCommands silently diverges — still returns old behavior
createCommands(controlStreamId: string): string {
  this.assertResourceAvailable('controlStreams');
  return this.buildResourceUrl('controlStreams', controlStreamId, 'commands');
}

Impact: Maintenance risk. Two public methods with different names promising different behavior but containing identical code. No other resource type has this singular/plural pair — createObservation() (line 1659) has no corresponding createObservations(), making this an inconsistent anomaly.

Ownership Verification

$ git diff upstream/main clean-fork/clean-pr -- src/ogc-api/csapi/url_builder.ts | head -5
diff --git a/src/ogc-api/csapi/url_builder.ts b/src/ogc-api/csapi/url_builder.ts
new file mode 100644
--- /dev/null
+++ b/src/ogc-api/csapi/url_builder.ts

Conclusion: This code is ours — url_builder.ts is a new file (new file mode 100644), 100% our code.

Files to Modify

File	Action	Est. Lines	Purpose
src/ogc-api/csapi/url_builder.ts	Modify	~3	Replace createCommands body with delegation to createCommand

Proposed Solutions

Option A: createCommands Delegates to createCommand (Recommended)

createCommands(controlStreamId: string): string {
  return this.createCommand(controlStreamId);
}

Pros: Eliminates body duplication; makes the relationship explicit; callers unchanged; both method names survive for API discoverability. Aligns with the intentional dual-method design documented in the contribution definition.
Cons: None.
Effort: Trivial | Risk: None

Option B: Merge Into One Method With JSDoc Note

Remove createCommands entirely and document on createCommand that the same URL serves both single and bulk POST:

/**
 * Returns the URL for creating command(s) within a control stream.
 * The OGC API uses the same endpoint for both single and bulk command
 * creation — multiplicity is determined by the request body, not the URL.
 */
createCommand(controlStreamId: string): string { ... }

Pros: Single source of truth; no duplication.
Cons: Breaking change if external callers use createCommands. Contradicts the deliberate dual-method design from the contribution definition.
Effort: Small | Risk: Low (internal API)

Scope — What NOT to Touch

• ❌ Do NOT modify files outside url_builder.ts
• ❌ Do NOT refactor other method pairs (e.g., update/delete patterns)
• ❌ Do NOT change the general assertResourceAvailable + buildResourceUrl pattern (tracked in [SPLIT] assertResourceAvailable() + buildResourceUrl() DRY violation (87 methods) → #158 + #159 + #160 #145)
• ❌ Do NOT rename methods or change public API signatures unless Option B is chosen

Acceptance Criteria

• createCommands delegates to createCommand (or vice versa) — a single implementation
• JSDoc on both methods clarifies the relationship and the OGC API shared-endpoint design
• Existing tests still pass (npm test)
• No lint errors (npm run lint)
• All modified files pass npx prettier --check

Dependencies

Blocked by: Nothing
Blocks: Nothing
Related: #145 — assertResourceAvailable() + buildResourceUrl() two-line pattern repeated 90× in url_builder.ts

---

Operational Constraints

⚠️ MANDATORY: Before starting work on this issue, review docs/governance/AI_OPERATIONAL_CONSTRAINTS.md.

Key constraints:

• Precedence: OGC specifications → AI Collaboration Agreement → This issue description → Existing code → Conversational context
• No scope expansion: Fix the finding, nothing more
• Minimal diffs: Prefer the smallest change that satisfies the acceptance criteria
• Ask when unclear: If intent is ambiguous, stop and ask for clarification
• Respect ownership: This code is ours — fix on the phase-6 branch

Ownership-Specific Constraints

Ours:

• Fix on the phase-6 branch (or the current working branch)
• Include in the next commit to clean-pr if the PR is still open
• Add tests that cover the finding

---

References

#	Document	What It Provides
	— Original Finding —
0	docs/code-review/014-pending-p3-create-command-duplicate.md	Original finding with full context
	— Affected Source Code —
1	src/ogc-api/csapi/url_builder.ts:2295–2298	createCommand() — first copy of the duplicated body
2	src/ogc-api/csapi/url_builder.ts:2326–2329	createCommands() — byte-identical second copy
3	src/ogc-api/csapi/url_builder.ts:1659–1662	createObservation() — no plural counterpart exists, confirming this pair is an inconsistent anomaly
	— OGC Standards (Normative) —
4	OGC 23-002 §Command Resources	Spec-correct endpoint for command creation — confirms same URL for single/bulk POST; multiplicity is request-body-driven
5	OGC 23-002 (full spec)	Part 2 Dynamic Data standard — defines ControlStreams, Commands, and bulk operations; authoritative source for whether separate endpoints exist
6	OGC 23-002 OpenAPI Spec	Machine-readable Part 2 API definition — verifiable single POST path for commands under controlstreams; no separate bulk endpoint
7	OGC 23-001 (Part 1 spec)	Part 1 Feature Resources standard — establishes CRUD URL patterns that Part 2 extends; confirms singular POST-to-collection convention
	— Related Issues —
8	#145	assertResourceAvailable() + buildResourceUrl() two-line pattern repeated 90× in url_builder.ts — same file, overlapping DRY concern at the pattern level
	— Requirements Research —
9	docs/research/requirements/csapi-crud-operations.md	CRUD operation matrix for all resources; has a "Batch Operations" section but describes a client-side utility pattern (wrapping multiple individual POSTs), not a spec-defined bulk endpoint — does not address why two separate methods exist
10	docs/research/requirements/csapi-part2-requirements.md	Part 2 specification analysis — covers command submission and POST /controlstreams/{csId}/commands endpoint; documents one POST endpoint with no separate bulk path, which can be inferred from the doc's silence on a plural endpoint
11	docs/research/requirements/csapi-subresource-navigation.md	Sub-resource navigation requirements — explicitly documents ControlStream → Commands with endpoint pattern /controlstreams/{id}/commands as a Compositional relationship; this is the nested URL both methods build
12	docs/research/requirements/csapi-query-parameters.md	Complete query parameter catalog — command-related parameters (executionTime, issueTime, cmdFormat, statusCode) are all filtering/GET parameters; none distinguish single vs bulk creation at the URL level
13	docs/research/requirements/csapi-usage-scenarios.md	Usage scenarios including single-command creation (Scenario 1.4); the 17 convenience methods include sendCommandAndWait and checkCommandFeasibility but no distinct bulk command creation method — no usage scenario justifying two separate URL methods
14	docs/research/requirements/csapi-datatype-schema-requirements.md	Defines Command and CommandCollection TypeScript interfaces (data types, not method contracts); indirect relevance — both methods return string, so the data types don't motivate a signature difference
15	docs/research/requirements/contribution-definition.md	Key context: Explicitly lists "POST create (bulk)" for Commands and names "Command bulk submission" as an acceptance criterion — confirms createCommands was an intentional API design choice, not copy-paste; strengthens Option A (delegation) over Option B (removal)
16	docs/research/requirements/lessons-learned-analysis.md	Documents "Duplicate simple helpers / Strong isolation between resources" as a deliberate strategy; "~75 lines duplicated (isolated)" accepted as a design choice; directly informs whether delegation (DRY fix) is the right approach vs keeping duplicated-but-isolated methods
17	docs/research/requirements/csapi-gap-analysis.md	Gap analysis from v1 focused exclusively on format/parsing gaps (SensorML, format abstraction); covers Commands only for "JSON + SWE Common parsing" — no mention of method duplication or URL builder design
	— Upstream Research —
18	docs/research/upstream/code-reuse-analysis.md	Code reuse vs duplication strategy — defines DRY vs isolation tiers, acceptable duplication thresholds by line count; directly governs whether intra-file method delegation is appropriate
19	docs/research/upstream/querybuilder-pattern-analysis.md	QueryBuilder lifecycle (Phases 1–4), state management, and method design patterns; does not mention createCommand/createCommands specifically but documents the architectural context in which both methods were designed
20	docs/research/upstream/url-building-analysis.md	URL construction patterns (links-over-construction, URL + searchParams, parameter assembly) and private helper delegation; note: buildResourceUrl is not named in this pre-implementation research — that method name emerged during implementation
21	docs/research/upstream/pr114-analysis.md	PR #114 (EDR) blueprint — EDR's query methods (buildPositionDownloadUrl, buildAreaDownloadUrl, etc.) are semantically distinct with different URL paths; unlike createCommand/createCommands which share a path — the EDR pattern does not justify duplicate implementations for the same URL
22	docs/research/upstream/csapi-architecture-analysis.md	CSAPI architecture decisions — documents "Single QueryBuilder class per API" constraint; command methods stay in the monolithic builder (implicitly answered by that constraint), not explicitly discussed
23	docs/research/upstream/architecture-patterns-analysis.md	Consistent patterns for adding new OGC API support (conformance check → capability getter → collection filter → factory method → QueryBuilder); does not discuss method duplication or pattern deviations — relevant only as general context
24	docs/research/upstream/file-organization-analysis.md	File organization strategy — documents where CSAPIQueryBuilder lives and method grouping; note: projected ~700–800 lines at research time vs 2,490 actual lines
25	docs/research/upstream/error-handling-analysis.md	Error handling design — documents EndpointError patterns thoroughly; note: assertResourceAvailable is not named in this pre-implementation research doc — that method emerged during implementation
26	docs/research/upstream/typescript-types-analysis.md	TypeScript type safety strategies (union types, const assertions, discriminated unions); does not discuss whether method signatures should differ — relevance is limited to general type system context
27	docs/research/upstream/integration-analysis.md	Integration with existing code — documents the minimal-touch integration approach (~62 lines added, 0 modified) and public API surface (export strategy); relevant for understanding the public-facing nature of both methods
	— Design Research —
28	docs/research/design/csapiquerybuilder/architecture-decision/ (22 plans)	CSAPIQueryBuilder architecture decision series — definitive architecture decisions including scope boundaries, method design rationale, and lessons from abandoned multi-class approach
29	docs/research/strategy/design-strategy-research.md	High-level architectural trade-offs; note: explicitly rejects a format abstraction layer ("no parsers, ~13 lines") — relevant for method design decisions at the QueryBuilder level, not format handling
	— Client Analysis Research —
30	docs/research/requirements/csapi-oshconnect-python-analysis.md	OSHConnect-Python analysis — Commands are marked ❌ (not implemented) in this client; evidence of absence rather than an affirmative analysis of single vs bulk command methods
31	docs/research/requirements/OSHConnect-Python-Analysis.md	OSHConnect-Python summary — shows a single publish_command() method on ControlStream objects; no discussion of single vs bulk command URL patterns
32	docs/research/requirements/csapi-owslib-analysis.md	OWSLib class-per-resource architecture with consistent CRUD for Commands; shows commands_of_control_stream() and general CRUD coverage but never explicitly discusses or compares single vs bulk creation method patterns
33	docs/research/requirements/csapi-oscarviewer-analysis.md	oscar-viewer TypeScript client — documents POST /controlstreams/{id}/commands for command submission; a single POST endpoint, no distinction between single/bulk — confirms the shared-endpoint design from a consumer perspective
34	docs/research/requirements/csapi-oshviewer-analysis.md	osh-viewer Vue.js client — shows command submission via POST /systems/{sysid}/controlstreams/{dsid}/commands; uses a single command POST pattern but never explicitly addresses the single vs bulk URL distinction
	— Planning Documents —
35	docs/planning/phase-5/P5-parser-completion-implementation-guide.md	Phase 5 parser implementation guide — covers parser signatures and field mappings; explicitly scopes out URL building ("QueryBuilder methods (complete)"); does not mention createCommands — relevant only as Phase 5 context, not for the duplication question
36	docs/planning/phase-5/P5-ROADMAP.md	Phase 5 roadmap — parser completion tasks; scopes out URL building as already complete; command URL builder methods were implemented in earlier phases, not Phase 5
37	docs/planning/csapi-implementation-guide.md	Top-level implementation guide — Commands section shows "Create single command" and "Bulk create" both mapping to POST /controlstreams/{id}/commands; createCommand() appears in a usage example but createCommands() is never named — useful context for intended API design
38	docs/planning/contribution-goal-and-definition.md	Contribution completeness definition at the "80 methods, 9 resource types" level; neither method is named individually — relevant only for high-level completeness context
39	docs/planning/phase-6/P6-implementation-guide.md	Phase 6 refactoring guide — explicitly states "zero CSAPI business logic changes" and "What this guide does NOT cover: CSAPI business logic, URL builder methods" — confirms DRY fixes to URL builder are out-of-scope for Phase 6
40	docs/planning/phase-6/P6-ROADMAP.md	Phase 6 roadmap — url_builder.ts appears only for a 1-line ESLint fix; scope explicitly excludes URL builder DRY fixes — confirms this issue is post-Phase 6 work
	— Governance Documents —
41	docs/governance/AI_OPERATIONAL_CONSTRAINTS.md	AI operational constraints — mandatory review before work; defines no-scope-expansion and minimal-diff constraints
42	docs/governance/AI_Collaboration_Agreement.md	AI collaboration agreement — quality standards and review requirements; governance for the fix
43	docs/governance/phase-3-lessons-learned.md	Phase 3 lessons — Lesson 4 ("Don't Build Parallel Systems") and Lesson 7 ("DRY violations compound") are transferable to this url_builder.ts duplication, though the document specifically addresses parser/format handler code
	— Implementation Records —
44	docs/implementation/final-project-code-review.md	Final project code review — reviewed url_builder.ts (A+ rating) but did not flag the createCommand/createCommands duplication; relevant as evidence the issue was missed, not previously flagged
45	docs/implementation/phase-5.1-code-review.md through phase-5.4-code-review.md	Phase 5 code reviews — Phase 5.3 mentions both methods for JSDoc additions (P4-F1/P4-F2 warnings) but none of the four reviews flag the byte-identical implementation as a DRY violation
46	docs/implementation/outstanding-findings-status-report.md	Outstanding findings status — tracks 47 Phase 5 findings and 5 deferred issues; the createCommand/createCommands duplication is not listed — confirms this is a newly discovered finding, not previously tracked
47	docs/implementation/deferred-findings-final-disposition.md	Deferred findings disposition — documents 6 deferred findings with detailed verdicts; this specific duplication is not among them — provides process context for how findings are deferred
	— Testing Research —
48	docs/research/testing/research-plans/12-querybuilder-testing-strategy.md	QueryBuilder testing strategy — planning document defining test patterns for all ~80 methods across 9 resource types; Commands listed with 10 methods; defines strategy but does not verify whether createCommand and createCommands have distinct tests
49	docs/research/testing/research-plans/13-resource-method-testing-patterns.md	Resource method testing patterns — systematic CRUD test template across all resource types including Commands; high-level pattern guidance, does not address singular/plural method distinction
	— Upstream PR —
50	camptocamp/ogc-client PR #136	The upstream PR — contains both createCommand and createCommands in the submitted diff; any fix must be forward-ported to clean-pr