fix: instinct promote leaves orphaned project-scoped copy#2549
fix: instinct promote leaves orphaned project-scoped copy#2549wouterschoonees wants to merge 2 commits into
Conversation
promote (both by-ID and auto) wrote the instinct to the global personal dir but never touched the project-scoped source file. Since load_all_instincts() dedupes by ID with project taking precedence over global, the leftover project copy silently shadowed the new global entry - `status` under-reported the global count until the stale file was removed by hand. Adds _remove_instinct_from_source(), which strips just the promoted instinct's block from its (possibly multi-instinct) source file, deleting the file only if it was the sole entry. Wired into both _promote_specific and _promote_auto (the latter cleans every project's copy that fed the promotion, not just the highest-confidence one used for the merged content).
📝 WalkthroughSummary by CodeRabbit
WalkthroughPromotion now removes promoted instincts from project-scoped source files. The cleanup helper removes matching blocks, deletes empty files, and preserves remaining entries. Both specific and automatic promotion paths invoke the helper when source metadata is available. ChangesInstinct promotion cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| skills/continuous-learning-v2/scripts/instinct-cli.py | Adds project-source cleanup after global promotion and avoids deleting a source when the target instinct is absent from the parsed result. |
Reviews (2): Last reviewed commit: "fix: don't delete source file on failed ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@skills/continuous-learning-v2/scripts/instinct-cli.py`:
- Around line 1333-1347: The promotion flow is not transactional and can leave
source and global files inconsistent after failures. In
skills/continuous-learning-v2/scripts/instinct-cli.py lines 1333-1347, stage
remaining-source content in a temporary file and atomically replace the source
instead of unlinking or directly rewriting it; in lines 1411-1415, publish the
global copy only after specific-source cleanup succeeds or roll back on failure;
and in lines 1489-1494, apply the same per-candidate transaction for automatic
promotion. Handle and propagate filesystem errors explicitly at each level, and
do not report partial cleanup as success.
- Around line 1333-1347: The remaining-instincts persistence flow must avoid
direct unlink/write_text mutations and handle failures explicitly. Update the
surrounding function that processes remaining instincts to stage serialized
content in a temporary file within source_file’s directory, atomically replace
the source only after a successful write, and coordinate the empty-result
deletion with promotion rollback. Preserve the existing output format and ensure
cleanup and errors are handled at each operation level.
- Around line 1340-1345: Update the instinct block serialization loop around the
`for key, value in inst.items()` logic to serialize every retained field with
the existing schema-aware YAML serializer, not only `trigger` via `_yaml_quote`.
Preserve the exclusions for `content` and underscore-prefixed keys, while
ensuring all remaining values are safely serialized according to the schema
before appending them to `block`.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5ba35339-d084-40db-8da3-c4823ed20976
📒 Files selected for processing (1)
skills/continuous-learning-v2/scripts/instinct-cli.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.{py,pyi}: Follow PEP 8 conventions in Python code
Use type annotations on all function signatures in Python
Prefer immutable data structures such as frozen dataclasses and NamedTuple in Python
**/*.{py,pyi}: Auto-format Python files using black/ruff after edit
Run type checking using mypy/pyright after editing Python files
**/*.{py,pyi}: Use Protocol from typing module for duck typing and defining object shapes in Python
Use dataclasses with@dataclassdecorator for DTOs (Data Transfer Objects) in Python
Use context managers (with statement) for resource management in Python
Use generators for lazy evaluation and memory-efficient iteration in Python
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.py: Use black for code formatting in Python
Use isort for import sorting in Python
Use ruff for linting Python codeAvoid using
print()statements in Python code; use theloggingmodule instead
**/*.py: Retrieve secrets and API keys from environment variables using os.environ with error handling (raise KeyError if missing) rather than hardcoding credentials
Use bandit for static security analysis in Python projects
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
skills/**
📄 CodeRabbit inference engine (AGENTS.md)
New workflow contributions should land in
skills/first;skills/is the canonical workflow surface.
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
skills/**/scripts/**
⚙️ CodeRabbit configuration file
skills/**/scripts/**: Review generated or imported scripts as untrusted-input tooling. Flag RCE, path traversal, network fetches without validation, and writes outside the expected workspace.
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
{skills,commands,agents,rules}/**
⚙️ CodeRabbit configuration file
{skills,commands,agents,rules}/**: Focus on prompt-injection resilience, tool-permission scope, destructive action guards, and secret exfiltration risks.
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
🧠 Learnings (1)
📚 Learning: 2026-06-27T15:41:48.588Z
Learnt from: gaurav0107
Repo: affaan-m/ECC PR: 2369
File: skills/continuous-learning-v2/scripts/test_parse_instinct.py:1056-1082
Timestamp: 2026-06-27T15:41:48.588Z
Learning: In this repo, the Ruff configuration for Python files under `skills/continuous-learning-v2/scripts/` selects only `E/F/I/N/W/UP`, and `flake8-pytest-style` (e.g., `PT018`) is not enabled. During code review for files in this directory, don’t assume `PT018` will be enforced by CI/ruff; only raise/test for issues that are covered by the enabled Ruff rules (or confirm the relevant rule is enabled before relying on it).
Applied to files:
skills/continuous-learning-v2/scripts/instinct-cli.py
| if not remaining: | ||
| source_file.unlink() | ||
| return | ||
|
|
||
| blocks = [] | ||
| for inst in remaining: | ||
| block = "---\n" | ||
| for key, value in inst.items(): | ||
| if key == 'content' or key.startswith('_'): | ||
| continue | ||
| block += f"{key}: {_yaml_quote(value) if key == 'trigger' else value}\n" | ||
| block += "---\n\n" | ||
| block += inst.get('content', '') + "\n" | ||
| blocks.append(block) | ||
| source_file.write_text("\n".join(blocks), encoding="utf-8") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Promotion is not transactional across global and source files.
The current sequence can lose remaining source data or leave stale project copies after a crash or filesystem error.
skills/continuous-learning-v2/scripts/instinct-cli.py#L1333-L1347: stage source replacements and atomically replace them instead of directly unlinking or truncating files.skills/continuous-learning-v2/scripts/instinct-cli.py#L1411-L1415: do not publish the global copy until specific-source cleanup succeeds, or roll back on failure.skills/continuous-learning-v2/scripts/instinct-cli.py#L1489-L1494: apply the same transaction per automatic-promotion candidate so partial multi-project cleanup cannot be reported as success.
As per coding guidelines, errors must be handled explicitly at every level.
📍 Affects 1 file
skills/continuous-learning-v2/scripts/instinct-cli.py#L1333-L1347(this comment)skills/continuous-learning-v2/scripts/instinct-cli.py#L1411-L1415skills/continuous-learning-v2/scripts/instinct-cli.py#L1489-L1494
🤖 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 `@skills/continuous-learning-v2/scripts/instinct-cli.py` around lines 1333 -
1347, The promotion flow is not transactional and can leave source and global
files inconsistent after failures. In
skills/continuous-learning-v2/scripts/instinct-cli.py lines 1333-1347, stage
remaining-source content in a temporary file and atomically replace the source
instead of unlinking or directly rewriting it; in lines 1411-1415, publish the
global copy only after specific-source cleanup succeeds or roll back on failure;
and in lines 1489-1494, apply the same per-candidate transaction for automatic
promotion. Handle and propagate filesystem errors explicitly at each level, and
do not report partial cleanup as success.
Source: Coding guidelines
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Make source cleanup atomic.
unlink() and write_text() mutate the source directly. A crash or short write can delete or truncate the remaining instincts, and callers have already published the global copy by this point. Stage replacements in the same directory and atomically replace them; coordinate deletion with promotion rollback.
As per coding guidelines, errors must be handled explicitly at every level.
🤖 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 `@skills/continuous-learning-v2/scripts/instinct-cli.py` around lines 1333 -
1347, The remaining-instincts persistence flow must avoid direct
unlink/write_text mutations and handle failures explicitly. Update the
surrounding function that processes remaining instincts to stage serialized
content in a temporary file within source_file’s directory, atomically replace
the source only after a successful write, and coordinate the empty-result
deletion with promotion rollback. Preserve the existing output format and ensure
cleanup and errors are handled at each operation level.
Source: Coding guidelines
| for key, value in inst.items(): | ||
| if key == 'content' or key.startswith('_'): | ||
| continue | ||
| block += f"{key}: {_yaml_quote(value) if key == 'trigger' else value}\n" | ||
| block += "---\n\n" | ||
| block += inst.get('content', '') + "\n" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Use YAML-safe serialization for every retained field.
Only trigger passes through _yaml_quote; every other value is inserted with Python string interpolation. A surviving value containing YAML syntax, a newline, or a non-scalar type can change content or type on the next parse, so cleanup does not reliably preserve the remaining instincts. Use the existing schema-aware serializer for every field.
As per coding guidelines, external data must be treated as untrusted and schema-based validation/serialization should be used.
🤖 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 `@skills/continuous-learning-v2/scripts/instinct-cli.py` around lines 1340 -
1345, Update the instinct block serialization loop around the `for key, value in
inst.items()` logic to serialize every retained field with the existing
schema-aware YAML serializer, not only `trigger` via `_yaml_quote`. Preserve the
exclusions for `content` and underscore-prefixed keys, while ensuring all
remaining values are safely serialized according to the schema before appending
them to `block`.
Source: Coding guidelines
Address review feedback on affaan-m#2549 (Greptile): _remove_instinct_from_source treated an empty parse_instinct_file() result as proof the source file was safe to delete. A malformed file or one modified concurrently by another session would also parse to [], so the guard was deleting content it never actually read. Now bails out unless the target ID is found in a successful parse - a parse failure is never treated as "nothing left to keep".
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
skills/continuous-learning-v2/scripts/instinct-cli.py (1)
1418-1422:⚠️ Potential issue | 🟠 MajorPromotion is still not transactional across global and source files.
Both paths publish the global copy before source cleanup completes, so filesystem or parsing failures can leave stale project copies and partial promotion state.
skills/continuous-learning-v2/scripts/instinct-cli.py#L1418-L1422: commit source cleanup before publishing the specific global copy, or roll back on failure.skills/continuous-learning-v2/scripts/instinct-cli.py#L1496-L1501: apply the same per-candidate transaction across every automatic-promotion source.🤖 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 `@skills/continuous-learning-v2/scripts/instinct-cli.py` around lines 1418 - 1422, Make promotion transactional in the per-instinct flow around _remove_instinct_from_source: complete and validate source cleanup before publishing the corresponding global copy, or restore both locations if either operation fails. Apply the same per-candidate transaction semantics to the automatic-promotion loop at skills/continuous-learning-v2/scripts/instinct-cli.py lines 1496-1501, ensuring every source leaves either both updates committed or the original state intact.
🤖 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 `@skills/continuous-learning-v2/scripts/instinct-cli.py`:
- Around line 1330-1335: Strengthen the removal flow around
parse_instinct_file() so source files are fully and losslessly validated before
modification. Do not rely solely on finding instinct_id, since filtered or
malformed blocks may be discarded; either strictly validate every source block
and abort on any unknown/malformed content, or preserve unknown/raw blocks when
rewriting. Ensure remaining being empty cannot delete the file unless the
complete source was safely parsed.
---
Outside diff comments:
In `@skills/continuous-learning-v2/scripts/instinct-cli.py`:
- Around line 1418-1422: Make promotion transactional in the per-instinct flow
around _remove_instinct_from_source: complete and validate source cleanup before
publishing the corresponding global copy, or restore both locations if either
operation fails. Apply the same per-candidate transaction semantics to the
automatic-promotion loop at
skills/continuous-learning-v2/scripts/instinct-cli.py lines 1496-1501, ensuring
every source leaves either both updates committed or the original state intact.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5cbdfa2c-b7c1-4e14-bdd7-e843d90018a5
📒 Files selected for processing (1)
skills/continuous-learning-v2/scripts/instinct-cli.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.{py,pyi}: Follow PEP 8 conventions in Python code
Use type annotations on all function signatures in Python
Prefer immutable data structures such as frozen dataclasses and NamedTuple in Python
**/*.{py,pyi}: Auto-format Python files using black/ruff after edit
Run type checking using mypy/pyright after editing Python files
**/*.{py,pyi}: Use Protocol from typing module for duck typing and defining object shapes in Python
Use dataclasses with@dataclassdecorator for DTOs (Data Transfer Objects) in Python
Use context managers (with statement) for resource management in Python
Use generators for lazy evaluation and memory-efficient iteration in Python
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
**/*.py
📄 CodeRabbit inference engine (.cursor/rules/python-coding-style.md)
**/*.py: Use black for code formatting in Python
Use isort for import sorting in Python
Use ruff for linting Python codeAvoid using
print()statements in Python code; use theloggingmodule instead
**/*.py: Retrieve secrets and API keys from environment variables using os.environ with error handling (raise KeyError if missing) rather than hardcoding credentials
Use bandit for static security analysis in Python projects
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
skills/**
📄 CodeRabbit inference engine (AGENTS.md)
New workflow contributions should land in
skills/first;skills/is the canonical workflow surface.
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
skills/**/scripts/**
⚙️ CodeRabbit configuration file
skills/**/scripts/**: Review generated or imported scripts as untrusted-input tooling. Flag RCE, path traversal, network fetches without validation, and writes outside the expected workspace.
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
{skills,commands,agents,rules}/**
⚙️ CodeRabbit configuration file
{skills,commands,agents,rules}/**: Focus on prompt-injection resilience, tool-permission scope, destructive action guards, and secret exfiltration risks.
Files:
skills/continuous-learning-v2/scripts/instinct-cli.py
🧠 Learnings (1)
📚 Learning: 2026-06-27T15:41:48.588Z
Learnt from: gaurav0107
Repo: affaan-m/ECC PR: 2369
File: skills/continuous-learning-v2/scripts/test_parse_instinct.py:1056-1082
Timestamp: 2026-06-27T15:41:48.588Z
Learning: In this repo, the Ruff configuration for Python files under `skills/continuous-learning-v2/scripts/` selects only `E/F/I/N/W/UP`, and `flake8-pytest-style` (e.g., `PT018`) is not enabled. During code review for files in this directory, don’t assume `PT018` will be enforced by CI/ruff; only raise/test for issues that are covered by the enabled Ruff rules (or confirm the relevant rule is enabled before relying on it).
Applied to files:
skills/continuous-learning-v2/scripts/instinct-cli.py
| parsed = parse_instinct_file(source_file.read_text(encoding="utf-8")) | ||
| if not any(i.get('id') == instinct_id for i in parsed): | ||
| # Target not found in a successful parse - either already removed, or the | ||
| # file is malformed/foreign content. Either way, don't touch it: a parse | ||
| # failure must never be treated as proof the file is safe to delete. | ||
| return |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Require a complete, lossless parse before modifying the source.
Checking that one parsed block has the target ID is insufficient: parse_instinct_file() filters out blocks without id and tolerates malformed content. A file containing the target plus unreadable/foreign content can therefore pass this guard; remaining may be empty and Line 1340 can delete the whole file, or the rewrite can drop those blocks. Require strict validation of every source block, or preserve unknown/raw blocks during removal.
As per coding guidelines, external data must be treated as untrusted and schema-validated before processing.
🤖 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 `@skills/continuous-learning-v2/scripts/instinct-cli.py` around lines 1330 -
1335, Strengthen the removal flow around parse_instinct_file() so source files
are fully and losslessly validated before modification. Do not rely solely on
finding instinct_id, since filtered or malformed blocks may be discarded; either
strictly validate every source block and abort on any unknown/malformed content,
or preserve unknown/raw blocks when rewriting. Ensure remaining being empty
cannot delete the file unless the complete source was safely parsed.
Source: Coding guidelines
daltino
left a comment
daltino
left a comment
There was a problem hiding this comment.
This PR addresses a significant issue where project-scoped copies of instincts remain after promotion, causing problems with global instinct recognition. The added function _remove_instinct_from_source appears to handle the cleanup effectively but could benefit from clarifications in its documentation and potentially additional safeguards. Consider adding a description of what happens if the instinct ID isn't found and test coverage to validate edge cases.
2 participants
Summary
instinct promote <id>(and the auto cross-project variant) writes the promoted instinct into the global personal dir but never removes it from the project-scoped source file it came from.load_all_instincts()dedupes by ID with project-scope taking precedence over global on collision, so the leftover project copy silently shadows the new global entry -statusunder-reports the global count (and shows the instinct as still project-scoped) until the stale file is deleted by hand.statusoutput before/after manually removing the stale source file.Fix
_remove_instinct_from_source(): re-parses the instinct's source file, strips just the promoted block, rewrites the remaining blocks in the same frontmatter format, or deletes the file outright if it was the only instinct in it._promote_specific(single-ID promote)._promote_autoas well - it iterates every project entry that contributed to the cross-project candidate, not just the highest-confidence one used for the merged output content, since each project's copy independently shadows the same global ID.Test plan
py_compileon the patched fileCLV2_HOMUNCULUS_DIRpointed at a temp dir): imported a 2-instinct file into a scratch git repo, promoted one ID, confirmed the promoted instinct disappears from project scope and appears under global instatus, while the un-promoted sibling in the same source file survives untouched_promote_specific/_promote_auto; happy to add one if there's a preferred harness/location forinstinct-cli.pytests