Skip to content

Optimize copilot usage #4581

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
atobiszei wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Optimize copilot usage #4581

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b415db9
Draft
atobiszei Sep 15, 2026
17c48db
Self-review
atobiszei Sep 15, 2026
28d5332
Update
atobiszei Sep 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions .github/agents/build-validate.agent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
---
description: "Use when: build, test, validate OVMS C++ code changes. Runs bazel build/test inside docker build container. Returns a compact PASS/FAIL report."
tools: [execute, read, search]
user-invocable: true
---
You are an OVMS build & validation agent. Your job is to build and test code changes and report results.

## Environment
- Run bazel commands inside the build container: `docker exec -w /ovms <container> bash -lc "<command>"`
- The workspace root is the OVMS repository checkout
- Write full command output to a log file under `logs/`; read only the tail to confirm status, and grep the file for details on failure. Never dump a whole log into the report.

## Workflow
1. Find the build container that mounts the current workspace, deterministically, in **one** command.
It lists all containers (running + stopped) whose mounts include this workspace, puts running
ones first, and prints a single name:
```bash
for id in $(docker ps -aq); do docker inspect "$id" --format '{{if .State.Running}}0{{else}}1{{end}} {{.Name}} {{range .Mounts}}{{.Source}};{{end}}'; done | grep -F "$(pwd)" | sort | sed -E 's#^[01] /([^ ]+).*#\1#' | head -1
Comment on lines +14 to +18
```
- This handles the case of 2–3 matching containers (running one wins) and always yields at most one name.
- If it prints a name, that is your container — use it. If it is stopped, `docker start <name>` once.
- Run this discovery **exactly once**. Do NOT re-scan or retry it after a later bazel error — a build/test failure is not a "container not found" problem.
- Only if it prints nothing: report "No build container found" and stop.
2. Build, redirecting all output to a log file, then check the tail:
```bash
mkdir -p logs
docker exec -w /ovms <container> bash -lc "bazel build --curses=no --color=no //src:ovms_test > /ovms/logs/build.log 2>&1; echo EXIT=\$?"
```
Read the last lines to confirm status: `tail -n 30 logs/build.log`.
3. If the build succeeds and a test filter is specified, run tests the same way:
```bash
docker exec -w /ovms <container> bash -lc "bazel test --test_summary=detailed --test_output=errors --curses=no --color=no --test_filter=\"<filter>\" //src:ovms_test > /ovms/logs/test.log 2>&1; echo EXIT=\$?"
```
4. Determine status from the `EXIT=` code and the log tail. On failure, search the **whole** log
file for the real error (it is often not in the last few lines):
```bash
grep -nE "^(FAILED|ERROR|\[ FAILED \])" logs/test.log | head -20
```
Then read only the specific matching line ranges. Apply the output trimming rules below to the report.

NOTE: Do NOT run clang-format, cpplint, or spell checks. Those are only run manually before commit, not during build validation.

## Output Trimming (CRITICAL)
Build and test logs can be enormous. You MUST minimize context usage:
- **Always redirect full output to `logs/build.log` / `logs/test.log`** (see Workflow). Never pipe raw build/test output into your own context.
- **On success**: report ONLY "PASS" — do not include any log output.
- **On failure**: grep the full log file for the failing lines (`FAILED`, `ERROR`, assertion text) — do not assume the error is in the tail. Report only those lines.
- **Never** dump full build or test logs into the report.
- **Truncate long lines** at 200 characters — test output often contains base64 images or serialized data. Use `cut -c1-200`.
- **For test failures**: report only the test name, the FAILED assertion line, and expected vs actual — not the full test output.
- **If the user asks** for more detail on a specific failure, THEN read the relevant range of the log file for that item only.

## Output Format
Return a compact structured report:
- **Build**: PASS/FAIL (if FAIL: last 20 lines of error output, lines truncated at 200 chars)
- **Tests**: PASS/FAIL (if FAIL: list of failing test names + assertion message only, one line each)

