Skip to content

feat: remove profiles support - #1183

Merged
twitchyliquid64 merged 1 commit into
mainfrom
tom/kill-profiles
Aug 7, 2026
Merged

feat: remove profiles support#1183
twitchyliquid64 merged 1 commit into
mainfrom
tom/kill-profiles

Conversation

@twitchyliquid64

@twitchyliquid64 twitchyliquid64 commented Aug 6, 2026

Copy link
Copy Markdown
Member

Yeets profiles into the sun once and for all.

Note

Remove profiles support from check pipeline, CLI, wire format, and decode/graph layers

  • Removes the profile checker, profile decoding, and profile graph logic entirely; only packages and stacks remain as check targets.
  • Drops the --profiles CLI flag from mip and minimald, removes the profiles field from CheckRequest proto (field 6 reserved), and stops emitting profile frames in wire streams.
  • The Nickel stdlib loosens the profiles array type to Array Dyn for compatibility with existing spec files that still declare profiles.
  • Adds deprecation warnings in mfile validation when defaults.profile or task.profile fields are set, noting removal in 0.5.1.
  • Risk: wire format and RPC changes mean older clients sending profile data (proto field 6) will have it silently ignored; task configs that rely on profiles will log warnings but profiles will not be applied.

Macroscope summarized e89d8bf.

Summary by CodeRabbit

  • Breaking Changes

    • Removed profile support from validation, task configuration, layer processing, and remote checks.
    • Check commands now cover packages and stacks only; the --profiles option is no longer available.
    • Existing profile settings are ignored and produce warnings where applicable.
    • Legacy profile data remains reserved but is not processed.
  • Documentation

    • Updated configuration, task, CLI, and sandbox documentation to reflect profile removal.
    • Removed the profiles concept documentation.
  • Maintenance

    • Updated the standard library package version.

@twitchyliquid64
twitchyliquid64 requested a review from a team as a code owner August 6, 2026 21:03
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Profile support was removed from decoding, graph construction, checks, remote requests, standard-library declarations, configuration warnings, and documentation. Package and stack checks remain supported. Legacy wire profile records are ignored.

Changes

Profile data and graph processing

Layer / File(s) Summary
Remove profile decoding
crates/decode/src/lib.rs
Profile and Layer::profiles were removed. Layer loading no longer parses profile records.
Remove graph profile state
crates/graph/src/graph.rs, crates/graph/src/loader.rs, crates/graph/src/wire.rs
Graph ingestion no longer loads or applies profiles. Legacy profile wire records are ignored.

Check and transport scope

Layer / File(s) Summary
Restrict checks to packages and stacks
crates/check/src/lib.rs, crates/mctx/src/env.rs, crates/minimald/src/session_sop.rs, crates/minimald/src/sessions.rs, crates/remote-client/src/lib.rs
Profile check scheduling, flags, arguments, response mapping, and test expectations were removed.
Remove remote profile fields
crates/remote-proto/build.rs, crates/remote-proto/protos/res/check.proto
Remote check requests and objects no longer include profiles. Reserved fields preserve protocol numbering.

Compatibility and documentation

Layer / File(s) Summary
Mark profile configuration as removed
crates/mfile/src/lib.rs, crates/mfile/src/tasks.rs, crates/stdlib/minimal-ncl/minimal.ncl, crates/stdlib/Cargo.toml
Configured profile fields now produce removal warnings. Standard-library profile declarations were removed, and the package version increased.
Update documentation
docs/reference/cli-mip.md, docs/reference/minimal-dot-toml.md, docs/reference/sandbox-operations.md, docs/reference/tasks.md
Profile-related command, layout, and task references were removed.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related PRs

Poem

A rabbit checks packages and stacks,
No profile paths remain in the tracks.
Old wire records pass by unseen,
Graphs hold only what they mean.
Hop through the simpler checks!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the main changes and risks but omits the required Testing section and Checklist. Add explicit Summary, Testing, and Checklist sections, including test commands with results and confirmation of documentation and breaking-change requirements.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the primary change: removing profile support.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch tom/kill-profiles

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/minimald/src/session_sop.rs (1)

250-273: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Remove --profiles from parser text.

The method documentation and the unknown-flag error still list --profiles. The parser now rejects that flag. Update both messages to list only supported flags.

🤖 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 `@crates/minimald/src/session_sop.rs` around lines 250 - 273, Update the
session check method’s documentation and the unknown-flag error in the argument
parser to remove `--profiles`; list only the currently supported `--packages`,
`--stacks`, and `--fix` flags.
🤖 Prompt for all review comments with 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.

