# Sprint: Spec Validation Fixes

> **Source**: `docs/spec.md` (2025-12-31 validation)
> **Quality Gates**: `docs/sprints/QUALITY_STANDARDS.md`
> **Status**: Planning

---

## Executive Summary

This sprint addresses 12 findings from the spec validation document, ranging from gRPC schema inconsistencies to performance issues and security gaps. Each finding has been validated against the current codebase with exact file locations and evidence.

---

## Priority Classification

### P0 - Critical (Security/Data Integrity)

| ID | Finding | Risk | Effort |
|----|---------|------|--------|
| #6 | ChunkedAssetReader lacks bounds checks | Data corruption, decryption failures | Medium |
| #10 | OTEL exporter uses `insecure=True` | Telemetry data exposed in transit | Low |

### P1 - High (API Contract/Correctness)

| ID | Finding | Risk | Effort |
|----|---------|------|--------|
| #1 | Timestamp representations inconsistent | Client/server mismatch, conversion errors | High |
| #2 | UpdateAnnotation sentinel defaults | Cannot clear fields intentionally | Medium |
| #3 | TranscriptUpdate ambiguous without `oneof` | Clients must defensively branch | Medium |
| #11 | Stringly-typed statuses | Typos, unsupported values at runtime | Medium |

### P2 - Medium (Reliability/Performance)

| ID | Finding | Risk | Effort |
|----|---------|------|--------|
| #4 | Background task tracking inconsistent | Sync tasks not cancelled on shutdown | Medium |
| #5 | Segmenter O(n) `pop(0)` in hot path | Performance degradation under load | Low |
| #7 | gRPC size limits in multiple places | Configuration drift | Low |
| #8 | Outlook adapter lacks timeouts/pagination | Hangs, incomplete data | Medium |
| #9 | Webhook delivery ID not recorded | Correlation impossible | Low |

### P3 - Low (Test Coverage)

| ID | Finding | Risk | Effort |
|----|---------|------|--------|
| #12 | Test targets for high-risk changes | Regression risk | Medium |

---

## Detailed Findings

### #1 Timestamp Representations Inconsistent

**Status**: Confirmed
**Locations**:
- `src/noteflow/grpc/proto/noteflow.proto:217` - `double created_at`
- `src/noteflow/grpc/proto/noteflow.proto:745` - `int64 start_time`
- `src/noteflow/grpc/proto/noteflow.proto:1203` - `string started_at` (ISO 8601)
- `src/noteflow/grpc/proto/noteflow.proto:149` - `double server_timestamp`

**Impact**: Multiple time encodings force per-field conversions and increase mismatch risk.

**Solution**:
1. Add `google.protobuf.Timestamp` fields in new/v2 messages
2. Deprecate legacy fields with comments
3. Add helper conversions in `src/noteflow/grpc/_mixins/converters.py`

---

### #2 UpdateAnnotation Sentinel Defaults

**Status**: Confirmed
**Locations**:
- `src/noteflow/grpc/proto/noteflow.proto:502` - Message definition
- `src/noteflow/grpc/_mixins/annotation.py:127` - Handler logic

**Impact**: Cannot clear text to empty string, set time to 0, or clear segment_ids.

**Solution**:
1. Add `optional` keyword to fields (proto3 presence tracking)
2. Use `HasField()` checks in handler instead of sentinel comparisons
3. Add `clear_*` flags for backward compatibility

---

### #3 TranscriptUpdate Ambiguous Without `oneof`

**Status**: Confirmed
**Location**: `src/noteflow/grpc/proto/noteflow.proto:136`

**Impact**: Schema allows both `partial_text` and `segment` or neither.

**Solution**:
1. Create `TranscriptUpdateV2` with `oneof payload`
2. Add new RPC `StreamTranscriptionV2`
3. Use `google.protobuf.Timestamp` for `server_timestamp`

---

### #4 Background Task Tracking Inconsistent

**Status**: Partially confirmed
**Locations**:
- `src/noteflow/grpc/_mixins/diarization/_jobs.py:130` - Tracked tasks
- `src/noteflow/grpc/_mixins/sync.py:109` - Untracked sync tasks

**Impact**: Sync tasks not cancelled on shutdown, exceptions not observed.

**Solution**:
1. Add shared `BackgroundTaskRegistry` in servicer
2. Register sync tasks for cancellation
3. Add done-callback for exception logging

---

### #5 Segmenter O(n) `pop(0)` in Hot Path

**Status**: Confirmed
**Location**: `src/noteflow/infrastructure/asr/segmenter.py:233`