## Constraints
- DO NOT edit any source files
- DO NOT run the full test suite unless explicitly asked — always prefer --test_filter
- ONLY report results, never attempt fixes
- If the build container is not running, report that and stop
- DO NOT dump raw logs — always filter and truncate
- Keep the total report under 50 lines
208 changes: 16 additions & 192 deletions .github/copilot-instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,194 +1,18 @@
# Copilot Instructions for OpenVINO Model Server (OVMS)

## Project Overview

OpenVINO Model Server (OVMS) is a high-performance inference serving platform built on top of **OpenVINO** and **OpenVINO GenAI**. The codebase is primarily **C++** with **Bazel** as the build system. Supporting infrastructure uses **Makefiles**, **Dockerfiles** (Ubuntu & Red Hat), and **batch files** (Windows).

**Performance is a top priority** — both **throughput** and **latency** are critical. Code changes should be evaluated for their performance impact. Avoid unnecessary copies, allocations, and blocking operations on the hot path.

## Repository Structure

- `src/` — Main C++ source code (server, gRPC/REST handlers, model management, pipelines, mediapipe, LLM, C API)
- `src/test/` — C++ unit tests (gtest-based); this is where most developer tests live
- `src/python/` — Python custom node bindings and related code
- `demos/` — End-user demo applications
- `client/` — Client libraries (C++, Python, Go, Java)
- `docs/` — Documentation
- `third_party/` — Third-party dependency definitions for Bazel
- `Dockerfile.ubuntu` / `Dockerfile.redhat` — Multi-stage Dockerfiles for Linux builds
- `Makefile` — Orchestrates Docker-based builds and test runs
- `*.bat` files — Windows build and setup scripts

## Code Style

- C++ style is enforced via `cpplint` and `clang-format`
- Run `make style` to check formatting
- Apache 2.0 license headers are required on all source files

## Expertise Areas

1. **OpenVINO Expertise:**
- Proficient with OpenVINO core libraries and `ov::genai` components
- Familiar with OpenVINO performance optimization techniques
2. **C++ Proficiency:**
- Strong C++17 skills
- Familiar with best practices in memory management, concurrency, and template programming
3. **Serving Infrastructure:**
- gRPC and REST API handler design
- Model management, pipeline orchestration, and MediaPipe integration
- C API (`libovms_shared.so`) surface and bindings
4. **Build System Awareness:**
- Bazel build configuration and dependency management
- Minimizing C++ build times: forward declarations, include-what-you-use, avoiding transitive header leakage
- Understanding of Bazel targets, build flags (`--//:distro`), and multi-stage Docker builds

## Code Review Instructions for PRs

When analyzing a Pull Request, follow this protocol:

1. Follow **C++ Core Guidelines** strictly. Include references in review comments.
2. Check for **hidden performance costs**: avoid `dynamic_cast` on the hot path; suggest `static_cast` or redesign if the type is known.
3. **Avoid copies**: ensure large data structures (tensors, buffers) are passed by reference or moved, not copied.
4. **Documentation**: ensure new public APIs have docstrings in C++ headers and Python bindings; update `docs/` as needed.
5. **Test coverage**: ensure that new features or changes have corresponding tests in `src/test/`.
6. **Formatting & safety:**
- No `using namespace std; using namespace ov;`. Prefer explicit using with specific symbols if needed, for readability.
- No `auto` for primitive types where it obscures readability.
- Use `const` and `constexpr` wherever possible.
7. Pass non-fundamental values by `const` reference wherever possible.
8. Prefer member initializer lists over direct assignments in constructor bodies.
9. Verify that the result of every newly introduced function is used in at least one call site (except `void` functions).
10. Use descriptive function and variable names. Avoid duplicate code — extract common functionality into reusable utilities.
11. When initial container values are known upfront, prefer initializer-list / brace-initialization over constructing an empty container and inserting.
12. Unused functions and includes are not allowed. Build times are already long — do not add unnecessary `#include` directives. Prefer forward declarations where possible and follow the include-what-you-use principle.
- **Forward-declare in headers, include in `.cpp`**: if a header only uses pointers or references to a type, use a forward declaration (`class Foo;`) instead of `#include "foo.hpp"`. Move the full `#include` to the `.cpp` file where the type is actually used.
- **Keep headers self-contained but minimal**: each header must compile on its own, but should not pull in transitive dependencies that callers don't need.
- **Prefer opaque types / Pimpl**: for complex implementation details, consider the Pimpl idiom to keep implementation-only types out of the public header entirely.
- **Never include a header solely for a typedef or enum**: forward-declare the enum (`enum class Foo;` in C++17) or relocate the typedef to a lightweight `fwd.hpp`-style header.
13. **No dangling references or temporaries bound to `const T&`**:
- Never use `const T&` parameters with default arguments that construct temporaries (e.g. `const std::string& param = ""`). This binds a reference to a temporary — use a function overload instead, or pass by value.
- When accepting `const T&` in constructors or functions that store the reference, verify that the referenced object's lifetime outlives the usage to avoid dangling references.
- Prefer overloads over default arguments for non-trivial types passed by reference.

