Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 14 additions & 2 deletions .github/PULL_REQUEST_TEMPLATE
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,20 @@ Thanks for sending a pull request (PR)! Here are some tips for you:
[Contributing to Texera](https://github.com/apache/texera/blob/main/CONTRIBUTING.md)
2. Ensure you have added or run the appropriate tests for your PR
3. If the PR is work in progress, mark it a draft on GitHub.
4. Please write your PR title to summarize what this PR proposes, we
are following Conventional Commits style for PR titles as well.
4. Please write your PR title to summarize what this PR proposes, we
are following Conventional Commits style for PR titles as well:
- `fix` is for behavior that worked before and no longer does; adding or
removing a functionality, or reworking one so that user-facing behavior
intentionally changes, is a `feat`; a change that leaves the user-facing
behavior unchanged is a `refactor`.
- A test-only PR is `test(<module>): ...`; repairing a broken test is
`fix(test, <module>): ...`.
- A dependency bump is `fix(deps, <module>): ...` when it patches a CVE
and `chore(deps, <module>): ...` otherwise; GitHub Actions bumps take
`ci` as their module, e.g. `chore(deps, ci): ...`.
- A PR targeting a release branch appends the version as the last scope
component, e.g. `fix(deps, frontend, v1.2): ...`.
See CONTRIBUTING.md for the full convention.
5. Be sure to keep the PR description updated to reflect all changes.
-->

Expand Down
54 changes: 44 additions & 10 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,18 +138,52 @@ Short, **Conventional Commits**, same shape for branch and commit subject.
| Feature | `feat/agent-workflow-edit` | `feat(agent-service): enable workflow edit` |
| Bug fix | `fix/marker-replay` | `fix(amber): marker replay during reconfiguration` |
| Tests | `test/pyamber-handlers` | `test(pyamber): add handler unit tests` |
| Chore | `chore/angular-21` | `chore(deps): upgrade frontend to Angular 21` |
| CI | `ci/cache-action-bump` | `ci: bump coursier/cache-action to v8.1.0` |
| Chore | `chore/angular-21` | `chore(deps, frontend): upgrade to Angular 21` |
| CI | `ci/merge-queue-stacking` | `ci: stack merge-queue builds by module` |

Both ≤ ~60 chars. For code changes, if you use a scope, use the module name
(`amber`, `pyamber`, `frontend`, `agent-service`, `file-service`, …) — not
`amber-python`. Dependency-only updates split by semantics: `fix(deps): ...` for
runtime/production dependency bumps (they ship to users),
`chore(deps): ...` for dev/toolchain-only bumps, and `ci: ...` for
CI-only changes (including GitHub Actions bumps). Append the module
as a second scope when the bump is module-specific, e.g.
`fix(deps, pyamber): ...`; omit it for cross-module bumps (sbt). No `Co-authored-by:` trailer for the repo
owner.
`amber-python`. No `Co-authored-by:` trailer for the repo owner.

**Choosing the type** turns on what happens to the behavior, not on how big
the diff is:

| The change | Type |
| --- | --- |
| Worked before, broken now | `fix` |
| Support never existed; adding it | `feat` |
| Support exists; removing it | `feat` |
| Reworked so user-facing behavior intentionally changes | `feat` |
| User-facing behavior unchanged | `refactor` |

Behavior is what the code does, not what a doc or an old PR description claims
it does: implementing something that was never actually there is a `feat`.

`refactor` claims the **user-facing** behavior is identical. Tests that pin a
user-facing API must pass untouched — editing one of those assertions means
the behavior moved, so it is a `feat` or a `fix`. Tests that pin internals (a
private helper's signature, call order between collaborators, the shape of an
intermediate value) mirror the implementation, so rewriting them alongside the
code they mirror is still a `refactor`.

**Tests.** A test-only PR is `test(<module>): ...`. Repairing a broken or
flaky test is a bug fix in test code: `fix(test, <module>): ...`.

**Dependencies.** `fix` only when the bump carries a security fix:

| Bump | Commit |
| --- | --- |
| Patches a CVE | `fix(deps, <module>): ...` |
| Everything else | `chore(deps, <module>): ...` |
| GitHub Actions | `chore(deps, ci): ...` |
Comment on lines +172 to +178

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Add the CVE exception to the GitHub Actions row.

Line 172 says that fix applies to security bumps, but Line 178 lists GitHub Actions updates as chore(deps, ci) without an exception. This conflicts with CONTRIBUTING.md Line 86 and docs/contribution-guidelines/_index.md Line 100. A contributor can select chore for a CVE-patching Actions update and miss fix-specific backport automation. Add the fix(deps, ci) form for CVE patches.

Proposed documentation fix
-| GitHub Actions | `chore(deps, ci): ...` |
+| GitHub Actions | `chore(deps, ci): ...`, or `fix(deps, ci): ...` when the bump patches a CVE |

As per coding guidelines, classify dependency bumps as fix only when they patch a CVE and use chore otherwise.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
**Dependencies.** `fix` only when the bump carries a security fix:
| Bump | Commit |
| --- | --- |
| Patches a CVE | `fix(deps, <module>): ...` |
| Everything else | `chore(deps, <module>): ...` |
| GitHub Actions | `chore(deps, ci): ...` |
**Dependencies.** `fix` only when the bump carries a security fix:
| Bump | Commit |
| --- | --- |
| Patches a CVE | `fix(deps, <module>): ...` |
| Everything else | `chore(deps, <module>): ...` |
| GitHub Actions | `chore(deps, ci): ...`, or `fix(deps, ci): ...` when the bump patches a CVE |
🤖 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 `@AGENTS.md` around lines 172 - 178, Update the GitHub Actions row in the
Dependencies commit-convention table to distinguish CVE-patching updates from
all other Actions bumps: document `fix(deps, ci): ...` for CVE patches and
retain `chore(deps, ci): ...` otherwise, matching the existing dependency-bump
guidance.

Source: Coding guidelines


Omit the module for cross-module bumps (sbt). GitHub Actions bumps take `ci`
as their module — that is what [`.github/renovate.json5`](.github/renovate.json5)
opens them with; a bare `ci: ...` is for hand-written CI and workflow changes.

**Backports.** A PR targeting `release/vX.Y` appends the version as the last
scope component — `fix(deps, frontend, v1.2): ...`. Version tags belong only
on release-branch PRs, never on one targeting `main`.

### Issues and PRs

Expand Down Expand Up @@ -193,7 +227,7 @@ write/adjust test (red) -> edit source (green) -> refactor
| New feature / behavior change | Failing test, then implement. |
| Bug fix | Regression test reproducing the bug, then fix. |
| Code with **no tests** | **Characterization tests** pin current behavior first; only then change source. |
| Refactor (no behavior change) | Tests stay green throughout — no assertion edits. |
| Refactor (no user-facing behavior change) | Tests stay green throughout. User-facing API assertions stay untouched; tests that mirror internals may be rewritten with the code. |

Every test must cover:

Expand Down
39 changes: 37 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,43 @@ Thank you for your interest in contributing to Texera! Please follow the steps b
- All PR titles will be used as the **squashed commit message** when merged into the `main` branch.
- Example PR titles:
- `feat: add a new join operator`
- `fix(ui): prevent racing of requests`
- `chore(deps): bump numpy to version 2.0.0`
- `fix(frontend): prevent racing of requests`
- `chore(deps, pyamber): bump numpy to version 2.0.0`

A scope names the module the change lands in — `amber`, `pyamber`, `frontend`, `agent-service`, `file-service`, and so on. Use the module's own name rather than an informal synonym, and when a PR spans modules, scope it to the one carrying the substantive change.

##### Choosing between `feat`, `fix`, and `refactor`

The type depends on what happens to the behavior, not on how large the change is.

| Your change | Type |
| ----------- | ---- |
| A functionality worked before and no longer does | `fix` |
| A functionality or a form of support never existed and you are adding it | `feat` |
| A functionality exists and you are removing support for it | `feat` |
| A functionality is reworked in a way that intentionally changes user-facing behavior | `feat` |
| The change leaves the user-facing behavior unchanged | `refactor` |
Comment on lines +63 to +69

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Fix the Markdown table style reported by markdownlint-cli2.

MD055 reports trailing pipes in both changed tables. Remove the final | from every row in Lines 63-69 and Lines 79-84 because the configured style is leading-only.

Proposed Markdown fix
-| Your change | Type |
-| ----------- | ---- |
+| Your change | Type
+| ----------- | ----

Also applies to: 79-84

🧰 Tools
🪛 markdownlint-cli2 (0.23.2)

[warning] 63-63: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


[warning] 64-64: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


[warning] 65-65: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


[warning] 66-66: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


[warning] 67-67: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


[warning] 68-68: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)


