Skip to content

fix(instinct-cli): preserve escaped frontmatter strings - #3605

Open
eneekoruiz wants to merge 2 commits into
affaan-m:mainfrom
eneekoruiz:codex/fix-instinct-string-roundtrip
Open

eneekoruiz wants to merge 2 commits into
affaan-m:mainfrom
eneekoruiz:codex/fix-instinct-string-roundtrip

Conversation

@eneekoruiz

@eneekoruiz eneekoruiz commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Serialize quoted frontmatter strings with JSON escaping and decode those escapes when parsing. Preserve the existing fallback for legacy non-JSON escapes.

Literal newlines currently split a trigger across frontmatter lines; control characters can produce invalid YAML or fold into spaces. Escaping keeps each field on one line and restores its exact contents when read.

Validation: regressions reproduced before the fix; 40 focused tests and the complete 126-test Linux instinct suite pass. Covers quotes, Windows paths, Unicode, C1/NEL controls, literal escapes and frontmatter delimiters inside values. Quote/parser line coverage: 100%/97.4%. Pi freshness, catalog checks, diff checks and independent review passed.

The context registry digest is refreshed without changing skill names, descriptions, retrieval triggers or their provenance.

RED: multiline/control serialization and newline decoding failed; C1/NEL cases exposed malformed or folded YAML. GREEN: 40 targeted tests and 126 complete Linux tests pass; quote/parser coverage 100%/97.4%. Unicode, Windows paths and legacy escapes survive round trips; Pi/catalog checks pass.
@eneekoruiz
eneekoruiz requested a review from affaan-m as a code owner October 16, 2026 13:18
@ecc-tools

ecc-tools Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / Security Evidence

Commit: 1b1119f4092259d0c733a98781432619d4ca8f55

Security evidence gate passed (success)

No security-sensitive scanner-evidence gap detected.

Mode: enforce

Scanned 2 changed file(s). No missing scanner-evidence signal was detected.

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@ecc-tools

ecc-tools Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / PR Risk Taxonomy

Commit: 1b1119f4092259d0c733a98781432619d4ca8f55

PR taxonomy review recommended (neutral)

Detected 4 PR taxonomy bucket(s): Harness Drift, Install Manifest Integrity, Reference Set Validation, Agent Config Review.

Scanned 2 changed file(s).

Roadmap taxonomy buckets:

Harness Drift

Harness-facing changes can drift across Claude Code, Codex, OpenCode, and shared adapter surfaces.

Signals:

  • 2 harness-facing path(s) changed

Paths:

  • skills/continuous-learning-v2/scripts/instinct-cli.py
  • skills/continuous-learning-v2/scripts/test_parse_instinct.py

Install Manifest Integrity

Install manifests, plugin metadata, and shipped skills should stay synchronized with user-facing setup guidance.

Signals:

  • Install or operator docs may drift from the shipped surface
  • 2 install or manifest path(s) changed

Paths:

  • skills/continuous-learning-v2/scripts/instinct-cli.py
  • skills/continuous-learning-v2/scripts/test_parse_instinct.py

Reference Set Validation

AI, analyzer, skill, agent, command, and harness guidance changes should be compared against a maintained eval, golden trace, benchmark, or reference set.

Signals:

  • AI or harness analysis changes may ship without reference-set validation
  • 2 reference-sensitive path(s) changed

Paths:

  • skills/continuous-learning-v2/scripts/instinct-cli.py
  • skills/continuous-learning-v2/scripts/test_parse_instinct.py

Agent Config Review

Agent, command, skill, MCP, and local instruction changes should be reviewed as executable agent configuration.

Signals:

  • 2 agent-config path(s) changed

Paths:

  • skills/continuous-learning-v2/scripts/instinct-cli.py
  • skills/continuous-learning-v2/scripts/test_parse_instinct.py

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@ecc-tools

ecc-tools Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / Reference Set Readiness

Commit: 1b1119f4092259d0c733a98781432619d4ca8f55

Reference set readiness gaps detected (neutral)

Reference evidence present for 0/7 areas (0%) across 2 changed file(s).

This check is based on files changed in this PR. Repository-level readiness is still reported by /ecc-tools analyze comments and generated manifests.

Area Status Evidence / Next Step
Deep analyzer corpus Missing Add analyzer fixture, golden, benchmark, or reference-set files that can catch analyzer regressions.
RAG/evaluator comparison Missing Add retrieval or evaluator reference-set comparison fixtures with expected ranking behavior.
PR salvage/review corpus Missing Add stale-PR, review-thread, reopen-flow, or salvage reference cases for queue cleanup automation.
Discussion triage corpus Missing Add public discussion triage fixtures, golden cases, or reference sets for informational, answered, and no-response classifications.
Harness compatibility Missing Add cross-harness, adapter-compliance, or harness-audit evidence for Claude, Codex, OpenCode, Zed, dmux, and agent surfaces.
Security evidence Missing Attach security evidence such as SBOMs, SARIF, audit reports, or AgentShield evidence packs.
CI failure-mode evidence Missing Add captured CI failure logs, dry-run fixtures, or troubleshooting docs for common workflow failure modes.

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@ecc-tools

