Optimize IcaoDecoder.isMilitary() by eliminating regex recompilation #291

kevinelliott · 2025-10-21T06:23:53Z

Fixed a critical performance issue where 30+ regex patterns were being compiled on every call to isMilitary(). The patterns are now compiled once as static readonly class fields and reused via .test() method.

Performance impact:

Before: 30+ regex compilations per isMilitary() call
After: Zero regex compilations (patterns compiled once at class load)

This significantly improves performance for high-throughput ACARS message processing scenarios.

🤖 Generated with Claude Code

Summary by CodeRabbit

Refactor
- Improved and simplified military ICAO address detection for faster, more reliable classification and expanded country/region coverage.
Tests
- Added comprehensive unit tests validating military-detection across many ranges and edge cases.
- Added thorough decoding tests for the label-15 plugin covering normal, partial, off, and negative scenarios with expected outputs.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Fixed a critical performance issue where 30+ regex patterns were being compiled on every call to isMilitary(). The patterns are now compiled once as static readonly class fields and reused via .test() method. Performance impact: - Before: 30+ regex compilations per isMilitary() call - After: Zero regex compilations (patterns compiled once at class load) This significantly improves performance for high-throughput ACARS message processing scenarios. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>

jazzberry-ai · 2025-10-21T06:23:57Z

Bug Report