## Build System

### Bazel (primary build tool)

Building and testing is done **inside a Docker `-build` container** with the repository mounted. Developers do not run Bazel on the host directly.

**Important:** Building the `-build` image from scratch is time-expensive, but may be required if dependencies or the build environment change. Before building a new one, check if one already exists:
```bash
docker images | grep -- -build
```
If a `-build` image exists, start a container from it with the repository mounted:
```bash
docker run -it -v $(pwd):/ovms \
-e http_proxy=$http_proxy -e https_proxy=$https_proxy -e no_proxy=$no_proxy \
<image_name> bash
```
If a container from a previous session is still available (`docker ps -a`), reuse it with `docker start -i <container>` or `docker exec -it <container> bash`.

**Key Bazel targets:**

| Target | Description |
|--------|-------------|
| `//src:ovms` | Main OVMS server binary |
| `//src:ovms_test` | C++ unit tests (gtest) |
| `//src:ovms_shared` | C API shared library (`libovms_shared.so`) |

**Build the server:**
```bash
bazel build //src:ovms
```

**Build and run unit tests:**
```bash
bazel build //src:ovms_test
bazel test --test_summary=detailed --test_output=streamed //src:ovms_test
```

### Red Hat builds — distro flag

For Red Hat (UBI9) builds, the `--//:distro=redhat` flag must be passed to all Bazel commands:
```bash
bazel build --//:distro=redhat //src:ovms
bazel test --//:distro=redhat //src:ovms_test
```

The default distro is `ubuntu`. Inside the `-build` container, the distro is auto-detected from `/etc/redhat-release`.

### Makefile (Docker-based workflow)

The Makefile orchestrates full Docker-based builds. Key targets:

| Target | Description |
|--------|-------------|
| `make docker_build` | Full build: builder image → package → release images (default target) |
| `make ovms_builder_image` | Build the `-build` Docker image (compilation container) |
| `make targz_package` | Extract `.tar.gz` release package |
| `make ovms_release_images` | Build CPU and GPU release Docker images |
| `make run_unit_tests` | Run C++ unit tests in the `-build` container |
| `make test_functional` | Run Python functional tests |
| `make style` / `make cpplint` | Code style checks |

**Red Hat build via Make:**
```bash
make docker_build BASE_OS=redhat
```

**Supported `BASE_OS` values:** `ubuntu24`, `ubuntu22`, `redhat`

### Dockerfile stages

Both `Dockerfile.ubuntu` and `Dockerfile.redhat` use multi-stage builds:

| Stage | Purpose |
|-------|---------|
| `base_build` | System dependencies, Boost, Azure SDK, OpenCV |
| `build` | Bazel + OpenVINO setup, compiles OVMS (the `-build` container) |
| `capi-build` | Builds C API shared library and examples |
| `pkg` | Packages everything into a `.tar.gz` |
| `release` | Minimal runtime image with entrypoint |

### Windows builds

Windows builds use batch files in the repository root:
- `windows_install_build_dependencies.bat` — Install build dependencies (MSVC 2022 Build Tools, etc.)
- `windows_build.bat` — Main build script
- `windows_test.bat` — Run tests

Windows-specific Bazel config: `--config=win_mp_on_py_off` (or `--config=win_mp_on_py_on` for Python support).

## Testing

