Streamline ppl-bugfix harness and CLAUDE.md after eval

Harness: 369→151 lines. Move fix paths, test templates, symptom
table, and case index to ppl-bugfix-reference.md (loaded on demand).
Add completion gate with 8 mandatory deliverables. Add guardrails
for runaway agents. Merge overlapping checklists.

CLAUDE.md: trim build commands, remove v2 engine references,
simplify ppl-bugfix section.

Command: add bypassPermissions allow-list note.

Signed-off-by: Heng Qian <qianheng@amazon.com>

Author: qianheng-aws

.claude/commands/ppl-bugfix.md

@@ -18,6 +18,8 @@ Optional mode flag (append to any of the above):
 - `--safe` — `acceptEdits` mode. Auto-approve file edits only, Bash commands require manual approval. (Most conservative)
 - `--yolo` — `bypassPermissions` mode. Fully trusted, no prompts. Subagent runs in an isolated worktree so this is safe. (Default)
 
+> **Note**: `bypassPermissions` skips the interactive prompt but still respects the allow-list in `~/.claude/settings.json`. Ensure git/gh write commands are in the global allow-list.
+
 Examples:
 - `/ppl-bugfix #1234` — single issue, defaults to yolo
 - `/ppl-bugfix #1234 #5678 --yolo` — two issues in parallel

.claude/harness/ppl-bugfix-harness.md

@@ -1,253 +1,95 @@
 # PPL Bugfix Harness
 
----
-
-## Phase 0: Triage & Classification
+## Phase 0: Triage
 
-### 0.0 Load Issue Context
+### 0.1 Load & Reproduce
 
 ```bash
 gh issue view <issue_number> --repo opensearch-project/sql
 ```
 
-Read the issue title, description, and any reproduction steps before proceeding.
-
-### 0.1 Reproduce the Bug
-
-```bash
-# Write a failing integration test, or run an existing one
-./gradlew :integ-test:integTest -Dtests.class="*<TestClass>" -Dtests.method="<testMethod>"
-```
+Write a failing test or run an existing one to reproduce the bug on `main`.
 
-**If the bug does not reproduce on latest `main`** — meaning the test scenario runs but produces correct results instead of the reported error — the bug is already fixed. This is distinct from "can't set up the scenario" (which is an infra issue).
+If the bug **does not reproduce** (correct results, not infra failure):
 
 | Finding | Action |
 |---------|--------|
-| Already fixed by another commit | `gh issue comment` with fixing commit/PR, then `gh issue close` |
-| Only on older version | `gh issue comment` with version where it's fixed, then `gh issue close` |
-| Intermittent / environment-specific | Label `flaky` or `needs-info`, do NOT close |
-| Insufficient info to reproduce | Comment asking for repro steps, label `needs-info` |
+| Already fixed | `gh issue comment` + `gh issue close` |
+| Older version only | `gh issue comment` + `gh issue close` |
+| Intermittent | Label `flaky` or `needs-info`, do NOT close |
+| Can't reproduce | Comment asking for repro steps, label `needs-info` |
 
-**HARD STOP**: Do NOT proceed to Phase 1, 2, or 3. Do NOT write tests. Do NOT create a PR. Report back with the finding and the action taken (comment/close). Your job is done.
+**HARD STOP** — do not proceed. Report back.
 
-### 0.2 Classify and Route
+### 0.2 Classify
 
-Identify the bug layer, then use the routing table to determine the fix path and required tests:
+Identify the bug layer (Grammar, AST/Functions, Type System, Optimizer, Execution, DI/Resource) and record it. Consult `.claude/harness/ppl-bugfix-reference.md` for fix-path-specific guidance if needed.
 
