Skip to content

feat: create markdown rules in notch #5945

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.

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
merged 6 commits into from
Jun 9, 2025
Merged

Conversation

Patrick-Erichsen
Copy link
Collaborator

Description

When clicking "Add Rule" from the notch, we now create a markdown rule instead of a YAML rule.

@Patrick-Erichsen Patrick-Erichsen requested a review from a team as a code owner June 2, 2025 21:34
@Patrick-Erichsen Patrick-Erichsen requested review from RomneyDa and removed request for a team June 2, 2025 21:34
Copy link

cubic-dev-ai bot commented Jun 2, 2025

Your cubic subscription is currently inactive. Please reactivate your subscription to receive AI reviews and use cubic.

Copy link

netlify bot commented Jun 2, 2025

Deploy Preview for continuedev ready!

Name Link
🔨 Latest commit 6406741
🔍 Latest deploy log https://app.netlify.com/projects/continuedev/deploys/68474127dcd7730008245e34
😎 Deploy Preview https://deploy-preview-5945--continuedev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@dosubot dosubot bot added the size:XL This PR changes 500-999 lines, ignoring generated files. label Jun 2, 2025
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This consolidates some of the rule creation logic we had in multiple places throughout the codebase

@@ -37,11 +37,6 @@ export const createRuleBlock: Tool = {
description:
"Optional file patterns to which this rule applies (e.g. ['**/*.{ts,tsx}'] or ['src/**/*.ts', 'tests/**/*.ts'])",
},
alwaysApply: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Decided to remove this since it's still relatively experimental.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's also confusing for the model and maybe we should use a different property name for the model and then map it to the underlying property

@chezsmithy
Copy link
Contributor

Could we offer the choice? We have a bunch of YAML rules now and this might be somewhat confusing.

@Patrick-Erichsen
Copy link
Collaborator Author

Patrick-Erichsen commented Jun 3, 2025

@chezsmithy thanks for the callout here! What are your general thoughts/feelings on markdown vs YAML syntax for rule creation?

We're thinking next week of rolling out a tool to easily convert all of your existing YAML rules to markdown. If we shipped that, 1) would you use it, and 2) would it then be non-disruptive to merge this PR?

If so I think we can leave this PR open until we ship the YAML -> markdown rule conversion.

@chezsmithy
Copy link
Contributor

@Patrick-Erichsen thanks for the heads up. I'll check in with my team. Are you considering deprecating YAML for rules all together, or just creation initially? Would the tool be command line, or something in the UI?

Maybe we could have a dialog on discord about the plan and save a bunch of back and forth here?

@Patrick-Erichsen
Copy link
Collaborator Author

RomneyDa
RomneyDa previously approved these changes Jun 9, 2025
@@ -37,11 +37,6 @@ export const createRuleBlock: Tool = {
description:
"Optional file patterns to which this rule applies (e.g. ['**/*.{ts,tsx}'] or ['src/**/*.ts', 'tests/**/*.ts'])",
},
alwaysApply: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's also confusing for the model and maybe we should use a different property name for the model and then map it to the underlying property

import { RULE_FILE_EXTENSION, createRuleMarkdown } from "../markdown";

export function blockTypeToSingular(blockType: BlockType): string {
switch (blockType) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

feels like an object with block info would be cleaner here, like

context:
  singular: Context
  plural: Context
  title: Context
mcpServers:
  singular: MCP Server
  plural: MCP Servers
  title: MCP Servers

Or similar

@github-project-automation github-project-automation bot moved this from Todo to In Progress in Issues and PRs Jun 9, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 9, 2025
@Patrick-Erichsen Patrick-Erichsen merged commit 8e9162b into main Jun 9, 2025
65 of 67 checks passed
@Patrick-Erichsen Patrick-Erichsen deleted the pe/add-rule-format branch June 9, 2025 21:17
@github-project-automation github-project-automation bot moved this from In Progress to Done in Issues and PRs Jun 9, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Jun 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm This PR has been approved by a maintainer size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants