Onboard the article of query performance tuning - #35
Open
Lei Jiang (lionelc) wants to merge 15 commits into
Open
Lei Jiang (lionelc) wants to merge 15 commits into
Lei Jiang (lionelc) wants to merge 15 commits into
Conversation
Add GitHub Actions workflow for secret scanning using Gitleaks.
Create secret-scan.yml for Gitleaks integration
Onboards https://devblogs.microsoft.com/documentdb/query-performance-tuning-guide/ into the kit: - New standalone skill `documentdb-query-performance-tuning` (end-to-end tuning methodology + `references/documentdb-explain-output.md` for DocumentDB's Postgres-backed explain format). - Merge the former `documentdb-query-optimization` rule into `documentdb-query-optimizer` (explain-first ordering + confounders); content preserved as `references/query-explain-plan.md`. - Extend the knowledge-base router to route to skills (Route B): `skills[]` + `routes_one_hop_skills` in kb.json, `rank_skills` + `--skills` in the engine; scripts routing unchanged. Regression-guarded (kb-router scenario). - Authoritative `VCoreMongoRequests` slow-query KQL in the monitoring rule. - Reproducible before/after test on the ecommerce dataset (`scenarios/ecommerce/query-perf-skill-test.{js,sh}`), an engineering write-up (`docs/query-perf-skill-test-instruction-*.md`), and a customer-facing quickstart (`docs/quickstart-find-and-fix-slow-queries.md`). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86722878-7b4c-4d57-933c-3d4188317703
Multi-word (bi/tri-gram) phrase matching for the diagnostic scripts is brittle: it requires the exact phrase as a substring, so paraphrases miss. Since the scripts are read-only, over-triggering is harmless — so favor recall. - `score_tool` gains an `onegram` mode: split each keyword into single tokens and score +1.5 per distinct matching query token. `rank_tools` uses it (Route A). - Skills (Route B) keep phrase-aware matching (`+3.0` full-phrase / `+1.5` token) — precision matters there, since routing to the wrong guidance is a real cost. - Tests: phrase-rule check moved to skills (`skill_phrase_case`); new `tool_onegram_case` guards the 1-gram token behavior; the "decline vague query" guarantee now asserts on skills (tools intentionally over-trigger). SCENARIO.md and knowledge-base/README.md document the tools=1-gram / skills=phrase split. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86722878-7b4c-4d57-933c-3d4188317703
…reproducible The quickstart jumped straight into `db.orders.find(...)` after seeding without telling the reader how to run it. Add "Step 3 — Open a Mongo shell" (interactive `docker exec -it ... mongosh`) so every `db.orders.…` command has an environment, and run the later steps at that prompt instead of burying the connection in `--eval` one-liners. Steps renumbered 3->4, 4->5, 5->6. Also make the hero example honest: the ecommerce seed uses unseeded Math.random(), so a specific customer id / exact match count are not reproducible. Add a "pick the busiest customer" helper, anchor on the always-true 50,000-document scan, frame the after-count as "the handful that match (~10)", and note that exact numbers vary with the random sample. Verified end-to-end against a live container. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86722878-7b4c-4d57-933c-3d4188317703
The Step 4 prompt was just a question plus a bare query, so the assistant had no idea which database was meant and no way to run explain() — it could only give generic advice. Replace it with a self-contained, copy-pasteable prompt that states the deployment (local documentdb-local container), database/collection, collection size, current indexes, and the orders document shape, plus a docker exec fallback so the assistant can run commands when no MCP connection exists. Add a short note explaining why the context matters and that MCP users can drop the exec line. Schema, status values, index state and the exec command all verified against the live container; the prompt still routes to the query-perf skill. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86722878-7b4c-4d57-933c-3d4188317703
Every doc, seeder, and diagnostic script hardcoded the same literal demo password, which teaches a bad habit and trips secret scanners. Read it from the DB_PASSWORD environment variable instead: - Docs (quickstart, DIAGNOSTICS, README, scenario READMEs, the query-perf test write-up) export a placeholder once up front, start the container with -e PASSWORD="$DB_PASSWORD", and use -p "$DB_PASSWORD" in later commands. The export was moved above `docker run` so the variable exists when it's referenced. - Script/seeder usage comments and "no password" error hints, plus the pytest skip message, now suggest a placeholder rather than a real value. - The quickstart's AI-assistant prompt says the password is in DB_PASSWORD instead of embedding it. Verified end-to-end on a fresh container created with a non-default password: seed, interactive shell, explain (COLLSCAN docsExamined=50000), createIndex, IXSCAN, the before/after harness, a live perf-advisor run, bash -n on every edited script, and the kb-router suite (38 passed). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86722878-7b4c-4d57-933c-3d4188317703
…report The query-perf test write-up was a one-off validation report, not documentation: ~40% of it duplicated the quickstart (container setup, mongosh install, seeding, harness command, managed-Azure note) and it had a single inbound link, so its most valuable content was effectively unreachable. Move that content to where it is actually consumed — the explain reference the `documentdb-query-performance-tuning` skill loads: - On `documentdb-local`, index-backed sort and covered queries do NOT reproduce. An ideal ESR index still yields `SORT -> FETCH -> IXSCAN`, and a covering projection does not drop the `FETCH`. Enabling the experimental engine flags via ALTER SYSTEM (and rebuilding the index) does not change the Mongo-API plan, because the gateway pins them per session. Includes the table of gated flags and how to inspect them, plus the guidance to judge a local index by scan-stage documents/keys examined rather than by the SORT/FETCH disappearing. - The companion gotcha that executionTimeMillis is noisy on small result sets. Delete docs/query-perf-skill-test-instruction.md and repoint scenarios/ecommerce/README.md at the harness + quickstart. The reproducible artifact (query-perf-skill-test.js/.sh) is unchanged. docs/DIAGNOSTICS.md is kept: it documents the diagnostic-script toolbox, a different subsystem from the quickstart, and is linked from README/AGENTS/SKILLS. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 86722878-7b4c-4d57-933c-3d4188317703
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.
-- utilizing the kb-routing mechanism to facilitate the onboarding
-- added a quickstart guide for this scenario
-- refactor for better harness