ecc-tools Bot commented Oct 16, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / Hosted Promotion Readiness

Commit: 1b1119f4092259d0c733a98781432619d4ca8f55

Hosted promotion readiness passed (success)

No hosted promotion evidence gaps detected across 2 changed file(s); 0 corpus scenarios had matching evidence.

This check compares PR file changes against the evaluator/RAG promotion corpus in src/analyzers/fixtures/evaluator-rag-corpus.ts.
Hosted output scoring inspected 0 completed cached hosted job results.

No evaluator corpus scenarios matched this PR.

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: affaan-m/ECC/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: e66d6e5b-5618-40e8-8e78-1208a89f0a4c
📥 Commits

Reviewing files that changed from the base of the PR and between 1b1119f and ae3c50c.

📒 Files selected for processing: 1
  • manifests/context-packs/skill-triggers@1.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

📜 Recent review details
⏰ Context from checks skipped due to timeout: 1
  • GitHub Check: Greptile Review
🧰 Additional context used
📚 Code guidelines: 2
.cursor/rules/common-hooks.md — auto-discovered
.cursor/rules/typescript-hooks.md — auto-discovered
📓 Path-based instructions: 2
Source excerpt: Use with caution: Source excerpt: Use with caution: Enable for trusted, well-defined plans Source excerpt: Use with caution: Disable for exploratory work Source excerpt: Use with caution: Never use dangerously-skip-permissio...

📄 CodeRabbit inference engine (.cursor/rules/common-hooks.md)

Files:

  • manifests/context-packs/skill-triggers@1.json
Source excerpt: **console.log audit**: Check all modified files for `console.log` before session ends

📄 CodeRabbit inference engine (.cursor/rules/typescript-hooks.md)

Files:

  • manifests/context-packs/skill-triggers@1.json
🔇 Additional comments: 1
manifests/context-packs/skill-triggers@1.json: 1

13-13: 🗄️ Data Integrity & Integration

The registry digest is current.

The canonical registry computation produces 3484b906cf27772bb0afb144344f443949b5cd5e6a9663aafecd71057fbdf735. The validator's stale-digest check does not reject this manifest.

📝 Summary

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of special characters in instinct text, including quotes, line breaks, tabs, Windows paths, Unicode, and control characters, so content is preserved when saving and reading it.
    • Preserved compatibility with existing escaped text formats.

The CLI now JSON-serializes quoted strings and decodes double-quoted values, with a fallback to its previous unescaping behavior. Tests cover control characters, Unicode, escaped newlines, and legacy backslash handling. The context-pack registry digest also changed.

Changes: Instinct String Handling

Layer / File(s) Summary
Quoted string serialization and parsing
skills/continuous-learning-v2/scripts/instinct-cli.py, skills/continuous-learning-v2/scripts/test_parse_instinct.py, manifests/context-packs/skill-triggers@1.json
_yaml_quote now uses JSON serialization to escape strings. parse_instinct_file decodes double-quoted values and falls back to the previous unescaping behavior if decoding fails. Tests cover round-trips, control characters, Unicode, and legacy backslashes. The context-pack registry digest changed.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Merge Risk: 🔵 Low · up to ae3c5

Some older Windows paths may be read incorrectly. The impact is limited, so the PR is mergeable with owner awareness of this compatibility gap.

### Architecture Summary

Architecture risk: 🔵 Low · up to ae3c5

The change affects 2 systems.

Changed systems: skills, manifests

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — skills (service) was modified; 2 changed files map to changed impact.
  • observed — manifests (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in skills/continuous-learning-v2/scripts/instinct-cli.py: _yaml_quote now returns JSON-serialized strings instead of manually escaping backslashes and double quotes; its docstring now notes control-character escaping and single-line serialization.
  • observed — Modified behavior in skills/continuous-learning-v2/scripts/instinct-cli.py: parse_instinct_file now JSON-decodes double-quoted values and falls back to the prior unescaping behavior when decoding raises JSONDecodeError.
  • observed — Modified behavior in skills/continuous-learning-v2/scripts/test_parse_instinct.py: Adds parameterized coverage asserting _yaml_quote output JSON-decodes to the original value and that parsing the resulting quoted trigger preserves it across varied string contents.
  • observed — Modified behavior in skills/continuous-learning-v2/scripts/test_parse_instinct.py: Adds a parser expectation that a quoted \n escape becomes an actual newline in the trigger.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly identifies the fix to escaped frontmatter strings and matches the main code and test changes.
Description check Passed The description directly explains the JSON escaping change, legacy fallback, affected behavior, validation, and registry digest refresh.
Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] No outstanding findings block merging.

