Skip to content

signage: Added guidance and skills files - #42

Draft
vkb1 wants to merge 2 commits into
intel-retail:mainfrom
vkb1:feature/vkb1/add-guidance-skills
Draft

signage: Added guidance and skills files#42
vkb1 wants to merge 2 commits into
intel-retail:mainfrom
vkb1:feature/vkb1/add-guidance-skills

Conversation

@vkb1

@vkb1 vkb1 commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

PULL DESCRIPTION

Basic Info

Info Please fill out this column
Environment used (e.g. Ubuntu 20.04)
ROS Version (e.g. Foxy)
Ticket Link (e.g. https://jira.devtools.intel.com/browse/SL6-1100)

REFERENCES

Reference URL for issue tracking (JIRA/HSD/Github): <URL to be filled>

  • Added label to the Pull Request following the template: ISDM_<Complexity>*
    Note-1: Depending on complexity of code changes, use the suitable word for complexity: Low/Medium/High
    Example: PR for Slim boot loader project with medium complexity can have the label as: ISDM_Medium
  • Added label to the Pull Request for easier discoverability and search

CODE MAINTAINABILITY

  • Commit Message meets guidelines as indicated in the URL* -> defined in Contributing
  • Every commit is a single defect fix and does not mix feature addition or changes*
  • Added required tests -> Added required new tests relevant to the changes
    • PR contains URL links to functional tests executed with the new tests
  • Updated relevant documentation
  • Updated build steps/commands
  • PR change contains code related to security
  • PR introduces breaking change. (If YES, please provide description)
  • Specific instructions or information for code reviewers (If any):

Maintainer Mandatory (to be filled by PR Reviewer/Approving Maintainer)

  • Maintainer who approves the Pull Request for merge is required to mark the checklist below as appropriate for the PR change reviewed as key proof of attestation indicating reasons for merge.
  • Those checklist items which are not marked are considered as not applicable for the PR change.
  • Items marked with an asterisk suffix are mandatory items to check and if not marked will be treated as non-compliant pull requests by the maintainers for ISDM compliance.

QUALITY CHECKS

  • Architectural and Design Fit
  • Quality of code (At least one should be checked as applicable)*
    • Commit Message meets guidelines
    • PR changes adhere to industry practices and standards
    • Upstream expectations are met
    • Adopted domain specific coding standards
    • Error and exception code paths implemented correctly
    • Code reviewed for domain or language specific anti-patterns
    • Code is adequately commented
    • Code copyright is correct
    • Tracing output are minimized and logic
    • Confusing logic is explained in comments
    • Commit comment can be used to design a new test case for the changes
  • Test coverage shows adequate coverage with required CI functional tests pass on all supported platforms*
  • Static code scan report shows zero critical issues*

CODE REVIEW IMPACT

  • Summary of Defects Detected in Code Review: <%P1xx,P2xx,P3xx,P4xx%>
    Note P1/P2/P3/P4 denotes severity of defects found (Showstopper/High/Medium/Low) and xx denotes number of defects found

SECURITY CHECKS

Please check if your PR fulfills the following requirements:

  • Avoid checks that rely on the result of undefined C behavior.
    Instead, clarify the check condition using defined behaviors. Example: if (x > INT_MAX-100) instead of if (x + 100 < x)
  • Use safe arithmetic to avoid integer overflow.
  • Follow best practices when handling primitive data types
  • Follow security best practices when dealing with pointers
  • Avoid uncontrolled format strings
  • Prevent buffer overflow/underflow
  • Avoid unsafe functions
  • Configure minimal permissions when opening pipes and ports
  • Check contents within input structures are valid before use
  • All forms of input validated
  • Avoid inter-process race conditions
  • Error and exception handling implemented
  • Use keywords and #pragma to constrain gcc compiler optimization
  • Prevent compiler from optimizing out security checks

Signed-off-by: B, Vinod K <vinod.k.b@intel.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds operator/developer guidance (“skills” + references), lightweight agent entrypoints (AGENTS/CLAUDE/GEMINI/Copilot/Cursor), and two helper scripts intended to automate model setup and stack health checks for the digital-signage repo.

Changes:

  • Added scripts/download-models.sh to download/prepare the required YOLO11s / SDXL‑Turbo / MiniLM models with basic verification.
  • Added scripts/check-stack.sh to report container health, scan recent logs, and probe key HTTP endpoints.
  • Added repository guidance + GitHub skills content (user + dev), including reference docs and eval prompts.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
scripts/download-models.sh New model download/export script for PID + AIG model assets with idempotency and verification.
scripts/check-stack.sh New stack health check script: container status, log scanning, and endpoint probes.
GEMINI.md Points Gemini guidance to AGENTS.md.
CLAUDE.md Points Claude guidance to AGENTS.md.
AGENTS.md New centralized project/agent guidelines including build/run workflow and repo conventions.
.github/skills/digital-signage-user/SKILL.md Operator-focused skill for deploying and troubleshooting the stack.
.github/skills/digital-signage-user/references/troubleshooting.md Symptom-driven troubleshooting guide for bring-up, video, ads, and performance.
.github/skills/digital-signage-user/references/env-reference.md Detailed .env variable reference including validation and known repo gaps.
.github/skills/digital-signage-user/references/customize.md How-to guide for changing video source, devices, models, and ad content.
.github/skills/digital-signage-user/references/api-usage.md Curl-driven reference for the AIG/ASe API surface and usage patterns.
.github/skills/digital-signage-user/evals/evals.json Eval prompts/assertions for the operator skill.
.github/skills/digital-signage-dev/SKILL.md Developer-focused skill describing architecture, conventions, and validation workflow.
.github/skills/digital-signage-dev/references/web-ui-service.md Detailed reference for web-ui behavior: MQTT ingestion, selection, polling contract.
.github/skills/digital-signage-dev/references/pipeline-and-config.md Reference for pipeline config, nginx routing, and compose wiring constraints.
.github/skills/digital-signage-dev/references/architecture.md Reference for service boundaries, seams, and startup-order consequences.
.github/skills/digital-signage-dev/references/aig-service.md Reference for the AIG service patterns, schemas, and known inconsistencies.
.github/skills/digital-signage-dev/evals/evals.json Eval prompts/assertions for the developer skill.
.github/copilot-instructions.md Points Copilot instructions to AGENTS.md.
.cursor/rules/project.mdc Points Cursor rules to AGENTS.md.
Comments suppressed due to low confidence (3)

scripts/download-models.sh:120

  • The script uses pip3 inside the venv. Some environments/venvs only provide pip (no pip3), which can cause the install step to fail even though the venv is active. Using python -m pip is more robust and guarantees you’re installing into the active venv.
    pip3 install --quiet --upgrade pip
    pip3 install --quiet -r "${workdir}/model_download_requirements.txt"

scripts/download-models.sh:152

  • Same as above: using pip3 inside the venv is less portable than python -m pip and can fail on systems where pip3 isn’t created in the venv.
    pip3 install --quiet --upgrade pip
    pip3 install --quiet -r "${workdir}/export-requirements.txt"

scripts/download-models.sh:103

  • usage() states the script requires git, but download_pid_model() only checks for wget and python3. If git is actually needed by the upstream download/quantize script, it will fail later with a harder-to-diagnose error. Consider enforcing the documented prerequisite here.
    log "Downloading and quantizing ${YOLO_MODEL_NAME} (detection)"
    require_cmd wget
    require_cmd python3


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +28 to +30
# Pinned so a moving upstream default cannot silently change what gets downloaded.
ASC_REF="v3.6.3"
ASC_SCRIPT_URL="https://raw.githubusercontent.com/intel-retail/automated-self-checkout/${ASC_REF}/download_models/downloadAndQuantizeModel.sh"
Comment thread AGENTS.md
Comment on lines +28 to +36
The Makefile is the interface for everything — prefer a target over running the underlying commands by hand. `make help` lists them all.

```bash
make download_models # Download/prepare YOLO11s, SDXL-Turbo, MiniLM (add FORCE=1 to redo)
make build # Build all Docker images
make up # Validate env + models, start all containers
make down # Stop and remove containers + volumes
make status # Stack health: containers, log errors, endpoint probes (alias: check_stack)
```
Comment thread scripts/check-stack.sh
Comment on lines +7 to +10
# Health check for the digital signage stack. Backs `make status` / `make check_stack`.
#
# Reports container state and restart counts, scans recent logs, and probes the AIG API and
# web UI so a live container that returns nothing is distinguishable from a dead one.
Comment on lines +63 to +79
## The Makefile is the interface

[Makefile](../../../Makefile) at the repo root drives everything — prefer a `make` target over
reconstructing the underlying commands, and quote the target to the user rather than a raw
script path. `make help` lists them all.

| Target | Does |
|---|---|
| `make download_models` | Downloads and prepares YOLO11s, SDXL-Turbo and MiniLM (`scripts/download-models.sh`) |
| `make download_models_pid` / `make download_models_aig` | Just the detection half / just the AIG half |
| `make ... FORCE=1` | Re-download even when the target directory is already populated |
| `make build` | Builds all images |
| `make up` | `check_models` → `check_env_variables` → `validate_host_ip` → `down` → `compose up -d` |
| `make status` (alias `make check_stack`) | Stack health: containers, restart counts, recent log errors, endpoint probes (`scripts/check-stack.sh`) |
| `make down` | Stops containers and removes volumes |
| `make check_models` / `check_env_variables` / `validate_host_ip` | The individual `make up` preconditions, runnable alone to isolate a failure |

Comment on lines +115 to +119
[Makefile](../../../Makefile) at the repo root is the interface for all of this — `make help`
lists every target. `make status` (alias `check_stack`) runs
[scripts/check-stack.sh](../../../scripts/check-stack.sh) and exits non-zero on failure, so it
works as a gate in a script. If the stack has never been brought up on this machine, models
come from `make download_models` (see the `digital-signage-user` skill).
Signed-off-by: B, Vinod K <vinod.k.b@intel.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