test(#830): add pytest suite for analyze-transcript.py

[fullsend-ai-coder]

Summary

Add a comprehensive pytest suite (111 tests) for skills/analyze-transcript/analyze-transcript.py, covering all key functions and subcommands that were previously untested. The script has ~820 lines of heuristic-based parsing logic that is easy to regress on.

Related Issue

Fixes #830

Changes

• New test file: skills/analyze-transcript/analyze_transcript_test.py with 111 tests organized by component:
  • parse_line_range, truncate, extract_content_blocks, get_tool_result_text — unit tests for utility functions
  • detect_file_type — OTLP telemetry detection, non-dict JSON, non-JSON lines, unreadable files, blank-line-heavy files, empty files, stdin bypass
  • parse_lines / iter_messages — line range filtering, malformed input resilience, non-UTF-8 content, queue-operation and last-prompt type handling
  • _accumulate_stats — token accounting, duration calculation, stop reasons, pre-iterated messages, edge cases (no timestamps, empty file)
  • Error detection (_is_error_result, _check_block_error, _collect_errors) — is_error flag, <error> tags, <tool_use_error> tags, _RESULT_ERROR_PATTERNS matching, assistant keyword mentions, false positive checks
  • Host matching (_host_matches, _match_host, _match_http_entry) — exact match, parent-domain, case-insensitive, combined method/host filters
  • parse_sandbox_log — OCSF entry parsing, field extraction, denied entries, non-OCSF line skipping
  • Subcommand e2e tests — summary, errors, audit, tools, conversation, search, network, network-search in both text and JSON output modes
• New test fixtures: crafted JSONL and log files under skills/analyze-transcript/testdata/ covering valid transcripts, OTLP telemetry, non-dict JSON, non-JSON lines, non-UTF-8 content, blank-heavy files, error patterns, and sandbox network logs
• Makefile: wired tests into the script-test target

Testing

• All 111 tests pass (python3 -m pytest skills/analyze-transcript/analyze_transcript_test.py -v)
• Tests added for all key areas listed in the issue

---

Closes #830

Post-script verification

• Branch is not main/master (agent/830-analyze-transcript-tests)
• Secret scan passed (gitleaks — c887fc47c59d8b87b5282ca44044d2933d30df41..HEAD)
• PR body secret scan passed (gitleaks — no-git)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:44 AM UTC · Completed 9:56 AM UTC
Commit: 30c4015 · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] skills/analyze-transcript/* — This PR adds 9 files under the skills/ protected path (test file skills/analyze-transcript/analyze_transcript_test.py and 8 test fixtures under skills/analyze-transcript/testdata/). The PR is linked to issue analyze-transcript: add test coverage for the analyzer script #830 and the description explains the rationale (adding comprehensive test coverage for the analyzer script). Human approval is always required for protected-path changes, regardless of context.

Previous run

Review

Findings

Medium

• [protected-path] skills/analyze-transcript/ — This PR modifies files under a protected path (skills/). The change is linked to issue analyze-transcript: add test coverage for the analyzer script #830 and the description explains the rationale (adding test coverage for analyze-transcript.py), but human approval is always required for protected-path changes. Affected paths: skills/analyze-transcript/analyze_transcript_test.py, skills/analyze-transcript/testdata/*.

Low

• [test-inadequate] skills/analyze-transcript/analyze_transcript_test.py:246 — test_non_utf8_resilience asserts len(results) >= 2 but the fixture deterministically produces exactly 3 parseable lines (lines 0, 1, and 3). The loose lower bound would still pass if a code change caused one valid line to be silently dropped, masking a regression.
  Remediation: Assert len(results) == 3 or at minimum >= 3.

• [test-inadequate] skills/analyze-transcript/analyze_transcript_test.py:861 — test_json_with_host_filter does not verify that entries outside the host filter are actually excluded. It only asserts that surviving non-DENIED entries contain github.com in the host string but never checks the result count or that excluded hosts (e.g., registry.npmjs.org) are absent.

[guyoron1]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 10:31 AM UTC · Completed 10:36 AM UTC
Commit: 30c4015 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 2 of 3 review findings by tightening test assertions. Finding 1 (protected-path) is informational and requires no code change. All 111 tests pass.

Fixed (2):

1. test_non_utf8_resilience uses loose lower bound (>= 2) instead of exact count (skills/analyze-transcript/analyze_transcript_test.py): Changed assertion from len(results) >= 2 to len(results) == 3. The fixture deterministically produces exactly 3 parseable JSON dict lines (indices 0, 1, and 3); line 2 is raw bad bytes and is skipped. The loose bound would mask a regression if a valid line were silently dropped.
2. test_json_with_host_filter does not verify excluded entries are absent (skills/analyze-transcript/analyze_transcript_test.py): Added three assertions: (1) assert len(data) == 3 to verify only matching entries survive the filter, (2) assert all hosts contain 'github.com', and (3) assert no host contains 'npmjs' to confirm excluded entries are truly absent.

Disagreed (1):

1. protected-path modification requires human approval: This is an informational finding, not a code change request. The PR intentionally adds test files under skills/analyze-transcript/ per issue analyze-transcript: add test coverage for the analyzer script #830. Human approval for protected-path changes is a process requirement already handled by the requires-manual-review label on this PR.

Tests: passed

Next steps:

• /fs-review — request a re-review of the changes
• /fs-fix <your instruction> — run another fix pass with specific guidance
• Push commits directly — review re-runs automatically on push
  _{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 10:40 AM UTC · Completed 10:54 AM UTC
Commit: 8da63ad · View workflow run →

[github-actions]

This pull request has been automatically marked as stale because it has not had any activity in the last month. It will be closed in 2 weeks if no further activity occurs. Remove the stale label to reset the inactivity timer.