Inline comments:
In `@crates/mctx/src/env.rs`:
- Around line 153-163: Update the argument parsing loop in env.rs to reject
unknown dash-prefixed tokens such as --profiles instead of adding them to
filter_names. Preserve name filtering for non-flag tokens and keep the existing
check_all behavior; alternatively reuse CheckOpts::from_args so parsing matches
the session_sop transport.

In `@crates/mfile/src/lib.rs`:
- Around line 296-298: Update File::hydrate_task_defaults to stop copying
Defaults::profile into task defaults, while retaining the profile field for
deserialization compatibility. Remove stale profile references from the
hydration documentation and leave other task-default assignments unchanged.

In `@crates/stdlib/minimal-ncl/minimal.ncl`:
- Around line 200-204: Update the Layer documentation near the profiles field to
state that profiles are retained as legacy compatibility data, accepted from old
spec files, and ignored during ingestion; remove the implication that profiles
are ingested.
- Around line 195-197: Restore the active profile compatibility binding in the
minimal standard library, using the existing identity function signature and
behavior. Leave the internal 'Profile type tag unchanged, then run the loader
test through the repository’s just recipe to verify the imported profile calls
resolve.

---

Outside diff comments:
In `@crates/minimald/src/session_sop.rs`:
- Around line 250-273: Update the session check method’s documentation and the
unknown-flag error in the argument parser to remove `--profiles`; list only the
currently supported `--packages`, `--stacks`, and `--fix` flags.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 084b9587-6255-4ab2-8486-7f139e202cb3

📥 Commits

Reviewing files that changed from the base of the PR and between f5a9a98 and e2de3fe.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (23)
  • crates/check/src/lib.rs
  • crates/check/src/profile.rs
  • crates/decode/src/lib.rs
  • crates/decode/src/profiles.rs
  • crates/graph/src/graph.rs
  • crates/graph/src/loader.rs
  • crates/graph/src/wire.rs
  • crates/mctx/src/env.rs
  • crates/mfile/src/lib.rs
  • crates/mfile/src/tasks.rs
  • crates/minimald/src/session_sop.rs
  • crates/minimald/src/sessions.rs
  • crates/mip/src/cmd_check.rs
  • crates/remote-client/src/lib.rs
  • crates/remote-proto/build.rs
  • crates/remote-proto/protos/res/check.proto
  • crates/stdlib/Cargo.toml
  • crates/stdlib/minimal-ncl/minimal.ncl
  • docs/concepts/profiles.md
  • docs/reference/cli-mip.md
  • docs/reference/minimal-dot-toml.md
  • docs/reference/sandbox-operations.md
  • docs/reference/tasks.md
💤 Files with no reviewable changes (6)
  • crates/decode/src/profiles.rs
  • crates/check/src/profile.rs
  • docs/concepts/profiles.md
  • crates/minimald/src/sessions.rs
  • crates/mip/src/cmd_check.rs
  • crates/remote-client/src/lib.rs

Comment thread crates/mctx/src/env.rs
Comment thread crates/mfile/src/lib.rs
Comment on lines +296 to 298
/// Removed feature, emits warning if set.
#[serde(default)]
pub profile: Option<String>,

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 | 🟠 Major | ⚡ Quick win

Stop applying the removed default profile.

Defaults::profile is documented as removed, but File::hydrate_task_defaults still copies it into every task. This preserves the removed default-profile inheritance and causes downstream task hydration to warn for each task. Keep the field for deserialization compatibility, but remove the assignment and the stale profile references from the hydration documentation.

Proposed fix
-        if let Some(default_profile) = &self.defaults.profile
-            && task.profile.is_none()
-        {
-            task.profile = Some(default_profile.clone());
-        }
🤖 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 `@crates/mfile/src/lib.rs` around lines 296 - 298, Update
File::hydrate_task_defaults to stop copying Defaults::profile into task
defaults, while retaining the profile field for deserialization compatibility.
Remove stale profile references from the hydration documentation and leave other
task-default assignments unchanged.

Comment on lines +195 to +197
# # Profiles were removed 2026-08-06, left to avoid breaking old spec-files
# profile | Dyn -> Dyn =
# fun spec => spec,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- target file ---'
cat -n crates/stdlib/minimal-ncl/minimal.ncl | sed -n '170,215p'

printf '%s\n' '--- references to profile/Profile and target file ---'
rg -n --glob '!target/**' --glob '!node_modules/**' '\bprofile\b|\bProfile\b|minimal-ncl|minimal\.ncl' .

