Address review: ATN v4 guard, startRuleIndex by name, test hardening

- Assert ATN serialization version 4 for both lexer and parser to enforce
  antlr4ng compatibility contract
- Resolve startRuleIndex by looking up "root" rule name instead of hardcoding 0
- Fix MockRestChannel.bytesOutput() to return real BytesStreamOutput
- Document nullable elements in literalNames/symbolicNames Javadoc
- Rename test methods to follow testXxx() convention per ppl/plugin modules

Signed-off-by: Eric Wei <mengwei.eric@gmail.com>

Author: mengweieric

plugin/src/test/java/org/opensearch/sql/plugin/rest/RestPPLGrammarActionTest.java

@@ -241,7 +241,7 @@ public XContentBuilder newBuilder(
 
     @Override
     public BytesStreamOutput bytesOutput() {
-      return null;
+      return new BytesStreamOutput();
     }
   }
 }

ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/GrammarBundle.java

@@ -44,9 +44,15 @@ public class GrammarBundle {
   /** Start rule index (0 = root rule). */
   private int startRuleIndex;
 
-  /** Literal token names indexed by token type (e.g. "'search'", "'|'"). */
+  /**
+   * Literal token names indexed by token type (e.g. "'search'", "'|'"). Elements may be null for
+   * tokens with no literal form; clients must handle sparse arrays.
+   */
   @NonNull private String[] literalNames;
 
-  /** Symbolic token names indexed by token type (e.g. "SEARCH", "PIPE"). */
+  /**
+   * Symbolic token names indexed by token type (e.g. "SEARCH", "PIPE"). Elements may be null for
+   * tokens with no symbolic name; clients must handle sparse arrays.
+   */
   @NonNull private String[] symbolicNames;
 }

ppl/src/main/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleBuilder.java

@@ -8,6 +8,7 @@
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
+import java.util.Arrays;
 import org.antlr.v4.runtime.CharStreams;
 import org.antlr.v4.runtime.CommonTokenStream;
 import org.antlr.v4.runtime.Vocabulary;
@@ -48,12 +49,17 @@ public GrammarBundle build() {
         .parserRuleNames(parser.getRuleNames())
         .channelNames(lexer.getChannelNames())
         .modeNames(lexer.getModeNames())
-        .startRuleIndex(0)
+        .startRuleIndex(resolveStartRuleIndex(parser.getRuleNames()))
         .literalNames(literalNames)
         .symbolicNames(symbolicNames)
         .build();
   }
 
+  private static int resolveStartRuleIndex(String[] ruleNames) {
+    int idx = Arrays.asList(ruleNames).indexOf("root");
+    return Math.max(idx, 0);
+  }
+
   private static String computeGrammarHash(int[] lexerATN, int[] parserATN) {
     try {
       MessageDigest digest = MessageDigest.getInstance("SHA-256");

ppl/src/test/java/org/opensearch/sql/ppl/autocomplete/PPLGrammarBundleBuilderTest.java

@@ -14,6 +14,8 @@
 
 public class PPLGrammarBundleBuilderTest {
 
+  private static final int EXPECTED_ATN_SERIALIZATION_VERSION = 4;
+
   private static GrammarBundle bundle;
 
   @BeforeClass
@@ -22,17 +24,17 @@ public static void buildBundle() {
   }
 
   @Test
-  public void bundleIsNotNull() {
+  public void testBuildBundleNotNull() {
     assertNotNull(bundle);
   }
 
   @Test
-  public void bundleVersionIsSet() {
+  public void testBuildBundleVersionIsSet() {
     assertEquals("1.0", bundle.getBundleVersion());
   }
 
   @Test
-  public void antlrVersionIsSet() {
+  public void testBuildAntlrVersionIsSet() {
     String version = bundle.getAntlrVersion();
     assertNotNull("antlrVersion should not be null", version);
     assertTrue(
@@ -41,64 +43,80 @@ public void antlrVersionIsSet() {
   }
 
   @Test
-  public void grammarHashHasExpectedFormat() {
+  public void testBuildGrammarHashHasSha256Format() {
     String hash = bundle.getGrammarHash();
     assertNotNull(hash);
     assertTrue("grammarHash should start with 'sha256:'", hash.startsWith("sha256:"));
     assertEquals("grammarHash should be 71 chars (sha256: + 64 hex)", 71, hash.length());
   }
 
   @Test
-  public void startRuleIndexIsZero() {
+  public void testBuildStartRuleIndexIsZero() {
     assertEquals(0, bundle.getStartRuleIndex());
   }
 
   @Test
-  public void lexerATNIsNonEmpty() {
+  public void testBuildLexerATNIsNonEmpty() {
     assertNotNull(bundle.getLexerSerializedATN());
     assertTrue(bundle.getLexerSerializedATN().length > 0);
   }
 
   @Test
-  public void parserATNIsNonEmpty() {
+  public void testBuildLexerATNIsSerializationVersion4() {
+    assertEquals(
+        "Lexer ATN must be serialization version 4 for antlr4ng compatibility",
+        EXPECTED_ATN_SERIALIZATION_VERSION,
+        bundle.getLexerSerializedATN()[0]);
+  }
+
+  @Test
+  public void testBuildParserATNIsNonEmpty() {
     assertNotNull(bundle.getParserSerializedATN());
     assertTrue(bundle.getParserSerializedATN().length > 0);
   }
 
   @Test
-  public void lexerRuleNamesAreNonEmpty() {
+  public void testBuildParserATNIsSerializationVersion4() {
+    assertEquals(
+        "Parser ATN must be serialization version 4 for antlr4ng compatibility",
+        EXPECTED_ATN_SERIALIZATION_VERSION,
+        bundle.getParserSerializedATN()[0]);
+  }
+
+  @Test
+  public void testBuildLexerRuleNamesAreNonEmpty() {
     assertNotNull(bundle.getLexerRuleNames());
     assertTrue(bundle.getLexerRuleNames().length > 0);
   }
 
   @Test
-  public void parserRuleNamesAreNonEmpty() {
+  public void testBuildParserRuleNamesAreNonEmpty() {
     assertNotNull(bundle.getParserRuleNames());
     assertTrue(bundle.getParserRuleNames().length > 0);
   }
 
   @Test
-  public void channelNamesAreNonEmpty() {
+  public void testBuildChannelNamesAreNonEmpty() {
     assertNotNull(bundle.getChannelNames());
     assertTrue(bundle.getChannelNames().length > 0);
   }
 
   @Test
-  public void modeNamesAreNonEmpty() {
+  public void testBuildModeNamesAreNonEmpty() {
     assertNotNull(bundle.getModeNames());
     assertTrue(bundle.getModeNames().length > 0);
   }
 
   @Test
-  public void vocabularyIsNonEmpty() {
+  public void testBuildVocabularyIsNonEmpty() {
     assertNotNull(bundle.getLiteralNames());
     assertNotNull(bundle.getSymbolicNames());
     assertTrue(bundle.getLiteralNames().length > 0);
     assertTrue(bundle.getSymbolicNames().length > 0);
   }
 
   @Test
-  public void buildIsDeterministic() {
+  public void testBuildIsDeterministic() {
     GrammarBundle second = new PPLGrammarBundleBuilder().build();
     assertEquals(
         "Two builds of the same grammar should produce the same hash",