Raise UnrecognizedPacketTypeError without assuming a PKT_APID parameter

[medley56]

Closes #276

Problem

When an abstract container had no valid inheritors, parse_bytes built the UnrecognizedPacketTypeError message by subscripting packet['PKT_APID']. XTCE has no notion of the CCSDS standard, so any definition without a parameter literally named PKT_APID raised KeyError while constructing the exception. The documented except UnrecognizedPacketTypeError pattern silently stopped working and partial_data was lost.

Fix

• The message now names the abstract container that had no valid inheritors.
• The APID is reported only when the packet actually has a PKT_APID value. If it does not, but the bytes are CCSDSPacketBytes, the CCSDS header printout is appended instead (same approach as the existing trailing-bits warning). Otherwise the message simply omits APID information.
• partial_data is populated in every case.
• CCSDSPacketBytes.__str__ no longer raises IndexError for inputs shorter than the six-byte primary header. It renders the bytes it has (CCSDSPacket Header: (incomplete, 1 of 6 bytes: 07)), so the header printout is safe to use in diagnostic messages, including the pre-existing trailing-bits warning. Construction is unchanged.
• No API change and no behavior change for CCSDS definitions beyond the message wording.

I scanned the package for sibling CCSDS-naming assumptions on error paths. This was the only PKT_APID subscript; the only other CCSDS field names outside the generators.ccsds module are the CLI display headers, which read from CCSDSPacketBytes rather than parsed parameters.

Tests

• test_parse_bytes_unrecognized_packet_without_pkt_apid: the reproduction from the issue. Asserts the intended exception type, that the message does not claim an APID, and that partial_data carries the parsed COUNTER.
• test_parse_bytes_unrecognized_packet_reports_apid: a CCSDS packet with an undefined APID still gets APID=... in the message and in partial_data.
• test_parse_bytes_unrecognized_packet_ccsds_bytes_without_pkt_apid: the CCSDSPacketBytes fallback branch, with a full header and with a single byte.
• test_ccsds_packet_bytes_str_short_input: str(CCSDSPacketBytes) on fewer than six bytes.

Full suite: 485 passed. ruff check and ruff format --check clean. Changelog entry added under Unreleased.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.62%. Comparing base (8c1ae21) to head (14e647b).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #282      +/-   ##
==========================================
+ Coverage   94.49%   94.62%   +0.12%     
==========================================
  Files          49       49              
  Lines        4163     4203      +40     
==========================================
+ Hits         3934     3977      +43     
+ Misses        229      226       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

🟡 Changes recommended

A moderate formatting issue can still raise IndexError, and the CCSDS fallback paths lack regression coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes unrecognized-packet handling for XTCE definitions without PKT_APID, preserving partial_data and improving diagnostics.

Changes:

• Removes the hard dependency on PKT_APID.
• Adds APID/header diagnostics where available.
• Adds regression tests and a changelog entry.

File summaries

File	Summary
tests/unit/test_xtce/test_definitions.py	Adds regression coverage. Nit (1 vote): missing coverage for the CCSDSPacketBytes fallback header path.
space_packet_parser/xtce/definitions.py	Updates exception construction. Moderate (3 votes): short CCSDSPacketBytes may raise IndexError while formatting. Nit (2 votes): the CCSDS fallback lacks regression coverage.
CHANGELOG.md	Documents the fix.

Review details

Suppressed comments (1)

tests/unit/test_xtce/test_definitions.py:710

• The new elif path that appends a CCSDS header when PKT_APID is absent is not covered: the no-APID regression passes plain bytes, while this test uses a parsed PKT_APID and exercises the preceding branch. Please add a case using a CCSDSPacketBytes input with a non-CCSDS definition and assert the header text is included, so this advertised diagnostic behavior is protected.

def test_parse_bytes_unrecognized_packet_reports_apid(test_data_dir):
    """Test that an unrecognized CCSDS packet still reports its APID in the UnrecognizedPacketTypeError message"""
    xdef = definitions.XtcePacketDefinition.from_xtce(test_data_dir / "test_xtce.xml")

    # APID 2047 is not defined in test_xtce.xml
    unknown_apid_packet = space_packet_parser.generators.ccsds.create_ccsds_packet(
        data=bytes(65), apid=2047, sequence_flags=space_packet_parser.generators.ccsds.SequenceFlags.UNSEGMENTED
    )

    with pytest.raises(UnrecognizedPacketTypeError, match="APID=2047") as exc_info:
        xdef.parse_bytes(unknown_apid_packet)

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[medley56]

Fix is correct and minimal. The KeyError path from #276 is gone, partial_data is populated on every raise, and the CCSDS message now names the abstract container as well as the APID. Both new tests pass and CI is green on all 20 checks. Two things stand before merge: CHANGELOG.md conflicts with #290 which landed on main after this branch was cut, and the two open Copilot threads are both valid (I reproduced the IndexError on a short CCSDSPacketBytes; the fallback branch is Codecov's one uncovered line).

Severity tally: 0 critical, 1 warning, 1 suggestion, 1 nitpick.

🤖 AI-assisted comment, reviewed and approved by @medley56 before posting.