**Impact**: O(n) behavior under sustained audio streaming.

**Solution**:
1. Replace `list` with `collections.deque`
2. Use `popleft()` for O(1) removals

---

### #6 ChunkedAssetReader Lacks Bounds Checks

**Status**: Partially confirmed
**Location**: `src/noteflow/infrastructure/security/crypto.py:279`

**Impact**: No guard for `chunk_length < NONCE_SIZE + TAG_SIZE`, invalid slices possible.

**Solution**:
1. Add `read_exact()` helper
2. Validate `chunk_length >= NONCE_SIZE + TAG_SIZE`
3. Treat partial length headers as errors
4. Consider optional AAD for chunk index

---

### #7 gRPC Size Limits in Multiple Places

**Status**: Confirmed
**Locations**:
- `src/noteflow/grpc/service.py:86` - `MAX_CHUNK_SIZE = 1MB`
- `src/noteflow/config/constants.py:27` - `MAX_GRPC_MESSAGE_SIZE = 100MB`
- `src/noteflow/grpc/server.py:158` - Hardcoded in options

**Impact**: Multiple sources of truth can drift.

**Solution**:
1. Move to `Settings` class
2. Use consistently in `server.py` and `service.py`
3. Enforce chunk size in streaming handlers
4. Surface in `ServerInfo`

---

### #8 Outlook Adapter Lacks Timeouts/Pagination

**Status**: Confirmed
**Location**: `src/noteflow/infrastructure/calendar/outlook_adapter.py:81`

**Impact**: No timeouts, no pagination via `@odata.nextLink`, unbounded error logging.

**Solution**:
1. Configure `httpx.AsyncClient(timeout=..., limits=...)`
2. Implement pagination with `@odata.nextLink`
3. Truncate error bodies before logging

---

### #9 Webhook Delivery ID Not Recorded

**Status**: Partially confirmed
**Locations**:
- `src/noteflow/infrastructure/webhooks/executor.py:255` - ID generated
- `src/noteflow/infrastructure/webhooks/executor.py:306` - Different ID in record
- `src/noteflow/infrastructure/webhooks/executor.py:103` - No client limits

**Impact**: Delivery ID sent to recipients not stored, correlation impossible.

**Solution**:
1. Reuse `delivery_id` as `WebhookDelivery.id`
2. Add `httpx.Limits` configuration
3. Include `delivery_id` in logs

---

### #10 OTEL Exporter Uses `insecure=True`

**Status**: Confirmed
**Location**: `src/noteflow/infrastructure/observability/otel.py:99`

**Impact**: TLS disabled unconditionally, even in production.

**Solution**:
1. Add `NOTEFLOW_OTEL_INSECURE` setting
2. Infer from endpoint scheme (`http://` vs `https://`)
3. Default to secure

---

### #11 Stringly-Typed Statuses

**Status**: Confirmed
**Locations**:
- `src/noteflow/grpc/proto/noteflow.proto:1191` - `string status` for sync
- `src/noteflow/grpc/proto/noteflow.proto:856` - `string status` for OAuth

**Impact**: Clients must match string literals, risk typos.

**Solution**:
1. Add `SyncRunStatus` enum
2. Add `OAuthConnectionStatus` enum
3. Migrate via new fields or v2 messages

---

### #12 Test Targets for High-Risk Changes

**Status**: Recommendation
**Existing Coverage**:
- `tests/stress/test_segmenter_fuzz.py`
- `tests/stress/test_audio_integrity.py`

**Suggested Additions**:
1. gRPC proto-level test for patch semantics on `UpdateAnnotation`
2. Sync task lifecycle test for shutdown cancellation
3. Outlook adapter test for `@odata.nextLink` pagination

---

## Success Criteria

- [ ] All P0 findings resolved
- [ ] All P1 findings resolved or have v2 migration path
- [ ] All P2 findings resolved
- [ ] Test coverage for high-risk changes
- [ ] Zero new quality threshold violations
- [ ] All type checks pass (`basedpyright`)
- [ ] Documentation updated

---

## Dependencies

- Proto regeneration affects Rust/TS clients
- Backward compatibility required for existing API consumers
- Feature flags for v2 migrations where applicable

---

## Risk Assessment

| Risk | Mitigation |
|------|------------|
| Proto changes break clients | Deprecate + add new fields (no removal) |
| Performance regression from deque | Benchmark before/after |
| OTEL secure default breaks dev | Make configurable with sane defaults |
| Task registry overhead | Lightweight set-based tracking |
