Skip to content

LCORE-3141: Docs models are PHONY targets #2225

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

Merged
tisnik merged 2 commits into from
Jul 23, 2026
Merged

LCORE-3141: Docs models are PHONY targets #2225

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bef849c
LCORE-3141: Docs models are PHONY targets
tisnik Jul 23, 2026
bcfdf8c
Update Makefile
tisnik Jul 23, 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
19 changes: 18 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,23 @@ LLAMA_STACK_IMAGE ?= lightspeed-llama-stack:local
LLAMA_STACK_PORT ?= 8321
CONTAINER_RUNTIME ?= $(shell command -v podman 2>/dev/null || command -v docker 2>/dev/null)

.PHONY: run run-stack build-llama-stack-image remove-llama-stack-container stop-llama-stack-container start-llama-stack-container wait-for-llama-stack-health clean-llama-stack
.PHONY: run \
run-stack \
build-llama-stack-image \
remove-llama-stack-container \
stop-llama-stack-container \
start-llama-stack-container \
wait-for-llama-stack-health \
clean-llama-stack \
docs/models \
docs/models/requests.puml \
docs/models/responses.puml \
docs/models/common.puml \
docs/models/database.puml \
docs/models/requests.svg \
docs/models/responses.svg \
docs/models/common.svg \
docs/models/database.svg
Comment on lines +20 to +36

@coderabbitai coderabbitai Bot Jul 23, 2026 •
edited
Loading

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -A6 '^docs/models/(requests|responses|common|database)\.puml:' Makefile

Repository: lightspeed-core/lightspeed-stack

Length of output: 1352

🏁 Script executed:

#!/bin/bash
set -u

echo "== Makefile relevant section =="
sed -n '1,45p;200,255p' Makefile

echo
echo "== all docs/models model generator lines =="
rg -n -A6 '^docs/models/(requests|responses|common|database)\.puml:' Makefile

echo
echo "== shared outputs in Makefile =="
rg -n -E 'docs/models/(classes|packages)\.puml|docs/models/classes\.puml|docs/models/packages\.puml' Makefile || true

Repository: lightspeed-core/lightspeed-stack

Length of output: 5617

Avoid reusing shared model-generation output files.

All four docs/models/*.puml targets are phony, so make -j docs/models can run them concurrently while each pyreverse invocation writes into the same docs/models/classes.puml before moving it to its final name. Serialize the targets or generate each model in an isolated temporary directory so generated diagrams cannot be overwritten or mismatched.

🧰 Tools
🪛 checkmake (0.3.2)

[warning] 21-21: Required target "all" is missing from the Makefile.

(minphony)

[warning] 21-21: Required target "clean" is missing from the Makefile.

(minphony)

[warning] 21-21: Required target "test" is missing from the Makefile.

(minphony)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Makefile` around lines 20 - 36, Update the docs/models generation targets and
their pyreverse output handling so parallel make execution cannot share or
overwrite docs/models/classes.puml. Either serialize the four .puml targets
through an explicit dependency chain or run each pyreverse invocation in an
isolated temporary directory before moving its result to the intended target,
preserving the existing generated filenames.

@tisnik tisnik Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@coderabbitai create issue

@coderabbitai coderabbitai Bot Jul 23, 2026

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.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

repo="lightspeed-core/lightspeed-stack"
title="Prevent parallel docs model generation from overwriting shared output"

body=$(cat <<'EOF'
## Summary
`make -j docs/models` can execute the phony PlantUML model targets concurrently. Each underlying `pyreverse` command writes its intermediate result to the same `docs/models/classes.puml` path before it is renamed, so diagrams can be overwritten, mismatched, or cause a target to fail.

## Required changes
Update the documentation model generation rules in `Makefile` so each of these targets has isolated intermediate output or is explicitly serialized:

- `docs/models/requests.puml`
- `docs/models/responses.puml`
- `docs/models/common.puml`
- `docs/models/database.puml`

Preserve the existing final generated filenames and the existing `docs/models` interface.

## Rationale
The model targets are declared `.PHONY`, which allows GNU Make to schedule them in parallel. Sharing `docs/models/classes.puml` makes the recipes unsafe under parallel execution.

## Affected area
- `Makefile` documentation model-generation targets

## Acceptance criteria
- Running `make -j docs/models` does not share or overwrite an intermediate `classes.puml` file between targets.
- Each target produces its matching final `.puml` diagram.
- The target behavior remains compatible with the SVG generation targets.

## Backlinks
- Reported from PR `#2225`: https://github.com/lightspeed-core/lightspeed-stack/pull/2225
- Review comment: https://github.com/lightspeed-core/lightspeed-stack/pull/2225#discussion_r3636257133
- Requested by `@tisnik`.
EOF
)

gh issue create \
  --repo "$repo" \
  --title "$title" \
  --body "$body" \
  --assignee "tisnik"

Length of output: 1916

@tisnik, created follow-up issue #2227: Prevent parallel docs model generation from overwriting shared output. It includes the PR and review-comment backlinks, required changes, and acceptance criteria.


run-stack: ## Run lightspeed-stack directly, without building dependent service/s
uv run src/lightspeed_stack.py -c $(CONFIG)
Expand Down Expand Up @@ -193,6 +209,7 @@ devel-doc: ## Generate documentation for developers
scripts/gen_doc.py

docs/models: docs/models/requests.puml docs/models/responses.puml docs/models/database.puml docs/models/common.puml ## Generate documentation about models
rm -f docs/models/packages.puml

docs/models/requests.puml: ## Generate PlantUML class diagram for requests data models
uv run pyreverse src/models/api/requests/ --output puml --output-directory=docs/models/
Expand Down
Loading