-| Layer | Typical Symptoms | Fix Path | Required Tests |
-|-------|-----------------|----------|---------------|
-| **Grammar/Parser** | `SyntaxCheckException`, unrecognized syntax | Path A | AstBuilderTest |
-| **AST/Functions** | Parses OK but AST wrong, function output wrong | Path B | CalcitePPL\*Test + IT + YAML |
-| **Semantic Analysis** | `SemanticCheckException`, type mismatch | Path C | Dedicated UT + CalcitePPLIT + YAML |
-| **Type System** | Field type lost, implicit conversion errors | Path C | Dedicated UT + CalcitePPLIT + YAML |
-| **Optimizer** | Plan bloat, predicate pushdown failure | Path D | CalcitePPL\*Test + CalcitePPLIT + NoPushdownIT + YAML |
-| **Execution** | Wrong results, OOM, memory leaks | Path E | IT + YAML |
-| **DI / Resource** | NPE, extensions not loaded, long-running OOM | Path E | IT + YAML |
+### 0.3 Guardrails
 
-Record before proceeding:
+Stop and report back if:
+- Root cause unclear after reading 15+ source files
+- Fix breaks 5+ unrelated tests
+- Same build error 3 times in a row
 
-```
-Bug Layer: <from table above>
-Fix Path:  <A / B / C / D / E>
-Issue:     #XXXX
-```
-
-### 0.3 Execution Flow
-
-Phases interleave TDD-style — write a failing test first, then fix, then complete test coverage:
+### 0.4 Execution Flow
 
 ```
-Phase 0: Triage → Classify → Route
-  → Phase 2 (partial): Write FAILING test reproducing the bug
-  → Phase 1: Implement fix via routed Path
-  → Phase 2 (remaining): Happy path, edge cases, YAML REST test
-  → Phase 3: Verify → Commit → PR → Decision Log
+Triage → Write FAILING test → Fix → Remaining tests → Verify → Commit → PR → Decision Log → Completion Gate
 ```
 
 ---
 
-## Phase 1: Fix Implementation
-
-### Path A — Grammar / Parser
-
-1. **Update grammar files** (must stay in sync):
-   - `language-grammar/src/main/antlr4/OpenSearchPPLParser.g4` (primary)
-   - `ppl/src/main/antlr/OpenSearchPPLParser.g4`
-   - `async-query-core/src/main/antlr/OpenSearchPPLParser.g4` (if applicable)
-2. **Regenerate**: `./gradlew generateGrammarSource`
-3. **Update AstBuilder**: `ppl/.../parser/AstBuilder.java` — modify `visit*()` to match new rule
-4. **Test**: `AstBuilderTest` using `assertEqual(pplQuery, expectedAstNode)` pattern
-
-> Ref: `734394d` — fixed `renameClasue` → `renameClause` across 3 grammars + AstBuilder
-
-### Path B — AST / Function Implementation
-
-1. **Locate**: AST nodes in `core/.../ast/tree/`, functions in `core/.../expression/function/` or `PPLBuiltinOperators`
-2. **Fix**: Watch Visitor pattern — changes may need syncing to `AbstractNodeVisitor`, `Analyzer`, `CalciteRelNodeVisitor`, `PPLQueryDataAnonymizer`
-3. **Test**: `verifyLogical()`, `verifyPPLToSparkSQL()`, `verifyResult()`
-
-> Ref: `26674f9` — rex nested capture group fix, ordinal index → named group extraction
-
-### Path C — Type System / Semantic Analysis
-
-1. **Locate**: `OpenSearchTypeFactory.java` (Calcite type factory), `Analyzer.java` / `ExpressionAnalyzer.java`
-2. **Fix**: Preserve nullable semantics when overriding Calcite methods; protect UDT from `leastRestrictive()` downgrade
-3. **Test (critical)**: Cover type preservation, nullable propagation, fallback to parent, mixed types — every edge case
-
-> Ref: `ada2e34` — UNION lost timestamp UDT, fixed `leastRestrictive()`, added 8 UTs + 4 ITs
-
-### Path D — Optimizer / Predicate Pushdown
-
-1. **Locate**: `PredicateAnalyzer.java`, `LogicalPlanOptimizer`, `QueryService.java`
-2. **Fix**: Watch `nullAs` semantics (TRUE/FALSE/UNKNOWN); for plan bloat consider Calcite rules like `FilterMergeRule`
-3. **Verify**: `EXPLAIN` output comparison + integration test result correctness
+## Phase 1: Fix
 
-> Ref: `b4df010` — `isnotnull()` not pushed down with multiple `!=`; `e045d15` — multi-filter OOM, inserted `FilterMergeRule`
-
-### Path E — Execution / Resource Management
-
-1. **Locate**: DQE operators in `dqe/`, `OpenSearchExecutionEngine.java`, `SQLPlugin.java`, `OpenSearchPluginModule.java`
-2. **Common patterns**:
-
-   | Problem | Fix | Example |
-   |---------|-----|---------|
-   | Cache key collision | `IndexReader.CacheHelper.getKey()` | `97d5d26` |
-   | Memory leak (no eviction) | Close listener + upper bound | `97d5d26` |
-   | Unbounded growth | `MAX_CAPACITY` check, throw with user guidance | `f024b4f` |
-   | Non-singleton repeated registration | `@Singleton`; `put` instead of `compute/append` | `90393bf` |
-   | DI not injected | Holder class → Guice → constructor injection | `f6be830` |
-
-> Ref: `f024b4f` — `LongOpenHashSet` capacity 1024→8 (8KB→64B per group), 8M cap on 5 HashMap variants
+Find and fix the root cause. Consult `.claude/harness/ppl-bugfix-reference.md` for path-specific patterns and examples.
 
 ---
 
-## Phase 2: Writing Tests
-
-### 2.1 Test Templates
-
-**Unit test** (extend `CalcitePPLAbstractTest`):
-```java
-public class CalcitePPLYourFixTest extends CalcitePPLAbstractTest {
-    public CalcitePPLYourFixTest() {
-        super(CalciteAssert.SchemaSpec.SCOTT_WITH_TEMPORAL);
-    }
-
-    @Before
-    public void init() {
-        doReturn(true).when(settings)
-            .getSettingValue(Settings.Key.CALCITE_ENGINE_ENABLED);
-    }
-
-    @Test
-    public void testBugScenario() {
-        verifyLogical("source=EMP | where SAL > 1000",
-            "LogicalFilter(condition=[>($5, 1000)])\n"
-            + "  LogicalTableScan(table=[[scott, EMP]])\n");
-    }
-}
-```
-
-**Integration test** (extend `CalcitePPLIT`):
-```java
-public class CalcitePPLYourFixIT extends CalcitePPLIT {
-    @Override
-    public void init() throws IOException {
-        super.init();
-        enableCalcite();
-    }
-
-    @Test
-    public void testBugFixEndToEnd() throws IOException {
-        JSONObject result = executeQuery("source=<index> | <your PPL>");
-        verifySchema(result, schema("field", "alias", "type"));
-        verifyDataRows(result, rows("expected_value_1"), rows("expected_value_2"));
-    }
-}
-```
-
-**YAML REST test** — place at `integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/<ISSUE>.yml`, auto-discovered by `RestHandlerClientYamlTestSuiteIT`:
-```yaml
-setup:
-  - do:
-      indices.create:
-        index: test_issue_<ISSUE>
-        body:
-          settings: { number_of_shards: 1, number_of_replicas: 0 }
-          mappings: { properties: { <field>: { type: <type> } } }
-  - do:
-      query.settings:
-        body: { transient: { plugins.calcite.enabled: true } }
----
-teardown:
-  - do:
-      query.settings:
-        body: { transient: { plugins.calcite.enabled: false } }
----
-"<Bug description for issue #ISSUE>":
-  - skip: { features: [headers, allowed_warnings] }
-  - do:
-      bulk: { index: test_issue_<ISSUE>, refresh: true, body: ['{"index": {}}', '{"<field>": "<value>"}'] }
-  - do:
-      headers: { Content-Type: 'application/json' }
-      ppl: { body: { query: "source=test_issue_<ISSUE> | <your PPL>" } }
-  - match: { total: <expected> }
-  - length: { datarows: <expected> }
-```
+## Phase 2: Tests
 