Name	Severity	Example test case	Description
Missing Spain Military Range End Anchor	Medium	Input: '3500001'	The code uses a range check `(i >= '350000' && i <= '37ffff')` for the Spain military range. The new regex implementation doesn't have an equivalent range check, which could lead to missed identifications if the input isn't exactly 6 characters long.
Incorrect Australia Military Regex	Critical	Input: '7c822e'	The regex `i.match(/^7c8([2-4]

_{Comments? Email us.}

coderabbitai · 2025-10-21T06:24:17Z

Warning

Rate limit exceeded

@makrsmark has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 38 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 27c0cc4 and 75a216b.

📒 Files selected for processing (2)

lib/IcaoDecoder.test.ts (1 hunks)
lib/IcaoDecoder.ts (1 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Replaced regex-based military ICAO detection with numeric-range checks by adding a private static MILITARY_RANGES array and updating isMilitary() to parse the ICAO hex value and test membership against those ranges. Added comprehensive tests for the new logic and plugin test cases.

Changes

Cohort / File(s)	Summary
ICAO military detection `lib/IcaoDecoder.ts`	Added `private static MILITARY_RANGES: Array<[number, number]>`. Replaced regex/string membership checks in `isMilitary()` with parsing `this.icao` as a hex number and iterating ranges to determine military status. Simplified `isMilitary()` signature and control flow; removed inline per-range regex logic.
IcaoDecoder tests `lib/IcaoDecoder.test.ts`	Added comprehensive unit tests covering construction, public properties, and extensive `isMilitary()` cases across many country/region ranges, including boundary and negative tests.
Label plugin tests `lib/plugins/Label_14.test.ts`	Added tests for the Label_15 plugin covering metadata, multiple decoding scenarios (short/off variants), partial decoding cases, and a negative/unrecognized message case.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Review MILITARY_RANGES for correctness and coverage of intended hex intervals.
Verify hex parsing edge cases (invalid/short inputs, casing, leading zeros).
Run and inspect new unit tests in lib/IcaoDecoder.test.ts for completeness and false positives/negatives.
Confirm Label_14.test.ts references the correct plugin and message decoding expectations.

Suggested reviewers

kevinelliott

Poem

I’m a rabbit with a tiny chart,
Hopping hex ranges, doing my part.
From edge to edge I bound and scan,
Codes fall in line — that’s my plan! 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main optimization: replacing regex-based checks with numeric range checks in isMilitary(), which eliminates regex recompilation overhead.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

lib/IcaoDecoder.ts (1)
108-109: Consider more descriptive variable names.

While the local aliases reduce verbosity, single-letter names like i and p reduce clarity. Consider using more descriptive names:
-    const i = this.icao;
-    const p = IcaoDecoder.MILITARY_PATTERNS;
+    const icaoAddress = this.icao;
+    const patterns = IcaoDecoder.MILITARY_PATTERNS;
Then update the references in the return statement accordingly (e.g., patterns.US_MIL_1.test(icaoAddress)).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b4b2d1f and 5a6ea2e.

📒 Files selected for processing (1)

lib/IcaoDecoder.ts (2 hunks)

🔇 Additional comments (2)

lib/IcaoDecoder.ts (2)

5-100: Excellent performance optimization!

The refactoring to static readonly pre-compiled patterns is exactly the right approach. This eliminates 30+ regex compilations per method call and improves performance for high-throughput scenarios.

111-229: Pattern usage is correct and efficient.

The refactored code correctly uses .test() method with the pre-compiled patterns. The logic flow and control structure are preserved while achieving the performance optimization goals.

Note: Some ranges still use string comparisons (lines 133, 212), which is fine and was present in the original code. String comparison works correctly for hex address ranges due to lexicographic ordering.

makrsmark · 2025-10-21T10:23:37Z

lib/IcaoDecoder.ts

+    const i = this.icao;
+    const p = IcaoDecoder.MILITARY_PATTERNS;
+
    return (


Make MILITARY_PATTERNS an array of regexp instead of a dictionary and then just return MILITARY_PATTERNS.some((p) => p.test(i));

all the comments can move up to the definition

makrsmark · 2025-10-21T10:24:24Z

lib/IcaoDecoder.ts

  isMilitary() {
-    let i = this.icao;
+    const i = this.icao;
+    const p = IcaoDecoder.MILITARY_PATTERNS;


If you take my other comment, remove as it's not saving anything

jazzberry-ai · 2025-12-07T23:56:32Z

Bug Report

Name	Severity	Example test case	Description
Incorrect Australia Military Range Classification	High	`new IcaoDecoder('7c87ff').isMilitary()` should return `true`, but returns `false`.	The Australia military ranges were incorrectly split which caused some ICAO addresses to be incorrectly classified.

_{Comments? Email us.}

Copilot

Pull request overview

This PR refactors the IcaoDecoder.isMilitary() method to eliminate a critical performance bottleneck by replacing 30+ regex pattern compilations per call with a static array of pre-compiled numeric ranges. The implementation now uses efficient integer range comparisons instead of repeated regex matching.

Key Changes:

Replaced regex-based pattern matching with numeric range comparisons using a static lookup array
Converted military ICAO ranges to a structured array of hex tuples for O(n) range checking
Added comprehensive test coverage for the refactored isMilitary() method across all military ranges

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
lib/IcaoDecoder.ts	Refactored `isMilitary()` to use static numeric range comparisons instead of regex patterns, significantly improving performance
lib/IcaoDecoder.test.ts	Added comprehensive test suite covering all military ICAO ranges with boundary and edge case testing
lib/plugins/Label_14.test.ts	Added test file for Label_15 position report decoding with multiple message format variants

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-07T23:57:21Z

lib/plugins/Label_14.test.ts

+import { MessageDecoder } from '../MessageDecoder';
+import { Label_15 } from './Label_15';
+
+describe('Label_15', () => {


The test file is named Label_14.test.ts but imports and tests Label_15. The filename should be Label_15.test.ts to match the plugin being tested.

Copilot · 2025-12-07T23:57:21Z

lib/plugins/Label_14.test.ts

+        });
+    });
+
+    test('decodes short variant missing unkown', () => {


Corrected spelling of 'unkown' to 'unknown'.

Suggested change

test('decodes short variant missing unkown', () => {

test('decodes short variant missing unknown', () => {

Copilot · 2025-12-07T23:57:21Z

lib/plugins/Label_14.test.ts

+        expect(decodeResult.remaining.text).toBe('--- 42');
+    });
+
+    test('decodes off variant no unkown', () => {


Corrected spelling of 'unkown' to 'unknown'.

Suggested change

test('decodes off variant no unkown', () => {

test('decodes off variant no unknown', () => {

lib/IcaoDecoder.ts

lib/IcaoDecoder.test.ts

jazzberry-ai · 2025-12-07T23:58:25Z

Bug Report

Name: Incorrect exclusion range for Australia military ICAO addresses
Severity: Low
Example test case: The Australia military ICAO address range 0x7c8400 - 0x7c87fe excludes 0x7c87ff correctly. However, there wasn't a specific test to validate that 0x7c87ff is excluded. I have added a test case in lib/IcaoDecoder.test.ts to explicitly check for this exclusion.
Description: The original regex patterns may have implicitly handled this exclusion, but the conversion to numeric ranges requires explicit handling and testing of boundary conditions. While the code seems correct, the lack of a specific test for this boundary condition increases the risk of a potential error.

_{Comments? Email us.}

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

jazzberry-ai · 2025-12-08T00:00:32Z

Bug Report

Name	Severity	Example test case	Description
Incorrect Australia and Austria military ranges	High	`new IcaoDecoder('7cc409').isMilitary()` should return `false`	The original ranges for Australia and Austria military ICAO addresses were incorrectly defined, leading to potential misidentification of military aircraft. The exception values were not handled correctly. The ranges have been updated, and exception checks added to `isMilitary()` to fix this.

_{Comments? Email us.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

lib/IcaoDecoder.ts (1)

5-59: Numeric range–based isMilitary looks correct and achieves the perf goal

The static MILITARY_RANGES table plus the simple numeric range scan give you exactly one parseInt and an O(N) check with early exit, and the ranges line up with the new tests (including the Austria/Australia exceptions). This is a solid improvement over per-call regex compilation.

Two entirely optional tweaks you might consider later:

Add a small guard for malformed ICAO strings (e.g., if (Number.isNaN(n)) return false;), just to make the intent explicit that non-hex / partially-hex inputs are always treated as non‑military.

If you prefer a more declarative style, you could express the loop as return IcaoDecoder.MILITARY_RANGES.some(([start, end]) => n >= start && n <= end); without changing behavior.

Also applies to: 66-75

lib/IcaoDecoder.test.ts (1)

3-302: Strong isMilitary coverage; consider tightening one test description

The test suite does a good job exercising boundaries and “just outside” values for all of the configured ranges, plus some clear negative cases — this should make future changes to MILITARY_RANGES relatively safe.

One small consistency nit: the Germany test case is named 'should match 3e8000-3ebfff', but MILITARY_RANGES encodes [0x3ea000, 0x3ebfff], and you only assert values starting at 3ea000. If 3ea000 is indeed the intended lower bound, it may be worth updating the description (or adding an assertion for 3e8000 if that’s supposed to be included) to avoid confusion for future readers.

lib/plugins/Label_14.test.ts (1)

1-123: Label_15 tests align well with decode logic; only minor naming nits

These tests exercise all the key branches of Label_15.decode (short vs OFF variants, with/without date/altitude, and the invalid-path handling) and validate both structured fields and remaining.text, which should give good confidence in the plugin behavior.

Two minor, purely cosmetic nits you might consider when you next touch this file:

The file is named Label_14.test.ts but only tests Label_15; renaming it to Label_15.test.ts would make it easier to find by convention.

A couple of test descriptions say unkown instead of unknown, which can make searching slightly less predictable.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a6ea2e and 27c0cc4.

📒 Files selected for processing (3)

lib/IcaoDecoder.test.ts (1 hunks)
lib/IcaoDecoder.ts (1 hunks)
lib/plugins/Label_14.test.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

lib/plugins/Label_14.test.ts (2)

lib/plugins/Label_15.ts (1)

Label_15 (8-61)

lib/utils/result_formatter.ts (1)

text (386-394)

lib/IcaoDecoder.test.ts (1)

lib/IcaoDecoder.ts (1)

IcaoDecoder (1-77)

jazzberry-ai · 2025-12-08T00:02:02Z

An error occured.

_{This error may be due to rate limits. If this error persists, please email us.}

jazzberry-ai · 2025-12-08T00:03:11Z

Bug Report

Name	Severity	Example test case	Description
Austria Range Exception	Low	`new IcaoDecoder('447ac7').isMilitary()`	The Austria range has an exception at `0x447ac7`. The current implementation uses two ranges to exclude this specific ICAO address. However, if the input ICAO is exactly `447ac7`, `parseInt(i, 16)` will parse "447ac7" and the code will proceed without specifically checking for and excluding this value. Thus, `0x447ac7` will not be flagged as military, which is the correct behavior. This is not a bug but a potential misunderstanding. The test suite has been updated to verify this behavior.

_{Comments? Email us.}

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lib/IcaoDecoder.ts

lib/IcaoDecoder.test.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

jazzberry-ai · 2025-12-08T00:06:54Z

Bug Report

Name	Severity	Example test case	Description
Incorrect Australia Ranges	High	`new IcaoDecoder('7c87ff').isMilitary()` should return `false`	The upper bounds of the Australia ranges were incorrectly defined, causing some non-military ICAO addresses to be misclassified as military.
Incorrect Australia Ranges	High	`new IcaoDecoder('7c8fff').isMilitary()` should return `false`	The upper bounds of the Australia ranges were incorrectly defined, causing some non-military ICAO addresses to be misclassified as military.

_{Comments? Email us.}

kevinelliott requested a review from makrsmark October 21, 2025 06:23

kevinelliott self-assigned this Oct 21, 2025

kevinelliott added the enhancement New feature or request label Oct 21, 2025

coderabbitai bot reviewed Oct 21, 2025

View reviewed changes

makrsmark reviewed Oct 21, 2025

View reviewed changes

makrsmark added 3 commits December 7, 2025 18:35

adding tests

cb805d9

PR feedback

04b8602

convert to numeric

27c0cc4

Copilot AI review requested due to automatic review settings December 7, 2025 23:56

Copilot AI reviewed Dec 7, 2025

View reviewed changes

Remove accidentally committed Label_14.test.ts

9ff7f5a

Apply suggestion from @Copilot

d6b0c65

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

coderabbitai bot reviewed Dec 8, 2025

View reviewed changes

style change

2ef83a8

merge US

d52be0a

makrsmark requested a review from Copilot December 8, 2025 00:03

Copilot AI reviewed Dec 8, 2025

View reviewed changes

lib/IcaoDecoder.ts Outdated Show resolved Hide resolved

lib/IcaoDecoder.test.ts Outdated Show resolved Hide resolved

Apply suggestions from code review

75a216b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

	test('decodes short variant missing unkown', () => {
	test('decodes short variant missing unknown', () => {

	test('decodes off variant no unkown', () => {
	test('decodes off variant no unknown', () => {

Uh oh!

Optimize IcaoDecoder.isMilitary() by eliminating regex recompilation #291

Are you sure you want to change the base?

Optimize IcaoDecoder.isMilitary() by eliminating regex recompilation #291

Uh oh!

Conversation

kevinelliott commented Oct 21, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

jazzberry-ai bot commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug Report

Uh oh!

coderabbitai bot commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Other AI code review bot(s) detected

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

makrsmark Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

makrsmark Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

jazzberry-ai bot commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jazzberry-ai bot commented Dec 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug Report

Uh oh!

jazzberry-ai bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

jazzberry-ai bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jazzberry-ai bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

jazzberry-ai bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

kevinelliott commented Oct 21, 2025 •

edited by coderabbitai bot

Loading

jazzberry-ai bot commented Oct 21, 2025 •

edited

Loading

coderabbitai bot commented Oct 21, 2025 •

edited

Loading

jazzberry-ai bot commented Dec 7, 2025 •

edited

Loading

jazzberry-ai bot commented Dec 7, 2025 •

edited

Loading

jazzberry-ai bot commented Dec 8, 2025 •

edited

Loading

jazzberry-ai bot commented Dec 8, 2025 •

edited

Loading

jazzberry-ai bot commented Dec 8, 2025 •

edited

Loading

jazzberry-ai bot commented Dec 8, 2025 •

edited

Loading