### Test setup

Before running tests, test models must be prepared:
```bash
make prepare_models
```

Models are exported using the `demos/common/export_models/export_model.py` script (used internally by the test setup).

If LLM test models change (e.g., new model version, OpenVINO version change or config update), you may need to remove and regenerate the LLM test data:
```bash
rm -rf src/test/llm_testing
make prepare_models
```

### Running tests

Running the full `//src:ovms_test` suite is **time-consuming**. During development, always run only the test fixtures relevant to your changes first using `--test_filter`:
```bash
bazel test --test_summary=detailed --test_output=streamed --test_filter="SuiteName.TestName" //src:ovms_test
```
Run the full test suite only after the targeted tests pass.

### Test structure

- **Unit tests** are in `src/test/` — gtest-based C++ tests covering all server components
- Test files follow the `*_test.cpp` naming convention
- Test utilities: `test_utils.hpp`, `light_test_utils.hpp`, `c_api_test_utils.hpp`
- Test models are stored in `src/test/` subdirectories (e.g., `dummy/`, `passthrough/`, `summator/`)
- Specialized test areas: `src/test/llm/`, `src/test/mediapipe/`, `src/test/python/`, `src/test/embeddings/`

> Canonical guidance for all AI agents lives in **[AGENTS.md](../AGENTS.md)** at the repository
> root. VS Code / Copilot reads that file automatically — start there.

`AGENTS.md` is a lean index: it holds the always-on principles (performance-first C++17,
include-what-you-use, style via `make` targets, license headers, security) and points to
task-scoped rule files.

Path-scoped rules auto-attach while you edit matching files and also live under
[.github/instructions/](instructions):

- [cpp-headers.instructions.md](instructions/cpp-headers.instructions.md) — `*.hpp` rules
- [cpp-sources.instructions.md](instructions/cpp-sources.instructions.md) — `*.cpp` rules
- [bazel-build.instructions.md](instructions/bazel-build.instructions.md) — `BUILD` file rules
- [build-workflow.instructions.md](instructions/build-workflow.instructions.md) — build & test workflow
- [code-review.instructions.md](instructions/code-review.instructions.md) — C++ review standards
- [ovms-auto-validate.instructions.md](instructions/ovms-auto-validate.instructions.md) — post-change validation
32 changes: 32 additions & 0 deletions .github/instructions/bazel-build.instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
applyTo: "**/BUILD"
---
# Bazel BUILD File Guidelines (OVMS)

## Target Granularity

- **One logical unit per `ovms_cc_library` target.** Each `.cpp` file should belong to its own target with its associated `.hpp` in `hdrs`.
- **Do not add sources to monolithic targets.** If you're tempted to add a file to an existing large target, create a new focused target instead.

## Macros

- Use `ovms_cc_library` for production code (sets standard copts, linkopts, local_defines).
- Use `ovms_cc_test_library` for test utility libraries (adds gtest dep automatically).
- Use `additional_copts` for feature-flag copts (`COPTS_MEDIAPIPE`, `COPTS_PYTHON`). Do not override `copts` directly.

## Dependencies

- **Include-what-you-use for deps**: list only direct dependencies. Do not rely on transitive deps.
- **Prefer narrow deps**: depend on interface targets (`servable_metadata_provider`, `model_instance_provider`) over heavy implementation targets (`modelmanager`) when possible.
- **Forward-declaration targets**: for lightweight `_fwd.hpp` headers, create a separate header-only target (e.g., `tensorinfo_fwd`) so consumers can depend on it without pulling the full implementation.
- **Use `select()` for optional deps**: MediaPipe, Python, Drogon features should be behind `select()` on the appropriate config setting.

## Visibility

- Default to no `visibility` (package-private). Only add `visibility = ["//visibility:public"]` when the target is genuinely needed outside its package.
- Tight visibility prevents accidental coupling between subsystems.

## Naming Conventions

- Target names: lowercase with descriptive names matching the source (e.g., `grpcservermodule`, `kfs_grpc_frontend`).
- Test targets: match the test file name pattern (e.g., `ensemble_tests` for `ensemble_tests.cpp`).
Loading
Loading