-### 2.2 Test Checklist
+Consult `.claude/harness/ppl-bugfix-reference.md` for test templates.
 
-- [ ] Failing test reproducing the original bug (write FIRST, before fixing)
-- [ ] Happy path after fix
-- [ ] Edge cases (null, empty, extreme volumes)
-- [ ] YAML REST test (named by issue number)
-- [ ] Optimizer bugs: both pushdown enabled and disabled
-- [ ] Type system bugs: nullable / non-nullable combinations
-- [ ] New AST nodes: update `PPLQueryDataAnonymizerTest`
+Required deliverables:
+- Failing test reproducing the bug (written BEFORE the fix)
+- Unit tests covering happy path and edge cases
+- Integration test (`*IT.java` extending `CalcitePPLIT`)
+- YAML REST test at `integ-test/src/yamlRestTest/resources/rest-api-spec/test/issues/<ISSUE>.yml`
 
 ---
 
-## Phase 3: Verification & Submission
+## Phase 3: Verify & Submit
 
-### 3.1 Local Verification
+### 3.1 Verify
 
 ```bash
-./gradlew spotlessApply                                      # 1. Format
-./gradlew :<module>:test --tests "<TestClass>"               # 2. Affected module UT
-./gradlew test                                               # 3. Full UT regression
-./gradlew :integ-test:integTest -Dtests.class="*<YourIT>"   # 4. New IT
-./gradlew :integ-test:integTest                              # 5. Full IT regression
-./gradlew :integ-test:yamlRestTest                           # 6. YAML REST tests
-# If grammar modified:
-./gradlew generateGrammarSource && ./gradlew :ppl:test       # 7. Parser tests
+./gradlew spotlessApply
+./gradlew :<module>:test --tests "<TestClass>"
+./gradlew test
+./gradlew :integ-test:integTest -Dtests.class="*<YourIT>"
 ```
 
-### 3.2 Commit & Push
+Run `./gradlew :integ-test:yamlRestTest` if YAML tests were added. Run `./gradlew generateGrammarSource && ./gradlew :ppl:test` if grammar was modified.
+
+### 3.2 Commit & PR
 
 ```bash
-# Commit with DCO sign-off — do NOT add Co-Authored-By lines
 git add <changed_files>
 git commit -s -m "[BugFix] Fix <description> (#<issue_number>)"
-
-# Sync with main (merge, not rebase — PRs use squash-merge)
 git fetch origin && git merge origin/main
-# Always re-run tests after merge — even a trivial merge can introduce regressions
 ./gradlew test && ./gradlew :integ-test:integTest -Dtests.class="*<YourIT>"
 
-# Resolve fork remote — find or add it
-# 1. Check if a fork remote already exists (not opensearch-project/sql)
-git remote -v
-# 2. If only origin (upstream) exists, add the fork:
+# Resolve fork remote (check git remote -v; add if missing)
 git remote add fork https://github.com/<fork_owner>/sql.git
-# Use the git user name to infer the fork owner, or fall back to "qianheng-aws"
-
-# Push
 git push -u fork <branch_name>
 ```
 
-### 3.3 Create Draft PR
+Do NOT add Co-Authored-By lines. Use the git user name to infer the fork owner, or fall back to "qianheng-aws".
 
 ```bash
-gh pr create --draft --repo opensearch-project/sql --title "[BugFix] Fix <description> (#<issue_number>)" --body "$(cat <<'EOF'
+gh pr create --draft --repo opensearch-project/sql \
+  --title "[BugFix] Fix <description> (#<issue_number>)" \
+  --body "$(cat <<'EOF'
 ### Description
 <Brief description of fix and root cause>
 
@@ -256,19 +98,17 @@ Resolves #<issue_number>
 
 ### Check List
 - [x] New functionality includes testing
-- [x] Javadoc added for new public API
 - [x] Commits signed per DCO (`-s`)
 - [x] `spotlessCheck` passed
 - [x] Unit tests passed
 - [x] Integration tests passed
-- [ ] Grammar changes: all three `.g4` files synced (if applicable)
 EOF
 )"
 ```
 
