Null-guard + cast boilerplate duplicated 5× across `part2.ts` parse functions

[Sam-Bolling]

Finding

All 5 parse functions in part2.ts begin with identical null-guard + cast boilerplate (4 lines each). This 5-way copy-paste is a bug-magnet — any future change to the error message format or the cast strategy must be applied to all 5 functions manually.

Review Source: Senior developer code review of clean-pr — docs/code-review/013-pending-p3-null-guard-duplicated-5x.md
Severity: P3-Minor
Category: Code Quality / DRY
Ownership: Ours

---

Problem Statement

Each of the 5 parse functions in src/ogc-api/csapi/formats/part2.ts opens with the same 4-line pattern:

if (typeof json !== 'object' || json === null) {
  throw new Error('<functionName>: input must be a non-null object');
}
const obj = json as Record<string, unknown>;

The only variation is the function name in the error message string. This is repeated at:

Function	Lines
parseDatastream()	115–120
parseControlStream()	215–220
parseCommand()	327–332
parseObservation()	408–413
parseCommandStatus()	492–497

That's 20 lines of boilerplate (4 lines × 5 functions) that could be reduced to 5 one-liners plus 1 helper definition.

Impact: No runtime risk — this is a maintainability/readability issue. If the guard logic ever needs to change (e.g., a different error class, additional validation, or a shared isRecord() guard), the change must be made 5 times identically.

Ownership Verification

part2.ts is a new file in our diff — it does not exist in upstream at all:

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

Conclusion: This code is 100% ours.

Files to Modify

File	Action	Est. Lines	Purpose
src/ogc-api/csapi/formats/part2.ts	Modify	~15 removals, ~6 additions	Extract requireObject helper, replace 5 copy-paste blocks

Proposed Solutions

Option A: Extract requireObject helper (Recommended)

function requireObject(json: unknown, fn: string): Record<string, unknown> {
  if (typeof json !== 'object' || json === null)
    throw new Error(`${fn}: input must be a non-null object`);
  return json as Record<string, unknown>;
}

// Usage (each function becomes):
export function parseDatastream(json: unknown): Datastream {
  const obj = requireObject(json, 'parseDatastream');
  // ...rest unchanged
}

Pros: Single source of truth for guard logic; each caller is one line; error messages preserved exactly
Cons: None
Effort: Small | Risk: None

