Compare commits
5 commits
73c80491be
...
e70909af47
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e70909af47 | ||
|
|
f4b7e95d15 | ||
| a37a93150b | |||
|
|
4846ab3bd6 | ||
|
|
22d0e4f537 |
4 changed files with 104 additions and 11 deletions
90
DEFINITION_OF_DONE.md
Normal file
90
DEFINITION_OF_DONE.md
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
# Definition of Done — feedsignal
|
||||
|
||||
A change is done when all of the following hold, not just when it compiles.
|
||||
|
||||
## Single Responsibility
|
||||
|
||||
- Each module/function does one thing. If a PR description needs "and" to
|
||||
describe what a function does, split it.
|
||||
- Crate boundaries stay honest: `core` has no I/O (see `crates/core` —
|
||||
domain models + scoring/affinity math only); `db` only talks to SQLite;
|
||||
`feeds` only fetches/parses; `llm` only talks to Ollama. A change that
|
||||
makes `core` reach for a DB handle or `db` parse RSS is a sign the code
|
||||
landed in the wrong crate.
|
||||
- Within `db`, one file per domain concern (`feeds.rs`, `articles.rs`,
|
||||
`reading_events.rs`, `affinities.rs`) — a new table gets its own file,
|
||||
not a growing `queries.rs`.
|
||||
- Data-model structs (domain models, query row types, DTOs/views) live in
|
||||
a crate's `models.rs` rather than the file that produces or consumes
|
||||
them, whenever a struct is used outside the function that builds it —
|
||||
matching the existing `crates/core/src/models.rs` pattern. This doesn't
|
||||
apply to structs that are inherently local to one file/component (e.g.
|
||||
a Dioxus component's `Props`/`Styles`/context struct, or a helper
|
||||
struct scoped to a single function's internals).
|
||||
|
||||
## DRY, but not premature
|
||||
|
||||
- No copy-pasted logic across call sites — extract a shared function once
|
||||
a second real call site exists, not in anticipation of one.
|
||||
- No new abstraction (trait, generic helper, config knob) unless at least
|
||||
two concrete cases need it today.
|
||||
|
||||
## Testing
|
||||
|
||||
- All non-trivial logic ships with unit tests wherever practical — not
|
||||
just `core`'s pure functions (scoring, affinity math — the existing
|
||||
pattern in `crates/core/src/affinity.rs`), but query/filter logic in
|
||||
`db` and non-passthrough logic in `web`'s `server::services`. "Non-trivial"
|
||||
excludes thin wrappers (a `#[server]` fn or service function that's just
|
||||
`db.some_query(args).await?` with no branching of its own) and glue code
|
||||
whose only failure mode is a compiler error.
|
||||
- When a DB/network dependency makes a case genuinely impractical to unit
|
||||
test (e.g. a Diesel query's SQL correctness), say so in the PR rather
|
||||
than silently skipping it — that's a real gap to track, not a pass.
|
||||
- Test rationale goes in a `///` doc comment above `#[test] fn`, not
|
||||
inline in the body.
|
||||
- A bug fix includes a regression test that fails on the old code.
|
||||
- `cargo test --workspace --exclude feedsignal-web` passes locally before
|
||||
pushing (CI runs this; `feedsignal-web` itself currently has no test
|
||||
target).
|
||||
|
||||
## Static checks (all of CI, not a subset)
|
||||
|
||||
- `cargo fmt --check` clean.
|
||||
- `cargo clippy --workspace --exclude feedsignal-web --all-targets -- -D warnings` clean.
|
||||
- `cargo clippy -p feedsignal-web --no-default-features --features server --all-targets -- -D warnings` clean.
|
||||
- `cargo clippy -p feedsignal-web --no-default-features --features web --target wasm32-unknown-unknown -- -D warnings` clean.
|
||||
- `cargo audit` clean, or a documented reason if it can't be (e.g. an
|
||||
unfixed transitive advisory).
|
||||
|
||||
## No half-finished work
|
||||
|
||||
- No `TODO`s standing in for a decision the PR itself was supposed to
|
||||
make (a `TODO` marking genuinely deferred, ticketed follow-up work is
|
||||
fine — see `LlmConfig`'s config-story TODO).
|
||||
- No dead code kept "just in case" — if it's unused, delete it. Library-
|
||||
surface exceptions (e.g. `dioxus-primitives`-derived components) get an
|
||||
explicit `#[allow(dead_code)]` with a one-line reason, matching the
|
||||
existing pattern in `components/button/component.rs`.
|
||||
- No commented-out code left in.
|
||||
|
||||
## Correctness at the boundary
|
||||
|
||||
- New `#[server]` fns validate/trim input the way `services::feeds::subscribe`
|
||||
does (`anyhow::ensure!` on empty URL) — don't trust the browser.
|
||||
- DB writes that must be atomic with respect to a failure path (e.g.
|
||||
"fetch before writing so a bad URL leaves no half-registered row") get
|
||||
a comment explaining why, like `subscribe`'s doc comment does.
|
||||
- Migrations are additive/reversible — no editing a shipped migration file.
|
||||
|
||||
## Docs
|
||||
|
||||
- A non-obvious "why" (a workaround, a chosen threshold, a design
|
||||
tradeoff) gets a comment; "what" is left to the code.
|
||||
- `README.md`'s architecture section gets updated if the change adds a
|
||||
crate, a pipeline stage, or a scoring input.
|
||||
|
||||
## PR hygiene
|
||||
|
||||
- Diff is scoped to the stated task — no drive-by refactors bundled in.
|
||||
- `Cargo.lock` changes match what the dependency change actually requires.
|
||||
|
|
@ -1,3 +1,4 @@
|
|||
use crate::models::RankedArticleRow;
|
||||
use crate::schema;
|
||||
use crate::Db;
|
||||
use anyhow::Result;
|
||||
|
|
@ -7,17 +8,6 @@ use diesel_async::RunQueryDsl;
|
|||
use feedsignal_core::Article;
|
||||
use uuid::Uuid;
|
||||
|
||||
/// One row of [`Db::list_ranked_articles`].
|
||||
pub struct RankedArticleRow {
|
||||
pub id: String,
|
||||
pub feed_id: String,
|
||||
pub title: String,
|
||||
pub url: String,
|
||||
pub summary: String,
|
||||
pub topics: Vec<String>,
|
||||
pub final_score: Option<f32>,
|
||||
}
|
||||
|
||||
impl Db {
|
||||
pub async fn insert_article(&self, article: &Article) -> Result<()> {
|
||||
use schema::articles::dsl;
|
||||
|
|
|
|||
|
|
@ -1,7 +1,10 @@
|
|||
mod affinities;
|
||||
mod articles;
|
||||
mod feeds;
|
||||
mod models;
|
||||
mod reading_events;
|
||||
|
||||
pub use models::RankedArticleRow;
|
||||
pub(crate) mod schema;
|
||||
|
||||
use anyhow::Result;
|
||||
|
|
|
|||
10
crates/db/src/models.rs
Normal file
10
crates/db/src/models.rs
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
/// One row of [`crate::Db::list_ranked_articles`].
|
||||
pub struct RankedArticleRow {
|
||||
pub id: String,
|
||||
pub feed_id: String,
|
||||
pub title: String,
|
||||
pub url: String,
|
||||
pub summary: String,
|
||||
pub topics: Vec<String>,
|
||||
pub final_score: Option<f32>,
|
||||
}
|
||||
Loading…
Reference in a new issue