-### 3.4 Persist Decision Context
+### 3.3 Decision Log
 
-Before the subagent completes, persist the *why* behind key decisions — the PR diff only shows the *what*. The **PR comment is the single source of truth** — it survives across sessions, GA runs, and is visible to both human reviewers and follow-up agents.
+Post as a PR comment:
 
 ```bash
 gh pr comment <pr_number> --body "$(cat <<'EOF'
@@ -284,48 +124,28 @@ EOF
 
 ---
 
-## Phase 4: Retrospective
+## Completion Gate
 
-After each bugfix, check if the harness itself needs updating:
+Do NOT report "done" until every item below is checked. List each in your final report:
 
-- **Template wrong** (API name, assertion field)? → Fix in Phase 2 templates
-- **New bug pattern** not covered? → Add fix path in Phase 1 or symptom in Quick Reference
-- **Verification gap** caused rework? → Add step to 3.1 or 2.2 checklist
-- **Representative fix**? → Add row to Case Index below
+- [ ] **Unit tests**: New test class or methods
+- [ ] **Integration test**: New `*IT.java` test
+- [ ] **YAML REST test**: `issues/<ISSUE>.yml`
+- [ ] **spotlessApply**: Ran successfully
+- [ ] **Tests pass**: Affected modules
+- [ ] **Commit**: DCO sign-off, `[BugFix]` prefix, no Co-Authored-By
+- [ ] **Draft PR**: `--draft`, body contains `Resolves #<issue>`
+- [ ] **Decision Log**: PR comment posted
 
-Include harness improvements in the same PR as the bugfix — they share the same review context.
+If any item is blocked, report which and why.
 
 ---
 
-## Quick Reference: Symptom → Fix Path
-
-```
-SyntaxCheckException / unrecognized syntax       → Path A: Grammar/Parser
-SemanticCheckException / type mismatch            → Path C: Type System / Analysis
-Field type wrong (timestamp→string)               → Path C: check leastRestrictive / coercion
-EXPLAIN shows predicate not pushed down           → Path D: Optimizer / Pushdown
-Multi-condition query: missing/extra rows         → Path D: PredicateAnalyzer nullAs handling
-OOM / memory growth over time                     → Path E: singletons, cache eviction, bounds
-NPE in Transport layer                            → Path E: DI / Guice injection chain
-"node must be boolean/number, found XXX"          → Path B: OpenSearchJsonContent parse*Value
-Regex/function extraction offset                  → Path B: ordinal vs named references
-```
+## Phase 4: Retrospective
 
----
+- [ ] Symptom in Quick Reference? Add if missing.
+- [ ] Classification correct? Fix routing if misleading.
+- [ ] Test template worked as-is? Fix if broken.
+- [ ] New pattern? Add to Case Index.
 
-## Appendix: Case Index
-
-| Commit | Bug | Layer | Tests |
-|--------|-----|-------|-------|
-| `ada2e34` | UNION loses UDT type | Type System | 8 UT + 4 IT |
-| `26674f9` | rex capture group index shift | AST/Functions | Multiple UTs |
-| `b4df010` | isnotnull not pushed down with != | Optimizer | 2 UT + IT |
-| `e045d15` | Multiple filters OOM | Optimizer | 26 output updates |
-| `f024b4f` | High-cardinality GROUP BY OOM | Execution | Benchmark |
-| `97d5d26` | OrdinalMap cache collision + leak | Execution | — |
-| `90393bf` | Non-singleton ExecutionEngine leak | Resource | — |
-| `f6be830` | Transport extensions not injected | DI | — |
-| `734394d` | Grammar rule typo | Grammar | — |
-| `246ed0d` | Float precision flaky test | Test Infra | — |
-| `d56b8fa` | Wildcard index type conflict | Value Parsing | 3 UT + 1 IT + 1 YAML |
-| `5a78b78` | Boolean coercion from numeric in wildcard queries | Value Parsing | 3 UT + 1 IT + 1 YAML |
+Include harness improvements in the same PR.