feat(packs): EcomOps workflow + run connector/pack tests in CI#19
Merged
Conversation
Tuần 2 of the Month-1 roadmap: the customer-service pipeline for Vietnamese
online shops.
Nhận tin -> Classify Intent -> Extract Order Info -> Fetch Order Status
-> Draft Response -> Guardrail Check -> Send hoặc Handoff
packs/ecomops/ (new):
- text.py — Vietnamese normalization. Customers routinely type without
diacritics, so both the message and the keyword lists are folded to a
diacritics-free form; "đơn hàng" and "don hang" match the same rule. đ/Đ
are handled explicitly since they don't decompose under NFD.
- intents.py — 7 EcomOps intents. RuleBasedIntentClassifier matches keyword
tokens in order allowing one filler word between them, because Vietnamese
slots pronouns inside fixed phrases ("khi nào EM nhận được hàng" must still
match "khi nao nhan"). A greeting only wins when it's the whole message, so
"chào shop, đơn em đâu" classifies as order_status. LLMIntentClassifier goes
through MagiC's own /api/v1/llm/chat gateway (cheapest routing) rather than a
provider SDK, so spend lands in the Cost Controller. HybridIntentClassifier
only pays for the LLM when the rules aren't confident, and keeps the rule
guess if the gateway is down.
- extraction.py — order code + phone. Phones are masked out before the bare
numeric order-code pattern runs, otherwise "sdt 0901234567" returns the phone
as the order id and every lookup silently misses.
- guardrails.py — an LLM draft is not automatically safe to send from a shop's
official account. Blocks leaked third-party contacts, hard delivery
guarantees, unapproved refunds/vouchers, and system-prompt/credential leaks.
Runs on normalized text so it can't be evaded by dropping diacritics.
- prompts.py — Vietnamese prompt library, kept as data so shops can retune tone
without touching pipeline code.
- workflow.py — the pipeline. Every dependency is injected, so it makes no
network calls and knows nothing about Zalo or Sheets. A blocked draft is
never returned to the caller: reply falls back to a safe message and action
becomes "handoff". Complaints skip drafting entirely.
- example_worker.py — end-to-end demo (Zalo webhook -> Sheet lookup -> guarded
reply), which is the stated Tuần 2 deliverable.
CI (this is the part that was quietly broken): the python job only ever ran
sdk/python. The 44 connector tests added in #17/#18 have never executed on CI,
and neither would these 76. Running `pytest connectors packs` from the root
failed outright because asyncio_mode lived only in per-directory pytest.ini
files — hence the new root pytest.ini. ruff was also weaker outside sdk/python
(E501/I aren't in ruff's default rule set), so a root ruff.toml now mirrors
sdk/python's settings; the 4 import-order issues that surfaced are fixed.
Tests: 76 new (packs) + 44 (connectors) + 36 (sdk) = 156, all passing. Go core
untouched, builds clean.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (26)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…rail Found while PR #19's CI was running: a shop that configures its hotline as "+84987654321" (or "84987654321") had its OWN number flagged as a leaked third-party contact, so any reply quoting the hotline was blocked and pushed to a human. The rule compared raw digit strings, so the +84 and 0-prefixed spellings of the same number never matched. Adds `canonical_phone()` — folds "+84 987-654-321", "84987654321", "987654321" and "0987-654-321" all to "0987654321" — and compares canonical forms on both sides. Detection is unaffected: a genuinely foreign number is still blocked (covered by a new test that allowlists the hotline and checks a different number is still caught). Also NOSONAR the `pip install ruff` lint line. Editing that line to widen the lint scope made SonarCloud treat it as new code and re-apply the same two supply-chain hotspots (S4830/S5445) already suppressed on the install lines above it — that's what dropped this PR's Security Rating to C. Tests: 14 new (canonical_phone table + hotline spellings + allowlist scoping), 170 total, all passing.
|
3 tasks
kienbui1995
pushed a commit
that referenced
this pull request
Jul 25, 2026
Closes a gap in what #19 shipped: the workflow accepted a `knowledge_lookup` but nothing implemented it, and the system prompt forbids answering from anything outside the "Dữ liệu" block. So shipping_fee and return_exchange — two of the seven intents — could never actually be answered; the bot could only promise to check. That's the safe failure mode, but it isn't a useful one. - knowledge.py — two interchangeable async lookups: * MagicKnowledgeLookup reads MagiC's Knowledge Hub (GET /api/v1/knowledge), so policies live alongside every other MagiC workload. * StaticKnowledgeLookup reads a YAML file, for shops running the pack against an in-memory core with no Hub. Entries carry two tags (`ecomops` + topic), and lookups require both. The Hub has no server-side tag filter, so without that a shipping question would get answered with the return policy whenever the keyword search overlapped — or worse, with unrelated org knowledge that merely shared a word. A Hub outage degrades to "no policy data" rather than dropping the message; the guardrails still catch anything the model invents to fill the gap. - sample_knowledge.yaml + seed_knowledge.py — Vietnamese starter KB (shipping fees, 7-day return policy, order statuses, product/complaint handling) and a seeder. Both the file and the seeder log loudly that this is sample data to be replaced with the shop's real policy, since wrong content here becomes a wrong answer to a customer. - CI: the install step now picks up every requirements.txt under connectors/ and packs/ instead of naming one file. Adding pyyaml for this feature would otherwise have gone missing on CI (it passed locally only because my venv happened to have it), and a future connector's deps would hit the same trap. Intentionally NOT included: the Nhanh.vn connector, which is the other Tuần 3-4 item. BUSINESS_MODEL.md and OPEN_CORE_STRATEGY.md both list the production Nhanh.vn connector as Commercial ("điểm khác biệt lớn"), and this is the public repo — that needs a deliberate decision, not a drive-by commit. Tests: 21 new (topic mapping, tag filtering, top_k, no-topic short-circuit, Hub-down degradation, malformed payloads, static YAML, seeding round-trip, plus a check that every topic in the sample YAML is reachable from some intent so the file can't drift into dead data). 191 total, all passing.
3 tasks
kienbui1995
added a commit
that referenced
this pull request
Jul 25, 2026
* feat(ecomops): knowledge base — shop policies for draft replies Closes a gap in what #19 shipped: the workflow accepted a `knowledge_lookup` but nothing implemented it, and the system prompt forbids answering from anything outside the "Dữ liệu" block. So shipping_fee and return_exchange — two of the seven intents — could never actually be answered; the bot could only promise to check. That's the safe failure mode, but it isn't a useful one. - knowledge.py — two interchangeable async lookups: * MagicKnowledgeLookup reads MagiC's Knowledge Hub (GET /api/v1/knowledge), so policies live alongside every other MagiC workload. * StaticKnowledgeLookup reads a YAML file, for shops running the pack against an in-memory core with no Hub. Entries carry two tags (`ecomops` + topic), and lookups require both. The Hub has no server-side tag filter, so without that a shipping question would get answered with the return policy whenever the keyword search overlapped — or worse, with unrelated org knowledge that merely shared a word. A Hub outage degrades to "no policy data" rather than dropping the message; the guardrails still catch anything the model invents to fill the gap. - sample_knowledge.yaml + seed_knowledge.py — Vietnamese starter KB (shipping fees, 7-day return policy, order statuses, product/complaint handling) and a seeder. Both the file and the seeder log loudly that this is sample data to be replaced with the shop's real policy, since wrong content here becomes a wrong answer to a customer. - CI: the install step now picks up every requirements.txt under connectors/ and packs/ instead of naming one file. Adding pyyaml for this feature would otherwise have gone missing on CI (it passed locally only because my venv happened to have it), and a future connector's deps would hit the same trap. Intentionally NOT included: the Nhanh.vn connector, which is the other Tuần 3-4 item. BUSINESS_MODEL.md and OPEN_CORE_STRATEGY.md both list the production Nhanh.vn connector as Commercial ("điểm khác biệt lớn"), and this is the public repo — that needs a deliberate decision, not a drive-by commit. Tests: 21 new (topic mapping, tag filtering, top_k, no-topic short-circuit, Hub-down degradation, malformed payloads, static YAML, seeding round-trip, plus a check that every topic in the sample YAML is reachable from some intent so the file can't drift into dead data). 191 total, all passing. * fix(ci): put NOSONAR on the pip line, not the block-scalar key My own mistake from the previous commit. Switching the install step to a `run: |` block moved the pip invocation onto a continuation line, but the NOSONAR marker stayed up on the `run: |` key — so it suppressed nothing and SonarCloud re-applied the two supply-chain hotspots (S4830/S5445), dropping the Security Rating to C. Back to a single-line `run:`, matching the neighbouring install/lint steps where this pattern is already proven: the marker is a YAML comment on the same line as the command, stripped before the shell ever sees it. Behaviour is unchanged — Python Tests passed on the previous commit, confirming the find/xargs install and the new pyyaml dependency both work. * fix(security): validate knowledge file paths before reading them SonarCloud flagged a high-severity path-injection vulnerability in the code I added in this PR — and it was right, so this fixes it rather than suppressing it. `seed_knowledge.py` passes sys.argv[1] straight into `knowledge_entries_for_seeding()`, which did `Path(path).read_text()` with no validation, and everything it reads is POSTed into the Knowledge Hub. That turns "seed the knowledge base" into "copy any file this process can read into a searchable store" — an arbitrary-file read whose output lands somewhere it can be queried back out. `StaticKnowledgeLookup.from_yaml` had the same hole. Both now go through `_resolve_yaml_path()`, which requires an existing regular file with a .yaml/.yml suffix. It calls `resolve()` first, deliberately: that collapses `..` and follows symlinks, so a `policy.yaml` symlinked at a private key is judged by its real name and fails the suffix check instead of sailing through it. The seeder CLI now exits 2 with a clear message instead of propagating a traceback. Verified by hand that /etc/hostname, a suffix-less file and a .yaml symlink to a key file are all refused, and that a genuine policy YAML still loads. Note on the previous commit: it moved a NOSONAR marker in ci.yml on the theory that the block-scalar placement was what dropped the Security Rating. That placement was genuinely wrong and is worth keeping fixed, but it was not the cause — this was. The rule key only became visible once the finding surfaced as a code-scanning review comment on knowledge.py:134. Tests: 7 new (non-YAML suffix, missing file, directory, symlink disguise, traversal, valid file, and the same validation on StaticKnowledgeLookup). 198 total, all passing. --------- Co-authored-by: Claude <noreply@anthropic.com>
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
2 participants
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.
What does this PR do?
Tuần 2 of the Month-1 roadmap (
MAGIC_MONTH1_TASK_LIST.md): the EcomOps customer-service pipeline for Vietnamese online shops, plus a CI gap fix.packs/ecomops/(new)text.py— Vietnamese normalization. Customers routinely type without diacritics, so both the message and the keyword lists fold to a diacritics-free form;"đơn hàng"and"don hang"hit the same rule.đ/Đare mapped explicitly because they don't decompose under NFD.intents.py— 7 EcomOps intents.RuleBasedIntentClassifiermatches keyword tokens in order allowing one filler word between them, because Vietnamese slots pronouns inside otherwise-fixed phrases ("khi nào **em** nhận được hàng"must still match the keyword"khi nao nhan"). A greeting only wins when it's the entire message, so"chào shop, đơn em đâu"classifies asorder_status, notgreeting.LLMIntentClassifiergoes through MagiC's own/api/v1/llm/chatgateway (cheapest routing) rather than a provider SDK, so token spend lands in the Cost Controller.HybridIntentClassifieronly pays for the LLM when the rules aren't confident, and keeps the rule guess if the gateway is down.extraction.py— order code + phone. Phones are masked out before the bare-numeric order-code pattern runs; otherwise"sdt 0901234567"returns the phone as the order id and every lookup silently misses.guardrails.py— an LLM draft is not automatically safe to send from a shop's official account. Blocks leaked third-party contacts, hard delivery guarantees, unapproved refunds/vouchers, and system-prompt/credential leaks. Runs on normalized text, so it can't be evaded by dropping diacritics.prompts.py— Vietnamese prompt library, kept as data so shops can retune tone without touching pipeline code.workflow.py— the pipeline. Every dependency is injected, so it makes no network calls and knows nothing about Zalo or Sheets. A guardrail-blocked draft is never returned to the caller:replyfalls back to a safe message andactionbecomes"handoff". Complaints skip drafting entirely.example_worker.py— end-to-end demo (Zalo webhook → Sheet lookup → guarded reply), which is the stated Tuần 2 deliverable: "AI trả lời được câu hỏi trạng thái đơn hàng qua Zalo, dùng dữ liệu từ Google Sheet".CI gap (worth flagging)
The
pythonjob only ever ransdk/python. The 44 connector tests added in #17/#18 have never actually executed on CI, and neither would these 76. Fixing that surfaced two more things:pytest connectors packsfrom the repo root failed outright —asyncio_modelived only in per-directorypytest.inifiles. Added a rootpytest.ini; each connector/pack keeps its own for standalone runs.sdk/python:E501/Iaren't in ruff's default rule set, so those directories were effectively unlinted for line length and import order. Added a rootruff.tomlmirroringsdk/python's settings, and fixed the 4 import-order issues it surfaced.CLAUDE.mdupdated so the documented structure and build commands match reality (connectors/,packs/, and how to run their tests).Related issue
Tracks Month 1, Tuần 2 (EcomOps Workflow cơ bản) of the technical roadmap in magic-business/technical-planning.
Checklist
cd core && go build ./...clean (Go core untouched)packs/ecomops/tests/covers intents (incl. diacritic-free spellings and LLM-down fallback), extraction (incl. the phone/order-code collision), every guardrail rule, and each workflow handoff pathpacks/README.md,packs/ecomops/README.md(Vietnamese),CLAUDE.mdGenerated by Claude Code