printf '%s\n' '--- available just recipes ---'
just --list 2>&1 || true

Repository: gominimal/minimal

Length of output: 34427


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- profile fixture and nearby test ---'
cat -n crates/decode/src/load.rs | sed -n '585,640p'

printf '%s\n' '--- profile-related Nickel files ---'
git ls-files | rg '(^|/)(profile\.ncl|.*profile.*\.ncl)$' || true

printf '%s\n' '--- exact Nickel profile type-annotation references ---'
rg -n --glob '*.ncl' '\|\s*Profile\b|\bProfile\b|\bprofile\s*\{' .

printf '%s\n' '--- justfile test recipes ---'
cat -n justfile | sed -n '1,80p;240,310p'

printf '%s\n' '--- decode test declarations around the fixture ---'
rg -n -C 8 'Make a profile called rust|profile_dir\.join|profile \{' crates/decode/src/load.rs

Repository: gominimal/minimal

Length of output: 12408


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- remainder of loader test ---'
cat -n crates/decode/src/load.rs | sed -n '626,705p'

printf '%s\n' '--- checked-in profile specs ---'
for f in .minimal/profiles/demo/profile.ncl .minimal/profiles/dev/profile.ncl; do
    if [ -f "$f" ]; then
        printf '%s\n' "--- $f ---"
        cat -n "$f"
    fi
done

printf '%s\n' '--- read-only compatibility verifier ---'
python3 - <<'PY'
from pathlib import Path
import re

stdlib = Path("crates/stdlib/minimal-ncl/minimal.ncl").read_text()
active_profile_binding = re.search(
    r"(?m)^\s*profile\s*\|\s*Dyn\s*->\s*Dyn\s*=",
    stdlib,
) is not None
commented_profile_binding = re.search(
    r"(?m)^\s*#\s*profile\s*\|\s*Dyn\s*->\s*Dyn\s*=",
    stdlib,
) is not None

profile_calls = []
profile_type_annotations = []
for path in Path(".").rglob("*.ncl"):
    text = path.read_text(errors="replace")
    if re.search(r'\blet\s*\{[^}]*\bprofile\b[^}]*\}\s*=\s*import\s+"minimal\.ncl"', text):
        profile_calls.append(str(path))
    if re.search(r"\|\s*Profile\b", text):
        profile_type_annotations.append(str(path))

print(f"active profile binding: {active_profile_binding}")
print(f"commented profile binding: {commented_profile_binding}")
print("legacy profile-import fixtures:")
for path in profile_calls:
    print(f"  {path}")
print("Nickel Profile type annotations:")
for path in profile_type_annotations:
    print(f"  {path}")
PY

Repository: gominimal/minimal

Length of output: 3591


Restore the active profile compatibility binding.

.minimal/profiles/demo/profile.ncl, .minimal/profiles/dev/profile.ncl, and crates/decode/src/load.rs import and call profile. The commented binding causes these specs to fail with an unknown identifier. No Nickel spec uses Profile as a type annotation; 'Profile in Tys is an internal type tag. Run the loader test through the repository’s just recipe.

🤖 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 `@crates/stdlib/minimal-ncl/minimal.ncl` around lines 195 - 197, Restore the
active profile compatibility binding in the minimal standard library, using the
existing identity function signature and behavior. Leave the internal 'Profile
type tag unchanged, then run the loader test through the repository’s just
recipe to verify the imported profile calls resolve.

Source: Coding guidelines

Comment on lines 200 to +204
Layer | doc "Implementation detail: the set of build-specs and profiles to be ingested." = {
ty | Tys = 'Layer,

builds | Array BuildSpec,
profiles | Array Profile,
profiles | Array Dyn, # Profiles were removed 2026-08-06.

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

Document retained profiles as ignored compatibility data.

The Layer documentation still describes profiles as data “to be ingested”. This change retains profiles as Array Dyn for old spec-file compatibility, while profile ingestion was removed. State that legacy profile records are accepted and ignored.

🤖 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 `@crates/stdlib/minimal-ncl/minimal.ncl` around lines 200 - 204, Update the
Layer documentation near the profiles field to state that profiles are retained
as legacy compatibility data, accepted from old spec files, and ignored during
ingestion; remove the implication that profiles are ingested.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@twitchyliquid64
twitchyliquid64 merged commit 70c9b59 into main Aug 7, 2026
30 checks passed
@twitchyliquid64
twitchyliquid64 deleted the tom/kill-profiles branch August 7, 2026 02:03
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