Summary

Preserves escaped strings in instinct frontmatter.

  • Instinct strings keep their exact contents in frontmatter.
  • The skill-trigger manifest records its refreshed registry digest.

Since the previous review, only registryDigest changed. No new actionable issues were found.

Reviews (2) · Last reviewed commit: "chore: refresh registry digest after ski..." · Reviewed by Greptile

@ecc-tools

ecc-tools Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / Security Evidence

Commit: ae3c50c6493408bd636406d17568d54fed264db3

Security evidence gate passed (success)

No security-sensitive scanner-evidence gap detected.

Mode: enforce

Scanned 3 changed file(s). No missing scanner-evidence signal was detected.

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@ecc-tools

ecc-tools Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / PR Risk Taxonomy

Commit: ae3c50c6493408bd636406d17568d54fed264db3

PR taxonomy review recommended (neutral)

Detected 4 PR taxonomy bucket(s): Harness Drift, Install Manifest Integrity, Reference Set Validation, Agent Config Review.

Scanned 3 changed file(s).

Roadmap taxonomy buckets:

Harness Drift

Harness-facing changes can drift across Claude Code, Codex, OpenCode, and shared adapter surfaces.

Signals:

  • 2 harness-facing path(s) changed

Paths:

  • skills/continuous-learning-v2/scripts/instinct-cli.py
  • skills/continuous-learning-v2/scripts/test_parse_instinct.py

Install Manifest Integrity

Install manifests, plugin metadata, and shipped skills should stay synchronized with user-facing setup guidance.

Signals:

  • Install or operator docs may drift from the shipped surface
  • 2 install or manifest path(s) changed

Paths:

  • skills/continuous-learning-v2/scripts/instinct-cli.py
  • skills/continuous-learning-v2/scripts/test_parse_instinct.py

Reference Set Validation

AI, analyzer, skill, agent, command, and harness guidance changes should be compared against a maintained eval, golden trace, benchmark, or reference set.

Signals:

  • AI or harness analysis changes may ship without reference-set validation
  • 2 reference-sensitive path(s) changed

Paths:

  • skills/continuous-learning-v2/scripts/instinct-cli.py
  • skills/continuous-learning-v2/scripts/test_parse_instinct.py

Agent Config Review

Agent, command, skill, MCP, and local instruction changes should be reviewed as executable agent configuration.

Signals:

  • 2 agent-config path(s) changed

Paths:

  • skills/continuous-learning-v2/scripts/instinct-cli.py
  • skills/continuous-learning-v2/scripts/test_parse_instinct.py

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@ecc-tools

ecc-tools Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / Reference Set Readiness

Commit: ae3c50c6493408bd636406d17568d54fed264db3

Reference set readiness gaps detected (neutral)

Reference evidence present for 0/7 areas (0%) across 3 changed file(s).

This check is based on files changed in this PR. Repository-level readiness is still reported by /ecc-tools analyze comments and generated manifests.

Area Status Evidence / Next Step
Deep analyzer corpus Missing Add analyzer fixture, golden, benchmark, or reference-set files that can catch analyzer regressions.
RAG/evaluator comparison Missing Add retrieval or evaluator reference-set comparison fixtures with expected ranking behavior.
PR salvage/review corpus Missing Add stale-PR, review-thread, reopen-flow, or salvage reference cases for queue cleanup automation.
Discussion triage corpus Missing Add public discussion triage fixtures, golden cases, or reference sets for informational, answered, and no-response classifications.
Harness compatibility Missing Add cross-harness, adapter-compliance, or harness-audit evidence for Claude, Codex, OpenCode, Zed, dmux, and agent surfaces.
Security evidence Missing Attach security evidence such as SBOMs, SARIF, audit reports, or AgentShield evidence packs.
CI failure-mode evidence Missing Add captured CI failure logs, dry-run fixtures, or troubleshooting docs for common workflow failure modes.

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

@ecc-tools

ecc-tools Bot commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

ECC Tools / Hosted Promotion Readiness

Commit: ae3c50c6493408bd636406d17568d54fed264db3

Hosted promotion readiness passed (success)

No hosted promotion evidence gaps detected across 3 changed file(s); 0 corpus scenarios had matching evidence.

This check compares PR file changes against the evaluator/RAG promotion corpus in src/analyzers/fixtures/evaluator-rag-corpus.ts.
Hosted output scoring inspected 0 completed cached hosted job results.

No evaluator corpus scenarios matched this PR.

Check publication was denied or unavailable. An app owner must enable Checks: read and write, and the installation owner must approve the updated permission.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant