Skip to content

Add standalone AI assistant chatbot (#1)#178

Open
Yamumsbumb wants to merge 1 commit into
Gracker:mainfrom
Yamumsbumb:main
Open

Add standalone AI assistant chatbot (#1)#178
Yamumsbumb wants to merge 1 commit into
Gracker:mainfrom
Yamumsbumb:main

Conversation

@Yamumsbumb

Copy link
Copy Markdown
  • Add standalone AI assistant chatbot

  • Clean up ai-assistant build config


Summary

Linked Issue

Change Type

  • feat (new capability)
  • fix (bug fix, no API change)
  • refactor (internal cleanup, no behavior change)
  • docs (documentation only)
  • chore (build, CI, tooling, dependency updates)
  • skill / strategy (YAML skill or .strategy.md / .template.md)

Contributor / AI Context

  • I read AGENTS.md before making this change
  • I read the relevant .claude/rules/* file(s) for the affected area
  • For non-trivial changes, I used an independent read-only review pass or documented why it was unavailable

Affected Areas

  • Web UI / committed frontend/
  • CLI / smp
  • Docker / portable package
  • HTTP/SSE API
  • Reports / exports / analysis-result snapshots
  • backend/src/agentv3/ or backend/src/agentOpenAI/ (agent runtime)
  • MCP tool registry / tool adapters
  • Provider Manager / runtime selection / session resume
  • backend/skills/ (YAML skills)
  • backend/strategies/ (*.strategy.md, .template.md, knowledge-*)
  • Code-aware analysis / CodeRef / source lookup
  • perfetto/ui/src/plugins/com.smartperfetto.AIAssistant/ (frontend)
  • scripts/ / backend/scripts/ (tooling, generators)
  • .github/workflows/ (CI)
  • Docs only
  • Tests only

Done Conditions

  • I preserved unrelated local changes
  • I did not hardcode prompt content, MCP tool lists, Skill counts, or scene lists
  • I kept chat/report/CLI/snapshot evidence boundaries intact where AI output is affected
  • I included any extra targeted test command needed for this change in the test plan below
  • New .ts / .yaml / .sh / .strategy.md files carry SPDX AGPL v3 header

Test Plan

Risk / Rollback

Notes for Reviewers

* Add standalone AI assistant chatbot

Co-authored-by: Yamumsbumb <Yamumsbumb@users.noreply.github.com>

* Clean up ai-assistant build config

Co-authored-by: Yamumsbumb <Yamumsbumb@users.noreply.github.com>

---------

Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Co-authored-by: Yamumsbumb <Yamumsbumb@users.noreply.github.com>

@Gracker Gracker left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. I don't think this should merge in its current form.

The main issue is architectural: this adds a second, standalone AI assistant stack under ai-assistant/ with its own FastAPI backend and Next.js frontend. The backend reads OPENAI_API_KEY, OPENAI_MODEL, and OPENAI_BASE_URL directly and calls Chat Completions itself. That bypasses SmartPerfetto's existing Provider Manager, runtime selection/session pinning, MCP tool/trace context, report/snapshot contracts, and the existing AI Assistant/API surfaces. If this is intended to be a SmartPerfetto assistant feature, it needs to integrate through the existing backend runtime/API instead of adding a parallel Python/Next app.

There is also a security blocker in the documented run path. The README and installer start Uvicorn with --host 0.0.0.0, while /api/chat is unauthenticated and spends the server-side OpenAI key for arbitrary requests. CORS does not protect this endpoint from same-network clients or curl. The default should bind to 127.0.0.1, or the service needs explicit authentication plus an opt-in LAN mode.

Finally, the new standalone build smoke is not wired into the root npm run verify:pr or GitHub Actions, and this PR currently has no reported checks. If a separate app remains in the tree, its install/build verification needs to be part of the normal PR gate.

Local checks I ran on the PR head (f25c2a5c):

  • git diff --check origin/main...origin/pr/178
  • bash -n on the added shell scripts
  • shellcheck -x ai-assistant/install.sh ai-assistant/scripts/test-build.sh
  • cd ai-assistant && ./scripts/test-build.sh
  • Python dependency install plus importing backend.main

Those smoke checks passed, but they don't address the architecture and security blockers above.

lab4ogtc pushed a commit to lab4ogtc/SmartPerfetto that referenced this pull request Jun 13, 2026
Land Plan 50 (Spark Gracker#34, Gracker#67, Gracker#105, Gracker#150, Gracker#176, Gracker#177, Gracker#178) — durable
App/Device/Build/CUJ baseline store with diff and CI regression gate.

Reuse boundary: `TraceSummaryBaselineRef` (Plan 02) and
`TraceSummaryMetric.metricId` namespace already exist. Plan 50 does not
re-define those — `BaselineRecord extends TraceSummaryBaselineRef` so
consumers see one shape, and metric ids stay in the same namespace.
Plan 50's contribution is durable persistence + cross-baseline diff +
CI gate semantics on top of Plan 02.

Types added to `backend/src/types/sparkContracts.ts`:

- `BaselineMetric` — per-metric aggregate (median/p95/p99/max +
  sampleCount). Carries `unsupportedReason` so devices that cannot
  collect a metric (e.g. GPU render stages on certain SoCs) don't
  silently zero-fill.
- `BaselineRecord` — extends SparkProvenance + TraceSummaryBaselineRef.
  Adds `key` (PerfBaselineKey), `status` (CurationStatus), and
  `redactionState` ('raw'|'partial'|'redacted'). Service layer
  enforces sampleCount >= 3 for status='published'; the schema does
  not so older snapshots remain readable.
- `BaselineDiffDelta` — per-metric delta with optional numeric fields
  + `severity: ... | 'unsupported'`. `unsupportedReason` is required
  when severity is 'unsupported' (missing on baseline, sample below 3,
  divide-by-zero, etc.).
- `BaselineDiffArtifact` — diff between two baselines or
  trace-vs-baseline.
- `RegressionGateResult` — CI gate output. `diff` is **optional** when
  status is 'skipped'; the gate must instead record `skipReason` so
  triagers can audit why the gate did not run.
- `BaselineStoreContract` — service surface with `baselines[]` and
  optional `matrix[]` for SoC/OEM cross-baseline comparison.

Seven new test cases in `__tests__/sparkContracts.test.ts` cover the
reuse boundary (BaselineRecord populates inherited fields), the
unsupported-data paths (metric and delta both), and the skipped-gate
contract. Test count: 50 (was 43).

Test tier: contract / type-only. `npx tsc --noEmit` clean,
sparkContracts.test.ts passes 50/50. Trace regression intentionally
not run per the tiered policy in commit d8529e1.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

2 participants