fix(hyundai): end the device name at a control byte, record the validation gap - #182
Merged
Conversation
…ation gap Follow-up to #181, which fixed the multi-phone parsing and stripped trailing \x01-\x05 from device friendly names. Three residual items: - The name capture still admitted control bytes. `.strip()` of a fixed set only reaches the ends, so a trailing byte outside that set survived ("Trailing BEL\x07") and an embedded one survived anywhere ("Embed\x02ded"), both reaching the HTML report and LAVA. The record delimits the name the same way it delimits the MAC, so the capture now ends at any C0 control or DEL and the strip chain is redundant. - Restore the docstrings on _present_columns. The helper looks like an indirection worth removing until you know a SELECT naming an absent column drops every row for that phone, which is the defect it exists to prevent. - Record in notes that these artifacts were validated against one head unit from an extraction that could not be shared, so the absence of a committed fixture is visible where an examiner reads it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WYbci6R9C5XJ14Sxorn2T2
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Follow-up to #181. Ends the
wireless_dev_list.datfriendly-name capture at any control byte rather than stripping a fixed set from the ends, restores the docstrings explaining why_present_columnsexists, and records in each artifact'snotesthat these were validated against an extraction that could not be shared.For a new or changed artifact
python admin/scripts/check_artifact_output.py <report folder>on that report and fixed or documented every finding.notes,descriptionandsample_datasay only what the data shows, and the numbers were re-derived from the finished run.The first three are deliberately unticked — see below. No Hyundai extraction was available for this change; it was verified against synthetic fixtures only.
Anything reviewers should know
What was wrong. #181 stripped
\x01-\x05from the ends of the device friendly name.str.strip(chars)only reaches the ends and only covers the listed set, so two cases still put control bytes into the HTML report and the LAVA database:Trailing BEL\x07Embed\x02dedThe fix. The name capture now ends at any C0 control or DEL (
[^\x00-\x1f\x7f]+), which makes the strip chain redundant and removes it. The record already delimits the friendly name the same way it delimits the MAC, so stopping at a control byte is consistent with how the rest of the record is parsed.A judgment call worth a second opinion. This truncates at an embedded control byte —
Embed\x02dedbecomesEmbed, notEmbedded. Filtering instead of truncating is equally defensible. Truncation was chosen for consistency with the\x00-delimited parse, but if a genuine device name can carry an embedded control byte, the tail is silently lost. Nothing available here settles which behaviour matches the format, so this is the reviewer's call to overrule.Validation gap. #181 was validated by its author against a single Hyundai/Kia head unit from an active case that could not be shared, so no fixture accompanies these artifacts. That is now stated in each artifact's
notes, where an examiner reads it, rather than only in the PR thread. This is the route the test-data bot offers for an unshareable extraction.How this was verified. Synthetic
CH_/MC_databases and awireless_dev_list.datbuilt to the observed record shape, run through the plugin loader:Trailing BEL\x07→Trailing BEL,Embed\x02ded→EmbedPixel 7,John's iPhone,José Galaxy(the latin-1 fallback path) unchangedphone_typestill returned, with the column empty and a log lineCH_*.db*skipped rather than fatalLocally green:
lint_changed(0 new warnings),check_claim_language,check_html_safety,check_report_local_paths,check_source_path,check_vendored,validate_sample_data, and the plugin-loader import test.These are synthetic fixtures encoding an assumed format, not evidence from a real head unit. They guard against regressions; they do not confirm the format is right.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WYbci6R9C5XJ14Sxorn2T2
Generated by Claude Code