Option B: Absorb into parseBaseStream (if #146 lands first)

If issue #146 (finding 010) creates a parseBaseStream helper for parseDatastream and parseControlStream, the guard for those 2 functions would move into parseBaseStream. The remaining 3 functions (parseCommand, parseObservation, parseCommandStatus) would still need requireObject or their own inline guards.

Pros: Fewer moving parts for the 2 stream functions
Cons: Only covers 2 of 5 functions; the other 3 still duplicate
Effort: Small (additive on top of #146) | Risk: None

Scope — What NOT to Touch

• ❌ Do NOT modify files outside part2.ts
• ❌ Do NOT refactor the body of the parse functions beyond replacing the guard boilerplate
• ❌ Do NOT change error message text or error class
• ❌ Do NOT change function signatures or return types
• ❌ Do NOT consolidate parseBaseStream in this issue — that's parseDatastream() and parseControlStream() share ~30 lines of identical base-stream extraction logic — DRY violation in part2.ts #146

Acceptance Criteria

• requireObject helper (or equivalent) extracted as a private function in part2.ts
• All 5 parse functions use the helper instead of inline guard + cast
• Error messages preserved exactly (function name prefix + "input must be a non-null object")
• All Part 2 parser tests pass (npm test)
• No lint errors (npm run lint)
• All modified files pass npx prettier --check

Dependencies

Blocked by: Nothing
Blocks: Nothing
Related: #146 — parseDatastream / parseControlStream share ~30 lines of identical base-stream extraction logic (finding 010). That issue covers the broader 2-function duplication; this issue covers the narrower 5-function guard boilerplate. If #146 creates parseBaseStream, Option B here becomes relevant.

---

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: Extract the guard helper, nothing more
• Minimal diffs: Replace the 5 inline blocks with 5 one-liners + 1 helper definition
• Ask when unclear: If intent is ambiguous, stop and ask for clarification
• Respect ownership: All affected code is ours (100% our fork)

Ownership-Specific Constraints

This code is 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
• No new tests needed — this is a zero-behavioral-change refactor; existing tests validate correctness

---

References

Code-Level References

#	Document	What It Provides
0	Finding 013 MD	Original senior developer finding with full context
1	formats/part2.ts	The sole affected file — all 5 duplicated guard blocks
2	formats/part2.spec.ts	Test suite — must pass after cleanup (acceptance criterion)

Standards

#	Document	What It Provides
3	OGC API - Connected Systems - Part 2: Dynamic Data (23-002)	The OGC standard governing all 5 resource types parsed in part2.ts — DataStreams (§9), Observations (§11), ControlStreams (§10), Commands (§12), CommandStatus (§13)
4	OGC API - Connected Systems - Part 2: OpenAPI Specification	Machine-readable schema definitions for all 5 resource types — the authoritative source for which fields each parser must extract

Requirements Research

#	Document	What It Provides
5	CSAPI Part 2 Requirements	Complete Part 2 specification analysis — documents DataStreams, Observations, ControlStreams, Commands, and CommandStatus requirements that drove these 5 parser functions
6	Data Type and Schema Requirements	TypeScript type system requirements (100+ schema definitions) — defines the Datastream, Observation, ControlStream, Command, CommandStatus interfaces that these parsers return
7	Common Format Requirements	Format negotiation and parsing depth requirements — explicitly discusses parse-vs-pass-through strategy that these functions implement
8	Comprehensive Format Requirements	Format specification analysis for GeoJSON, SensorML, and SWE Common — background on the format landscape the Part 2 parsers operate in
9	Gap Analysis from Previous Iteration	Lessons from failed first iteration — discusses strategic importance of comprehensive format abstraction, though it does not address individual parser code structure like the null-guard pattern
10	Lessons Learned from Previous Iterations	Documents what was over-engineered (shared utility modules, excessive abstractions) and under-engineered (tests, documentation). The document warns against shared utility modules, but the proposed requireObject is a file-private helper — a much smaller-scale refactor than what the document cautions about
11	Upstream Library Expectations	Architectural patterns camptocamp/ogc-client expects (Endpoint class pattern, Web Worker support, caching, TypeScript types) — does not mention DRY or code duplication standards specifically, but establishes the overall quality bar for contributions

Upstream Research

#	Document	What It Provides
12	Architecture Patterns Analysis	Blueprint for maintaining upstream consistency — code quality norms the parsers should align with
13	Code Reuse vs Duplication Strategy	Cross-module reuse policy (import vs. duplicate across files/directories). Its size thresholds (1–5 lines = always duplicate) technically apply, but the document addresses inter-file/inter-module reuse, not intra-file DRY extraction. A file-private helper is a lower-stakes refactor than the cross-module coupling the document focuses on
14	Error Handling Design Analysis	Error handling patterns in ogc-client — documents both EndpointError and generic new Error(...) for parameter validation, with a decision matrix for when to use each. The null-guard's throw new Error(...) follows the "generic Error for parameter validation" pattern documented here
15	TypeScript Type System Design	Type hierarchy and organization patterns — context for the json as Record<string, unknown> cast pattern and type narrowing approach
16	File Organization Strategy	File-level organization patterns (flat structure, helpers.ts for shared utilities, model.ts for types). The EDR helpers.ts pattern is tangentially relevant, but the document doesn't address the specific question of whether a file-private helper should stay in its own file or move to a shared utility
17	QueryBuilder Pattern Analysis	Core pattern for CSAPIQueryBuilder — provides context for the consumer of these parser outputs
18	PR #114 (EDR Implementation) Analysis	Direct blueprint that upstream accepted — documents EDR's URL-builder validation patterns (query type support, parameter existence checks). EDR has no equivalent JSON parse functions, so it does not provide precedent for null-guard patterns specifically

Design Research

#	Document	What It Provides
19	CSAPIQueryBuilder Architecture Decisions	Architecture decision series — LESSONS-LEARNED-multi-class-failure.md documents how code duplication across 9 classes (1,200–1,600 lines) caused architectural failures. The scale is much larger than 20 lines of boilerplate, but the underlying DRY principle is the same

Testing Research

#	Document	What It Provides
20	Testing Strategy Research	Overall testing approach and quality gates — governs the "all tests pass" acceptance criterion
21	Testing Research Plan 03 — TypeScript Testing Standards	TypeScript-specific testing requirements — relevant to verifying the refactored guard preserves type narrowing behavior
22	Testing Strategy Review — Parser Testing vs Spec Validation	Defines what "parser tests pass" means in this project: tests verify correct extraction of spec-defined properties into typed TypeScript output

Planning Documents

#	Document	What It Provides
23	CSAPI Implementation Guide	Definitive architecture reference — format handler design requirements that the Part 2 parsers implement
24	Phase 5: Parser Completion — Implementation Guide	Parser function signatures, field mappings, and contracts — the specification these 5 parsers were built to (dedicated sections §4.2–§4.6 for each function)
25	Phase 5: Parser Completion — Roadmap	Phase 5 task sequencing — prescribes when and what to build for these 5 parsers (Tasks 2–6), with dependency ordering and complexity ratings
26	Phase 5: Parser Completion — Contribution Goal	Per-parser scope descriptions (field handling for each function) and project-wide quality standards (unit tests, type safety, JSDoc, >80% coverage). Acceptance criteria are general, not per-function
27	Phase 6: Implementation Guide	Phase 6 module boundary refactoring specs — covers Prettier/ESLint CI compliance. Phase 6 scope explicitly excludes CSAPI business logic changes; DRY refactoring would be out of Phase 6 scope, but finding this issue during Phase 6 code review is within the phase's quality oversight
28	Contribution Goal and Definition	PR acceptance criteria — defines the quality bar these parsers must meet for upstream submission

Governance Documents

#	Document	What It Provides
29	AI Collaboration Agreement	Foundational governance — quality standards and collaboration protocols that apply to all code changes
30	AI Operational Constraints	Operational safety boundaries — already referenced in Operational Constraints section above
31	Phase 3 Implementation Lessons Learned	Lessons from Phase 3 format handler development — covers parser design principles (Postel's Law, recognition-before-extraction) and a DRY lesson (Lesson 7). Note: Phase 3 covered GeoJSON/Part 1 handlers; the Part 2 parsers were developed in Phase 5. The design principles carry over, but the phase attribution is indirect
32	Code Review Prompt Templates	Systematic defect detection methodology — the review framework under which this finding was discovered
33	Issue Creation Prompt Template — Code Review	Template C used to create this issue — defines the ownership verification and severity framework applied here

Implementation Records

#	Document	What It Provides
34	Final Project Code Review	End-to-end quality assessment (59 files) — inventories Part 2 parsers (under format/part2/*.ts structure). Identifies three code duplication groups (F-NEW-04) in Part 2 parsers but does not specifically flag the null-guard boilerplate pattern
35	Phase 5.1–5.2 Code Review Reports	Phase 5.1 covers parseDatastream and parseObservation; Phase 5.2 covers parseControlStream, parseCommand, and parseCommandStatus — these reviews were conducted when all 5 functions were first written. Notably, the 5.1 review identifies the identical guard pattern as finding [F1] POSITIVE (consistent structure), not as a concern
36	Outstanding Findings Status Report	Process reference (47 findings, F1–F47) — shows how code quality findings are tracked and resolved, with disposition for every finding

Testing Documentation

#	Document	What It Provides
37	Test Fixtures Guide	Fixture sourcing and management — context for the test fixtures used by the Part 2 parser test suite

Upstream PR Preparation

#	Document	What It Provides
38	Upstream PR #136	The actual upstream PR where this code will be reviewed — jahow will see the 5× duplication if not cleaned up
39	CI Formatting Lessons Learned	Prettier formatting alignment with upstream — relevant to the acceptance criterion "all modified files pass npx prettier --check"
40	Rebase Plan — Clean PR	Commit sequence design for PR reviewability — context for the commit history this cleanup will need to fit into

Code Repositories

#	Document	What It Provides
41	camptocamp/ogc-client	Upstream library — part2.ts does not exist here, confirming 100% ownership; also establishes the code quality baseline our parsers must match
42	OS4CSAPI/ogc-client-CSAPI_2	This development repository — all affected files live here on the phase-6 branch

Related Issues

#	Document	What It Provides
43	Issue #146 — parseDatastream / parseControlStream base-stream duplication	Related finding 010 — broader DRY refactor of the same file affecting 2 of the 5 functions; if parseBaseStream is extracted, Option B becomes relevant
44	Issue #148 — Redundant as Record casts after isRecord narrowing	Sibling finding — redundant cast hygiene in the SWE Common parsers; same theme of unnecessary as Record<string, unknown> casts
45	Issue #141 — parseCollectionResponse casts raw JSON to T[] without validation	Related type safety finding — unchecked generic casts (different file, similar theme of input validation at parse boundaries)
46	Issue #143 — extractCSAPIFeature casts properties without null check	Related type safety finding — missing null guard before cast (the inverse problem: this issue has the guard but duplicated; #143 is missing the guard entirely)