[warning] 69-69: Table pipe style
Expected: leading_only; Actual: leading_and_trailing; Unexpected trailing pipe

(MD055, table-pipe-style)

🤖 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 `@CONTRIBUTING.md` around lines 63 - 69, Update the Markdown tables in the
contribution guidance around the change-type table and the other affected table
to use leading-only pipes: remove the trailing `|` from every row in both table
sections while preserving their contents and alignment.

Source: Linters/SAST tools


Behavior is defined by the code, not by what a document or an old PR description says the code does. A functionality that was never implemented does not exist, so implementing it is a `feat` even when the docs already described it as present.

`refactor` is a strong claim: it says the **user-facing** behavior is identical. The test suite is how you check that, but not every test carries the same weight. A test that pins a user-facing API is the contract — if you had to change one of its assertions to make the suite green, the behavior moved, and the PR is a `feat` or a `fix`. A test that pins internals, such as a private helper's signature, the call order between two collaborators, or the shape of an intermediate value, is mirroring the implementation; rewriting it alongside the code it mirrors is expected and still a `refactor`.

##### Tests and dependency bumps

Test and dependency PRs take the titles below. Where the table shows a two-part scope, it is written as `<type>(<area>, <module>): <description>`:

| Your change | Title |
| ----------- | ----- |
| A test-only PR — adding or updating tests | `test(<module>): ...`, e.g. `test(amber): add marker replay specs` |
| Repairing a broken or flaky test | `fix(test, <module>): ...`, e.g. `fix(test, frontend): stabilize the dashboard spec` |
| A dependency bump that patches a CVE | `fix(deps, <module>): ...`, e.g. `fix(deps, pyamber): bump protobuf for CVE-2025-4565` |
| Any other dependency bump | `chore(deps, <module>): ...`, e.g. `chore(deps, pyamber): bump numpy to 2.0.0` |

Omit the module for bumps that span modules. GitHub Actions bumps are dependency bumps too and take the `ci` module — `chore(deps, ci): ...`, or `fix(deps, ci): ...` when the bump patches a CVE — which is the form [`.github/renovate.json5`](.github/renovate.json5) opens them with. Reserve a bare `ci: ...` for hand-written CI and workflow changes.

##### Backports

A PR targeting a release branch appends the release version as the **last scope component**, so a backport of `fix(deps, frontend): ...` to `release/v1.2` is titled `fix(deps, frontend, v1.2): ...`. Version tags belong only on release-branch PRs — never put one on a PR targeting `main`.

> 💡 You can use the [Conventional Commits plugin](https://plugins.jetbrains.com/plugin/13389-conventional-commit) in IntelliJ to help format commit messages correctly.

Expand Down
31 changes: 30 additions & 1 deletion docs/contribution-guidelines/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,39 @@ Fork the [Texera repository](https://github.com/apache/texera) on GitHub and clo
We use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/):
- Example PR titles:
- `feat: add new join operator`
- `fix(ui): resolve workflow panel crash`
- `fix(frontend): resolve workflow panel crash`
- `chore(deps): bump dependency versions`
- The PR title becomes the final squashed commit message upon merge.

A scope names the module the change lands in — `amber`, `pyamber`, `frontend`, `agent-service`, `file-service`, and so on. Use the module's own name rather than an informal synonym, and when a PR spans modules, scope it to the one carrying the substantive change.
Comment on lines 72 to +75

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Clarify the unscoped dependency example.

Line 72 shows a bare chore(deps) title. Lines 75 and 93-100 require a module scope for module-specific dependency bumps. Line 100 allows omitting the module only for cross-module bumps. Replace the example with a module-scoped form or state that it represents a cross-module bump.

Proposed documentation fix
-  - `chore(deps): bump dependency versions`
+  - `chore(deps, pyamber): bump numpy to version 2.0.0`
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- `chore(deps): bump dependency versions`
- The PR title becomes the final squashed commit message upon merge.
A scope names the module the change lands in — `amber`, `pyamber`, `frontend`, `agent-service`, `file-service`, and so on. Use the module's own name rather than an informal synonym, and when a PR spans modules, scope it to the one carrying the substantive change.
- `chore(deps, pyamber): bump numpy to version 2.0.0`
- The PR title becomes the final squashed commit message upon merge.
A scope names the module the change lands in — `amber`, `pyamber`, `frontend`, `agent-service`, `file-service`, and so on. Use the module's own name rather than an informal synonym, and when a PR spans modules, scope it to the one carrying the substantive change.
🤖 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 `@docs/contribution-guidelines/_index.md` around lines 72 - 75, Clarify the
dependency-bump example in the PR title guidance by either replacing
`chore(deps): bump dependency versions` with a module-scoped example or
explicitly labeling it as a cross-module bump. Keep the surrounding scope rules
consistent with the documented exception for cross-module changes.


Pick the type by what happens to the behavior, not by how large the change is:

| Your change | Type |
| ----------- | ---- |
| A functionality worked before and no longer does | `fix` |
| A functionality or a form of support never existed and you are adding it | `feat` |
| A functionality exists and you are removing support for it | `feat` |
| A functionality is reworked in a way that intentionally changes user-facing behavior | `feat` |
| The change leaves the user-facing behavior unchanged | `refactor` |

Behavior is defined by the code, not by what a document or an old PR description says the code does. A functionality that was never implemented does not exist, so implementing it is a `feat` even when the docs already described it as present.

`refactor` claims the **user-facing** behavior is identical. A test that pins a user-facing API must keep passing untouched — changing one of its assertions means the behavior moved, so the PR is a `feat` or a `fix`. A test that pins internals mirrors the implementation and may be rewritten alongside the code it mirrors.

Test and dependency PRs take the titles below. Where the table shows a two-part scope, it is written as `<type>(<area>, <module>): <description>`:

| Your change | Title |
| ----------- | ----- |
| A test-only PR — adding or updating tests | `test(<module>): ...` |
| Repairing a broken or flaky test | `fix(test, <module>): ...` |
| A dependency bump that patches a CVE | `fix(deps, <module>): ...` |
| Any other dependency bump | `chore(deps, <module>): ...` |

Omit the module for bumps that span modules. GitHub Actions bumps are dependency bumps too and take the `ci` module — `chore(deps, ci): ...`, or `fix(deps, ci): ...` when the bump patches a CVE — which is the form Renovate opens them with. Reserve a bare `ci: ...` for hand-written CI and workflow changes.

A PR targeting a release branch appends the version as the last scope component — a backport of `fix(deps, frontend): ...` to `release/v1.2` becomes `fix(deps, frontend, v1.2): ...`. Never put a version tag on a PR targeting `main`.

#### PR Description Should Include:
- **Purpose:** use `Closes #1234` to auto-close an issue.
- **Summary:** short overview of your changes.
Expand Down